refactor: raise errors in validate()

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

@@ -705,7 +705,6 @@ def _run_cli_html(args: argparse.Namespace) -> None:
                            args.chunk_toc_depth, args.section_toc_depth, args.media_dir),
             json.load(manpage_urls), Redirects(redirects, redirects_script.read()))
         md.convert(args.infile, args.outfile)
-        md._redirects.raise_errors()
 
 def build_cli(p: argparse.ArgumentParser) -> None:
     formats = p.add_subparsers(dest='format', required=True)

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

@@ -1,5 +1,5 @@
 import json
-from dataclasses import dataclass, field
+from dataclasses import dataclass
 
 from .manual_structure import XrefTarget
 
@@ -13,8 +13,6 @@ class Redirects:
     _raw_redirects: dict[str, list[str]]
     _redirects_script: str
 
-    _errors: list[RedirectsError] = field(default_factory=list)
-
     def validate(self, xref_targets: dict[str, XrefTarget]):
         """
         Validate redirection mappings against element locations in the output
@@ -28,10 +26,9 @@ def validate(self, xref_targets: dict[str, XrefTarget]):
         """
         identifiers_without_redirects = xref_targets.keys() - self._raw_redirects.keys()
         orphan_identifiers = self._raw_redirects.keys() - xref_targets.keys()
-        if orphan_identifiers:
-            self._errors.append(OrphanIdentifiersError(orphan_identifiers))
 
         client_side_redirects = {}
+        errors = []
         server_side_redirects = {}
         all_client_locations = set()
         conflicting_anchors = set()
@@ -66,41 +63,38 @@ def validate(self, xref_targets: dict[str, XrefTarget]):
                         server_side_redirects[location] = xref_targets[identifier].path
                     else:
                         divergent_redirects.add(location)
-        if divergent_redirects:
-            self._errors.append(DivergentRedirectError(divergent_redirects))
-        if identifiers_missing_current_outpath:
-            self._errors.append(InvalidCurrentPathError(identifiers_missing_current_outpath))
-        if identifiers_without_redirects:
-            self._errors.append(MissingRedirectError(identifiers_without_redirects))
 
         for target in {location.split('#')[0] for location in all_client_locations}:
             identifiers = [identifier for identifier, xref in xref_targets.items() if xref.path == target]
             anchors = {location.split('#')[1] for location in all_client_locations}
             conflicting_anchors |= anchors.intersection(identifiers)
-        if conflicting_anchors:
-            self._errors.append(ConflictingAnchorsError(conflicting_anchors))
 
         client_paths_with_server_redirects = {}
         for server_from, server_to in server_side_redirects.items():
             for client_from, client_to in client_side_redirects.items():
                 path, anchor = client_from.split('#')
                 if server_from == path:
                     client_paths_with_server_redirects[client_from] = f"{server_to}#{anchor}"
+
         if client_paths_with_server_redirects:
-            self._errors.append(ClientPathWithServerRedirectError(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))
 
-    def raise_errors(self, error_type: RedirectsError = None):
-        """
-        Raises all errors if error_type is not specified.
-        If error_type is specified, only raises error of that type.
-        """
-        errors_to_raise = [error for error in self._errors if error_type is None or isinstance(error, error_type)]
-        if len(errors_to_raise) == 0:
-            return
-        if len(errors_to_raise) == 1:
-            raise errors_to_raise[0]
+        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_to_raise))}")
+            raise RedirectsError(f"Multiple validation errors occurred:\n{'\n'.join(map(str, errors))}")
 
     def get_client_redirects(self, redirection_target: str):
         client_redirects = {}

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

@@ -1,13 +1,14 @@
 import json
 import unittest
 from pathlib import Path
+from typing import Type
 
 from nixos_render_docs.manual import HTMLConverter, HTMLParameters
-from nixos_render_docs.redirects import Redirects, ClientPathWithServerRedirectError, ConflictingAnchorsError, DivergentRedirectError, InvalidCurrentPathError, MissingRedirectError, OrphanIdentifiersError
+from nixos_render_docs.redirects import Redirects, RedirectsError, ClientPathWithServerRedirectError, ConflictingAnchorsError, DivergentRedirectError, InvalidCurrentPathError, MissingRedirectError, OrphanIdentifiersError
 
 
 class TestRedirects(unittest.TestCase):
-    def setup_boilerplate(self, sources, redirects):
+    def setup_test(self, sources, raw_redirects):
         with open(Path(__file__).parent / 'index.md', 'w') as infile:
             indexHTML = ["# Redirects test suite {#redirects-test-suite}\n## Setup steps"]
             for path in sources.keys():
@@ -19,156 +20,161 @@ def setup_boilerplate(self, sources, redirects):
             with open(Path(__file__).parent / filename, 'w') as infile:
                 infile.write(content)
 
