refactor: squash errors into single type

RedirectsError will handle all of the string parsing while giving unit tests all of the freedom to test granularly

pkgs/tools/nix/nixos-render-docs/src/nixos_render_docs/redirects.py

@@ -5,7 +5,106 @@
 
 
 class RedirectsError(Exception):
-    pass
+    def __init__(
+        self,
+        client_paths_with_server_redirects=None,
+        conflicting_anchors=None,
+        divergent_redirects=None,
+        identifiers_missing_current_outpath=None,
+        identifiers_without_redirects=None,
+        orphan_identifiers=None
+    ):
+        self.client_paths_with_server_redirects = client_paths_with_server_redirects or []
+        self.conflicting_anchors = conflicting_anchors or []
+        self.divergent_redirects = divergent_redirects or []
+        self.identifiers_missing_current_outpath = identifiers_missing_current_outpath or []
+        self.identifiers_without_redirects = identifiers_without_redirects or []
+        self.orphan_identifiers = orphan_identifiers or []
+
+    def __str__(self):
+        error_messages = []
+        if self.client_paths_with_server_redirects:
+            error_messages.append(f"""
+**Client Paths with Server Redirects Found**
+A client redirect from a path that has a server-side redirect must not exist.
+
+The following identifiers violate the above rule:
+- {"\n- ".join(f"{source} -> {dest}" for source, dest in self.client_paths_with_server_redirects.items())}
+
+This can generally happen when:
+- A redirect was added that redirects to another redirect
+
+This is problematic because:
+- It could lead to undefined behaviour. If a user goes to such a link, the server-side redirect would activate asynchronously along with the client-side redirect which then would trigger another server-side redirect.
+""")
+        if self.conflicting_anchors:
+            error_messages.append(f"""
+**Conflicting Anchors Found**
+Identifiers must not be identical to any historical location's anchor of the same output path.
+
+The following identifiers violate the above rule:
+- {"\n- ".join(self.conflicting_anchors)}
+
+This can generally happen when:
+- An identifier was added
+
+This is problematic because:
+- It can cause confusion and potentially break links or redirects.
+""")
+        if self.divergent_redirects:
+            error_messages.append(f"""
+**Divergent Redirects Found**
+A given historical path must correspond to only one identifier.
+
+The following paths violate the above rule:
+- {"\n- ".join(self.divergent_redirects)}
+
+This can generally happen when:
+- A redirect was added
+
+This is problematic because:
+- It leads to inconsistent behavior depending on which redirect is applied.
+""")
+        if self.identifiers_missing_current_outpath:
+            error_messages.append(f"""
+**Invalid Current Paths Found**
+The head element of an identifier's corresponding historical location must be its current output location.
+
+The following identifiers violate the above rule:
+- {"\n- ".join(self.identifiers_missing_current_outpath)}
+
+This is problematic because:
+- We use the head of each historical locations list to track if an identifier moved across to another output location.
+""")
+        if self.identifiers_without_redirects:
+            error_messages.append(f"""
+**Redirects Missing**
+Identifiers present in the source must have a mapping in the redirects.
+
+The following identifiers violate the above rule:
+- {"\n- ".join(self.identifiers_without_redirects)}
+
+This can generally happen when:
+- An identifier was added
+- An identifier was renamed
+
+This is problematic because:
+- We cannot track the movement of identifiers reliably if we don't have a reference point.
+""")
+        if self.orphan_identifiers:
+            error_messages.append(f"""
+**Orphan Identifiers Found**
+Keys of the redirects mapping must correspond to some identifier in the source.
+
+The following identifiers violate the above rule:
+- {"\n- ".join(self.orphan_identifiers)}
+
+This can generally happen when:
+- An identifier was removed
+- An identifier was renamed
+""")
+
+        return "\n".join(error_messages)
 
 
 @dataclass
@@ -76,25 +175,22 @@ def validate(self, xref_targets: dict[str, XrefTarget]):
                 if server_from == path:
                     client_paths_with_server_redirects[client_from] = f"{server_to}#{anchor}"
 
-        if client_paths_with_server_redirects:
-            errors.append(ClientPathWithServerRedirectError(client_paths_with_server_redirects))
-        if conflicting_anchors:
-            errors.append(ConflictingAnchorsError(conflicting_anchors))
-        if divergent_redirects:
-            errors.append(DivergentRedirectError(divergent_redirects))
-        if identifiers_missing_current_outpath:
-            errors.append(InvalidCurrentPathError(identifiers_missing_current_outpath))
-        if identifiers_without_redirects:
-            errors.append(MissingRedirectError(identifiers_without_redirects))
-        if orphan_identifiers:
-            errors.append(OrphanIdentifiersError(orphan_identifiers))
-
-        if len(errors) == 0:
-            pass
-        elif len(errors) == 1:
-            raise errors[0]
-        else:
-            raise RedirectsError(f"Multiple validation errors occurred:\n{'\n'.join(map(str, errors))}")
+        if any([
+            client_paths_with_server_redirects,
+            conflicting_anchors,
+            divergent_redirects,
+            identifiers_missing_current_outpath,
+            identifiers_without_redirects,
+            orphan_identifiers
+        ]):
+            raise RedirectsError(
+                client_paths_with_server_redirects=client_paths_with_server_redirects,
+                conflicting_anchors=conflicting_anchors,
+                divergent_redirects=divergent_redirects,
+                identifiers_missing_current_outpath=identifiers_missing_current_outpath,
+                identifiers_without_redirects=identifiers_without_redirects,
+                orphan_identifiers=orphan_identifiers
+            )
 
     def get_client_redirects(self, redirection_target: str):
         client_redirects = {}
