refactor(spdx-utils): Remove disjunctiveNormalForm()

Remove all DNF-related code as it is not actually necessary in order to determine the valid license choices anymore as of the recent changes to `validChoicesForDnf()`. Signed-off-by: Sebastian Schuberth <[email protected]>
oss-review-toolkit · Jan 18, 2024 · 3c795a1 · 3c795a1
1 parent 1e22bc4
commit 3c795a1
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 70 deletions.
diff --git a/utils/spdx/src/main/kotlin/SpdxExpression.kt b/utils/spdx/src/main/kotlin/SpdxExpression.kt
@@ -122,13 +122,6 @@ sealed class SpdxExpression {
             simpleExpression.toString()
         }
 
-    /**
-     * Return the [disjunctive normal form][1] of this expression.
-     *
-     * [1]: https://en.wikipedia.org/wiki/Disjunctive_normal_form
-     */
-    open fun disjunctiveNormalForm(): SpdxExpression = this
-
     /**
      * Normalize all license IDs using a mapping containing common misspellings of license IDs. If [mapDeprecated] is
      * `true`, also deprecated IDs are mapped to their current counterparts. The result of this function is not
@@ -151,15 +144,9 @@ sealed class SpdxExpression {
     abstract fun validate(strictness: Strictness)
 
     /**
-     * Return all valid license choices for this SPDX expression, by converting it to the
-     * [disjunctive normal form][disjunctiveNormalForm] and collecting all disjunct expressions.
-     */
-    fun validChoices(): Set<SpdxExpression> = disjunctiveNormalForm().validChoicesForDnf()
-
-    /**
-     * Internal implementation of [validChoices], assuming that this expression is already in disjunctive normal form.
+     * Return all valid license choices for this SPDX expression by collecting all disjunct expressions.
      */
