Streamline license normalization #1403

* Clarify the cascade of conditions for readability.
* Do not deduplicate detections (do not use a set)
* Format code

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>

Author: pombredanne

src/packagedcode/maven.py

@@ -150,61 +150,80 @@ def compute_normalized_license(self):
         return compute_normalized_license(self.declared_license)
 
 
-def compute_normalized_license(listed_license_dictionary):
+def compute_normalized_license(declared_license):
     """
     Return a detected license expression from a declared license mapping.
     """
-    if listed_license_dictionary:
-        licensing = Licensing()
-        # Use set instead of list to avoid duplication.
-        detected_licenses = set()
-        for license_declaration in listed_license_dictionary:
-            name = license_declaration.get('name')
-            url = license_declaration.get('url')
-            comments = license_declaration.get('comments')
-            # 1. try detection on the value of name if not empty and keep this
-            # 2. try detection on the value of url  if not empty and keep this
-            # 3. try detection on the value of comment  if not empty and keep this
-            # 4. if the three detection are for the same license, this becomes the kept license for that one licenses item
-            # 5. if not, the name should have precedence and any unknowns
+    if not declared_license:
+        return
+
+    licensing = Licensing()
+
+    detected_licenses = []
+
+    for license_declaration in declared_license:
+        # 1. try detection on the value of name if not empty and keep this
+        name = license_declaration.get('name')
+        via_name = models.compute_normalized_license(name)
+
+        # 2. try detection on the value of url  if not empty and keep this
+        url = license_declaration.get('url')
+        via_url = models.compute_normalized_license(url)
+
+        # 3. try detection on the value of comment  if not empty and keep this
+        comments = license_declaration.get('comments')
+        via_comments = models.compute_normalized_license(comments)
+
+
+        if via_name:
+            # The name should have precedence and any unknowns
             # in url and comment should be ignored.
-            via_name = models.compute_normalized_license(name)
-            via_url = models.compute_normalized_license(url)
-            via_comments = models.compute_normalized_license(comments)
-
-            if via_name:
-                # The name should have precedence and any unknowns
-                # in url and comment should be ignored.
-                if via_url == 'unknown':
-                    via_url = None
-                if via_comments == 'unknown':
-                    via_comments = None
-
-            if via_name and ((via_name == via_url and via_comments == via_url) or (via_name == via_url and not via_comments) or (via_name == via_comments and not via_url)):
-                # if three detection are the same and not empty, return the value
-                # or one of url or comments is empty and the non-empty one equals to the name value
-                detected_licenses.add(via_name)
+            if via_url == 'unknown':
+                via_url = None
+            if via_comments == 'unknown':
+                via_comments = None
+
+        # Check the three detections to decide which license to keep
+        name_and_url = via_name == via_url
+        name_and_comment = via_name == via_comments
+        all_same = name_and_url and name_and_comment
+
+        if via_name:
+            if all_same:
+                detected_licenses.append(via_name)
+
+            # name and (url or comment) are same
+            elif name_and_url and not via_comments:
+                detected_licenses.append(via_name)
+            elif name_and_comment and not via_url:
+                detected_licenses.append(via_name)
+
             else:
-                # Form a list and the element does not contain any None value, since the None value means 'unknown' or real empty value from above assignment.
-                detected_items = [item for item in (via_name, via_url, via_comments) if item]
-                if detected_items:
-                    if len(detected_items) == 1:
-                        detected_licenses.add(detected_items[0])
+                # we have some non-unknown license detected in url or comment
+                detections = via_name, via_url, via_comments
+                detections = [l for l in detections if l]
+                if detections:
+                    if len(detections) == 1:
+                        combined_expression = detections[0]
                     else:
-                        # Combine if name, url and comments are different licenses
-                        licensing = Licensing()
-                        total_license_expression = [licensing.parse(detected_item, simple=True) for detected_item in detected_items]
-                        combined_expression_object = licensing.AND(*total_license_expression)
-                        detected_licenses.add(str(combined_expression_object))
-        if detected_licenses:
-            if len(detected_licenses) == 1:
-                return str(detected_licenses.pop())
-            else:
-                # Combine if pom contains more than 1 licenses declarations.
-                licensing = Licensing()
-                total_license_expression = [licensing.parse(detected_license, simple=True) for detected_license in detected_licenses]
-                combined_expression_object = licensing.AND(*total_license_expression)
-                return str(combined_expression_object)
+                        expressions = [
+                            licensing.parse(le, simple=True) for le in detections]
+                        combined_expression = str(licensing.AND(*expressions))
+                    detected_licenses.append(combined_expression)
+
+        elif via_url:
+            detected_licenses.append(via_url)
+        elif via_comments:
+            detected_licenses.append(via_comments)
+
+    if len(detected_licenses) == 1:
+        return detected_licenses[0]
+
+    if detected_licenses:
+        # Combine if pom contains more than one licenses declarations.
+        expressions = [licensing.parse(le, simple=True) for le in detected_licenses]
+        combined_expression = licensing.AND(*expressions)
+        return str(combined_expression)
 
 
 def build_url(group_id, artifact_id, version, filename, baseurl='http://repo1.maven.org/maven2'):

tests/packagedcode/data/maven2/aopalliance-1.0.pom.package.json

@@ -21,7 +21,7 @@
   "code_view_url": null, 
   "vcs_url": null, 
   "copyright": null, 
