Scancode: Fix false positive reported by scancode output analyser script

ScanCode can possibly return many licenses found for a single file scanned.
This commit ensures that the file is not reported as lacking a permissive license
if at least one license found in it is permissive.
Previously the script was reporting an issue if it found at least one license
in a file that was not permissive.

Additionally catch more errors and provide specific details about failures.
Provide unitest.

.travis.yml

@@ -70,7 +70,7 @@ matrix:
               | ( grep -v '^tools/test/toolchains/api_test.py' || true ) \
               | while read file; do cp --parents "${file}" SCANCODE; done
         - scancode -l --json-pp scancode.json SCANCODE
-        - python ./tools/test/travis-ci/scancode-evaluate.py -f scancode.json || true
+        - python ./tools/test/travis-ci/scancode-evaluate.py scancode.json || true
         # run the same but for new files. All new files must have SPDX
         - >-
             git diff --name-only --diff-filter=A FETCH_HEAD..HEAD \
@@ -78,10 +78,10 @@ matrix:
               | ( grep -v '^tools/test/toolchains/api_test.py' || true ) \
               | while read file; do cp --parents "${file}" SCANCODE_NEW_FILES; done
         - scancode -l --json-pp scancode_new_files.json SCANCODE_NEW_FILES
-        - python ./tools/test/travis-ci/scancode-evaluate.py -f scancode_new_files.json || true
+        - python ./tools/test/travis-ci/scancode-evaluate.py scancode_new_files.json || true
         - cat scancode-evaluate.log
         - COUNT=$(cat scancode-evaluate.log | grep 'File:' | wc -l) || true
-        - python ./tools/test/travis-ci/scancode-evaluate.py -f scancode_new_files.json
+        - python ./tools/test/travis-ci/scancode-evaluate.py scancode_new_files.json
         - cat scancode-evaluate.log
         - COUNT_NEW_FILES=$(cat scancode-evaluate.log | grep 'File:' | wc -l) || true
         - |

tools/test/travis-ci/scancode-evaluate.py

@@ -16,134 +16,149 @@ See the License for the specific language governing permissions and
 limitations 
 """
 
-# Asumptions for this script:
-# 1. directory_name is scanned directory.
-#  Files are copied to this directory with full tree. As result, if we find
-#  license offender, we can have full path (just scrape directory_name). We do this
-#  magic because scancode allows to scan directories/one file.
-# 2. SPDX and license text is a must for all code files
-
-import json
 import argparse
-import sys
+import json
-import os.path
 import logging
+import os.path
 import re
-
+import sys
-userlog = logging.getLogger("scancode-evaluate")
+from enum import Enum
-userlog.setLevel(logging.INFO)
-logfile = os.path.join(os.getcwd(), 'scancode-evaluate.log')
-log_file_handler = logging.FileHandler(logfile, mode='w')
-userlog.addHandler(log_file_handler)
 
 MISSING_LICENSE_TEXT = "Missing license header"
-MISSING_PERMISIVE_LICENSE_TEXT = "Non-permissive license"
+MISSING_PERMISSIVE_LICENSE_TEXT = "Non-permissive license"
 MISSING_SPDX_TEXT = "Missing SPDX license identifier"
 
-def license_check(directory_name, file):
+userlog = logging.getLogger("scancode-evaluate")
-    """ Check licenses in the scancode json file for specified directory
+
+class ReturnCode(Enum):
+    """Return codes."""
+
+    SUCCESS = 0
+    ERROR = -1
+
+
+def init_logger():
+    """Initialise the logger."""
+    userlog.setLevel(logging.INFO)
+    userlog.addHandler(
+        logging.FileHandler(
+            os.path.join(os.getcwd(), 'scancode-evaluate.log'), mode='w'
+        )
+    )
+
+
+def path_leaf(path):
+    """Return the leaf of a path."""
+    head, tail = os.path.split(path)
+    # Ensure the correct file name is returned if the file ends with a slash
+    return tail or os.path.basename(head)
+
+
+def has_permissive_text_in_scancode_output(scancode_output_data_file_licenses):
+    """Returns true if at list one license in the scancode output is permissive."""
+    return any(
+        scancode_output_data_file_license['category'] == 'Permissive'
+        for scancode_output_data_file_license in scancode_output_data_file_licenses
+    )
+
+
+def has_spdx_text_in_scancode_output(scancode_output_data_file_licenses):
+    """Returns true if at least one license in the scancode output has the spdx identifier."""
+    return any(
+        'spdx' in scancode_output_data_file_license['matched_rule']['identifier']
+        for scancode_output_data_file_license in scancode_output_data_file_licenses
+    )
+
+
+def has_spdx_text_in_analysed_file(scanned_file_content):
+    """Returns true if the file analysed by ScanCode contains SPDX identifier."""
+    return bool(re.findall("SPDX-License-Identifier:?", scanned_file_content))
+
+
+def license_check(scancode_output_path):
+    """Check licenses in the scancode json file for specified directory.
 
     This function does not verify if file exists, should be done prior the call.
 
     Args:
-    directory_name - where scancode was run, used to scrape this from paths
+    scancode_output_path: path to the scancode json output file (output from scancode --license --json-pp)
-    file - scancode json output file (output from scancode --license --json-pp)
 
     Returns:
     0 if nothing found
     >0 - count how many license isses found
-    -1 if any error in file licenses found
+    ReturnCode.ERROR.value if any error in file licenses found
     """
 
     offenders = []
     try:
