Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
23 changes: 21 additions & 2 deletions src/reuse/lint.py
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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__
Expand All @@ -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

Expand Down Expand Up @@ -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"
)
Expand Down Expand Up @@ -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()
Expand Down
103 changes: 97 additions & 6 deletions src/reuse/report.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -113,6 +114,7 @@ def to_dict_lint(self) -> Dict[str, Any]:
"summary": {
"used_licenses": [],
},
"recommendations": self.recommendations,
}

# Populate 'files'
Expand Down Expand Up @@ -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}

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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."""
Expand Down
32 changes: 32 additions & 0 deletions tests/test_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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
Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion tests/test_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down