-        md = HTMLConverter("1.0.0", HTMLParameters("", [], [], 2, 2, 2, Path("")), {}, Redirects({"redirects-test-suite": ["index.html"]} | redirects, ''))
+        redirects = Redirects({"redirects-test-suite": ["index.html"]} | raw_redirects, '')
+        return HTMLConverter("1.0.0", HTMLParameters("", [], [], 2, 2, 2, Path("")), {}, redirects)
+
+    def run_test(self, md: HTMLConverter):
         md.convert(Path(__file__).parent / 'index.md', Path(__file__).parent / 'index.html')
 
-        return md._redirects
+    def assert_redirect_error(self, error: Type[RedirectsError], md: HTMLConverter):
+        with self.assertRaises(RuntimeError) as context:
+            self.run_test(md)
+        self.assertIsInstance(context.exception.__cause__, error)
 
     def test_identifier_added(self):
         """Test adding a new identifier to the source."""
-        before = self.setup_boilerplate(
+        before = self.setup_test(
             sources={"foo.md": "# Foo {#foo}"},
-            redirects={"foo": ["foo.html"]},
+            raw_redirects={"foo": ["foo.html"]},
         )
-        before.raise_errors()
+        self.run_test(before)
 
-        intermediate = self.setup_boilerplate(
+        intermediate = self.setup_test(
             sources={"foo.md": "# Foo {#foo}\n## Bar {#bar}"},
-            redirects={"foo": ["foo.html"]},
+            raw_redirects={"foo": ["foo.html"]},
         )
-        self.assertRaises(MissingRedirectError, lambda: intermediate.raise_errors(MissingRedirectError))
+        self.assert_redirect_error(MissingRedirectError, intermediate)
 
-        after = self.setup_boilerplate(
+        after = self.setup_test(
             sources={"foo.md": "# Foo {#foo}\n## Bar {#bar}"},
-            redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
+            raw_redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
         )
-        after.raise_errors()
+        self.run_test(after)
 
     def test_identifier_removed(self):
         """Test removing an identifier from the source."""
-        before = self.setup_boilerplate(
+        before = self.setup_test(
             sources={"foo.md": "# Foo {#foo}\n## Bar {#bar}"},
-            redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
+            raw_redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
         )
-        before.raise_errors()
+        self.run_test(before)
 
-        intermediate = self.setup_boilerplate(
+        intermediate = self.setup_test(
             sources={"foo.md": "# Foo {#foo}"},
-            redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
+            raw_redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
         )
-        self.assertRaises(OrphanIdentifiersError, lambda: intermediate.raise_errors(OrphanIdentifiersError))
+        self.assert_redirect_error(OrphanIdentifiersError, intermediate)
 
-        after = self.setup_boilerplate(
+        after = self.setup_test(
             sources={"foo.md": "# Foo {#foo}"},
-            redirects={"foo": ["foo.html"]},
+            raw_redirects={"foo": ["foo.html"]},
         )
-        after.raise_errors()
+        self.run_test(after)
 
     def test_identifier_renamed(self):
         """Test renaming an identifier in the source."""
-        before = self.setup_boilerplate(
+        before = self.setup_test(
             sources={"foo.md": "# Foo {#foo}\n## Bar {#bar}"},
-            redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
+            raw_redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
         )
-        before.raise_errors()
+        self.run_test(before)
 
-        intermediate = self.setup_boilerplate(
+        intermediate = self.setup_test(
             sources={"foo.md": "# Foo Prime {#foo-prime}\n## Bar {#bar}"},
-            redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
+            raw_redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
         )
-        self.assertRaises(MissingRedirectError, lambda: intermediate.raise_errors(MissingRedirectError))
-        self.assertRaises(OrphanIdentifiersError, lambda: intermediate.raise_errors(OrphanIdentifiersError))
+        self.assert_redirect_error(RedirectsError, intermediate)
 
-        after = self.setup_boilerplate(
+        after = self.setup_test(
             sources={"foo.md": "# Foo Prime {#foo-prime}\n## Bar {#bar}"},
-            redirects={"foo-prime": ["foo.html", "foo.html#foo"], "bar": ["foo.html"]},
+            raw_redirects={"foo-prime": ["foo.html", "foo.html#foo"], "bar": ["foo.html"]},
         )
-        after.raise_errors()
+        self.run_test(after)
 
     def test_leaf_identifier_moved_to_different_file(self):
         """Test moving a leaf identifier to a different output path."""
-        before = self.setup_boilerplate(
+        before = self.setup_test(
             sources={"foo.md": "# Foo {#foo}\n## Bar {#bar}"},
-            redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
+            raw_redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
         )
-        before.raise_errors()
+        self.run_test(before)
 
-        intermediate = self.setup_boilerplate(
+        intermediate = self.setup_test(
             sources={
                 "foo.md": "# Foo {#foo}",
                 "bar.md": "# Bar {#bar}"
             },
-            redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
+            raw_redirects={"foo": ["foo.html"], "bar": ["foo.html"]},
         )
-        self.assertRaises(InvalidCurrentPathError, lambda: intermediate.raise_errors(InvalidCurrentPathError))
+        self.assert_redirect_error(InvalidCurrentPathError, intermediate)
 
-        after = self.setup_boilerplate(
+        after = self.setup_test(
             sources={
                 "foo.md": "# Foo {#foo}",
                 "bar.md": "# Bar {#bar}"
             },
-            redirects={"foo": ["foo.html"], "bar": ["bar.html", "foo.html#bar"]},
+            raw_redirects={"foo": ["foo.html"], "bar": ["bar.html", "foo.html#bar"]},
         )
-        after.raise_errors()
+        self.run_test(after)
 
     def test_non_leaf_identifier_moved_to_different_file(self):
         """Test moving a non-leaf identifier to a different output path."""
-        before = self.setup_boilerplate(
+        before = self.setup_test(
             sources={"foo.md": "# Foo {#foo}\n## Bar {#bar}\n### Baz {#baz}"},
-            redirects={"foo": ["foo.html"], "bar": ["foo.html"], "baz": ["foo.html"]},
+            raw_redirects={"foo": ["foo.html"], "bar": ["foo.html"], "baz": ["foo.html"]},
         )
-        before.raise_errors()
+        self.run_test(before)
 
-        intermediate = self.setup_boilerplate(
+        intermediate = self.setup_test(
             sources={
                 "foo.md": "# Foo {#foo}",
                 "bar.md": "# Bar {#bar}\n## Baz {#baz}"
             },
-            redirects={"foo": ["foo.html"], "bar": ["foo.html"], "baz": ["foo.html"]},
+            raw_redirects={"foo": ["foo.html"], "bar": ["foo.html"], "baz": ["foo.html"]},
         )
-        self.assertRaises(InvalidCurrentPathError, lambda: intermediate.raise_errors(InvalidCurrentPathError))
+        self.assert_redirect_error(InvalidCurrentPathError, intermediate)
 
-        after = self.setup_boilerplate(
+        after = self.setup_test(
             sources={
                 "foo.md": "# Foo {#foo}",
                 "bar.md": "# Bar {#bar}\n## Baz {#baz}"
             },
-            redirects={"foo": ["foo.html"], "bar": ["bar.html", "foo.html#bar"], "baz": ["bar.html", "foo.html#baz"]},
+            raw_redirects={"foo": ["foo.html"], "bar": ["bar.html", "foo.html#bar"], "baz": ["bar.html", "foo.html#baz"]},
         )
-        after.raise_errors()
+        self.run_test(after)
 
     def test_conflicting_anchors(self):
         """Test for conflicting anchors."""
-        with self.assertRaises(ConflictingAnchorsError):
-            self.setup_boilerplate(
-                sources={"foo.md": "# Foo {#foo}\n## Bar {#bar}"},
-                redirects={
-                    "foo": ["foo.html", "foo.html#bar"],
-                    "bar": ["foo.html"],
-                }
-            ).raise_errors(ConflictingAnchorsError)
+        md = self.setup_test(
+            sources={"foo.md": "# Foo {#foo}\n## Bar {#bar}"},
+            raw_redirects={
+                "foo": ["foo.html", "foo.html#bar"],
+                "bar": ["foo.html"],
+            }
+        )
+        self.assert_redirect_error(ConflictingAnchorsError, md)
 
     def test_divergent_redirect(self):
         """Test for divergent redirects."""
-        with self.assertRaises(DivergentRedirectError):
-            self.setup_boilerplate(
-                sources={
-                    "foo.md": "# Foo {#foo}",
-                    "bar.md": "# Bar {#bar}"
-                },
-                redirects={
-                    "foo": ["foo.html", "old-foo.html"],
-                    "bar": ["bar.html", "old-foo.html"]
-                }
-            ).raise_errors(DivergentRedirectError)
+        md = self.setup_test(
+            sources={
+                "foo.md": "# Foo {#foo}",
+                "bar.md": "# Bar {#bar}"
+            },
+            raw_redirects={
+                "foo": ["foo.html", "old-foo.html"],
+                "bar": ["bar.html", "old-foo.html"]
+            }
+        )
+        self.assert_redirect_error(DivergentRedirectError, md)
 
     def test_client_path_with_server_redirect(self):
         """Test for client paths with server redirects."""
-        with self.assertRaises(ClientPathWithServerRedirectError):
-            self.setup_boilerplate(
-                sources={"foo.md": "# Foo {#foo}"},
-                redirects={"foo": ["foo.html", "bar.html", "bar.html#foo"]}
-            ).raise_errors(ClientPathWithServerRedirectError)
+        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)
 
 
 class TestGetClientRedirects(unittest.TestCase):