-        # find all licenses in the files, must be licensed and permissive
+        with open(scancode_output_path, 'r') as read_file:
-        with open(file, 'r') as scancode_output:
+            scancode_output_data = json.load(read_file)
-            results = json.load(scancode_output)
+    except json.JSONDecodeError as jex:
-    except ValueError:
+        userlog.warning("JSON could not be decoded, Invalid JSON in body: %s", jex)
-        userlog.warning("JSON could not be decoded")
+        return ReturnCode.ERROR.value
-        return -1
 
+    if 'files' not in scancode_output_data:
+        userlog.warning("Missing `files` attribute in %s" % (scancode_output_path))
+        return ReturnCode.ERROR.value
+
+    for scancode_output_data_file in scancode_output_data['files']:
+        if scancode_output_data_file['type'] != 'file':
+            continue
+
+        if not scancode_output_data_file['licenses']:
+            scancode_output_data_file['fail_reason'] = MISSING_LICENSE_TEXT
+            offenders.append(scancode_output_data_file)
+            # check the next file in the scancode output
+            continue
+
+        if not has_permissive_text_in_scancode_output(scancode_output_data_file['licenses']):
+            scancode_output_data_file['fail_reason'] = MISSING_PERMISSIVE_LICENSE_TEXT
+            offenders.append(scancode_output_data_file)
+
+        if not has_spdx_text_in_scancode_output(scancode_output_data_file['licenses']):
+            # Scancode does not recognize license notice in Python file headers.
+            # Issue: https://github.com/nexB/scancode-toolkit/issues/1913
+            # Therefore check if the file tested by ScanCode actually has a licence notice.
+            file_path = os.path.abspath(scancode_output_data_file['path'])
             try:
-        for file in results['files']:
+                with open(file_path, 'r') as read_file:
-            license_offender = {}
+                    scanned_file_content = read_file.read()
-            license_offender['file'] = file
-            # ignore directory, not relevant here
-            if license_offender['file']['type'] == 'directory':
-                continue
-            if not license_offender['file']['licenses']:
-                license_offender['reason'] = MISSING_LICENSE_TEXT
-                offenders.append(license_offender.copy())
-                continue
-
-            found_spdx = spdx_check(offenders, license_offender)
-
-            if not found_spdx:
-                try:
-                    # Issue reported here https://github.com/nexB/scancode-toolkit/issues/1913
-                    # We verify here if SPDX is not really there as SDPX is part of the license text
-                    # scancode has some problems detecting it properly
-                    with open(os.path.join(os.path.abspath(license_offender['file']['path'])), 'r') as spdx_file_check:
-                        filetext = spdx_file_check.read()
-                    matches = re.findall("SPDX-License-Identifier:?", filetext)
-                    if matches:
-                        continue
-                    license_offender['reason'] = MISSING_SPDX_TEXT
-                    offenders.append(license_offender.copy())
             except UnicodeDecodeError:
