Get errant source from exception instead of find_global_licensing

In _main, find_global_licensing was called to find the file that
contained some parsing error. This may have contained false information
in the case of multiple REUSE.tomls.

Instead of needlessly calling this function, the errant file is now an
attribute on the exception.

Signed-off-by: Carmen Bianca BAKKER <[email protected]>

src/reuse/_main.py

@@ -29,7 +29,7 @@
 from ._format import INDENT, fill_all, fill_paragraph
 from ._util import PathType, setup_logging
 from .global_licensing import GlobalLicensingParseError
-from .project import GlobalLicensingConflict, GlobalLicensingFound, Project
+from .project import GlobalLicensingConflict, Project
 from .vcs import find_root
 
 _LOGGER = logging.getLogger(__name__)
@@ -257,18 +257,12 @@ def main(args: Optional[List[str]] = None, out: IO[str] = sys.stdout) -> int:
         project = Project.from_directory(root)
     # FileNotFoundError and NotADirectoryError don't need to be caught because
     # argparse already made sure of these things.
-    except UnicodeDecodeError:
-        found = cast(GlobalLicensingFound, Project.find_global_licensing(root))
-        main_parser.error(
-            _("'{path}' could not be decoded as UTF-8.").format(path=found.path)
-        )
     except GlobalLicensingParseError as error:
-        found = cast(GlobalLicensingFound, Project.find_global_licensing(root))
         main_parser.error(
             _(
                 "'{path}' could not be parsed. We received the following error"
                 " message: {message}"
-            ).format(path=found.path, message=str(error))
+            ).format(path=error.source, message=str(error))
         )
     except GlobalLicensingConflict as error:
         main_parser.error(str(error))

src/reuse/global_licensing.py

@@ -39,7 +39,7 @@
 from debian.copyright import Error as DebianError
 from license_expression import ExpressionError
 
-from . import ReuseInfo, SourceType
+from . import ReuseException, ReuseInfo, SourceType
 from ._util import _LICENSING, StrPath
 from .covered_files import all_files
 from .vcs import VCSStrategy
@@ -68,13 +68,17 @@ class PrecedenceType(Enum):
     OVERRIDE = "override"
 
 
-class GlobalLicensingParseError(Exception):
+class GlobalLicensingParseError(ReuseException):
     """An exception representing any kind of error that occurs when trying to
     parse a :class:`GlobalLicensing` file.
     """
 
+    def __init__(self, *args: Any, source: Optional[str] = None):
+        super().__init__(*args)
+        self.source = source
 
-class GlobalLicensingParseTypeError(TypeError, GlobalLicensingParseError):
+
+class GlobalLicensingParseTypeError(GlobalLicensingParseError, TypeError):
     """An exception representing a type error while trying to parse a
     :class:`GlobalLicensing` file.
     """
@@ -104,6 +108,7 @@ def __call__(
             attr_name = instance.TOML_KEYS[attribute.name]
         else:
             attr_name = attribute.name
+        source = getattr(instance, "source", None)
 
         if not isinstance(value, self.collection_type):
             raise GlobalLicensingParseTypeError(
@@ -115,7 +120,8 @@ def __call__(
                     type_name=self.collection_type.__name__,
                     value=repr(value),
                     value_class=repr(value.__class__),
-                )
+                ),
+                source=source,
             )
         for item in value:
             if not isinstance(item, self.value_type):
@@ -128,13 +134,15 @@ def __call__(
                         type_name=self.value_type.__name__,
                         item_value=repr(item),
                         item_class=repr(item.__class__),
-                    )
+                    ),
+                    source=source,
                 )
         if not self.optional and not value:
             raise GlobalLicensingParseValueError(
                 _("{attr_name} must not be empty.").format(
-                    attr_name=repr(attr_name)
-                )
+                    attr_name=repr(attr_name),
+                ),
+                source=source,
             )
 
 
@@ -162,7 +170,8 @@ def __call__(self, inst: Any, attr: attrs.Attribute, value: _T) -> None:
                     type=repr(error.args[2].__name__),
                     value=repr(error.args[3]),
                     value_type=repr(error.args[3].__class__),
-                )
+                ),
+                source=getattr(inst, "source", None),
             ) from error
 
 
@@ -175,7 +184,7 @@ def _instance_of(
 def _str_to_global_precedence(value: Any) -> PrecedenceType:
     try:
         return PrecedenceType(value)
-    except ValueError as err:
+    except ValueError as error:
         raise GlobalLicensingParseValueError(
             _(
                 "The value of 'precedence' must be one of {precedence_vals}"
@@ -186,7 +195,7 @@ def _str_to_global_precedence(value: Any) -> PrecedenceType:
                 ),
                 received=repr(value),
             )
-        ) from err
+        ) from error
 
 
 @overload
