From 6ad4d0aacc3116513ff2231929a5c5538527a3f2 Mon Sep 17 00:00:00 2001
From: Sebastian Schuberth <sebastian@doubleopen.org>
Date: Thu, 11 Jan 2024 22:25:58 +0100
Subject: [PATCH] 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.

Reword affected tests according to the fact that returned choices are
not simplified, but explicit, though still deduplicated. Commonly
introduce a `choice` variable along the way to make tests for long
license expressions more readable.

Fixes #8082.

Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>
---
 utils/spdx/src/main/kotlin/SpdxExpression.kt  | 13 +++++++-
 .../src/test/kotlin/SpdxExpressionTest.kt     | 33 ++++++++++++-------
 2 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/utils/spdx/src/main/kotlin/SpdxExpression.kt b/utils/spdx/src/main/kotlin/SpdxExpression.kt
index 5bd9e00af9067..96bb6eb6c342e 100644
--- 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
index e29096c3500e1..2df9268f0b3a3 100644
--- a/utils/spdx/src/test/kotlin/SpdxExpressionTest.kt
+++ b/utils/spdx/src/test/kotlin/SpdxExpressionTest.kt
@@ -422,8 +422,10 @@ class SpdxExpressionTest : WordSpec({
     }
 
     "validChoices()" should {
-        "list the valid choices for a complex expression" {
-            "(a OR b) AND c AND (d OR e)".toSpdx().validChoices().map { it.toString() } should containExactlyInAnyOrder(
+        "return the valid choices for a complex expression" {
+            val choices = "(a OR b) AND c AND (d OR e)".toSpdx().validChoices()
+
+            choices.map { it.toString() } should containExactlyInAnyOrder(
                 "a AND c AND d",
                 "a AND c AND e",
                 "b AND c AND d",
@@ -431,20 +433,27 @@ class SpdxExpressionTest : WordSpec({
             )
         }
 
-        "not contain a duplicate valid choice for a simple expression" {
-            "a AND a".toSpdx().validChoices().map { it.toString() } should containExactly("a")
-        }
+        "return the valid choices for a nested expression" {
+            val choices = "(a OR (b AND (c OR d))) AND (e OR f)".toSpdx().validChoices()
 
-        "not contain duplicate valid choice for a complex expression" {
-            "(a OR b) AND (a OR b)".toSpdx().validChoices().map { it.toString() } should containExactlyInAnyOrder(
-                "a",
-                "b",
-                "a AND b"
+            choices.map { it.toString() } should containExactlyInAnyOrder(
+                "a AND e",
+                "a AND f",
+                "b AND c AND e",
+                "b AND c AND f",
+                "b AND d AND e",
+                "b AND d AND f"
             )
         }
 
-        "not contain duplicate valid choice different left and right expressions" {
-            "a AND a AND b".toSpdx().validChoices().map { it.toString() } should containExactly("a AND b")
+        "be explicit about the choices even if they could be simplified" {
+            val choices = "(a OR b) AND (a OR b)".toSpdx().validChoices()
+
+            choices.map { it.toString() } should containExactlyInAnyOrder(
+                "a AND a",
+                "a AND b",
+                "b AND b"
+            )
         }
     }