@@ -111,126 +207,3 @@ def get_client_redirects(self, redirection_target: str):
     def get_redirect_script(self, redirection_target: str) -> str:
         client_redirects = self.get_client_redirects(redirection_target)
         return self._redirects_script.replace('REDIRECTS_PLACEHOLDER', json.dumps(client_redirects))
-
-
-class ClientPathWithServerRedirectError(RedirectsError):
-    def __init__(self, client_paths_with_server_redirects: dict[str, str]):
-        self.client_paths_with_server_redirects = client_paths_with_server_redirects
-        super().__init__(self.__str__())
-
-    def __str__(self):
-        client_paths_with_server_redirects_list = "\n- ".join(f"{source} -> {dest}" for source, dest in self.client_paths_with_server_redirects.items())
-        return f"""
-**Client Paths with Server Redirects Found**
-A client redirect from a path that has a server-side redirect must not exist.
-
-The following identifiers violate the above rule:
-- {client_paths_with_server_redirects_list}
-
-This can generally happen when:
-- A redirect was added that redirects to another redirect
-
-This is problematic because:
-- It could lead to undefined behaviour. If a user goes to such a link, the server-side redirect would activate asynchronously along with the client-side redirect which then would trigger another server-side redirect.
-"""
-
-class ConflictingAnchorsError(RedirectsError):
-    def __init__(self, conflicts: set[str]):
-        self.conflicts = conflicts
-        super().__init__(self.__str__())
-
-    def __str__(self):
-        conflict_list = "\n- ".join(sorted(self.conflicts))
-        return f"""
-**Conflicting Anchors Found**
-Identifiers must not be identical to any historical location's anchor of the same output path.
-
-The following identifiers violate the above rule:
-- {conflict_list}
-
-This can generally happen when:
-- An identifier was added
-
-This is problematic because:
-- It can cause confusion and potentially break links or redirects.
-"""
-
-class DivergentRedirectError(RedirectsError):
-    def __init__(self, divergent: set[str]):
-        self.divergent = divergent
-        super().__init__(self.__str__())
-
-    def __str__(self):
-        divergent_list = "\n- ".join(sorted(self.divergent))
-        return f"""
-**Divergent Redirects Found**
-A given historical path must correspond to only one identifier.
-
-The following paths violate the above rule:
-- {divergent_list}
-
-This can generally happen when:
-- A redirect was added
-
-This is problematic because:
-- It leads to inconsistent behavior depending on which redirect is applied.
-"""
-
-class InvalidCurrentPathError(RedirectsError):
-    def __init__(self, invalid: set[str]):
-        self.invalid = invalid
-        super().__init__(self.__str__())
-
-    def __str__(self):
-        invalid_list = "\n- ".join(sorted(self.invalid))
-        return f"""
-**Invalid Current Paths Found**
-The head element of an identifier's corresponding historical location must be its current output location.
-
-The following identifiers violate the above rule:
-- {invalid_list}
-
-This is problematic because:
-- We use the head of each historical locations list to track if an identifier moved across to another output location.
-"""
-
-class MissingRedirectError(RedirectsError):
-    def __init__(self, missing: set[str]):
-        self.missing = missing
-        super().__init__(self.__str__())
-
-    def __str__(self):
-        missing_list = "\n- ".join(sorted(self.missing))
-        return f"""
-**Redirects Missing**
-Identifiers present in the source must have a mapping in the redirects.
-
-The following identifiers violate the above rule:
-- {missing_list}
-
-This can generally happen when:
-- An identifier was added
-- An identifier was renamed
-
-This is problematic because:
-- We cannot track the movement of identifiers reliably if we don't have a reference point.
-"""
-
-class OrphanIdentifiersError(RedirectsError):
-    def __init__(self, orphans: set[str]):
-        self.orphans = orphans
-        super().__init__(self.__str__())
-
-    def __str__(self):
-        orphan_list = "\n- ".join(sorted(self.orphans))
-        return f"""
-**Orphan Identifiers Found**
-Keys of the redirects mapping must correspond to some identifier in the source.
-
-The following identifiers violate the above rule:
-- {orphan_list}
-
-This can generally happen when:
-- An identifier was removed
-- An identifier was renamed
-"""

pkgs/tools/nix/nixos-render-docs/src/tests/test_redirects.py