@@ -240,7 +249,6 @@ def from_file(cls, path: StrPath, **kwargs: Any) -> "GlobalLicensing":
 
         Raises:
             FileNotFoundError: file doesn't exist.
-            UnicodeDecodeError: could not decode file as UTF-8.
             OSError: some other error surrounding I/O.
             GlobalLicensingParseError: file could not be parsed.
         """
@@ -269,13 +277,17 @@ def from_file(cls, path: StrPath, **kwargs: Any) -> "ReuseDep5":
         try:
             with path.open(encoding="utf-8") as fp:
                 return cls(str(path), Copyright(fp))
-        except UnicodeDecodeError:
-            raise
+        except UnicodeDecodeError as error:
+            raise GlobalLicensingParseError(
+                str(error), source=str(path)
+            ) from error
         # TODO: Remove ValueError once
         # <https://salsa.debian.org/python-debian-team/python-debian/-/merge_requests/123>
         # is closed
         except (DebianError, ValueError) as error:
-            raise GlobalLicensingParseError(str(error)) from error
+            raise GlobalLicensingParseError(
+                str(error), source=str(path)
+            ) from error
 
     def reuse_info_of(
         self, path: StrPath
@@ -421,10 +433,14 @@ def from_dict(cls, values: Dict[str, Any], source: str) -> "ReuseTOML":
         new_dict["source"] = source
 
         annotation_dicts = values.get("annotations", [])
-        annotations = [
-            AnnotationsItem.from_dict(annotation)
-            for annotation in annotation_dicts
-        ]
+        try:
+            annotations = [
+                AnnotationsItem.from_dict(annotation)
+                for annotation in annotation_dicts
+            ]
+        except GlobalLicensingParseError as error:
+            error.source = source
+            raise error from error
 
         new_dict["annotations"] = annotations
 
@@ -436,13 +452,20 @@ def from_toml(cls, toml: str, source: str) -> "ReuseTOML":
         try:
             tomldict = tomlkit.loads(toml)
         except tomlkit.exceptions.TOMLKitError as error:
-            raise GlobalLicensingParseError(str(error)) from error
+            raise GlobalLicensingParseError(
+                str(error), source=source
+            ) from error
         return cls.from_dict(tomldict, source)
 
     @classmethod
     def from_file(cls, path: StrPath, **kwargs: Any) -> "ReuseTOML":
-        with Path(path).open(encoding="utf-8") as fp:
-            return cls.from_toml(fp.read(), str(path))
+        try:
+            with Path(path).open(encoding="utf-8") as fp:
+                return cls.from_toml(fp.read(), str(path))
+        except UnicodeDecodeError as error:
+            raise GlobalLicensingParseError(
+                str(error), source=str(path)
+            ) from error
 
     def find_annotations_item(self, path: StrPath) -> Optional[AnnotationsItem]:
         """Find a :class:`AnnotationsItem` that matches *path*. The latest match

tests/test_global_licensing.py

@@ -347,33 +347,37 @@ def test_simple(self, annotations_item):
 
     def test_version_not_int(self, annotations_item):
         """Version must be an int"""
-        with pytest.raises(GlobalLicensingParseTypeError):
+        with pytest.raises(GlobalLicensingParseTypeError) as exc_info:
             ReuseTOML(
                 version=1.2, source="REUSE.toml", annotations=[annotations_item]
             )
+        assert exc_info.value.source == "REUSE.toml"
 
     def test_source_not_str(self, annotations_item):
         """Source must be a str."""
-        with pytest.raises(GlobalLicensingParseTypeError):
+        with pytest.raises(GlobalLicensingParseTypeError) as exc_info:
             ReuseTOML(version=1, source=123, annotations=[annotations_item])
+        assert exc_info.value.source == 123
 
     def test_annotations_must_be_list(self, annotations_item):
         """Annotations must be in a list, not any other collection."""
         # TODO: Technically we could change this to 'any collection that is
         # ordered', but let's not split hairs.
-        with pytest.raises(GlobalLicensingParseTypeError):
+        with pytest.raises(GlobalLicensingParseTypeError) as exc_info:
             ReuseTOML(
                 version=1,
                 source="REUSE.toml",
                 annotations=iter([annotations_item]),
             )
+        assert exc_info.value.source == "REUSE.toml"
 
     def test_annotations_must_be_object(self):
         """Annotations must be AnnotationsItem objects."""
-        with pytest.raises(GlobalLicensingParseTypeError):
+        with pytest.raises(GlobalLicensingParseTypeError) as exc_info:
             ReuseTOML(
                 version=1, source="REUSE.toml", annotations=[{"foo": "bar"}]
             )
+        assert exc_info.value.source == "REUSE.toml"
 
 
 class TestReuseTOMLFromDict:
@@ -428,6 +432,24 @@ def test_no_version(self):
                 "REUSE.toml",
             )
 