-    internal open fun validChoicesForDnf(): Set<SpdxExpression> = setOf(this)
+    open fun validChoices(): Set<SpdxExpression> = setOf(this)
 
     /**
      * Return whether this expression contains [present][SpdxConstants.isPresent] licenses, i.e. not all licenses in
@@ -250,30 +237,6 @@ class SpdxCompoundExpression(
 ) : SpdxExpression() {
     override fun decompose() = left.decompose() + right.decompose()
 
-    override fun disjunctiveNormalForm(): SpdxExpression {
-        val leftDnf = left.disjunctiveNormalForm()
-        val rightDnf = right.disjunctiveNormalForm()
-
-        return when (operator) {
-            SpdxOperator.OR -> SpdxCompoundExpression(leftDnf, SpdxOperator.OR, rightDnf)
-
-            SpdxOperator.AND -> when {
-                leftDnf is SpdxCompoundExpression && leftDnf.operator == SpdxOperator.OR &&
-                    rightDnf is SpdxCompoundExpression && rightDnf.operator == SpdxOperator.OR ->
-                    ((leftDnf.left and rightDnf.left) or (leftDnf.left and rightDnf.right)) or
-                        ((leftDnf.right and rightDnf.left) or (leftDnf.right and rightDnf.right))
-
-                leftDnf is SpdxCompoundExpression && leftDnf.operator == SpdxOperator.OR ->
-                    (leftDnf.left and rightDnf) or (leftDnf.right and rightDnf)
-
-                rightDnf is SpdxCompoundExpression && rightDnf.operator == SpdxOperator.OR ->
-                    (leftDnf and rightDnf.left) or (leftDnf and rightDnf.right)
-
-                else -> SpdxCompoundExpression(leftDnf, operator, rightDnf)
-            }
-        }
-    }
-
     override fun normalize(mapDeprecated: Boolean) =
         SpdxCompoundExpression(left.normalize(mapDeprecated), operator, right.normalize(mapDeprecated))
 
@@ -312,11 +275,11 @@ class SpdxCompoundExpression(
         right.validate(strictness)
     }
 
-    override fun validChoicesForDnf(): Set<SpdxExpression> =
+    override fun validChoices(): Set<SpdxExpression> =
         when (operator) {
             SpdxOperator.AND -> {
-                val leftChoices = left.validChoicesForDnf()
-                val rightChoices = right.validChoicesForDnf()
+                val leftChoices = left.validChoices()
+                val rightChoices = right.validChoices()
 
                 // Cartesian product of choices on the left and right.
                 leftChoices.flatMapTo(mutableSetOf()) { leftChoice ->
@@ -326,7 +289,7 @@ class SpdxCompoundExpression(
                 }
             }
 
-            SpdxOperator.OR -> left.validChoicesForDnf() + right.validChoicesForDnf()
+            SpdxOperator.OR -> left.validChoices() + right.validChoices()
         }
 
     override fun offersChoice(): Boolean =

diff --git a/utils/spdx/src/test/kotlin/SpdxExpressionTest.kt b/utils/spdx/src/test/kotlin/SpdxExpressionTest.kt
@@ -379,30 +379,6 @@ class SpdxExpressionTest : WordSpec({
         }
     }
 
-    "disjunctiveNormalForm()" should {
-        "not change an expression already in DNF" {
-            "a AND b OR c AND d".toSpdx().disjunctiveNormalForm() should beString("(a AND b) OR (c AND d)")
-        }
-
-        "correctly convert an OR on the left side of an AND expression" {
-            "(a OR b) AND c".toSpdx().disjunctiveNormalForm() should beString("(a AND c) OR (b AND c)")
-        }
-
-        "correctly convert an OR on the right side of an AND expression" {
-            "a AND (b OR c)".toSpdx().disjunctiveNormalForm() should beString("(a AND b) OR (a AND c)")
-        }
-
-        "correctly convert ORs on both sides of an AND expression" {
-            "(a OR b) AND (c OR d)".toSpdx().disjunctiveNormalForm() should
-                beString("(a AND c) OR (a AND d) OR (b AND c) OR (b AND d)")
-        }
-
-        "correctly convert a complex expression" {
-            "(a OR b) AND c AND (d OR e)".toSpdx().disjunctiveNormalForm() should
-                beString("(a AND c AND d) OR (a AND c AND e) OR (b AND c AND d) OR (b AND c AND e)")
-        }
-    }
-
     "sort()" should {
         "not change already sorted expressions" {
             "a AND b".toSpdx().sorted() should beString("a AND b")
@@ -555,7 +531,7 @@ class SpdxExpressionTest : WordSpec({
             shouldThrow<InvalidLicenseChoiceException> { expression.applyChoice(choice) }
         }
 
-        "apply the choice if the expression is not in DNF" {
+        "apply the choice even if not literally contained in the expression" {
             val expression = "(a OR b) AND c".toSpdx()
             val choice = "a AND c".toSpdx()
 
@@ -564,7 +540,7 @@ class SpdxExpressionTest : WordSpec({
             result shouldBe "a AND c".toSpdx()
         }
 
-        "return the reduced subExpression in DNF if the choice was valid" {
+        "return the reduced subExpression if the choice was valid" {
             val expression = "(a OR b) AND c AND (d OR e)".toSpdx()
             val choice = "a AND c AND d".toSpdx()
             val subExpression = "a AND c AND d OR a AND c AND e".toSpdx()
@@ -590,7 +566,7 @@ class SpdxExpressionTest : WordSpec({
             shouldThrow<InvalidSubExpressionException> { expression.applyChoice(choice, subExpression) }
         }
 
-        "throw an exception if the subExpression does not match and needs to be converted to a DNF" {
+        "throw an exception if the subExpression does not match" {
             val expression = "(a OR b) AND c AND (d OR e)".toSpdx()
             val choice = "a AND c AND d".toSpdx()
             val subExpression = "(a AND c AND d) OR (x AND y AND z)".toSpdx()