Bugfix/check fails with NoneType (#64)

- fixes #57

---------

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Signed-off-by: Anton Utz <uta5fe@bosch.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Anton Utz <96201379+ant-u@users.noreply.github.com>
Co-authored-by: Anton Utz <uta5fe@bosch.com>

README.md

@@ -86,6 +86,29 @@ options:
   -q, --quiet           disable most output
 ```
 
+Additionally, there is an option to ignore single files, folders and types of files.
+If there exists a `.scanignore` in the **top level directory** of a package, 
+everything in it is going to be ignored. 
+The file entries work similar to a `.gitignore` file, including making comments with `#`.
+One Example for a custom `.scanignore` file:
+
+```
+.git/* # folder
+README.txt # file
+README.* # file pattern
+```
+
+Per default, ros_license_toolkit ignores the following:
+
+```
+.scanignore
+package.xml
+setup.py
+setup.cfg
+CMakeLists.txt
+.git/*
+```
+
 ### Using it as a GitHub action
 
 You can use `ros_license_toolkit` inside your GitHub workflow in order to check licenses in your

how_to_update.md

@@ -1,5 +1,13 @@
 # Note for me
 
+## Requirements
+
+```bash
+pip install bumpver build twine
+```
+
+## Steps
+
 How to update the project:
 
 1. Increment version

src/ros_license_toolkit/checks.py

@@ -205,7 +205,16 @@ def _check_licenses(self, package: Package) -> None:
                     f"License text file '{license_text_file}' is " +\
                     f"of license {actual_license} but tag is " +\
                     f"{license_tag.get_license_id()}."
-                self.missing_license_texts_status[license_tag] = Status.WARNING
+                # If Tag and File both are in SPDX but don't match -> Error
+                if is_license_name_in_spdx_list(license_tag.get_license_id()):
+                    self.missing_license_texts_status[license_tag] =\
+                        Status.FAILURE
+                else:
+                    self.missing_license_texts_status[license_tag] =\
+                        Status.WARNING
+                self.files_with_wrong_tags[license_tag] = \
+                    {'actual_license': actual_license,
+                        'license_tag': license_tag.get_license_id()}
                 continue
 
     def _evaluate_results(self):
@@ -257,6 +266,8 @@ def _check_license_files(self, package: Package) -> None:
                 continue
             found_licenses_str = found_licenses[
                 'detected_license_expression_spdx']
+            if not found_licenses_str:
+                continue
             licenses = found_licenses_str.split(' AND ')
             for license_str in licenses:
                 if license_str not in self.declared_licenses:
@@ -302,14 +313,6 @@ def _evaluate_result(self, package: Package) -> None:
                 self.files_not_matched_by_any_license_tag,
                 package,
             )
-        elif self.files_with_inofficial_tag:
-            info_str = ''
-            info_str += 'For the following files, please change the ' +\
-                'License Tag in the package file to SPDX format:\n' +\
-                '\n'.join(
-                    [f"  '{x[0]}' is of {x[1][0]} but its Tag is {x[1][1]}."
-                     for x in self.files_with_inofficial_tag.items()])
-            self._warning(info_str)
         elif len(self.files_not_matched_by_any_license_tag) > 0:
             info_str = ''
             info_str += '\nThe following files contain licenses that ' +\
@@ -323,6 +326,14 @@ def _evaluate_result(self, package: Package) -> None:
                 self.files_not_matched_by_any_license_tag,
                 package,
             )
+        elif self.files_with_inofficial_tag:
+            info_str = ''
+            info_str += 'For the following files, please change the ' +\
+                'License Tag in the package file to SPDX format:\n' +\
+                '\n'.join(
+                    [f"  '{x[0]}' is of {x[1][0]} but its Tag is {x[1][1]}."
+                     for x in self.files_with_inofficial_tag.items()])
+            self._warning(info_str)
         else:
             self._success('All licenses found in the code are covered by a '
                           'license declaration.')
@@ -337,3 +348,60 @@ def _print_info(self, info_str, files_with_uncovered_licenses,
                     lambda x: x[0] in files_with_uncovered_licenses or (
                         x[0] in files_not_matched_by_any_license_tag),
                     package.found_files_w_licenses.items()))))
+
+
+class LicenseFilesReferencedCheck(Check):
+    """Check if all found License file have a reference in package.xml."""
+
+    def _check(self, package: Package):
+        not_covered_texts: Dict[str, str] = {}
+        inofficial_covered_texts: Dict[str, List[str]] = {}
+        for filename, license_text in package.found_license_texts.items():
+            # skipping all declarations above the package
+            if not is_in_package(package, filename):
+                continue
+            if 'detected_license_expression_spdx' in license_text and \
+               license_text['detected_license_expression_spdx'] not in \
+               package.license_tags:
+                spdx_expression = license_text[
+                    'detected_license_expression_spdx']
+                inofficial_licenses = {
+                    lic_tag.id_from_license_text: key
+                    for key, lic_tag in package.license_tags.items()
+                    if lic_tag.id_from_license_text != ''}
+                if spdx_expression in inofficial_licenses:
+                    inofficial_covered_texts[filename] = \
+                        [spdx_expression,
+                         inofficial_licenses[spdx_expression]]
+                else:
+                    not_covered_texts[filename] = \
+                        spdx_expression
+        if not_covered_texts:
+            info_str = ''
+            info_str += 'The following license files are not' +\
+                ' mentioned by any tag:\n' +\
+                '\n'.join(
+                    [f"  '{x[0]}' is of {x[1]}."
+                     for x in not_covered_texts.items()])
+            self._failed(info_str)
+        elif inofficial_covered_texts:
+            info_str = ''
+            info_str += 'The following license files are not' +\
+                ' mentioned by any tag:\n' +\
+                '\n'.join(
+                    [f"  '{x[0]}' is of {x[1][0]} but its tag is {x[1][1]}."
+                     for x in inofficial_covered_texts.items()])
+            self._warning(info_str)
+        else:
+            self._success("All license declaration are referenced by a tag.")
+
+
+def is_in_package(package: Package, file: str) -> bool:
+    """Return TRUE if the file is underneath the absolute package path.
+    Return FALSE if file is located above package."""
+    parent = os.path.abspath(package.abspath)
+    child = os.path.abspath(package.abspath + '/' + file)
+
+    comm_parent = os.path.commonpath([parent])
+    comm_child_parent = os.path.commonpath([parent, child])
+    return comm_parent == comm_child_parent

