diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index b299a3a..0930b77 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -301,6 +301,19 @@ - markdown - rst - tex +- id: check-repo-links + name: Check @repo links + description: |- + Check that `@repo` relative links in text files point to existing files or directories. + + Mark a link with the literal `@repo` marker followed by whitespace and a path, e.g. `// see @repo src/foo.hpp`. + Paths starting with `/` are resolved from the repository root, all others relative to the file the link appears in. + Existence is checked against `git ls-files`, so links may point to any tracked file or directory. + URLs, mailto links and anchor-only targets are ignored. + entry: check-repo-links + language: python + types: + - text - id: check-ownership name: Check CODEOWNERS consistency (can take a while) description: | diff --git a/README.md b/README.md index 1461123..ab80b07 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,7 @@ These tools are used to help developers in their day-to-day tasks. - [`sync-vscode-config`](#sync-vscode-config) - [`sync-tool-versions`](#sync-tool-versions) - [`check-max-one-sentence-per-line`](#check-max-one-sentence-per-line) + - [`check-repo-links`](#check-repo-links) - [`check-ownership`](#check-ownership) - [Contributing](#contributing) @@ -232,6 +233,15 @@ Sentences are split on `.`, `!`, or `?` followed by a space and a capital letter This hook doesn't respect surrounding indentation, so be sure to combine it with or a similar formatter that fixes indentation. +### `check-repo-links` + +Check that `@repo` relative links in text files point to existing files or directories. + +Mark a link with the literal `@repo` marker followed by whitespace and a path, e.g. `// see @repo src/foo.hpp`. +Paths starting with `/` are resolved from the repository root, all others relative to the file the link appears in. +Existence is checked against `git ls-files`, so links may point to any tracked file or directory. +URLs, mailto links and anchor-only targets are ignored. + ### `check-ownership` Check if all folders in the `CODEOWNERS` file exist, there are no duplicates, and it has acceptable codeowners. diff --git a/dev_tools/check_repo_links.py b/dev_tools/check_repo_links.py new file mode 100644 index 0000000..7bcea33 --- /dev/null +++ b/dev_tools/check_repo_links.py @@ -0,0 +1,147 @@ +# Copyright (c) Luminar Technologies, Inc. All rights reserved. +# Licensed under the MIT License. + +"""Verify that ``@repo`` relative links in text files point to existing files or directories.""" + +from __future__ import annotations + +import posixpath +import re +import subprocess +import sys +from pathlib import Path +from typing import TYPE_CHECKING, NamedTuple + +from dev_tools.utils.git_hook_utils import parse_arguments + +if TYPE_CHECKING: + from collections.abc import Callable, Iterable, Sequence + +# The marker must not be preceded by a word character or another ``@`` (so ``@report``, ``foo@repo`` +# and ``@@repo`` never match) and must be followed by same-line whitespace and a non-whitespace path. +# Requiring whitespace (rather than any delimiter) avoids the common npm/pnpm scope ``@repo/...``. +MARKER = re.compile(rb"(?" + + +class BrokenLink(NamedTuple): + """A relative ``@repo`` link that does not resolve to a tracked file or directory.""" + + file_path: str + line: int + column: int + link: str + + +def resolve_link_target(link: str, file_path: str) -> str | None: + """Resolve a raw ``@repo`` link to a repo-root-relative path, or None if it should be skipped. + + Links starting with ``/`` are resolved from the repository root, everything else relative to the + file the link appears in. URLs, mailto links and anchor-only targets are skipped. + """ + link = link.split("#", 1)[0].split("?", 1)[0].rstrip(_TRAILING_PUNCTUATION) + if not link or "://" in link or link.startswith("mailto:"): + return None + if link.startswith("/"): + return posixpath.normpath(link[1:]) + return posixpath.normpath(posixpath.join(posixpath.dirname(file_path), link)) + + +def offset_to_line_and_column(content: bytes, offset: int) -> tuple[int, int]: + """Return the 1-based (line, column) of a byte offset within content.""" + line = content.count(b"\n", 0, offset) + 1 + column = offset - content.rfind(b"\n", 0, offset) + return line, column + + +def find_broken_links_in_content( + content: bytes, + file_path: str, + does_target_exist: Callable[[str], bool], +) -> list[BrokenLink]: + """Scan a single file's bytes and return the broken ``@repo`` links it contains.""" + # Fast pre-filter: skip the overwhelming majority of files without running the regex. + if b"@repo" not in content: + return [] + broken_links: list[BrokenLink] = [] + for match in MARKER.finditer(content): + link = match.group(1).decode("utf-8", errors="replace") + target = resolve_link_target(link, file_path) + if target is None or does_target_exist(target): + continue + # Only for a confirmed broken link do we pay for computing the precise location. + line, column = offset_to_line_and_column(content, match.start(1)) + broken_links.append(BrokenLink(file_path, line, column, link)) + return broken_links + + +def find_broken_links( + files_to_check: list[Path], + repository_root: Path, + does_target_exist: Callable[[str], bool], +) -> list[BrokenLink]: + broken_links: list[BrokenLink] = [] + for file in files_to_check: + repo_relative_path = file.resolve().relative_to(repository_root).as_posix() + broken_links.extend(find_broken_links_in_content(file.read_bytes(), repo_relative_path, does_target_exist)) + return broken_links + + +def build_set_of_valid_link_targets(tracked_files: Iterable[str]) -> set[str]: + """Build the set of valid link targets: every tracked file plus all of their parent directories.""" + valid_targets = set(tracked_files) + for path in list(valid_targets): + parent = posixpath.dirname(path) + while parent: + valid_targets.add(parent) + parent = posixpath.dirname(parent) + valid_targets.add(".") # the repository root itself + return valid_targets + + +def build_target_existence_check(tracked_files: Iterable[str]) -> Callable[[str], bool]: + """Return a predicate reporting whether a link target is a tracked file or directory.""" + return build_set_of_valid_link_targets(tracked_files).__contains__ + + +def get_repository_root() -> Path: + output = subprocess.run( + ["git", "rev-parse", "--show-toplevel"], # noqa: S607 + capture_output=True, + check=True, + text=True, + ).stdout + return Path(output.strip()) + + +def list_tracked_files(repository_root: Path) -> list[str]: + output = subprocess.run( + ["git", "-C", str(repository_root), "ls-files", "-z"], # noqa: S607 + capture_output=True, + check=True, + ).stdout + return [path.decode("utf-8", "surrogateescape") for path in output.split(b"\0") if path] + + +def print_broken_links(broken_links: list[BrokenLink]) -> None: + if not broken_links: + return + print("Found broken links:") + for link in broken_links: + print(f"{link.file_path}:{link.line}:{link.column} {link.link}") + + +def main(argv: Sequence[str] | None = None) -> int: + files = parse_arguments(argv).filenames + repository_root = get_repository_root() + does_target_exist = build_target_existence_check(list_tracked_files(repository_root)) + broken_links = find_broken_links(files, repository_root, does_target_exist) + print_broken_links(broken_links) + return 1 if broken_links else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/pyproject.toml b/pyproject.toml index f100f4a..0623ae5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,6 +39,7 @@ check-forbidden-tags = "dev_tools.check_forbidden_tags:main" check-max-one-sentence-per-line = "dev_tools.check_max_one_sentence_per_line:main" check-number-of-lines-count = "dev_tools.check_number_of_lines_count:main" check-ownership = "dev_tools.check_ownership:main" +check-repo-links = "dev_tools.check_repo_links:main" check-shellscript-set-options = "dev_tools.check_shellscript_set_options:main" check-useless-exclude-paths-hooks = "dev_tools.check_useless_exclude_paths_hooks:main" generate-hook-docs = "dev_tools.generate_hook_docs:main" diff --git a/tests/test_check_repo_links.py b/tests/test_check_repo_links.py new file mode 100644 index 0000000..38cd64e --- /dev/null +++ b/tests/test_check_repo_links.py @@ -0,0 +1,145 @@ +# Copyright (c) Luminar Technologies, Inc. All rights reserved. +# Licensed under the MIT License. + +from __future__ import annotations + +from pathlib import Path +from typing import TYPE_CHECKING + +import pytest + +from dev_tools.check_repo_links import ( + BrokenLink, + build_set_of_valid_link_targets, + build_target_existence_check, + find_broken_links_in_content, + main, + offset_to_line_and_column, + resolve_link_target, +) + +if TYPE_CHECKING: + from pyfakefs.fake_filesystem import FakeFilesystem + +TRACKED = ["README.md", "docs/readme.md", "src/foo.hpp", "src/util/helper.h", "src/c.h"] + + +def scan(text: str, file_path: str = "src/foo.cpp", *, tracked: tuple[str, ...] = ()) -> list[BrokenLink]: + return find_broken_links_in_content(text.encode(), file_path, build_target_existence_check(tracked)) + + +# --- resolve_link_target -------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("link", "file_path", "expected"), + [ + ("foo.hpp", "src/foo.cpp", "src/foo.hpp"), + ("../c.h", "src/util/helper.h", "src/c.h"), + ("/src/foo.hpp", "docs/readme.md", "src/foo.hpp"), + ("readme.md", "README.md", "readme.md"), + ("foo.hpp#section", "src/foo.cpp", "src/foo.hpp"), + ("foo.hpp?v=1", "src/foo.cpp", "src/foo.hpp"), + ("foo.hpp.", "src/foo.cpp", "src/foo.hpp"), + ], + ids=[ + "sibling file", + "parent traversal", + "leading slash is repo root", + "file at repo root", + "anchor stripped", + "query stripped", + "trailing punctuation stripped", + ], +) +def test_resolve_link_target(link: str, file_path: str, expected: str) -> None: + assert resolve_link_target(link, file_path) == expected + + +@pytest.mark.parametrize("link", ["https://example.com/x", "http://example.com", "mailto:a@b.com", "#anchor", ""]) +def test_resolve_link_target_skips_non_relative_links(link: str) -> None: + assert resolve_link_target(link, "src/foo.cpp") is None + + +# --- offset_to_line_and_column -------------------------------------------------------------- + + +def test_offset_to_line_and_column() -> None: + content = b"line1\nline2\nXhere" + assert offset_to_line_and_column(content, 0) == (1, 1) + assert offset_to_line_and_column(content, 12) == (3, 1) # 'X' after two newlines + + +# --- build_set_of_valid_link_targets -------------------------------------------------------- + + +def test_build_set_of_valid_link_targets_includes_files_dirs_and_root() -> None: + assert build_set_of_valid_link_targets(["src/util/helper.h", "README.md"]) == { + "src/util/helper.h", + "src/util", + "src", + "README.md", + ".", + } + + +# --- find_broken_links_in_content ----------------------------------------------------------- + + +def test_broken_link_is_reported_with_location() -> None: + assert scan("// see @repo missing/file.md here") == [BrokenLink("src/foo.cpp", 1, 14, "missing/file.md")] + + +@pytest.mark.parametrize( + ("text", "tracked"), + [ + pytest.param("// see @repo foo.hpp for details", ("src/foo.hpp",), id="valid link"), + pytest.param(" * @report_new: populate the report", (), id="doxygen @report tag"), + pytest.param('import { Button } from "@repo/ui";', (), id="npm @repo scope"), + pytest.param("foo@repo missing.md", (), id="marker preceded by word char"), + pytest.param("// @repo https://example.com/page", (), id="url target"), + ], +) +def test_scan_reports_no_broken_links(text: str, tracked: tuple[str, ...]) -> None: + assert scan(text, tracked=tracked) == [] + + +def test_leading_slash_resolves_from_repo_root() -> None: + assert scan("// @repo /src/foo.hpp", file_path="docs/readme.md", tracked=("src/foo.hpp",)) == [] + assert [b.link for b in scan("// @repo /src/nope.hpp", file_path="docs/readme.md")] == ["/src/nope.hpp"] + + +def test_multiple_findings_with_correct_lines_and_columns() -> None: + broken = scan( + "// @repo foo.hpp\n// @repo missing1.md\ncode();\n/* @repo missing2.md */\n", tracked=("src/foo.hpp",) + ) + assert [(b.line, b.column, b.link) for b in broken] == [(2, 10, "missing1.md"), (4, 10, "missing2.md")] + + +# --- main ----------------------------------------------------------------------------------- + + +def test_main_reports_only_broken_links_across_files( + fs: FakeFilesystem, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture, +) -> None: + fs.create_file(Path("src/a.cpp"), contents="// @repo missing.h\n") + fs.create_file(Path("src/b.cpp"), contents="// @repo a.cpp\n") # valid link to the tracked src/a.cpp + monkeypatch.setattr("dev_tools.check_repo_links.get_repository_root", Path.cwd) + monkeypatch.setattr("dev_tools.check_repo_links.list_tracked_files", lambda _root: ["src/a.cpp", "src/b.cpp"]) + assert main(["src/a.cpp", "src/b.cpp"]) == 1 + assert capsys.readouterr().out == "Found broken links:\nsrc/a.cpp:1:10 missing.h\n" + + +def test_main_returns_zero_and_is_silent_for_valid_links( + fs: FakeFilesystem, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture, +) -> None: + fs.create_file(Path("src/a.cpp"), contents="// @repo b.h\n") + fs.create_file(Path("src/b.h"), contents="") + monkeypatch.setattr("dev_tools.check_repo_links.get_repository_root", Path.cwd) + monkeypatch.setattr("dev_tools.check_repo_links.list_tracked_files", lambda _root: ["src/a.cpp", "src/b.h"]) + assert main(["src/a.cpp"]) == 0 + assert capsys.readouterr().out == ""