-                    # not valid file for license check
+                userlog.warning("Unable to look for SPDX text in `{}`:".format(file_path))
+                # Ignore files that cannot be decoded
+                # check the next file in the scancode output
                 continue
-    except KeyError:
+
-        userlog.warning("Invalid scancode json file")
+            if not has_spdx_text_in_analysed_file(scanned_file_content):
-        return -1
+                scancode_output_data_file['fail_reason'] = MISSING_SPDX_TEXT
+                offenders.append(scancode_output_data_file)
 
     if offenders:
         userlog.warning("Found files with missing license details, please review and fix")
         for offender in offenders:
-            userlog.warning("File: " + offender['file']['path'][len(directory_name):] + " " + "reason: " + offender['reason'])
+            userlog.warning("File: %s reason: %s" % (path_leaf(offender['path']), offender['fail_reason']))
     return len(offenders)
 
 
-def spdx_check(offenders, license_offender):
-        """ Parse through list of licenses to determine whether licenses are permissive
-                @input list of offender, individual offender dict
-                @output none
-        """
-        found_spdx = False
-        # iterate through licenses, stop once permissive license has been found
-        for i in range(len(license_offender['file']['licenses'])):
-                # is any of the licenses permissive ?
-                if license_offender['file']['licenses'][i]['category'] == 'Permissive':
-                        # confirm that it has spdx license key
-                        if license_offender['file']['licenses'][i]['matched_rule']['identifier'].find("spdx") != -1:
-                                found_spdx = True
-                        # if no spdx found return anyway
-                        return found_spdx
-        # otherwise file is missing permissive license
-        license_offender['reason'] = MISSING_PERMISIVE_LICENSE_TEXT
-        offenders.append(license_offender.copy())
-
-        # missing spdx and permissive license
-        return found_spdx
-
 def parse_args():
-    parser = argparse.ArgumentParser(
+    """Parse command line arguments."""
-        description="License check.")
+    parser = argparse.ArgumentParser(description="License check.")
-    parser.add_argument('-f', '--file',
+    parser.add_argument(
-                        help="scancode-toolkit output json file")
+        'scancode_output_path',
-    parser.add_argument('-d', '--directory_name', default="SCANCODE",
+        help="scancode-toolkit output json file"
-                        help='Directory name where are files being checked')
+    )
     return parser.parse_args()
 
 
 if __name__ == "__main__":
+    init_logger()
     args = parse_args()
-    if args.file and os.path.isfile(args.file):
+    if os.path.isfile(args.scancode_output_path):
-        count = license_check(args.directory_name, args.file)
+        sys.exit(
-        if count == 0:
+            ReturnCode.SUCCESS.value
-            sys.exit(0)
+            if license_check(args.scancode_output_path) == 0
-        else:
+            else ReturnCode.ERROR.value
-            sys.exit(-1)
+        )
     else:
         userlog.warning("Could not find the scancode json file")
-        sys.exit(-1)
+        sys.exit(ReturnCode.ERROR.value)

tools/test/travis-ci/scancode_evaluate_test.py

@@ -4,29 +4,23 @@
 # SPDX-License-Identifier: Apache-2.0
 import importlib
 import os
-import sys
+import pytest
-from unittest import TestCase
 
-# TODO: fix scancode to match python naming conventROOTi
+license_check = importlib.import_module("scancode-evaluate").license_check
-SCANCODE_EVALUATE = importlib.import_module("scancode-evaluate")
-license_check = SCANCODE_EVALUATE.license_check
 
