Address feedback and suggestions

Reference: #3620

Signed-off-by: Ayan Sinha Mahapatra <[email protected]>

Author: AyanSinhaMahapatra

src/licensedcode/detection.py

@@ -165,14 +165,15 @@ class LicenseDetection:
     license_expression = attr.ib(
         default=None,
         metadata=dict(
-            help='Full license expression string '
-            'using the SPDX license expression syntax and ScanCode license keys.')
+            help='A license expression string using the SPDX license expression'
+            ' syntax and ScanCode license keys, the effective license expression'
+            ' for this license detection.')
     )
 
     license_expression_spdx = attr.ib(
         default=None,
         metadata=dict(
-            help='Full license expression string with SPDX license keys.')
+            help='SPDX license expression string with SPDX ids.')
     )
 
     matches = attr.ib(
@@ -955,7 +956,7 @@ def dict_fields(attr, value):
             return True
 
         detection_mapping = attr.asdict(self, filter=dict_fields)
-        detection_mapping["sample_matches"] = [
+        detection_mapping["reference_matches"] = [
             match.to_dict(
                 include_text=include_text,
                 license_text_diagnostics=license_text_diagnostics,

src/licensedcode/licenses_reference.py

@@ -97,6 +97,7 @@ def collect_license_and_rule_references(codebase, include_packages=True, include
         if TRACE:
             logger_debug(f'collect_references_from_packages: license keys: {pks}')
             logger_debug(f'collect_references_from_packages: rules by id: {prules}')
+        license_keys.update(pks)
         rules_by_identifier.update(prules)
 
     if include_files:

src/licensedcode/models.py

@@ -2128,6 +2128,8 @@ def from_file(cls, rule_file, is_builtin=True):
 
     @property
     def pysafe_expression(self):
+        """
+        Return a python safe identifier, for use in rule identifiers"""
         return python_safe_name(self.license_expression)
 
     def load_data(self, rule_file):

src/packagedcode/debian_copyright.py

@@ -1259,7 +1259,6 @@ def parse_paras_with_license_text(paras_with_license):
                     matches=text_matches, reference_match=common_license_match
                 ):
                     # TODO: Add unknown matches if matches are not consistent
-                    # raise Exception(f'Inconsistent Licenses: {common_license_match} {matches}')
                     pass
 
                 # TODO: Add unknown matches if matches are weak

src/packagedcode/models.py

@@ -1336,7 +1336,6 @@ def assemble_from_many_datafiles_in_directory(
                         package_data = PackageData.from_dict(package_data)
                         pkgdata_resources.append((package_data, resource,))
 
-        #raise Exception(pkgdata_resources)
         if pkgdata_resources:
             if TRACE:
                 logger_debug(f' assemble_from_many_datafiles: pkgdata_resources: {pkgdata_resources!r}')
@@ -1499,6 +1498,11 @@ def from_package_data(cls, package_data, datafile_path):
         package_data_mapping['datafile_paths'] = [datafile_path]
         package_data_mapping['datasource_ids'] = [dsid]
 
+        license_detections = package_data_mapping['license_detections']
+        for detection in license_detections:
+            for match in detection['matches']:
+                match['from_file'] = datafile_path
+
         return cls.from_dict(package_data_mapping)
 
     @classmethod

src/packagedcode/plugin_package.py

@@ -38,6 +38,8 @@
 from packagedcode.models import PackageWithResources
 
 TRACE = os.environ.get('SCANCODE_DEBUG_PACKAGE_API', False)
+TRACE_ASSEMBLY = os.environ.get('SCANCODE_DEBUG_PACKAGE_ASSEMBLY', False)
+TRACE_LICENSE = os.environ.get('SCANCODE_DEBUG_PACKAGE_LICENSE', False)
 
 
 def logger_debug(*args):
@@ -46,7 +48,7 @@ def logger_debug(*args):
 
 logger = logging.getLogger(__name__)
 
-if TRACE:
+if TRACE or TRACE_LICENSE or TRACE_ASSEMBLY:
     import sys
 
     logging.basicConfig(stream=sys.stdout)
@@ -207,7 +209,7 @@ def process_codebase(self, codebase, strip_root=False, license_text=False, licen
             # If we don't detect license in package_data but there is license detected in file
             # we add the license expression from the file to a package
             modified = add_license_from_file(resource, codebase)
-            if TRACE and modified:
+            if TRACE_LICENSE and modified:
                 logger_debug(f'packagedcode: process_codebase: add_license_from_file: modified: {modified}')
 
             if codebase.has_single_resource:
@@ -216,25 +218,26 @@ def process_codebase(self, codebase, strip_root=False, license_text=False, licen
             # If there is referenced files in a extracted license statement, we follow
             # the references, look for license detections and add them back
             modified = list(add_referenced_license_matches_for_package(resource, codebase))
-            if TRACE and modified:
+            if TRACE_LICENSE and modified:
                 logger_debug(f'packagedcode: process_codebase: add_referenced_license_matches_for_package: modified: {modified}')
 
             # If there is a LICENSE file on the same level as the manifest, and no license
             # is detected in the package_data, we add the license from the file
             modified = add_license_from_sibling_file(resource, codebase)
-            if TRACE and modified:
+            if TRACE_LICENSE and modified:
                 logger_debug(f'packagedcode: process_codebase: add_license_from_sibling_file: modified: {modified}')
 
         # Create codebase-level packages and dependencies
         create_package_and_deps(codebase, strip_root=strip_root, **kwargs)
+        #raise Exception()
 
         if has_licenses:
             # This step is dependent on top level packages
             for resource in codebase.walk(topdown=False):
                 # If there is a unknown reference to a package we add the license
                 # from the package license detection
                 modified = list(add_referenced_license_detection_from_package(resource, codebase))
-                if TRACE and modified:
+                if TRACE_LICENSE and modified:
                     logger_debug(f'packagedcode: process_codebase: add_referenced_license_matches_from_package: modified: {modified}')
 
 
@@ -244,15 +247,15 @@ def add_license_from_file(resource, codebase):
     and the file has license detections, and if so, populate the package_data license
     expression and detection fields from the file license.
     """
-    if TRACE:
+    if TRACE_LICENSE:
         logger_debug(f'packagedcode.plugin_package: add_license_from_file: resource: {resource.path}')
 
     if not resource.is_file:
         return
 
     license_detections_file = resource.license_detections
 
-    if TRACE:
+    if TRACE_LICENSE:
         logger_debug(f'add_license_from_file: license_detections_file: {license_detections_file}')
     if not license_detections_file:
         return
@@ -263,7 +266,7 @@ def add_license_from_file(resource, codebase):
 
     for pkg in package_data:
         license_detections_pkg = pkg["license_detections"]
-        if TRACE:
+        if TRACE_LICENSE:
             logger_debug(f'add_license_from_file: license_detections_pkg: {license_detections_pkg}')
 
         if not license_detections_pkg:
@@ -329,16 +332,7 @@ def create_package_and_deps(codebase, package_adder=add_to_package, strip_root=F
         **kwargs
     )
 
-    package_mappings = []
-    for package in packages:
-        for detection in package.license_detections:
-            populate_matches_with_path(
-                matches=detection["matches"],
-                path=package.datafile_paths[0],
-            )
-        package_mappings.append(package.to_dict())
-
-    codebase.attributes.packages.extend(package_mappings)
+    codebase.attributes.packages.extend(package.to_dict() for package in packages)
     codebase.attributes.dependencies.extend(dep.to_dict() for dep in dependencies)
 
 
@@ -361,20 +355,20 @@ def get_package_and_deps(codebase, package_adder=add_to_package, strip_root=Fals
         if resource.path in seen_resource_paths:
             continue
 
-        if TRACE:
+        if TRACE_ASSEMBLY:
             logger_debug('get_package_and_deps: location:', resource.location)
 
         for package_data in resource.package_data:
             try:
                 package_data = PackageData.from_dict(mapping=package_data)
 
-                if TRACE:
+                if TRACE_ASSEMBLY:
                     logger_debug('  get_package_and_deps: package_data:', package_data)
 
                 # Find a handler for this package datasource to assemble collect
                 # packages and deps
                 handler = get_package_handler(package_data)
-                if TRACE:
+                if TRACE_ASSEMBLY:
                     logger_debug('  get_package_and_deps: handler:', handler)
 
                 items = handler.assemble(
@@ -385,7 +379,7 @@ def get_package_and_deps(codebase, package_adder=add_to_package, strip_root=Fals
                 )
 
                 for item in items:
-                    if TRACE:
+                    if TRACE_ASSEMBLY:
                         logger_debug('    get_package_and_deps: item:', item)
 
                     if isinstance(item, Package):
@@ -404,7 +398,7 @@ def get_package_and_deps(codebase, package_adder=add_to_package, strip_root=Fals
                     elif isinstance(item, Resource):
                         seen_resource_paths.add(item.path)
 
-                        if TRACE:
+                        if TRACE_ASSEMBLY:
                             logger_debug(
                                 '    get_package_and_deps: seen_resource_path:',
                                 seen_resource_paths,

src/summarycode/todo.py

@@ -105,7 +105,7 @@ def process_codebase(self, codebase, **kwargs):
             license_text_diagnostics = kwargs.get("license_text_diagnostics")
             if not license_diagnostics or not license_text or not license_text_diagnostics:
                 usage_suggestion_message = (
-                    "The --review option, whe paired with --license option should be used with the folowing "
+                    "The --todo option, whe paired with --license option should be used with the folowing "
                     "additional CLI options for maximum benifit: [`--license-text`, `--license-text-diagnostics`,"
                     "--license-diagnostics`] as these show additional diagnostic information to help review the issues."
                 )
@@ -118,7 +118,7 @@ def process_codebase(self, codebase, **kwargs):
 
         if not has_packages and not has_licenses:
             usage_suggestion_message = (
-                "The --review option should be used with atleast one of the license [`--license`], "
+                "The --todo option should be used with atleast one of the license [`--license`], "
                 "or package [`--package`] options."
             )
             warnings.simplefilter('always', ToDoPluginUsageWarning)

tests/cluecode/data/plugin_filter_clues/filtered-expected.json

@@ -5,7 +5,7 @@
       "license_expression": "apache-1.1",
       "license_expression_spdx": "Apache-1.1",
       "detection_count": 1,
-      "sample_matches": [
+      "reference_matches": [
         {
           "license_expression": "apache-1.1",
           "license_expression_spdx": "Apache-1.1",

tests/cluecode/data/plugin_filter_clues/filtered-expected2.json

@@ -5,7 +5,7 @@
       "license_expression": "pygres-2.2",
       "license_expression_spdx": "LicenseRef-scancode-pygres-2.2",
       "detection_count": 1,
-      "sample_matches": [
+      "reference_matches": [
         {
           "license_expression": "pygres-2.2",
           "license_expression_spdx": "LicenseRef-scancode-pygres-2.2",

tests/cluecode/data/plugin_filter_clues/filtered-expected3.json

@@ -5,7 +5,7 @@
       "license_expression": "pcre",
       "license_expression_spdx": "LicenseRef-scancode-pcre",
       "detection_count": 1,
-      "sample_matches": [
+      "reference_matches": [
         {
           "license_expression": "pcre",
           "license_expression_spdx": "LicenseRef-scancode-pcre",

tests/formattedcode/data/common/manifests-expected.json

@@ -483,7 +483,7 @@
       "license_expression": "apache-2.0",
       "license_expression_spdx": "Apache-2.0",
       "detection_count": 3,
-      "sample_matches": [
+      "reference_matches": [
         {
           "license_expression": "apache-2.0",
           "license_expression_spdx": "Apache-2.0",
@@ -505,7 +505,7 @@
       "license_expression": "apache-2.0",
       "license_expression_spdx": "Apache-2.0",
       "detection_count": 1,
-      "sample_matches": [
+      "reference_matches": [
         {
           "license_expression": "apache-2.0",
           "license_expression_spdx": "Apache-2.0",
@@ -527,7 +527,7 @@
       "license_expression": "cddl-1.0",
       "license_expression_spdx": "CDDL-1.0",
       "detection_count": 1,
-      "sample_matches": [
+      "reference_matches": [
         {
           "license_expression": "unknown-license-reference",
           "license_expression_spdx": "LicenseRef-scancode-unknown-license-reference",
@@ -563,7 +563,7 @@
       "license_expression": "cddl-1.0",
       "license_expression_spdx": "CDDL-1.0",
       "detection_count": 1,
-      "sample_matches": [
+      "reference_matches": [
         {
           "license_expression": "cddl-1.0",
           "license_expression_spdx": "CDDL-1.0",
@@ -599,7 +599,7 @@
       "license_expression": "lgpl-3.0",
       "license_expression_spdx": "LGPL-3.0-only",
       "detection_count": 2,
-      "sample_matches": [
+      "reference_matches": [
         {
           "license_expression": "lgpl-3.0",
           "license_expression_spdx": "LGPL-3.0-only",
@@ -621,7 +621,7 @@
       "license_expression": "lgpl-3.0",
       "license_expression_spdx": "LGPL-3.0-only",
       "detection_count": 1,
-      "sample_matches": [
+      "reference_matches": [
         {
           "license_expression": "lgpl-3.0",
           "license_expression_spdx": "LGPL-3.0-only",
@@ -643,7 +643,7 @@
       "license_expression": "lgpl-3.0",
       "license_expression_spdx": "LGPL-3.0-only",
       "detection_count": 1,
-      "sample_matches": [
+      "reference_matches": [
         {
           "license_expression": "lgpl-3.0",
           "license_expression_spdx": "LGPL-3.0-only",
@@ -665,7 +665,7 @@
       "license_expression": "lgpl-3.0",
       "license_expression_spdx": "LGPL-3.0-only",
       "detection_count": 1,
-      "sample_matches": [
+      "reference_matches": [
         {
           "license_expression": "lgpl-3.0",
           "license_expression_spdx": "LGPL-3.0-only",
@@ -687,7 +687,7 @@
       "license_expression": "mit",
       "license_expression_spdx": "MIT",
       "detection_count": 1,
-      "sample_matches": [
+      "reference_matches": [
         {
           "license_expression": "mit",
           "license_expression_spdx": "MIT",
@@ -709,7 +709,7 @@
       "license_expression": "mit",
       "license_expression_spdx": "MIT",
       "detection_count": 1,
-      "sample_matches": [
+      "reference_matches": [
         {
           "license_expression": "mit",
           "license_expression_spdx": "MIT",