+    def test_annotations_error(self):
+        """If there is an error in the annotations, the error get ReuseTOML's
+        source.
+        """
+        with pytest.raises(GlobalLicensingParseTypeError) as exc_info:
+            ReuseTOML.from_dict(
+                {
+                    "version": 1,
+                    "annotations": [
+                        {
+                            "path": {1},
+                        }
+                    ],
+                },
+                "REUSE.toml",
+            )
+        assert exc_info.value.source == "REUSE.toml"
+
 
 class TestReuseTOMLFromToml:
     """Test the from_toml method of ReuseTOML."""
@@ -1097,14 +1119,19 @@ def test_unicode_decode_error(self, fake_repository_dep5):
         shutil.copy(
             RESOURCES_DIRECTORY / "fsfe.png", fake_repository_dep5 / "fsfe.png"
         )
-        with pytest.raises(UnicodeDecodeError):
+        with pytest.raises(GlobalLicensingParseError) as exc_info:
             ReuseDep5.from_file(fake_repository_dep5 / "fsfe.png")
+        error = exc_info.value
+        assert error.source == str(fake_repository_dep5 / "fsfe.png")
+        assert "'utf-8' codec can't decode byte" in str(error)
 
     def test_parse_error(self, empty_directory):
         """Raise GlobalLicensingParseError on parse error."""
         (empty_directory / "foo").write_text("foo")
-        with pytest.raises(GlobalLicensingParseError):
+        with pytest.raises(GlobalLicensingParseError) as exc_info:
             ReuseDep5.from_file(empty_directory / "foo")
+        error = exc_info.value
+        assert error.source == str(empty_directory / "foo")
 
     def test_double_copyright_parse_error(self, empty_directory):
         """Raise GlobalLicensingParseError on double Copyright lines."""
@@ -1123,8 +1150,10 @@ def test_double_copyright_parse_error(self, empty_directory):
                 """
             )
         )
-        with pytest.raises(GlobalLicensingParseError):
+        with pytest.raises(GlobalLicensingParseError) as exc_info:
             ReuseDep5.from_file(empty_directory / "foo")
+        error = exc_info.value
+        assert error.source == str(empty_directory / "foo")
 
 
 def test_reuse_dep5_reuse_info_of(reuse_dep5):

tests/test_main.py

@@ -205,15 +205,49 @@ def test_lint_dep5_decode_error(fake_repository_dep5, capsys):
     )
     with pytest.raises(SystemExit):
         main(["lint"])
-    assert "could not be decoded" in capsys.readouterr().err
+    error = capsys.readouterr().err
+    assert str(fake_repository_dep5 / ".reuse/dep5") in error
+    assert "could not be parsed" in error
+    assert "'utf-8' codec can't decode byte" in error
 
 
 def test_lint_dep5_parse_error(fake_repository_dep5, capsys):
     """Display an error if there's a dep5 parse error."""
     (fake_repository_dep5 / ".reuse/dep5").write_text("foo")
     with pytest.raises(SystemExit):
         main(["lint"])
-    assert "could not be parsed" in capsys.readouterr().err
+    error = capsys.readouterr().err
+    assert str(fake_repository_dep5 / ".reuse/dep5") in error
+    assert "could not be parsed" in error
+
+
+def test_lint_toml_parse_error_version(fake_repository_reuse_toml, capsys):
+    """If version has the wrong type, print an error."""
+    (fake_repository_reuse_toml / "REUSE.toml").write_text("version = 'a'")
+    with pytest.raises(SystemExit):
+        main(["lint"])
+    error = capsys.readouterr().err
+    assert str(fake_repository_reuse_toml / "REUSE.toml") in error
+    assert "could not be parsed" in error
+
+
+def test_lint_toml_parse_error_annotation(fake_repository_reuse_toml, capsys):
+    """If there is an error in an annotation, print an error."""
+    (fake_repository_reuse_toml / "REUSE.toml").write_text(
+        cleandoc_nl(
+            """
+            version = 1
+
+            [[annotations]]
+            path = 1
+            """
+        )
+    )
+    with pytest.raises(SystemExit):
+        main(["lint"])
+    error = capsys.readouterr().err
+    assert str(fake_repository_reuse_toml / "REUSE.toml") in error
+    assert "could not be parsed" in error
 
 
 def test_lint_json(fake_repository, stringio):

tests/test_project.py

@@ -50,7 +50,7 @@ def test_project_conflicting_global_licensing(empty_directory):
     """
     (empty_directory / "REUSE.toml").write_text("version = 1")
     (empty_directory / ".reuse").mkdir()
-    (empty_directory / ".reuse/dep5").touch()
+    shutil.copy(RESOURCES_DIRECTORY / "dep5", empty_directory / ".reuse/dep5")
     with pytest.raises(GlobalLicensingConflict):
         Project.from_directory(empty_directory)