refactor: split the site validator into its two passes

validate_site reached complexity 43 doing three things at once. The
per-page checks and the link resolution are now separate functions and
the caller keeps only the cross-page duplicate check, which cannot run
until every page has been seen.

The first attempt left the link pass calling a closure that had moved,
and running it against the real site did not catch that: no page there
has a broken link, so the error path never ran. The unit test covering
a deliberately broken fragment did.
This commit is contained in:
Abdessamad Derraz committed 2026-08-11 20:16:11 +02:00
1 parent 00f10b0379
commit 0a941acc43
1 file changed
+46 -17
+46 -17
View File
@@ -147,20 +147,17 @@ def _local_target(
return candidates, unquote(parsed.fragment) return candidates, unquote(parsed.fragment)
def validate_site(site: Path, config_path: Path) -> list[str]: def _check_pages(
site = site.resolve() pages: dict, site: Path, titles: dict, descriptions: dict
if not site.is_dir(): ) -> list[str]:
return [f"site directory does not exist: {site}"] """Every per-page check: title, description, links, JSON-LD.
base_path = _base_path(config_path) Split out of validate_site, which reached complexity 43. The
html_paths = sorted(site.rglob("*.html")) cross-page duplicate check stays with the caller, since it can only
if not html_paths: run once every page has been seen. *titles* and *descriptions* are
return [f"no HTML pages found in {site}"] filled here for it.
"""
pages = {path.resolve(): _parse_page(path) for path in html_paths}
issues: list[str] = [] issues: list[str] = []
titles: dict[str, list[Path]] = defaultdict(list)
descriptions: dict[str, list[Path]] = defaultdict(list)
def report(path: Path, message: str) -> None: def report(path: Path, message: str) -> None:
issues.append(f"{path.relative_to(site)}: {message}") issues.append(f"{path.relative_to(site)}: {message}")
@@ -207,11 +204,15 @@ def validate_site(site: Path, config_path: Path) -> list[str]:
if not isinstance(payload, dict) or payload.get("@context") != "https://schema.org": if not isinstance(payload, dict) or payload.get("@context") != "https://schema.org":
report(path, "JSON-LD is not a schema.org object") report(path, "JSON-LD is not a schema.org object")
for label, values in (("title", titles), ("description", descriptions)): return issues
for value, paths in values.items():
if len(paths) > 1:
rendered = ", ".join(str(path.relative_to(site)) for path in paths[:5]) def _check_links(pages: dict, site: Path, base_path: str) -> list[str]:
issues.append(f"duplicate {label} {value!r}: {rendered}") """Every local link and fragment resolves to something on disk."""
issues: list[str] = []
def report(path: Path, message: str) -> None:
issues.append(f"{path.relative_to(site)}: {message}")
for path, page in pages.items(): for path, page in pages.items():
for href in page.links: for href in page.links:
@@ -234,6 +235,34 @@ def validate_site(site: Path, config_path: Path) -> list[str]:
return issues return issues
def validate_site(site: Path, config_path: Path) -> list[str]:
site = site.resolve()
if not site.is_dir():
return [f"site directory does not exist: {site}"]
base_path = _base_path(config_path)
html_paths = sorted(site.rglob("*.html"))
if not html_paths:
return [f"no HTML pages found in {site}"]
pages = {path.resolve(): _parse_page(path) for path in html_paths}
issues: list[str] = []
titles: dict[str, list[Path]] = defaultdict(list)
descriptions: dict[str, list[Path]] = defaultdict(list)
issues.extend(_check_pages(pages, site, titles, descriptions))
for label, values in (("title", titles), ("description", descriptions)):
for value, paths in values.items():
if len(paths) > 1:
rendered = ", ".join(str(path.relative_to(site)) for path in paths[:5])
issues.append(f"duplicate {label} {value!r}: {rendered}")
issues.extend(_check_links(pages, site, base_path))
return issues
def main() -> None: def main() -> None:
parser = argparse.ArgumentParser(description=__doc__) parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("--site-dir", default="site") parser.add_argument("--site-dir", default="site")