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.

Author: hugueskamba

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

@@ -13,22 +13,30 @@
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
-limitations
+limitations 
 """
 
-# Asumptions for this script:
+# 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 os.path
+import json
 import logging
+import os.path
 import re
+import sys
+from enum import Enum
+
+
+class ReturnCode(Enum):
+    """Return codes."""
+
+    SUCCESS = 0
+    ERROR = -1
 
 userlog = logging.getLogger("scancode-evaluate")
 userlog.setLevel(logging.INFO)
@@ -37,95 +45,113 @@
 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):
-    """ Check licenses in the scancode json file for specified directory
+
+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):
+    """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
     file - scancode json output file (output from scancode --license --json-pp)
 
-    Returns:
+    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(file, 'r') as scancode_output:
-            results = json.load(scancode_output)
-    except ValueError:
-        userlog.warning("JSON could not be decoded")
-        return -1
-
-    try:
-        for file in results['files']:
-            license_offender = {}
-            license_offender['file'] = file
-            # ignore directory, not relevant here
-            if license_offender['file']['type'] == 'directory':
+        with open(scancode_output, 'r') as read_file:
+            scancode_output_data = json.load(read_file)
+    except json.JSONDecodeError as jex:
+        userlog.warning("JSON could not be decoded, Invalid JSON in body: %s", jex)
+        return ReturnCode.ERROR.value
+
+    if 'files' not in scancode_output_data:
+        userlog.warning("Missing `files` attribute in %s" % (scancode_output))
+        return ReturnCode.ERROR.value
+
+    for scancode_output_data_file in scancode_output_data['files']:
+        try:
+            if scancode_output_data_file['type'] != 'file':
                 continue
-            if not license_offender['file']['licenses']:
-                license_offender['reason'] = MISSING_LICENSE_TEXT
-                offenders.append(license_offender.copy())
+        except KeyError as e:
+            userlog.warning("Could not find %s attribute in %s" % (str(e), scancode_output))
+            return ReturnCode.ERROR.value
+
+        try:
+            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
-
-            found_spdx = spdx_check(offenders, license_offender)
-
-            if not found_spdx:
+        except KeyError as e:
+            userlog.warning("Could not find %s attribute in %s" % (str(e), scancode_output))
+            return ReturnCode.ERROR.value
+
+        try:
+            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)
+        except KeyError as e:
+            userlog.warning("Could not find %s attribute in %s" % (str(e), scancode_output))
+            return ReturnCode.ERROR.value
+
+        try:
+            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:
-                    # 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())
+                    with open(file_path, 'r') as read_file:
+                        scanned_file_content = read_file.read()
                 except UnicodeDecodeError:
-                    # not valid file for license check
+                    userlog.warning("Unable to look for SPDX text in `{}`:".format(file))
+                    # Ignore files that cannot be decoded
+                    # check the next file in the scancode output
                     continue
-    except KeyError:
-        userlog.warning("Invalid scancode json file")
-        return -1
+                if not has_spdx_text_in_analysed_file(scanned_file_content):
+                    scancode_output_data_file['fail_reason'] = MISSING_SPDX_TEXT
+                    offenders.append(scancode_output_data_file)
+        except KeyError as e:
+            userlog.warning("Could not find %s attribute in %s" % (str(e), scancode_output))
+            return ReturnCode.ERROR.value
 
     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(
         description="License check.")
@@ -135,15 +161,15 @@ def parse_args():
                         help='Directory name where are files being checked')
     return parser.parse_args()
 
-
 if __name__ == "__main__":
+
     args = parse_args()
     if args.file and os.path.isfile(args.file):
-        count = license_check(args.directory_name, args.file)
+        count = license_check(args.file)
         if count == 0:
-            sys.exit(0)
+            sys.exit(ReturnCode.SUCCESS.value)
         else:
-            sys.exit(-1)
+            sys.exit(ReturnCode.ERROR.value)
     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
-from unittest import TestCase
+import pytest
 
-# TODO: fix scancode to match python naming conventROOTi
-SCANCODE_EVALUATE = importlib.import_module("scancode-evaluate")
-license_check = SCANCODE_EVALUATE.license_check
+license_check = importlib.import_module("scancode-evaluate").license_check
 
-ROOT = os.path.abspath(
-	os.path.join(os.path.dirname(__file__))
+STUBS_PATH = os.path.join(
+    os.path.abspath(os.path.join(os.path.dirname(__file__))), "scancode_test"
 )
 
-# path to stub files
-stub_path = ROOT + "/scancode_test/"
-
-# template copyright notices
-invalid_header_1 = "/* Copyright (C) Arm Limited, Inc - All Rights Reserved\
+HEADER_WITHOUT_SPDX = "/* 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 @@
  * limitations under the License.\
  */"
 
-
-# implement test class
-class TestScancodeEvaluate(TestCase):
-	""" Test scancode evaluation script """
-
-	def test_scancode_case_1(self):
-		""" Test Case 1 -- faulty json file
-			@inputs scancode_test_1.json
-			@outputs -1 if any error in file licenses found
-		"""
-		expected_result = -1
-		test_json = ROOT + "/scancode_test/scancode_test_1.json"
-
-		# pass json path to test function
-		result = license_check(ROOT, test_json)
-
-		self.assertEqual(expected_result, result)
-
-	def test_scancode_case_2(self):
-		""" 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)
-
-		expected_result = 7
-		test_json = ROOT + "/scancode_test/scancode_test_3.json"
-
-		result = license_check(ROOT, test_json)
-
-		self.assertEqual(expected_result, result, "False Positive(s)")
-		# delete stub files
-		os.remove(stub_path + "test3.h")
-		os.remove(stub_path + "test4.h")
-		os.remove(stub_path + "test5.h")
-
-	def test_scancode_case_4(self):
-		""" Test Case 4 -- license header permissive and non-permissive 'license' [FP]
-			@inputs scancode_test_4.json
-			@outputs 0
-		"""
-		expected_result = 0
-		test_json = ROOT + "/scancode_test/scancode_test_4.json"
-
-		result = license_check(ROOT, test_json)
-
-		self.assertEqual(expected_result, result, "Non-Permissive Header False Positive")
+@pytest.fixture()
+def create_scanned_files():
+    """Create stub files.
+    test3.h missing license notice
+    test4.h with license notice
+    test5.h with license notice
+    """
+    file_paths = [
+        os.path.join(STUBS_PATH, "test3.h"),
+        os.path.join(STUBS_PATH, "test4.h"),
+        os.path.join(STUBS_PATH, "test5.h")
+    ]
+    for file_path in file_paths:
+        with open(file_path, "w") as new_file:
+            if file_path in [os.path.join(STUBS_PATH, "test3.h")]:
+                new_file.write(HEADER_WITHOUT_SPDX)
+            else:
+                new_file.write(HEADER_WITH_SPDX)
+    yield "create_scanned_files"
+    for file_path in file_paths:
+        os.remove(file_path)
+
+
+class TestScancodeEvaluate:
+
+    def test_missing_files_attribute(self):
+        """ Missing `files` attribute in JSON.
+            @inputs scancode_test/scancode_test_1.json
+            @outputs -1
+        """
+        assert license_check(os.path.join(STUBS_PATH, "scancode_test_1.json")) == -1
+
+    def test_various_combinations_permissive_license_with_spdx(self):
+        """ Various combinations where at least one license in
+            a file is permissive and has spdx in the match.identifier
+            attribute.
+            @inputs scancode_test/scancode_test_2.json
+            @outputs 0
+        """
+        assert license_check(os.path.join(STUBS_PATH, "scancode_test_2.json")) == 0
+
+    def test_missing_license_permissive_license_and_spdx(self, create_scanned_files):
+        """ Test 5 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 5
+        """
+        assert license_check(os.path.join(STUBS_PATH, "scancode_test_3.json")) == 4
+
+    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 0
+        """
+        assert license_check(os.path.join(STUBS_PATH, "scancode_test_4.json")) == 1