Simplify redundant SPDX choices

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
@@ -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
@@ -300,6 +306,8 @@ class SpdxCompoundExpression(
     override fun validChoices(): Set<SpdxExpression> =
         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,13 +325,11 @@ class SpdxCompoundExpression(
                 }
             }
 
-            SpdxOperator.OR -> children.flatMapTo(mutableSetOf()) { it.validChoices() }
-        }
-
-    override fun offersChoice(): Boolean =
-        when (operator) {
-            SpdxOperator.OR -> true
-            SpdxOperator.AND -> children.any { it.offersChoice() }
+            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 applyChoice(choice: SpdxExpression, subExpression: SpdxExpression): SpdxExpression {

utils/spdx/src/test/kotlin/SpdxCompoundExpressionTest.kt

@@ -59,25 +59,54 @@ class SpdxCompoundExpressionTest : WordSpec({
             )
 
             // Compare string representations to not rely on semantic equality.
-            expression.simplify().toString() shouldBe SpdxCompoundExpression(
+            expression.simplify().sorted().toString() shouldBe "Apache-2.0 AND MIT"
+        }
+
+        "create a single expression for equal AND-operands" {
+            val expression = SpdxCompoundExpression(
                 SpdxOperator.AND,
                 listOf(
                     SpdxLicenseIdExpression("MIT"),
-                    SpdxLicenseIdExpression("Apache-2.0")
+                    SpdxLicenseIdExpression("MIT")
                 )
-            ).toString()
+            )
+
+            // Compare string representations to not rely on semantic equality.
+            expression.simplify().toString() shouldBe "MIT"
         }
 
-        "create a single expression for equal operands" {
+        "create a single expression for equal OR-operands" {
             val expression = SpdxCompoundExpression(
-                SpdxOperator.AND,
+                SpdxOperator.OR,
                 listOf(
                     SpdxLicenseIdExpression("MIT"),
                     SpdxLicenseIdExpression("MIT")
                 )
             )
 
-            expression.simplify() shouldBe SpdxLicenseIdExpression("MIT")
+            // 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"
         }
     }
 })

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 {