-ROOT = os.path.abspath(
+STUBS_PATH = os.path.join(
-	os.path.join(os.path.dirname(__file__))
+    os.path.abspath(os.path.join(os.path.dirname(__file__))), "scancode_test"
 )
 
-# path to stub files
+HEADER_WITHOUT_SPDX = "/* Copyright (C) Arm Limited, Inc - All Rights Reserved\
-stub_path = ROOT + "/scancode_test/"
-
-# template copyright notices
-invalid_header_1 = "/* Copyright (C) Arm Limited, Inc - All Rights Reserved\
  * Unauthorized copying of this. file, via any medium is strictly prohibited\
  * Proprietary and confidential\
  */"
 
-invalid_header_2 = "/* mbed Microcontroller Library\
+HEADER_WITH_SPDX = "/* mbed Microcontroller Library\
  * Copyright (c) 2006-2013 ARM Limited\
  *\
+ * SPDX-License-Identifier: Apache-2.0\
  * Licensed under the Apache License, Version 2.0 (the \"License\");\
  * you may not use this file except in compliance with the License.\
  * You may obtain a copy of the License at\
@@ -40,67 +34,62 @@ invalid_header_2 = "/* mbed Microcontroller Library\
  * limitations under the License.\
  */"
 
-
+@pytest.fixture()
-# implement test class
+def create_scanned_files():
-class TestScancodeEvaluate(TestCase):
+    """Create stub files.
-	""" Test scancode evaluation script """
+    test3.h missing license notice
-
+    test4.h with license notice
-	def test_scancode_case_1(self):
+    test5.h with license notice
-		""" Test Case 1 -- faulty json file
-			@inputs scancode_test_1.json
-			@outputs -1 if any error in file licenses found
     """
-		expected_result = -1
+    file_paths = [
-		test_json = ROOT + "/scancode_test/scancode_test_1.json"
+        os.path.join(STUBS_PATH, "test3.h"),
-
+        os.path.join(STUBS_PATH, "test4.h"),
-		# pass json path to test function
+        os.path.join(STUBS_PATH, "test5.h")
-		result = license_check(ROOT, test_json)
+    ]
-
+    for file_path in file_paths:
-		self.assertEqual(expected_result, result)
+        with open(file_path, "w") as new_file:
-
+            if file_path in [os.path.join(STUBS_PATH, "test3.h")]:
-	def test_scancode_case_2(self):
+                new_file.write(HEADER_WITHOUT_SPDX)
-		""" Test Case 2 -- no errors in license headers, try multiple types i.e Apache-2.0, BSD3
-			@inputs scancode_test_2.json [4 Apache-2.0, 4 BSD-3.0]
-			@outputs 0
-		"""
-		expected_result = 0
-		test_json = ROOT + "/scancode_test/scancode_test_2.json"
-
-		result = license_check(ROOT, test_json)
-		self.assertEqual(expected_result, result, "False Negative(s)")
-
-	def test_scancode_case_3(self):
-		""" Test Case 3 -- all files containing errors
-			@inputs scancode_test_3.json [2 no header, 2 non-permissive + spdx, 1 missing SPDX]
-			@output 5
-		"""
-		# create stub files with a non-permissive license and missing spdx
-		for i in range(3, 6):
-			with open(stub_path + "test" + str(i) + ".h", "w") as file:
-				if i == 5:
-					file.write(invalid_header_2)
             else:
-					file.write(invalid_header_1)
+                new_file.write(HEADER_WITH_SPDX)
+    yield
+    for file_path in file_paths:
+        os.remove(file_path)
 
-		expected_result = 7
-		test_json = ROOT + "/scancode_test/scancode_test_3.json"
 
-		result = license_check(ROOT, test_json)
+class TestScancodeEvaluate:
 
-		self.assertEqual(expected_result, result, "False Positive(s)")
+    def test_missing_files_attribute(self):
-		# delete stub files
+        """ Missing `files` attribute in JSON.
-		os.remove(stub_path + "test3.h")
+            @inputs scancode_test/scancode_test_1.json
-		os.remove(stub_path + "test4.h")
+            @outputs -1
-		os.remove(stub_path + "test5.h")
+        """
+        assert license_check(os.path.join(STUBS_PATH, "scancode_test_1.json")) == -1
 
-	def test_scancode_case_4(self):
+    def test_various_combinations_permissive_license_with_spdx(self):
-		""" Test Case 4 -- license header permissive and non-permissive 'license' [FP]
+        """ Various combinations where at least one license in
-			@inputs scancode_test_4.json
+            a file is permissive and has spdx in the match.identifier
+            attribute.
+            @inputs scancode_test/scancode_test_2.json
             @outputs 0
         """
-		expected_result = 0
+        assert license_check(os.path.join(STUBS_PATH, "scancode_test_2.json")) == 0
-		test_json = ROOT + "/scancode_test/scancode_test_4.json"
 
-		result = license_check(ROOT, test_json)
+    def test_missing_license_permissive_license_and_spdx(self, create_scanned_files):
+        """ Test four files scanned with various issues.
+            test.h: Missing license text (error count += 1)
+            test3.h: Missing `Permissive` license text and `spdx` in match.identifier and not in file tested by ScanCode (error count += 2)
+            test4.h: Missing `Permissive` license text and `spdx` in match.identifier but found in file tested by ScanCode (error count += 1)
+            test5.h: Missing `spdx` in match.identifier but found in file tested by ScanCode. (error count += 0)
+            @inputs scancode_test/scancode_test_2.json
+            @output 4
+        """
+        assert license_check(os.path.join(STUBS_PATH, "scancode_test_3.json")) == 4
 
-		self.assertEqual(expected_result, result, "Non-Permissive Header False Positive")
+    def test_permissive_license_no_spdx(self, create_scanned_files):
+        """ Multiple `Permissive` licenses in one file but none with `spdx` in
+            match.identifier and not in file tested by ScanCode (error count += 1)
+            @inputs scancode_test/scancode_test_2.json
+            @outputs 1
+        """
+        assert license_check(os.path.join(STUBS_PATH, "scancode_test_4.json")) == 1

tools/test/travis-ci/scancode_test/scancode_test_2.json

@@ -21,7 +21,9 @@
         "start_timestamp":"2020-10-01T135040.106260",
         "end_timestamp":"2020-10-01T135047.793497",
         "message":null,
-        "errors": [],
+        "errors":[
+           
+        ],
         "extra_data":{
            "files_count":8
         }
@@ -134,7 +136,9 @@
            "bsd-new",
            "bsd-new"
         ],
