diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index b299a3a..1f6ab43 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -281,6 +281,16 @@ always_run: true additional_dependencies: - pre-commit==4.6.0 +- id: check-stale-references + name: Check for stale references to deleted/renamed files + description: |- + When files are deleted or renamed, check that no remaining tracked file still references the old path. + Searches for both the full repo-relative path and the basename of deleted files. + This catches broken cross-references in dead code such as comments, documentation, and configs. + entry: check-stale-references + language: python + pass_filenames: false + always_run: true - id: check-max-one-sentence-per-line name: Check max one sentence per line description: |- diff --git a/README.md b/README.md index 0328328..b8a77bb 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,6 @@ These tools are used to help developers in their day-to-day tasks. > I try to contribute back if possible but since I am no longer working at Luminar, it is not guaranteed that my contributions are accepted. - - [Tools](#tools) - [Configure VS Code for Bazel](#configure-vs-code-for-bazel) - [Whoowns](#whoowns) @@ -39,10 +38,10 @@ These tools are used to help developers in their day-to-day tasks. - [`print-pre-commit-metrics`](#print-pre-commit-metrics) - [`sync-vscode-config`](#sync-vscode-config) - [`sync-tool-versions`](#sync-tool-versions) + - [`check-stale-references`](#check-stale-references) - [`check-max-one-sentence-per-line`](#check-max-one-sentence-per-line) - [`check-ownership`](#check-ownership) - [Contributing](#contributing) - ## Tools @@ -225,6 +224,12 @@ sync_versions: version_override: '314' ``` +### `check-stale-references` + +When files are deleted or renamed, check that no remaining tracked file still references the old path. +Searches for both the full repo-relative path and the basename of deleted files. +This catches broken cross-references in dead code such as comments, documentation, and configs. + ### `check-max-one-sentence-per-line` This hook is a simplified version of [rumdl MD013 sentence-per-line-mode](https://github.com/rvben/rumdl/blob/main/docs/md013.md#sentence-per-line-mode). diff --git a/dev_tools/check_stale_references.py b/dev_tools/check_stale_references.py new file mode 100644 index 0000000..8908f54 --- /dev/null +++ b/dev_tools/check_stale_references.py @@ -0,0 +1,89 @@ +from __future__ import annotations + +import re +import subprocess +import sys +from dataclasses import dataclass + + +@dataclass(frozen=True) +class StaleReference: + """A reference in a tracked file that points to a deleted/renamed path.""" + + from_file: str + from_line: int + to_path: str + + def __str__(self) -> str: + """Format as file:line references deleted_path.""" + return f"{self.from_file}:{self.from_line} references {self.to_path}" + + +def _run_git(*args: str, check: bool = True) -> str: + return subprocess.run( + ["git", *args], # noqa: S607 + capture_output=True, + text=True, + check=check, + ).stdout + + +def get_deleted_paths() -> set[str]: + """Return repo-relative paths of files being deleted or renamed away in the staged commit.""" + output = _run_git("diff", "--cached", "--diff-filter=DR", "--name-status") + # Both D(eleted) and R(enamed) have the vanishing path in column 1 + return {line.split("\t")[1] for line in output.splitlines()} + + +def build_path_pattern(deleted_path: str) -> str: + """Build a PCRE pattern for a deleted path that matches references to it. + + For path "a/b/c.txt", matches (with non-word/dot/dash boundaries): + - /?a/b/c.txt (full path, optional leading /) + - (../)*b/c.txt (intermediate suffix with optional ../ prefix) + - (../)*c.txt (basename with optional ../ prefix) + """ + segments = deleted_path.split("/") + alternatives: list[str] = [] + for i in range(len(segments)): + suffix = "/".join(segments[i:]) + escaped_suffix = re.escape(suffix) + if i == 0: + alternatives.append(rf"/?{escaped_suffix}") + else: + alternatives.append(rf"(?:\.\./)*{escaped_suffix}") + return rf"(? list[tuple[str, int]]: + """Run git grep with PCRE and return (file, line_number) tuples.""" + output = _run_git("grep", "-nP", pattern, check=False) + matches: list[tuple[str, int]] = [] + for line in output.splitlines(): + file, line_no = line.split(":", 1) + matches.append((file, int(line_no))) + return matches + + +def find_stale_references() -> list[StaleReference]: + """Search tracked files for references to deleted paths.""" + deleted_paths = get_deleted_paths() + return [ + StaleReference(from_file=file, from_line=line_no, to_path=deleted_path) + for deleted_path in deleted_paths + for file, line_no in git_grep(build_path_pattern(deleted_path)) + if file not in deleted_paths + ] + + +def main() -> int: + if stale_refs := find_stale_references(): + print("Stale references to deleted/renamed files:") + for ref in stale_refs: + print(f" {ref}") + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/pyproject.toml b/pyproject.toml index 0ff56d6..b4d8c7e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,6 +45,7 @@ generate-hook-docs = "dev_tools.generate_hook_docs:main" print-pre-commit-metrics = "dev_tools.print_pre_commit_metrics:main" sync-vscode-config = "dev_tools.sync_vscode_config:main" sync-tool-versions = "dev_tools.sync_tool_versions:main" +check-stale-references = "dev_tools.check_stale_references:main" [tool.pytest.ini_options] addopts = "--cov-report term-missing --cov=dev_tools --cov=packages/whoowns/whoowns --cov=packages/pre_commit_excludes/pre_commit_excludes --cov=packages/configure_vscode_for_bazel/configure_vscode_for_bazel -vv" diff --git a/tests/test_check_stale_references.py b/tests/test_check_stale_references.py new file mode 100644 index 0000000..441122a --- /dev/null +++ b/tests/test_check_stale_references.py @@ -0,0 +1,104 @@ +from __future__ import annotations + +import re +from unittest.mock import patch + +from dev_tools.check_stale_references import ( + StaleReference, + build_path_pattern, + find_stale_references, +) + + +def test_matches_full_path() -> None: + pattern = build_path_pattern("src/lib/foo.hpp") + assert re.search(pattern, "see src/lib/foo.hpp for details") + + +def test_matches_full_path_with_leading_slash() -> None: + pattern = build_path_pattern("src/lib/foo.hpp") + assert re.search(pattern, "see /src/lib/foo.hpp for details") + + +def test_matches_partial_path() -> None: + pattern = build_path_pattern("src/lib/foo.hpp") + assert re.search(pattern, "see lib/foo.hpp here") + + +def test_matches_partial_path_with_dotdot() -> None: + pattern = build_path_pattern("src/lib/foo.hpp") + assert re.search(pattern, "see ../lib/foo.hpp here") + + +def test_matches_basename() -> None: + pattern = build_path_pattern("src/lib/foo.hpp") + assert re.search(pattern, "see foo.hpp for details") + + +def test_matches_basename_with_dotdot() -> None: + pattern = build_path_pattern("src/lib/foo.hpp") + assert re.search(pattern, "see ../../foo.hpp for details") + + +def test_no_substring_match() -> None: + pattern = build_path_pattern("src/lib/foo.hpp") + assert not re.search(pattern, "notfoo.hpp") + + +def test_no_match_with_dot_suffix() -> None: + pattern = build_path_pattern("src/lib/foo.hpp") + assert not re.search(pattern, "foo.hpp.bak") + + +def test_no_match_with_dash_suffix() -> None: + pattern = build_path_pattern("src/lib/foo.hpp") + assert not re.search(pattern, "foo.hpp-old") + + +def test_dot_in_extension_is_literal() -> None: + pattern = build_path_pattern("src/lib/foo.hpp") + assert not re.search(pattern, "fooXhpp") + + +@patch("dev_tools.check_stale_references.git_grep") +@patch("dev_tools.check_stale_references.get_deleted_paths") +def test_finds_reference_to_deleted_file(mock_deleted, mock_grep) -> None: + mock_deleted.return_value = {"src/utils/helper.hpp"} + mock_grep.return_value = [("src/main.cpp", 1)] + result = find_stale_references() + assert result == [StaleReference("src/main.cpp", 1, "src/utils/helper.hpp")] + + +@patch("dev_tools.check_stale_references.git_grep") +@patch("dev_tools.check_stale_references.get_deleted_paths") +def test_no_reference_no_finding(mock_deleted, mock_grep) -> None: + mock_deleted.return_value = {"src/utils/helper.hpp"} + mock_grep.return_value = [] + result = find_stale_references() + assert result == [] + + +@patch("dev_tools.check_stale_references.git_grep") +@patch("dev_tools.check_stale_references.get_deleted_paths") +def test_skips_deleted_file_itself(mock_deleted, mock_grep) -> None: + mock_deleted.return_value = {"src/utils/helper.hpp"} + mock_grep.return_value = [("src/utils/helper.hpp", 1)] + result = find_stale_references() + assert result == [] + + +@patch("dev_tools.check_stale_references.get_deleted_paths") +def test_empty_deleted_paths_returns_empty(mock_deleted) -> None: + mock_deleted.return_value = set() + assert find_stale_references() == [] + + +@patch("dev_tools.check_stale_references.git_grep") +@patch("dev_tools.check_stale_references.get_deleted_paths") +def test_multiple_files_with_references(mock_deleted, mock_grep) -> None: + mock_deleted.return_value = {"src/helper.hpp"} + mock_grep.return_value = [ + ("a.cpp", 1), + ("c.md", 3), + ] + assert {reference.from_file for reference in find_stale_references()} == {"a.cpp", "c.md"}