fix(spdx-utils): Correctly determine choices for AND expressions

The original code was clearly wrong in that it recursively decomposed an expression and composed the contained expressions always with `AND` although there could be nested `OR` expressions. Instead, if there is a top-level `AND` in an expressions, create all combinations of choices on the left and choices on the right to get the total valid choices. Fixes #8082. Signed-off-by: Sebastian Schuberth <[email protected]>
oss-review-toolkit · Jan 12, 2024 · 541a9a8 · 541a9a8
1 parent 6b1aa3c
commit 541a9a8
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 1 deletion.
diff --git a/utils/spdx/src/main/kotlin/SpdxExpression.kt b/utils/spdx/src/main/kotlin/SpdxExpression.kt
@@ -314,7 +314,18 @@ class SpdxCompoundExpression(
 
     override fun validChoicesForDnf(): Set<SpdxExpression> =
         when (operator) {
-            SpdxOperator.AND -> setOf(decompose().reduce(SpdxExpression::and))
+            SpdxOperator.AND -> {
+                val leftChoices = left.validChoicesForDnf()
+                val rightChoices = right.validChoicesForDnf()
+
+                // Cartesian product of choices on the left and right.
+                leftChoices.flatMapTo(mutableSetOf()) { leftChoice ->
+                    rightChoices.map { rightChoice ->
+                        leftChoice and rightChoice
+                    }
+                }
+            }
+
             SpdxOperator.OR -> left.validChoicesForDnf() + right.validChoicesForDnf()
         }
 

diff --git a/utils/spdx/src/test/kotlin/SpdxExpressionTest.kt b/utils/spdx/src/test/kotlin/SpdxExpressionTest.kt
@@ -446,6 +446,26 @@ class SpdxExpressionTest : WordSpec({
         "not contain duplicate valid choice different left and right expressions" {
             "a AND a AND b".toSpdx().validChoices() should containExactly("a AND b".toSpdx())
         }
+
+        "work for nested AND expressions" {
+            val license = "(MIT OR GPL-2.0-only) AND (MIT OR BSD-3-Clause OR GPL-1.0-or-later) AND " +
+                "(MIT OR BSD-3-Clause OR GPL-2.0-only)"
+
+            val choices = license.toSpdx().validChoices().map { it.toString() }
+
+            choices should containExactlyInAnyOrder(
+                "MIT AND MIT AND MIT",
+                "MIT AND MIT AND BSD-3-Clause",
+                "MIT AND MIT AND GPL-2.0-only",
+                "MIT AND BSD-3-Clause AND GPL-2.0-only",
+                "MIT AND GPL-1.0-or-later AND MIT",
+                "MIT AND GPL-1.0-or-later AND BSD-3-Clause",
+                "MIT AND GPL-1.0-or-later AND GPL-2.0-only",
+                "GPL-2.0-only AND BSD-3-Clause AND BSD-3-Clause",
+                "GPL-2.0-only AND GPL-1.0-or-later AND BSD-3-Clause",
+                "GPL-2.0-only AND GPL-1.0-or-later AND GPL-2.0-only"
+            )
+        }
     }
 
     "offersChoice()" should {