-            "scan_errors": []
+        "scan_errors":[
+           
+        ]
      },
      {
         "path":"tools/test/travis-ci/scancode_test/test2.h",
@@ -242,7 +246,9 @@
            "bsd-new",
            "bsd-new"
         ],
-        "scan_errors": []
+        "scan_errors":[
+           
+        ]
      },
      {
         "path":"tools/test/travis-ci/scancode_test/test3.h",
@@ -350,7 +356,9 @@
            "bsd-new",
            "bsd-new"
         ],
-            "scan_errors": []
+        "scan_errors":[
+           
+        ]
      },
      {
         "path":"tools/test/travis-ci/scancode_test/test4.h",
@@ -458,7 +466,9 @@
            "bsd-new",
            "bsd-new"
         ],
-        "scan_errors": []
+        "scan_errors":[
+           
+        ]
      },
      {
         "path":"tools/test/travis-ci/scancode_test/test5.c",
@@ -533,7 +543,9 @@
            "apache-2.0",
            "apache-2.0"
         ],
-            "scan_errors": []
+        "scan_errors":[
+           
+        ]
      },
      {
         "path":"tools/test/travis-ci/scancode_test/test6.c",
@@ -608,7 +620,9 @@
            "apache-2.0",
            "apache-2.0"
         ],