-  "license_expression": "gpl-1.0-plus AND public-domain", 
+  "license_expression": "public-domain AND gpl-1.0-plus", 
   "declared_license": [
     {
       "name": "Public Domain", 

tests/packagedcode/test_maven.py

@@ -222,7 +222,7 @@ def test_package_root_is_properly_returned_for_metainf_poms(self):
         resource_attributes = dict(packages=attr.ib(default=attr.Factory(list), repr=False))
 
         codebase = Codebase(test_dir, resource_attributes=resource_attributes)
-        manifest_resource = [r for r in codebase.walk() if r.name=='pom.xml'][0]
+        manifest_resource = [r for r in codebase.walk() if r.name == 'pom.xml'][0]
         package = maven.MavenPomPackage.recognize(manifest_resource.location)
         manifest_resource.packages.append(package.to_dict())
         manifest_resource.save(codebase)
@@ -359,82 +359,92 @@ class TestMavenComputeNormalizedLicense(testcase.FileBasedTesting):
     test_data_dir = os.path.join(os.path.dirname(__file__), 'data')
 
     def test_compute_normalized_license_two_names_only(self):
-        declared_license = [{'name': 'apache-2.0'},
-                                   {'name': 'mit'}
-                                   ]
+        declared_license = [
+            {'name': 'apache-2.0'},
+            {'name': 'mit'}
+        ]
         result = maven.compute_normalized_license(declared_license)
         expected = 'apache-2.0 AND mit'
         assert expected == result
 
     def test_compute_normalized_license_tree_nodes(self):
-        declared_license = [{'name': 'apache-2.0'},
-                                   {'name': 'mit'}
-                                   ]
+        declared_license = [
+            {'name': 'apache-2.0'},
+            {'name': 'mit'}
+        ]
         result = maven.compute_normalized_license(declared_license)
         expected = 'apache-2.0 AND mit'
         assert expected == result
-        
+
     def test_compute_normalized_license_with_unknown_url(self):
-        declared_license = [{'name': 'apache-2.0', 'url': 'unknown'},
-                                   {'name': 'mit'}
-                                   ]
+        declared_license = [
+            {'name': 'apache-2.0', 'url': 'unknown'},
+            {'name': 'mit'}
+        ]
         result = maven.compute_normalized_license(declared_license)
         expected = 'apache-2.0 AND mit'
         assert expected == result
 
     def test_compute_normalized_license_with_unknown_url_known_comments(self):
-        declared_license = [{'name': 'apache-2.0', 'url': 'unknown', 'comments': 'apache-2.0'},
-                                   {'name': 'mit'}
-                                   ]
+        declared_license = [
+            {'name': 'apache-2.0', 'url': 'unknown', 'comments': 'apache-2.0'},
+            {'name': 'mit'}
+        ]
         result = maven.compute_normalized_license(declared_license)
         expected = 'apache-2.0 AND mit'
         assert expected == result
 
     def test_compute_normalized_license_with_unknown_url_unknown_comments(self):
-        declared_license = [{'name': 'apache-2.0', 'url': 'unknown', 'comments': 'unknown'},
-                                   {'name': 'mit'}
-                                   ]
+        declared_license = [
+            {'name': 'apache-2.0', 'url': 'unknown', 'comments': 'unknown'},
+            {'name': 'mit'}
+        ]
         result = maven.compute_normalized_license(declared_license)
         expected = 'apache-2.0 AND mit'
         assert expected == result
-        
+
     def test_compute_normalized_license_unknown_name(self):
-        declared_license = [{'name': 'unknown', 'url': 'apache-2.0'},
-                                   {'name': 'mit'}
-                                   ]
+        declared_license = [
+            {'name': 'unknown', 'url': 'apache-2.0'},
+            {'name': 'mit'}
+        ]
         result = maven.compute_normalized_license(declared_license)
         expected = '(unknown AND apache-2.0) AND mit'
         assert expected == result
 
     def test_compute_normalized_license_same_name_and_url(self):
-        declared_license = [{'name': 'apache-2.0', 'url': 'apache-2.0'},
-                                   {'name': 'mit'}
-                                   ]
+        declared_license = [
+            {'name': 'apache-2.0', 'url': 'apache-2.0'},
+            {'name': 'mit'}
+        ]
         result = maven.compute_normalized_license(declared_license)
         expected = 'apache-2.0 AND mit'
         assert expected == result
 
     def test_compute_normalized_license_same_name_url_comments(self):
-        declared_license = [{'name': 'apache-2.0', 'url': 'apache-2.0', 'comments': 'apache-2.0'},
-                                   {'name': 'mit'}
-                                   ]
+        declared_license = [
+            {'name': 'apache-2.0', 'url': 'apache-2.0', 'comments': 'apache-2.0'},
+            {'name': 'mit'}
+        ]
         result = maven.compute_normalized_license(declared_license)
         expected = 'apache-2.0 AND mit'
         assert expected == result
 
     def test_compute_normalized_license_with_url_invalid(self):
-        declared_license = [{'name': 'MIT', 'url': 'LICENSE.txt'},
-                                   ]
+        declared_license = [
+            {'name': 'MIT', 'url': 'LICENSE.txt'},
+        ]
         result = maven.compute_normalized_license(declared_license)
         expected = 'mit'
         assert expected == result
 
     def test_compute_normalized_license_with_duplicated_license(self):
-        declared_license = [{'name': 'LGPL'},
-                                   {'name': 'GNU Lesser General Public License', 'url': 'http://www.gnu.org/licenses/lgpl.html'},
-                                   ]
+        declared_license = [
+            {'name': 'LGPL'},
+            {'name': 'GNU Lesser General Public License', 'url': 'http://www.gnu.org/licenses/lgpl.html'},
+        ]
         result = maven.compute_normalized_license(declared_license)
-        expected = 'lgpl-2.0-plus'
+        expected = 'lgpl-2.0-plus AND lgpl-2.0-plus'
         assert expected == result