Merge pull request #698 from fsfe/summary-help

Add a "Recommendations" section in lint output based on found issues

CHANGELOG.md

@@ -46,6 +46,7 @@ CLI command and its behaviour. There are no guarantees of stability for the
 - More file types are recognised:
   - Julia (`.jl`) (#815)
   - Modern Fortran (`.f90`) (#836)
+- Display recommendations for steps to fix found issues during a lint. (#698)
 
 ### Changed
 

src/reuse/lint.py

@@ -1,5 +1,6 @@
 # SPDX-FileCopyrightText: 2017 Free Software Foundation Europe e.V. <https://fsfe.org>
 # SPDX-FileCopyrightText: 2022 Florian Snow <florian@familysnow.net>
+# SPDX-FileCopyrightText: 2023 DB Systel GmbH
 #
 # SPDX-License-Identifier: GPL-3.0-or-later
 
@@ -13,6 +14,7 @@
 from gettext import gettext as _
 from io import StringIO
 from pathlib import Path
+from textwrap import TextWrapper
 from typing import IO, Any
 
 from . import __REUSE_version__
@@ -37,7 +39,7 @@ def add_arguments(parser: ArgumentParser) -> None:
     )
 
 
-# pylint: disable=too-many-branches, too-many-statements
+# pylint: disable=too-many-branches,too-many-statements,too-many-locals
 def format_plain(report: ProjectReport) -> str:
     """Formats data dictionary as plaintext string to be printed to sys.stdout
 
@@ -118,7 +120,7 @@ def format_plain(report: ProjectReport) -> str:
             files_without_licenses_excl
         )
 
-        if files_without_either:
+        if files_without_either or files_without_both:
             header = (
                 "# " + _("MISSING COPYRIGHT AND LICENSING INFORMATION") + "\n\n"
             )
@@ -201,6 +203,23 @@ def format_plain(report: ProjectReport) -> str:
                 "{} of the REUSE Specification :-("
             ).format(__REUSE_version__)
         )
+
+        # Write recommendations in a nicely wrapped format
+        output.write("\n\n\n# ")
+        output.write(_("RECOMMENDATIONS"))
+        output.write("\n\n")
+
+        wrapper = TextWrapper(
+            width=80,
+            drop_whitespace=True,
+            break_long_words=False,
+            initial_indent="* ",
+            subsequent_indent="  ",
+        )
+        for help_text in report.recommendations:
+            output.write("\n".join(wrapper.wrap(help_text)))
+            output.write("\n")
+
     output.write("\n")
 
     return output.getvalue()

src/reuse/report.py

@@ -1,6 +1,7 @@
 # SPDX-FileCopyrightText: 2017 Free Software Foundation Europe e.V. <https://fsfe.org>
 # SPDX-FileCopyrightText: 2022 Florian Snow <florian@familysnow.net>
 # SPDX-FileCopyrightText: 2022 Pietro Albini <pietro.albini@ferrous-systems.com>
+# SPDX-FileCopyrightText: 2023 DB Systel GmbH
 # SPDX-FileCopyrightText: 2023 Carmen Bianca BAKKER <carmenbianca@fsfe.org>
 #
 # SPDX-License-Identifier: GPL-3.0-or-later
@@ -113,6 +114,7 @@ def to_dict_lint(self) -> Dict[str, Any]:
             "summary": {
                 "used_licenses": [],
             },
+            "recommendations": self.recommendations,
         }
 
         # Populate 'files'
@@ -140,15 +142,21 @@ def to_dict_lint(self) -> Dict[str, Any]:
         }
 
         # Sort dictionary keys while keeping the top three keys at the beginning
+        # and the recommendations on the bottom
         sorted_keys = sorted(list(unsorted_data.keys()))
         sorted_keys.remove("lint_version")
         sorted_keys.remove("reuse_spec_version")
         sorted_keys.remove("reuse_tool_version")
-        sorted_keys = [
-            "lint_version",
-            "reuse_spec_version",
-            "reuse_tool_version",
-        ] + sorted_keys
+        sorted_keys.remove("recommendations")
+        sorted_keys = (
+            [
+                "lint_version",
+                "reuse_spec_version",
+                "reuse_tool_version",
+            ]
+            + sorted_keys
+            + ["recommendations"]
+        )
 
         sorted_data = {key: unsorted_data[key] for key in sorted_keys}
 
@@ -333,7 +341,7 @@ def unused_licenses(self) -> Set[str]:
 
     @property
     def files_without_licenses(self) -> Set[Path]:
-        """Set of paths that have no license information."""
+        """Set of paths that have no licensing information."""
         if self._files_without_licenses is not None:
             return self._files_without_licenses
 
@@ -380,6 +388,89 @@ def is_compliant(self) -> bool:
 
         return self._is_compliant
 
+    @property
+    def recommendations(self) -> List[str]:
+        """Generate help for next steps based on found REUSE issues"""
+        recommendations = []
+
+        # These items should be ordered in the same way as in the summary.
+        if self.bad_licenses:
+            recommendations.append(
+                _(
+                    "Fix bad licenses: At least one license in the LICENSES"
+                    " directory and/or provided by 'SPDX-License-Identifier'"
+                    " tags is invalid. They are either not valid SPDX License"
+                    " Identifiers or do not start with 'LicenseRef-'. FAQ about"
+                    " custom licenses:"
+                    " https://reuse.software/faq/#custom-license"
+                )
+            )
+        if self.deprecated_licenses:
+            recommendations.append(
+                _(
+                    "Fix deprecated licenses: At least one of the licenses in"
+                    " the LICENSES directory and/or provided by an"
+                    " 'SPDX-License-Identifier' tag or in '.reuse/dep5' has"
+                    " been deprecated by SPDX. The current list and their"
+                    " respective recommended  new identifiers can be found"
+                    " here: <https://spdx.org/licenses/#deprecated>"
+                )
+            )
+        if self.licenses_without_extension:
+            recommendations.append(
+                _(
+                    "Fix licenses without file extension: At least one license"
+                    " text file in the 'LICENSES' directory does not have a"
+                    " '.txt' file extension. Please rename the file(s)"
+                    " accordingly."
+                )
+            )
+        if self.missing_licenses:
+            recommendations.append(
+                _(
+                    "Fix missing licenses: For at least one of the license"
+                    " identifiers provided by the 'SPDX-License-Identifier'"
+                    " tags, there is no corresponding license text file in the"
+                    " 'LICENSES' directory. For SPDX license identifiers, you"
+                    " can simply run 'reuse download --all' to get any missing"
+                    " ones. For custom licenses (starting with 'LicenseRef-'),"
+                    " you need to add these files yourself."
+                )
+            )
+        if self.unused_licenses:
+            recommendations.append(
+                _(
+                    "Fix unused licenses: At least one of the license text"
+                    " files in 'LICENSES' is not referenced by any file, e.g."
+                    " by an 'SPDX-License-Identifier' tag. Please make sure"
+                    " that you either tag the accordingly licensed files"
+                    " properly, or delete the unused license text if you are"
+                    " sure that no file or code snippet is licensed as such."
+                )
+            )
+        if self.read_errors:
+            recommendations.append(
+                _(
+                    "Fix read errors: At least one of the files in your"
+                    " directory cannot be read by the tool. Please check the"
+                    " file permissions. You will find the affected files at the"
+                    " top of the output as part of the logged error messages."
+                )
+            )
+        if self.files_without_copyright or self.files_without_licenses:
+            recommendations.append(
+                _(
+                    "Fix missing copyright/licensing information: For one or"
+                    " more files, the tool cannot find copyright and/or"
+                    " licensing information. You typically do this by adding"
+                    " 'SPDX-FileCopyrightText' and 'SPDX-License-Identifer'"
+                    " tags to each file. The tutorial explains additional ways"
+                    " to do this: <https://reuse.software/tutorial/>"
+                )
+            )
+
+        return recommendations
+
 
 class FileReport:  # pylint: disable=too-many-instance-attributes
     """Object that holds a linting report about a single file."""

tests/test_lint.py

@@ -86,7 +86,10 @@ def test_lint_deprecated(fake_repository):
     result = format_plain(report)
 
     assert ":-(" in result
+    assert "# DEPRECATED LICENSES" in result
     assert "GPL-3.0" in result
+    assert "Fix deprecated licenses:" in result
+    assert "spdx.org/licenses/#deprecated" in result
 
 
 def test_lint_bad_license(fake_repository):
@@ -99,8 +102,26 @@ def test_lint_bad_license(fake_repository):
     result = format_plain(report)
 
     assert ":-(" in result
+    assert "# BAD LICENSES" in result
     assert "foo.py" in result
     assert "bad-license" in result
+    assert "Fix bad licenses:" in result
+    assert "reuse.software/faq/#custom-license" in result
+
+
+def test_lint_licenses_without_extension(fake_repository):
+    """A license without file extension is detected."""
+    (fake_repository / "LICENSES/GPL-3.0-or-later.txt").rename(
+        fake_repository / "LICENSES/GPL-3.0-or-later"
+    )
+    project = Project(fake_repository)
+    report = ProjectReport.generate(project)
+    result = format_plain(report)
+
+    assert ":-(" in result
+    assert "# LICENSES WITHOUT FILE EXTENSION" in result
+    assert "GPL-3.0-or-later" in result
+    assert "Fix licenses without file extension:" in result
 
 
 def test_lint_missing_licenses(fake_repository):
@@ -111,8 +132,10 @@ def test_lint_missing_licenses(fake_repository):
     result = format_plain(report)
 
     assert ":-(" in result
+    assert "# MISSING LICENSES" in result
     assert "foo.py" in result
     assert "MIT" in result
+    assert "Fix missing licenses:" in result
 
 
 def test_lint_unused_licenses(fake_repository):
@@ -123,7 +146,9 @@ def test_lint_unused_licenses(fake_repository):
     result = format_plain(report)
 
     assert ":-(" in result
+    assert "# UNUSED LICENSES" in result
     assert "Unused licenses: MIT" in result
+    assert "Fix unused licenses:" in result
 
 
 @cpython
@@ -137,8 +162,10 @@ def test_lint_read_errors(fake_repository):
     result = format_plain(report)
 
     assert ":-(" in result
+    assert "# READ ERRORS" in result
     assert "Could not read:" in result
     assert "foo.py" in result
+    assert "Fix read errors:" in result
 
 
 def test_lint_files_without_copyright_and_licensing(fake_repository):
@@ -149,11 +176,14 @@ def test_lint_files_without_copyright_and_licensing(fake_repository):
     result = format_plain(report)
 
     assert ":-(" in result
+    assert "# MISSING COPYRIGHT AND LICENSING INFORMATION" in result
     assert (
         "The following files have no copyright and licensing information:"
         in result
     )
     assert "foo.py" in result
+    assert "Fix missing copyright/licensing information:" in result
+    assert "reuse.software/tutorial" in result
 
 
 def test_lint_json_output(fake_repository):
@@ -172,9 +202,11 @@ def test_lint_json_output(fake_repository):
     assert "non_compliant" in json_result
     assert "files" in json_result
     assert "summary" in json_result
+    assert "recommendations" in json_result
     # Test length of resulting list values
     assert len(json_result["files"]) == 9
     assert len(json_result["summary"]) == 5
+    assert len(json_result["recommendations"]) == 2
     # Test result
     assert json_result["summary"]["compliant"] is False
     # Test license path

tests/test_report.py

@@ -439,8 +439,11 @@ def test_generate_project_report_to_dict_lint(fake_repository, multiprocessing):
         "reuse_tool_version",
     ]
 
+    # Check if the recommendation key is at the bottom of the dictionary
+    assert list(result.keys())[-1] == "recommendations"
+
     # Check if the rest of the keys are sorted alphabetically
-    assert list(result.keys())[3:] == sorted(list(result.keys())[3:])
+    assert list(result.keys())[3:-1] == sorted(list(result.keys())[3:-1])
 
 
 def test_bill_of_materials(fake_repository, multiprocessing):