From 2b3d07746caa76e7ad782651d7a19ac8c09c1d08 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Wed, 18 Dec 2024 17:13:51 +0100 Subject: [PATCH 1/6] test(spdx-utils): Simplify comparing a string representation With sorting applied, the string representation can be hard-coded. Signed-off-by: Sebastian Schuberth --- utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt b/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt index 7c3f93479fc41..951fa3d05bcdd 100644 --- a/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt +++ b/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt @@ -59,13 +59,7 @@ class SpdxCompoundExpressionTest : WordSpec({ ) // Compare string representations to not rely on semantic equality. - expression.simplify().toString() shouldBe SpdxCompoundExpression( - SpdxOperator.AND, - listOf( - SpdxLicenseIdExpression("MIT"), - SpdxLicenseIdExpression("Apache-2.0") - ) - ).toString() + expression.simplify().sorted().toString() shouldBe "Apache-2.0 AND MIT" } "create a single expression for equal operands" { From 78bbedd35827215134be87d2aa713bf2a3c65720 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Wed, 18 Dec 2024 15:39:47 +0100 Subject: [PATCH 2/6] test(spdx-utils): Compare strings to not rely on semantic equality Be extra sure that the actual string representation becomes simpler. Signed-off-by: Sebastian Schuberth --- utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt b/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt index 951fa3d05bcdd..fd726f789b30e 100644 --- a/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt +++ b/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt @@ -71,7 +71,8 @@ class SpdxCompoundExpressionTest : WordSpec({ ) ) - expression.simplify() shouldBe SpdxLicenseIdExpression("MIT") + // Compare string representations to not rely on semantic equality. + expression.simplify().toString() shouldBe "MIT" } } }) From 31dec8812ffc9603d5cf6e328b7303f18cc9b9fd Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Wed, 18 Dec 2024 15:40:58 +0100 Subject: [PATCH 3/6] test(spdx-utils): Add a test for simplifying OR-operands Disambiguate the AND-operator's test name accordingly. Signed-off-by: Sebastian Schuberth --- .../src/test/kotlin/SpdxCompoundExpressionTest.kt | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt b/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt index fd726f789b30e..fa6dc6f589579 100644 --- a/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt +++ b/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt @@ -62,7 +62,7 @@ class SpdxCompoundExpressionTest : WordSpec({ expression.simplify().sorted().toString() shouldBe "Apache-2.0 AND MIT" } - "create a single expression for equal operands" { + "create a single expression for equal AND-operands" { val expression = SpdxCompoundExpression( SpdxOperator.AND, listOf( @@ -74,5 +74,18 @@ class SpdxCompoundExpressionTest : WordSpec({ // Compare string representations to not rely on semantic equality. expression.simplify().toString() shouldBe "MIT" } + + "create a single expression for equal OR-operands" { + val expression = SpdxCompoundExpression( + SpdxOperator.OR, + listOf( + SpdxLicenseIdExpression("MIT"), + SpdxLicenseIdExpression("MIT") + ) + ) + + // Compare string representations to not rely on semantic equality. + expression.simplify().toString() shouldBe "MIT" + } } }) From e9d6dd45dd8be9cab0e3ebe94c0f151fd2270e16 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Wed, 18 Dec 2024 15:53:11 +0100 Subject: [PATCH 4/6] docs(spdx-utils): Add comments about the `validChoices()` algorithm While this was partly explained in the commit message that introduced the code, make this more visible. Signed-off-by: Sebastian Schuberth --- utils/spdx/src/main/kotlin/SpdxExpression.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/utils/spdx/src/main/kotlin/SpdxExpression.kt b/utils/spdx/src/main/kotlin/SpdxExpression.kt index bc7f4ec9e6354..45cc29e438a0f 100644 --- a/utils/spdx/src/main/kotlin/SpdxExpression.kt +++ b/utils/spdx/src/main/kotlin/SpdxExpression.kt @@ -300,6 +300,8 @@ class SpdxCompoundExpression( override fun validChoices(): Set = when (operator) { SpdxOperator.AND -> { + // If there is a top-level `AND` in an expression, create the combinations of all choices on the left + // and all choices on the right to get the overall valid choices. children.fold(setOf()) { acc, child -> if (acc.isEmpty()) { child.validChoices() @@ -317,7 +319,11 @@ class SpdxCompoundExpression( } } - SpdxOperator.OR -> children.flatMapTo(mutableSetOf()) { it.validChoices() } + SpdxOperator.OR -> { + // If there is a top-level `OR` in an expression, the operands already are the choices and just need to + // be checked themselves for choices. + children.flatMapTo(mutableSetOf()) { it.validChoices() } + } } override fun offersChoice(): Boolean = From 1cf1bf389bfd9971ee6f95aaa33b8cde8c2c0764 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Wed, 18 Dec 2024 15:59:23 +0100 Subject: [PATCH 5/6] feat(spdx-utils): Make `simplify()` remove redundant choices Extract flattening to a function and reuse it to either flatten an only choice, or the original expression. Signed-off-by: Sebastian Schuberth --- utils/spdx/src/main/kotlin/SpdxExpression.kt | 10 +++++++-- .../test/kotlin/SpdxCompoundExpressionTest.kt | 21 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/utils/spdx/src/main/kotlin/SpdxExpression.kt b/utils/spdx/src/main/kotlin/SpdxExpression.kt index 45cc29e438a0f..36f78ac50b482 100644 --- a/utils/spdx/src/main/kotlin/SpdxExpression.kt +++ b/utils/spdx/src/main/kotlin/SpdxExpression.kt @@ -249,13 +249,13 @@ class SpdxCompoundExpression( override fun normalize(mapDeprecated: Boolean) = SpdxCompoundExpression(operator, children.map { it.normalize(mapDeprecated) }) - override fun simplify(): SpdxExpression { + private fun flatten(): SpdxExpression { val flattenedChildren = children.flatMapTo(mutableSetOf()) { child -> val simplifiedChild = child.simplify() if (simplifiedChild is SpdxCompoundExpression && simplifiedChild.operator == operator) { // Inline nested children of the same operator. - simplifiedChild.children.map { it.simplify() } + simplifiedChild.children.map { if (it is SpdxCompoundExpression) it.flatten() else it } } else { setOf(simplifiedChild) } @@ -264,6 +264,12 @@ class SpdxCompoundExpression( return flattenedChildren.singleOrNull() ?: SpdxCompoundExpression(operator, flattenedChildren) } + override fun simplify(): SpdxExpression = + // Eliminate redundant choices by creating the set of unique choices and using the choice if there is only one. + validChoices().singleOrNull()?.let { + if (it is SpdxCompoundExpression) it.flatten() else it + } ?: flatten() + override fun sorted(): SpdxExpression { /** * Get all transitive children of this expression that are concatenated with the same operator as this compound diff --git a/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt b/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt index fa6dc6f589579..2c5e76e162772 100644 --- a/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt +++ b/utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt @@ -87,5 +87,26 @@ class SpdxCompoundExpressionTest : WordSpec({ // Compare string representations to not rely on semantic equality. expression.simplify().toString() shouldBe "MIT" } + + "remove redundant choices with a nested expression at the beginning" { + val expression = "(Apache-2.0 OR MIT) AND MIT AND Apache-2.0 AND Apache-2.0 AND MIT".toSpdx() + + // Compare string representations to not rely on semantic equality. + expression.simplify().sorted().toString() shouldBe "Apache-2.0 AND MIT" + } + + "remove redundant choices with a nested expression in the middle" { + val expression = "Apache-2.0 AND (Apache-2.0 OR MIT) AND MIT AND MIT".toSpdx() + + // Compare string representations to not rely on semantic equality. + expression.simplify().sorted().toString() shouldBe "Apache-2.0 AND MIT" + } + + "remove redundant choices with a nested expression at the end" { + val expression = "MIT AND Apache-2.0 AND Apache-2.0 AND MIT AND (Apache-2.0 OR MIT)".toSpdx() + + // Compare string representations to not rely on semantic equality. + expression.simplify().sorted().toString() shouldBe "Apache-2.0 AND MIT" + } } }) From e33e1413d185c92b0d4b367f2c7e3eeb57f6f309 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Wed, 18 Dec 2024 17:45:19 +0100 Subject: [PATCH 6/6] fix(spdx-utils): Fix `offersChoice()` for equal `OR`-operands If there is a "choice" between "a" or "a", that is not really a choice. Signed-off-by: Sebastian Schuberth --- utils/spdx/src/main/kotlin/SpdxExpression.kt | 8 +------- utils/spdx/src/test/kotlin/SpdxExpressionChoiceTest.kt | 4 ++++ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/utils/spdx/src/main/kotlin/SpdxExpression.kt b/utils/spdx/src/main/kotlin/SpdxExpression.kt index 36f78ac50b482..9598150720ab3 100644 --- a/utils/spdx/src/main/kotlin/SpdxExpression.kt +++ b/utils/spdx/src/main/kotlin/SpdxExpression.kt @@ -164,7 +164,7 @@ sealed class SpdxExpression { * Return true if this expression offers a license choice. This can only be true if this expression contains the * [OR operator][SpdxOperator.OR]. */ - open fun offersChoice(): Boolean = false + fun offersChoice(): Boolean = validChoices().size > 1 /** * Apply a license [choice], optionally limited to the given [subExpression], and return an [SpdxExpression] where @@ -332,12 +332,6 @@ class SpdxCompoundExpression( } } - override fun offersChoice(): Boolean = - when (operator) { - SpdxOperator.OR -> true - SpdxOperator.AND -> children.any { it.offersChoice() } - } - override fun applyChoice(choice: SpdxExpression, subExpression: SpdxExpression): SpdxExpression { if (!subExpression.isSubExpression(choice)) { throw InvalidLicenseChoiceException( diff --git a/utils/spdx/src/test/kotlin/SpdxExpressionChoiceTest.kt b/utils/spdx/src/test/kotlin/SpdxExpressionChoiceTest.kt index 1eafc42e2ddc8..907d6a07745ea 100644 --- a/utils/spdx/src/test/kotlin/SpdxExpressionChoiceTest.kt +++ b/utils/spdx/src/test/kotlin/SpdxExpressionChoiceTest.kt @@ -286,6 +286,10 @@ class SpdxExpressionChoiceTest : WordSpec({ "a AND b".toSpdx().offersChoice() shouldBe false "a AND b AND c".toSpdx().offersChoice() shouldBe false } + + "return false if the expression contains the OR operator with equal operands" { + "a OR a".toSpdx().offersChoice() shouldBe false + } } "validChoices()" should {