src/ros_license_toolkit/common.py

@@ -16,14 +16,44 @@
 
 """Common utility functions."""
 
-from typing import Any, Dict, Optional
+import os
+from typing import Any, Dict, List, Optional
 
 REQUIRED_PERCENTAGE_OF_LICENSE_TEXT = 95.0
 
+# files we ignore in scan results
+IGNORED = [
+    ".scanignore",
+    "package.xml",
+    "setup.py",
+    "setup.cfg",
+    "CMakeLists.txt",
+    ".git/*"
+]
+
 
 def get_spdx_license_name(scan_results: Dict[str, Any]) -> Optional[str]:
     """Get the SPDX license name from scan results."""
     if scan_results['percentage_of_license_text'] \
        >= REQUIRED_PERCENTAGE_OF_LICENSE_TEXT:
         return scan_results['detected_license_expression_spdx']
     return None
+
+
+def get_ignored_content(pkg_abspath: str) -> List[str]:
+    """Return all ignored patterns from '.scanignore'
+    and local IGNORED definition."""
+    ignored_content: List[str] = []
+    scanignore_path = pkg_abspath + "/.scanignore"
+    if os.path.exists(scanignore_path):
+        with open(scanignore_path, 'r', encoding="utf-8") as f:
+            for line in f:
+                line_contents = line.split('#')
+                ignore_pattern = line_contents[0].rstrip()
+                if len(ignore_pattern) > 0:
+                    ignored_content.append(ignore_pattern)
+            f.close()
+    for pattern in IGNORED:
+        if pattern not in ignored_content:
+            ignored_content.append(pattern)
+    return ignored_content

src/ros_license_toolkit/main.py

@@ -24,7 +24,8 @@
 import timeit
 from typing import Optional, Sequence
 
-from ros_license_toolkit.checks import (LicensesInCodeCheck,
+from ros_license_toolkit.checks import (LicenseFilesReferencedCheck,
+                                        LicensesInCodeCheck,
                                         LicenseTagExistsCheck,
                                         LicenseTagIsInSpdxListCheck,
                                         LicenseTextExistsCheck, Status)
@@ -134,7 +135,8 @@ def process_one_pkg(rll_print, package):
         LicenseTagExistsCheck(),
         LicenseTagIsInSpdxListCheck(),
         LicenseTextExistsCheck(),
-        LicensesInCodeCheck()]
+        LicensesInCodeCheck(),
+        LicenseFilesReferencedCheck()]
 
     for check in checks_to_perform:
         check.check(package)

src/ros_license_toolkit/package.py

@@ -26,20 +26,12 @@
 from rospkg.common import PACKAGE_FILE
 from scancode.api import get_licenses
 
-from ros_license_toolkit.common import get_spdx_license_name
+from ros_license_toolkit.common import (get_ignored_content,
+                                        get_spdx_license_name)
 from ros_license_toolkit.copyright import get_copyright_strings_per_pkg
 from ros_license_toolkit.license_tag import LicenseTag
 from ros_license_toolkit.repo import NotARepoError, Repo
 
-# files we ignore in scan results
-IGNORED = [
-    "package.xml",
-    "setup.py",
-    "setup.cfg",
-    "CMakeLists.txt",
-    ".git/*",
-]
-
 
 class PackageException(Exception):
     """Exception raised when a package is invalid."""
@@ -88,6 +80,9 @@ def __init__(self, path: str, repo: Optional[Repo] = None):
         # this is Optional, because it is only evaluated on the first call
         self._license_tags: Optional[Dict[str, LicenseTag]] = None
 
+        # All ignored files and folders
+        self._ignored_content: List[str] = get_ignored_content(self.abspath)
+
     def _get_path_relative_to_pkg(self, path: str) -> str:
         """Get path relative to pkg root"""
         return os.path.relpath(path, self.abspath)
@@ -121,7 +116,7 @@ def _run_scan_and_save_results(self):
         for (root, _, files) in os.walk(self.abspath):
             files_rel_to_pkg = [self._get_path_relative_to_pkg(
                 os.path.join(root, f)) for f in files]
-            for pattern in IGNORED:
+            for pattern in self._ignored_content:
                 matched = fnmatch.filter(files_rel_to_pkg, pattern)
                 for m in matched:
                     files_rel_to_pkg.remove(m)