-          "scan_errors": []
+        "scan_errors":[
+           
+        ]
      },
      {
         "path":"tools/test/travis-ci/scancode_test/test7.c",
@@ -683,7 +697,9 @@
            "apache-2.0",
            "apache-2.0"
         ],
-          "scan_errors": []
+        "scan_errors":[
+           
+        ]
      },
      {
         "path":"tools/test/travis-ci/scancode_test/test8.c",
@@ -758,7 +774,9 @@
            "apache-2.0",
            "apache-2.0"
         ],
-          "scan_errors": []
+        "scan_errors":[
+           
+        ]
      }
   ]
 }

tools/test/travis-ci/scancode_test/scancode_test_3.json

@@ -18,7 +18,9 @@
         "start_timestamp":"2020-10-01T135040.106260",
         "end_timestamp":"2020-10-01T135047.793497",
         "message":null,
-      "errors": [],
+        "errors":[
+           
+        ],
         "extra_data":{
            "files_count":5
         }
@@ -28,16 +30,15 @@
      {
         "path":"tools/test/travis-ci/scancode_test/test.h",
         "type":"file",
-      "licenses": [],
+        "licenses":[
-      "license_expressions": [],
+           
-      "scan_errors": []
+        ],
-    },
+        "license_expressions":[
-    {
+           
-      "path": "tools/test/travis-ci/scancode_test/test2.h",
+        ],
-      "type": "file",
+        "scan_errors":[
-      "licenses": [],
+           
-      "license_expressions": [],
+        ]
-      "scan_errors": []
      },
      {
         "path":"tools/test/travis-ci/scancode_test/test3.h",
@@ -79,7 +80,9 @@
         "license_expressions":[
            "proprietary-license"
         ],
-      "scan_errors": []
+        "scan_errors":[
+           
+        ]
      },
      {
         "path":"tools/test/travis-ci/scancode_test/test4.h",
@@ -121,7 +124,9 @@
         "license_expressions":[
            "proprietary-license"
         ],
-      "scan_errors": []
+        "scan_errors":[
+           
+        ]
      },
      {
       "path":"tools/test/travis-ci/scancode_test/test5.h",
@@ -163,7 +168,9 @@
       "license_expressions":[
          "apache-2.0"
       ],
-        "scan_errors": []
+      "scan_errors":[
+         
+      ]
    }
   ]
 }

tools/test/travis-ci/scancode_test/scancode_test_4.json

@@ -5,7 +5,7 @@
         "tool_version":"3.1.1",
         "options":{
            "input":[
-            "test.h"
+              "test4.h"
            ],
            "--json-pp":"scancode.json",
            "--license":true
@@ -14,7 +14,9 @@
         "start_timestamp":"2020-10-01T135040.106260",
         "end_timestamp":"2020-10-01T135047.793497",
         "message":null,
-        "errors": [],
+        "errors":[
+           
+        ],
         "extra_data":{
            "files_count":1
         }
@@ -22,7 +24,7 @@
   ],
   "files":[
      {
-        "path": "tools/test/travis-ci/scancode_test/test.h",
+        "path":"tools/test/travis-ci/scancode_test/test3.h",
         "type":"file",
         "licenses":[
            {
@@ -41,7 +43,7 @@
               "start_line":2,
               "end_line":2,
               "matched_rule":{
-              "identifier": "spdx-license-identifier: bsd-new",
+                 "identifier":"sprdx-license-identifier: bsd-new",
                  "license_expression":"bsd-new",
                  "licenses":[
                     "bsd-new"
@@ -160,7 +162,9 @@
            "elastic-license-2018",
            "bsd-new"
         ],
-        "scan_errors": []
+        "scan_errors":[
+           
+        ]
      }
   ]
 }