@@ -4,7 +4,7 @@
 from typing import Type
 
 from nixos_render_docs.manual import HTMLConverter, HTMLParameters
-from nixos_render_docs.redirects import Redirects, RedirectsError, ClientPathWithServerRedirectError, ConflictingAnchorsError, DivergentRedirectError, InvalidCurrentPathError, MissingRedirectError, OrphanIdentifiersError
+from nixos_render_docs.redirects import Redirects, RedirectsError
 
 
 class TestRedirects(unittest.TestCase):
@@ -26,10 +26,17 @@ def setup_test(self, sources, raw_redirects):
     def run_test(self, md: HTMLConverter):
         md.convert(Path(__file__).parent / 'index.md', Path(__file__).parent / 'index.html')
 
-    def assert_redirect_error(self, error: Type[RedirectsError], md: HTMLConverter):
+    def assert_redirect_error(self, expected_errors: dict, md: HTMLConverter):
         with self.assertRaises(RuntimeError) as context:
             self.run_test(md)
-        self.assertIsInstance(context.exception.__cause__, error)
+
+        exception = context.exception.__cause__
+        self.assertIsInstance(exception, RedirectsError)
+
+        for attr, expected_values in expected_errors.items():
+            self.assertTrue(hasattr(exception, attr))
+            actual_values = getattr(exception, attr)
+            self.assertEqual(set(actual_values), set(expected_values))
 
     def test_identifier_added(self):
         """Test adding a new identifier to the source."""
@@ -43,7 +50,7 @@ def test_identifier_added(self):
             sources={"foo.md": "# Foo {#foo}\n## Bar {#bar}"},
             raw_redirects={"foo": ["foo.html"]},
         )
-        self.assert_redirect_error(MissingRedirectError, intermediate)
+        self.assert_redirect_error({"identifiers_without_redirects": ["bar"]}, intermediate)
 
         after = self.setup_test(
             sources={"foo.md": "# Foo {#foo}\n## Bar {#bar}"},
@@ -63,7 +70,7 @@ def test_identifier_removed(self):
             sources={"foo.md": "# Foo {#foo}"},
             raw_redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
         )
-        self.assert_redirect_error(OrphanIdentifiersError, intermediate)
+        self.assert_redirect_error({"orphan_identifiers": ["bar"]}, intermediate)
 
         after = self.setup_test(
             sources={"foo.md": "# Foo {#foo}"},
@@ -83,7 +90,13 @@ def test_identifier_renamed(self):
             sources={"foo.md": "# Foo Prime {#foo-prime}\n## Bar {#bar}"},
             raw_redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
         )
-        self.assert_redirect_error(RedirectsError, intermediate)
+        self.assert_redirect_error(
+            {
+                "identifiers_without_redirects": ["foo-prime"],
+                "orphan_identifiers": ["foo"]
+            },
+            intermediate
+        )
 
         after = self.setup_test(
             sources={"foo.md": "# Foo Prime {#foo-prime}\n## Bar {#bar}"},
@@ -106,7 +119,7 @@ def test_leaf_identifier_moved_to_different_file(self):
             },
             raw_redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
         )
-        self.assert_redirect_error(InvalidCurrentPathError, intermediate)
+        self.assert_redirect_error({"identifiers_missing_current_outpath": ["bar"]}, intermediate)
 
         after = self.setup_test(
             sources={
@@ -132,7 +145,7 @@ def test_non_leaf_identifier_moved_to_different_file(self):
             },
             raw_redirects={"foo": ["foo.html"], "bar": ["foo.html"], "baz": ["foo.html"]},
         )
-        self.assert_redirect_error(InvalidCurrentPathError, intermediate)
+        self.assert_redirect_error({"identifiers_missing_current_outpath": ["bar", "baz"]}, intermediate)
 
         after = self.setup_test(
             sources={
@@ -152,7 +165,7 @@ def test_conflicting_anchors(self):
                 "bar": ["foo.html"],
             }
         )
-        self.assert_redirect_error(ConflictingAnchorsError, md)
+        self.assert_redirect_error({"conflicting_anchors": ["bar"]}, md)
 
     def test_divergent_redirect(self):
         """Test for divergent redirects."""
@@ -166,15 +179,15 @@ def test_divergent_redirect(self):
                 "bar": ["bar.html", "old-foo.html"]
             }
         )
-        self.assert_redirect_error(DivergentRedirectError, md)
+        self.assert_redirect_error({"divergent_redirects": ["old-foo.html"]}, md)
 
     def test_client_path_with_server_redirect(self):
         """Test for client paths with server redirects."""
         md = self.setup_test(
             sources={"foo.md": "# Foo {#foo}"},
             raw_redirects={"foo": ["foo.html", "bar.html", "bar.html#foo"]}
         )
-        self.assert_redirect_error(ClientPathWithServerRedirectError, md)
+        self.assert_redirect_error({"client_paths_with_server_redirects": ["bar.html#foo"]}, md)
 
 
 class TestGetClientRedirects(unittest.TestCase):