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,10 +13,10 @@
 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
@@ -29,6 +29,7 @@
 import os.path
 import logging
 import re
+import ntpath
 
 userlog = logging.getLogger("scancode-evaluate")
 userlog.setLevel(logging.INFO)
@@ -40,92 +41,117 @@
 MISSING_PERMISIVE_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
+class FileDecodeError(Exception):
+    """An exception for a failure to decode a file being tested."""
+
+def path_leaf(path):
+    """Return the leaf of a path."""
+    head, tail = ntpath.split(path)
+    # Ensure the correct file name is returned if the file ends with a slash
+    return tail or ntpath.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(file):
+    """Returns true if the file analysed by ScanCode contains SPDX identifier."""
+    try:
+        with open(file, 'r') as read_file:
+            filetext = read_file.read()
+    except UnicodeDecodeError:
+        raise FileDecodeError(
+            "Unable to look for SPDX text in `{}`:".format(file)
+        )
+
+    return re.findall("SPDX-License-Identifier:?", filetext)
+
+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
     """
 
     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")
+        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 -1
 
-    try:
-        for file in results['files']:
-            license_offender = {}
-            license_offender['file'] = file
-            # ignore directory, not relevant here
-            if license_offender['file']['type'] == 'directory':
+    if 'files' not in scancode_output_data:
+        userlog.warning("Missing `files` attribute in %s" % (scancode_output))
+        return -1
+
+    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 -1
+
+        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 -1
+
+        try:
+            if not has_permissive_text_in_scancode_output(scancode_output_data_file['licenses']):
+                scancode_output_data_file['fail_reason'] = MISSING_PERMISIVE_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 -1
+
+        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.
                 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
+                    file_path = os.path.abspath(scancode_output_data_file['path'])
+                    if not has_spdx_text_in_analysed_file(file_path):
+                        scancode_output_data_file['fail_reason'] = MISSING_SPDX_TEXT
+                        offenders.append(scancode_output_data_file)
+                except FileDecodeError:
+                    # 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
+        except KeyError as e:
+            userlog.warning("Could not find %s attribute in %s" % (str(e), scancode_output))
+            return -1
 
     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,11 +161,11 @@ 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)
         else:

tools/test/travis-ci/scancode_evaluate_test.py

@@ -4,29 +4,28 @@
 # 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__))
+    os.path.join(os.path.dirname(__file__))
 )
 
 # 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 +39,83 @@
  * 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 resource_test3():
+    """Create stub files.
+    test3.h missing license notice
+    test4.h with license notice
+    test5.h with license notice
+    """
+    file_paths = [
+        stub_path + "test3.h",
+        stub_path + "test4.h",
+        stub_path + "test5.h",
+    ]
+    for file_path in file_paths:
+        with open(file_path, "w") as new_file:
+            if file_path in [stub_path + "test4.h", stub_path + "test5.h"]:
+                new_file.write(header_with_spdx)
+            else:
+                new_file.write(header_without_spdx)
+    yield "resource_test3"
+    # delete stub files
+    for file_path in file_paths:
+        os.remove(file_path)
+
+
+@pytest.fixture()
+def resource_test4():
+    """Create stub files.
+    test.h missing license notice
+    """
+    file_paths = [
+        stub_path + "test.h",
+    ]
+    for file_path in file_paths:
+        with open(file_path, "w") as new_file:
+                new_file.write(header_without_spdx)
+    yield "resource_test4"
+    # delete stub files
+    for file_path in file_paths:
+        os.remove(file_path)
+
+
+class TestScancodeEvaluate:
+    """ Test scancode evaluation script """
+    def test_scancode_case_1(self):
+        """ Test Case 1 -- Missing `files` attribute in JSON
+            @inputs scancode_test/scancode_test_1.json
+            @outputs -1
+        """
+        scancode_output = ROOT + "/scancode_test/scancode_test_1.json"
+        assert license_check(scancode_output) == -1
+
+    def test_scancode_case_2(self):
+        """ Test Case 2 -- 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
+        """
+        scancode_output = ROOT + "/scancode_test/scancode_test_2.json"
+        assert license_check(scancode_output) == 0
+
+    def test_scancode_case_3(self, resource_test3):
+        """ Test Case 3 -- Five file 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
+        """
+        scancode_output = ROOT + "/scancode_test/scancode_test_3.json"
+        assert license_check(scancode_output) == 4
+
+    def test_scancode_case_4(self, resource_test4):
+        """ Test Case 4 -- 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
+        """
+        scancode_output = ROOT + "/scancode_test/scancode_test_4.json"
+        assert license_check(scancode_output) == 1, "Non-Permissive Header False Positive"