🚑 Add unit tests for sorting corner cases, and fix code to match

pmonks · Dec 9, 2024 · 6ea4c16 · 6ea4c16
1 parent 863722e
commit 6ea4c16
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 4 deletions.
diff --git a/src/spdx/expressions.clj b/src/spdx/expressions.clj
@@ -240,12 +240,28 @@
   "Compares two license maps, as found in a parse tree."
   [x y]
   (cond
-    ; License-ids first
-    (and (:license-id x) (:license-id y))   (compare (license-map->sortable-string x) (license-map->sortable-string y))
+    ; license-ids first
+    (and (:license-id x) (:license-id y))   (if (and (= (s/lower-case (:license-id x)) (s/lower-case (:license-id y)))
+                                                     (= (:or-later? x) (:or-later? y)))
+                                              (cond
+                                                ; exception-ids first
+                                                (and (:license-exception-id x) (:addition-ref         y)) -1
+                                                ; then AdditionRefs
+                                                (and (:addition-ref         x) (:license-exception-id y)) 1
+                                                :else                          (compare (license-map->sortable-string x) (license-map->sortable-string y)))
+                                              (compare (license-map->sortable-string x) (license-map->sortable-string y)))
     (:license-id x)                         -1
     (:license-id y)                         1
     ; then LicenseRefs
-    (and (:license-ref x) (:license-ref y)) (compare (license-map->sortable-string x) (license-map->sortable-string y))
+    (and (:license-ref x) (:license-ref y)) (if (and (= (:license-ref x)  (:license-ref y))
+                                                     (= (:document-ref x) (:document-ref y)))
+                                              (cond
+                                                ; exception-ids first
+                                                (and (:license-exception-id x) (:addition-ref         y)) -1
+                                                ; then AdditionRefs
+                                                (and (:addition-ref         x) (:license-exception-id y)) 1
+                                                :else                          (compare (license-map->sortable-string x) (license-map->sortable-string y)))
+                                              (compare (license-map->sortable-string x) (license-map->sortable-string y)))
     (:license-ref x)                        -1
     (:license-ref y)                        1
     :else                                   1))

diff --git a/test/spdx/expressions_test.clj b/test/spdx/expressions_test.clj
@@ -270,7 +270,11 @@
     (is (= (parse "(GPL-2.0-only OR Apache-2.0) AND LicenseRef-foo")                                                                                      ; Sub-clauses after LicenseRefs
                                                               [:and {:license-ref "foo"} [:or {:license-id "Apache-2.0"} {:license-id "GPL-2.0-only"}]]))
     (is (= (parse "MIT WITH AdditionRef-foo AND MIT WITH AdditionRef-FOO")                                                                                ; Case *in*sensitive license id sorting, with case sensitive AdditionRef sorting
-                                                              [:and {:license-id "MIT" :addition-ref "FOO"} {:license-id "MIT" :addition-ref "foo"}]))))
+                                                              [:and {:license-id "MIT" :addition-ref "FOO"} {:license-id "MIT" :addition-ref "foo"}]))
+    (is (= (parse "GPL-2.0-only WITH AdditionRef-foo AND GPL-2.0-only WITH Classpath-exception-2.0")                                                      ; AdditionRefs sort after license exceptions
+                                                              [:and {:license-id "GPL-2.0-only" :license-exception-id "Classpath-exception-2.0"} {:license-id "GPL-2.0-only" :addition-ref "foo"}]))
+    (is (= (parse "LicenseRef-foo WITH AdditionRef-foo AND LicenseRef-foo WITH Classpath-exception-2.0")                                                  ; AdditionRefs sort after license exceptions (LicenseRef variant)
+                                                              [:and {:license-ref "foo" :license-exception-id "Classpath-exception-2.0"} {:license-ref "foo" :addition-ref "foo"}]))))
 
 (deftest unnormalised-parse-tests
   (testing "Simple expressions - normalisation"