Merge pull request #859 from carmenbianca/revert-logic

Remove --exit-if-unrecognised, add --fallback-dot-license

CHANGELOG.md

@@ -50,6 +50,10 @@ CLI command and its behaviour. There are no guarantees of stability for the
 - Display recommendations for steps to fix found issues during a lint. (#698)
 - Add support for Pijul VCS. Pijul support is not added to the Docker image.
   (#858)
+- When running `annotate` on a file with an unrecognised file path, the tool
+  currently exits early. To automatically create a .license file for
+  unrecognised files, `--fallback-dot-license` has been added. (#823, #851,
+  #853, #859; this took a while to get right.)
 
 ### Changed
 
@@ -60,9 +64,6 @@ CLI command and its behaviour. There are no guarantees of stability for the
   "[repositories] that were cloned independently and later added as a submodule
   or old setups", which "have the submodule's git directory inside the submodule
   instead of embedded into the superproject's git directory". (#687)
-- When running `annotate` on a file with an unrecognised file path,
-  automatically create a `.license` file. The old behaviour of exiting on
-  unrecognised paths is now behind `--exit-if-unrecognised`. (#851, #853)
 - No longer scan binary or uncommentable files for their contents in search of
   REUSE information. (#825)
 - `--force-dot-license` and `--skip-unrecognised` are now mutually exclusive on

src/reuse/_annotate.py

@@ -23,6 +23,7 @@
 from pathlib import Path
 from typing import IO, Iterable, Optional, Set, Tuple, Type, cast
 
+from binaryornot.check import is_binary
 from jinja2 import Environment, FileSystemLoader, Template
 from jinja2.exceptions import TemplateNotFound
 
@@ -35,7 +36,7 @@
     _determine_license_suffix_path,
     _get_comment_style,
     _has_style,
-    _is_commentable,
+    _is_uncommentable,
     contains_reuse_info,
     detect_line_endings,
     make_copyright_line,
@@ -116,6 +117,7 @@ def add_header_to_file(
     force_multi: bool = False,
     skip_existing: bool = False,
     skip_unrecognised: bool = False,
+    fallback_dot_license: bool = False,
     merge_copyrights: bool = False,
     replace: bool = True,
     out: IO[str] = sys.stdout,
@@ -130,17 +132,19 @@ def add_header_to_file(
         comment_style = _get_comment_style(path)
     if comment_style is None:
         if skip_unrecognised:
-            out.write(_("Skipped unrecognised file {path}").format(path=path))
+            out.write(_("Skipped unrecognised file '{path}'").format(path=path))
             out.write("\n")
             return result
-        out.write(
-            _("{path} is not recognised; creating {path}.license").format(
-                path=path
+        if fallback_dot_license:
+            out.write(
+                _(
+                    "'{path}' is not recognised; creating '{path}.license'"
+                ).format(path=path)
             )
-        )
-        out.write("\n")
-        path = _determine_license_path(path)
-        comment_style = EmptyCommentStyle
+            out.write("\n")
+            path = _determine_license_suffix_path(path)
+            path.touch()
+            comment_style = EmptyCommentStyle
 
     with open(path, "r", encoding="utf-8", newline="") as fp:
         text = fp.read()
@@ -338,10 +342,14 @@ def verify_write_access(
 
 
 def verify_paths_comment_style(args: Namespace, paths: Iterable[Path]) -> None:
-    """Exit if --exit-if-unrecognised is enabled and one of the paths has an
-    unrecognised style.
+    """Exit if --fallback-dot-license or --skip-unrecognised is not enabled and
+    one of the paths has an unrecognised style.
     """
-    if args.exit_if_unrecognised:
+    if (
+        not args.fallback_dot_license
+        and not args.skip_unrecognised
+        and not args.force_dot_license
+    ):
         unrecognised_files = []
 
         for path in paths:
@@ -353,8 +361,8 @@ def verify_paths_comment_style(args: Namespace, paths: Iterable[Path]) -> None:
                 "{}\n\n{}".format(
                     _(
                         "The following files do not have a recognised file"
-                        " extension. Please use --style, --force-dot-license or"
-                        " --skip-unrecognised:"
+                        " extension. Please use --style, --force-dot-license,"
+                        " --fallback-dot-license, or --skip-unrecognised:"
                     ),
                     "\n".join(str(path) for path in unrecognised_files),
                 )
@@ -433,12 +441,6 @@ def add_arguments(parser: ArgumentParser) -> None:
         action="store_true",
         help=_("force multi-line comment style, optional"),
     )
-    style_mutex_group = parser.add_mutually_exclusive_group()
-    style_mutex_group.add_argument(
-        "--force-dot-license",
-        action="store_true",
-        help=_("write a .license file instead of a header inside the file"),
-    )
     parser.add_argument(
         "--recursive",
         "-r",
@@ -454,11 +456,19 @@ def add_arguments(parser: ArgumentParser) -> None:
             "do not replace the first header in the file; just add a new one"
         ),
     )
+    style_mutex_group = parser.add_mutually_exclusive_group()
+    style_mutex_group.add_argument(
+        "--force-dot-license",
+        action="store_true",
+        help=_(
+            "always write a .license file instead of a header inside the file"
+        ),
+    )
     style_mutex_group.add_argument(
-        "--exit-if-unrecognised",
+        "--fallback-dot-license",
         action="store_true",
         help=_(
-            "exit prematurely if one of the files has no defined comment style"
+            "write a .license file to files with unrecognised comment styles"
         ),
     )
     style_mutex_group.add_argument(
@@ -498,15 +508,10 @@ def run(args: Namespace, project: Project, out: IO[str] = sys.stdout) -> int:
 
     result = 0
     for path in paths:
-        commentable = _is_commentable(path)
-        if not _has_style(path) and not args.force_dot_license:
-            # TODO: This is an awful check.
-            _LOGGER.debug(
-                _("{path} has no style, skipping it.").format(path=path)
-            )
-        elif not commentable or args.force_dot_license:
+        binary = is_binary(str(path))
+        if binary or _is_uncommentable(path) or args.force_dot_license:
             new_path = _determine_license_suffix_path(path)
-            if not commentable:
+            if binary:
                 _LOGGER.info(
                     _(
                         "'{path}' is a binary, therefore using '{new_path}'"
@@ -524,6 +529,7 @@ def run(args: Namespace, project: Project, out: IO[str] = sys.stdout) -> int:
             force_multi=args.multi_line,
             skip_existing=args.skip_existing,
             skip_unrecognised=args.skip_unrecognised,
+            fallback_dot_license=args.fallback_dot_license,
             merge_copyrights=args.merge_copyrights,
             replace=not args.no_replace,
             out=out,

tests/test_main_annotate.py

@@ -399,28 +399,28 @@ def test_annotate_implicit_style_filename(
     assert simple_file.read_text() == expected
 
 
-def test_annotate_unrecognised_style(fake_repository, stringio):
+def test_annotate_unrecognised_style(fake_repository, capsys):
     """Add a header to a file that has an unrecognised extension."""
     simple_file = fake_repository / "foo.foo"
     simple_file.write_text("pass")
 
-    result = main(
-        [
-            "annotate",
-            "--license",
-            "GPL-3.0-or-later",
-            "--copyright",
-            "Jane Doe",
-            "foo.foo",
-        ],
-        out=stringio,
-    )
+    with pytest.raises(SystemExit):
+        main(
+            [
+                "annotate",
+                "--license",
+                "GPL-3.0-or-later",
+                "--copyright",
+                "Jane Doe",
+                "foo.foo",
+            ],
+        )
 
-    assert result == 0
+    stdout = capsys.readouterr().err
     assert (
-        "foo.foo is not recognised; creating foo.foo.license"
-        in stringio.getvalue()
+        "The following files do not have a recognised file extension" in stdout
     )
+    assert "foo.foo" in stdout
 
 
 def test_annotate_skip_unrecognised(fake_repository, stringio):
@@ -442,7 +442,7 @@ def test_annotate_skip_unrecognised(fake_repository, stringio):
     )
 
     assert result == 0
-    assert "Skipped unrecognised file foo.foo" in stringio.getvalue()
+    assert "Skipped unrecognised file 'foo.foo'" in stringio.getvalue()
 
 
 def test_annotate_skip_unrecognised_and_style(
@@ -761,6 +761,56 @@ def test_annotate_uncommentable_json(
     )
 
 
+def test_annotate_fallback_dot_license(
+    fake_repository, stringio, mock_date_today
+):
+    """Add a header to .license if --fallback-dot-license is given, and no style
+    yet exists.
+    """
+    (fake_repository / "foo.py").write_text("Foo")
+    (fake_repository / "foo.foo").write_text("Foo")
+
+    expected_py = cleandoc(
+        """
+        # SPDX-FileCopyrightText: 2018 Jane Doe
+        #
+        # SPDX-License-Identifier: GPL-3.0-or-later
+        """
+    )
+    expected_foo = cleandoc(
+        """
+        SPDX-FileCopyrightText: 2018 Jane Doe
+
+        SPDX-License-Identifier: GPL-3.0-or-later
+        """
+    )
+
+    result = main(
+        [
+            "annotate",
+            "--license",
+            "GPL-3.0-or-later",
+            "--copyright",
+            "Jane Doe",
+            "--fallback-dot-license",
+            "foo.py",
+            "foo.foo",
+        ],
+        out=stringio,
+    )
+
+    assert result == 0
+    assert expected_py in (fake_repository / "foo.py").read_text()
+    assert (fake_repository / "foo.foo.license").exists()
+    assert (
+        fake_repository / "foo.foo.license"
+    ).read_text().strip() == expected_foo
+    assert (
+        "'foo.foo' is not recognised; creating 'foo.foo.license'"
+        in stringio.getvalue()
+    )
+
+
 def test_annotate_force_dot_license(fake_repository, stringio, mock_date_today):
     """Add a header to a .license file if --force-dot-license is given."""
     simple_file = fake_repository / "foo.py"
@@ -1104,6 +1154,7 @@ def test_annotate_multi_line_not_supported_custom_style(
                 "--copyright",
                 "Jane Doe",
                 "--multi-line",
+                "--force-dot-license",
                 "--style",
                 "python",
                 "foo.foo",