From c8534a1e987e3502ea4c7468e3275f86283ef454 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Thu, 22 Oct 2026 11:29:57 +0000 Subject: [PATCH 1/8] feat: clang-tidy checks only changed lines and skips without a compilation database - Report only diagnostics on the lines a commit adds or changes, using clang-tidy's --line-filter (so --fix is limited too). The lines come from the staged changes, or from PRE_COMMIT_FROM_REF...PRE_COMMIT_TO_REF for pre-push and run --from-ref/--to-ref. Every line is checked when a file has no changes (run --all-files) or outside git; --all-lines turns the filter off. - Without a compilation database, skip clang-tidy locally with a notice and fail when CI is set. clang-tidy's own compile_commands.json and compile_flags.txt lookup next to the files, -p and --extra-arg count as a database. - Also look in builddir/, cmake-build-*/, build-*/, build/*/ and out/build/*/ for compile_commands.json. - Treat all file types pre-commit tags as c/c++ (.mm, .ino, module units) as source files. --- AGENTS.md | 6 +- README.md | 65 +++++- cpp_linter_hooks/changed_lines.py | 110 +++++++++++ cpp_linter_hooks/clang_tidy.py | 187 +++++++++++++++--- examples/README.md | 9 +- examples/cmake/README.md | 6 +- examples/meson/README.md | 9 +- tests/conftest.py | 39 ++++ tests/test_changed_lines.py | 217 ++++++++++++++++++++ tests/test_clang_tidy.py | 319 +++++++++++++++++++++++++++++- 10 files changed, 923 insertions(+), 44 deletions(-) create mode 100644 cpp_linter_hooks/changed_lines.py create mode 100644 tests/conftest.py create mode 100644 tests/test_changed_lines.py diff --git a/AGENTS.md b/AGENTS.md index c84bc81..fdf5eb8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -26,7 +26,8 @@ A pre-commit hook repo that auto-installs and runs `clang-format` and `clang-tid | `pyproject.toml` | Build config, deps (`pip>=26.1`), dev deps, entry points | | `cpp_linter_hooks/util.py` | Installed-wheel check, PyPI version resolution + pip install (prefix matching, offline fallback) | | `cpp_linter_hooks/clang_format.py` | clang-format wrapper (`--verbose`, `--dry-run`) | -| `cpp_linter_hooks/clang_tidy.py` | clang-tidy wrapper (`--compile-commands`, `--jobs`, `--fix`, `-quiet` + status-line filter, `--checks` repair, error hints) | +| `cpp_linter_hooks/clang_tidy.py` | clang-tidy wrapper (`--compile-commands`, missing-database policy, `--jobs`, `--fix`, `--all-lines`, `-quiet` + status-line filter, `--checks` repair, error hints) | +| `cpp_linter_hooks/changed_lines.py` | Added line ranges per file from `git diff` (staged changes, or `PRE_COMMIT_FROM_REF...PRE_COMMIT_TO_REF`) | | `.pre-commit-hooks.yaml` | Hook metadata for pre-commit framework | | `testing/` | Test fixtures (`main.c`, `good.c`, `.clang-format`, `.clang-tidy`, CMakeLists) | | `examples/` | Example configs (CMake, Meson, large-project scoping) | @@ -44,7 +45,8 @@ bash testing/run.sh # Integration tests (full pre-commit pipeline) - **Version resolution**: A wheel already installed in the hook environment (found by its `-.dist-info` directory in site-packages, e.g. from `additional_dependencies`, with its executable in the environment's scripts directory) is used without network access when it matches `--version` (exact or prefix) or when no `--version` is given. Otherwise the version is resolved dynamically from PyPI at runtime. No hardcoded version lists. Prefix match supported (`--version=20` → `20.1.8`). - **Return values**: All hooks return `tuple[int, str]` — `(retval, output)`, where `retval` is the tool's exit code (`0` on success, non-zero on failure; `--dry-run` auto-adds `--Werror` so formatting issues fail). -- **clang-tidy compile DB**: Auto-detected from `build/`, `out/`, `cmake-build-debug/`, `_build/`. Override with `--compile-commands=`, disable with `--no-compile-commands`. +- **clang-tidy compile DB**: Auto-detected from `build/`, `out/`, `cmake-build-debug/`, `_build/`, `builddir/`, then `cmake-build-*/`, `build-*/`, `build/*/`, `out/build/*/` (newest wins). Override with `--compile-commands=`, disable with `--no-compile-commands`. If none is found, no `-p`/`--extra-arg*` is given and clang-tidy finds no `compile_commands.json`/`compile_flags.txt` next to the files or in their parents, the hook skips clang-tidy (retval 0, message on stderr), or fails when `CI` is set. +- **clang-tidy changed lines**: By default only diagnostics on added lines are reported, via clang-tidy's `--line-filter` (so `-fix` is limited too). Files without added lines are not checked. If any passed file has no changes (`--all-files`), or git fails, every line is checked. `--all-lines` or a user `--line-filter` turns this off. Tests: `tests/conftest.py` disables this and the missing-database check by default; use the `real_changed_lines` / `real_compile_db_check` fixtures to test them. - **clang-tidy parallel**: `--jobs=N` runs per-file in parallel. Forced serial when `--export-fixes`, `-fix`, or `-fix-errors` is used. - **clang-tidy output**: Without `--verbose`, the hook adds `-quiet` (unless the user set it) and strips status lines (`N warnings generated.`, `Suppressed N warnings ...`, `Use -header-filter=...`, `[1/N] Processing file ...`). With `--verbose`, output is unchanged. - **clang-tidy check lists**: `--checks=` / `--warnings-as-errors=` lists that YAML split at commas are joined again, quotes inside the value are removed, and `--checks=` (e.g. `.clang-tidy`) gets a note. Notes are appended to the output, which pre-commit shows when the hook fails. diff --git a/README.md b/README.md index 3dfeaad..bbe9ccc 100644 --- a/README.md +++ b/README.md @@ -29,10 +29,11 @@ repos: Run `pre-commit install` once in each clone. `--style=file` loads the style from your `.clang-format` file, and clang-tidy reads your `.clang-tidy` file by itself. The clang-tidy hook -needs a `compile_commands.json`, which it looks for in `build/` and a few other directories (see -[Compilation database](https://github.com/cpp-linter/cpp-linter-hooks#compilation-database)); -leave it out if you only run clang-tidy in CI, for example with -[cpp-linter-action](https://cpp-linter.github.io/cpp-linter-action/). +reports only problems on the lines that your commit adds or changes (see +[Only changed lines](https://github.com/cpp-linter/cpp-linter-hooks#only-changed-lines)). It needs +a `compile_commands.json`, which it looks for in `build/` and a few other directories; without one +it skips clang-tidy on your machine and fails in CI (see +[Compilation database](https://github.com/cpp-linter/cpp-linter-hooks#compilation-database)). ## Usage @@ -163,6 +164,31 @@ The hook passes `-quiet` to clang-tidy and drops status lines such as `522 warni which count warnings in system headers that clang-tidy does not show. Add `--verbose` to `args` to keep clang-tidy's full output. +#### Only changed lines + +By default the hook reports only the problems on lines that the commit adds or changes, so old +warnings elsewhere in a file do not block your commit. It reads the changes with `git diff`: + +- On `git commit` and `pre-commit run`: the staged changes. +- With `pre-commit run --from-ref A --to-ref B` and in the `pre-push` stage: the changes between + the two commits. + +The hook passes these lines to clang-tidy's `--line-filter`, so `--fix` also changes only those +lines. Compiler errors are always reported. Files with no added lines, such as a pure rename, are +not checked. + +The hook checks every line when the files it gets have no changes, for example with +`pre-commit run --all-files` on a clean working tree, or outside a git repository. To always check +every line, for example in a manual stage, add `--all-lines`: + +```yaml + - id: clang-tidy + args: [--all-lines] +``` + +Checking only changed lines does not make clang-tidy faster: it still analyzes the whole file. +See [Performance optimization](https://github.com/cpp-linter/cpp-linter-hooks#performance-optimization). + > [!NOTE] > Add `--fix` to `args` to automatically apply clang-tidy fixes in place (equivalent to > passing `-fix` to clang-tidy directly). This is **opt-in** and **not the default** because @@ -187,9 +213,13 @@ For CMake or Meson projects, clang-tidy works best with a `compile_commands.json file that records the exact compiler flags used for each file. Without it, clang-tidy may report false positives from missing include paths or wrong compiler flags. -The hook auto-detects `compile_commands.json` in common build directories (`build/`, -`out/`, `cmake-build-debug/`, `_build/`) and passes `-p ` to clang-tidy -automatically — no configuration needed for most projects: +The hook auto-detects `compile_commands.json` in common build directories and passes `-p ` +to clang-tidy automatically — no configuration needed for most projects. It looks in `build/`, +`out/`, `cmake-build-debug/`, `_build/` and `builddir/` first, then in `cmake-build-*/`, +`build-*/`, `build/*/` and `out/build/*/` (CMake presets, Visual Studio), where the most recently +written one wins. Without `-p`, clang-tidy also finds a `compile_commands.json` or +`compile_flags.txt` next to each file or in its parent directories, so a database in the +repository root works too. ```yaml repos: @@ -207,7 +237,19 @@ To specify the build directory explicitly: args: [--compile-commands=build] ``` -To disable auto-detection (e.g. in a monorepo where auto-detect might pick the wrong database): +When no compilation database is found, clang-tidy would report missing headers instead of +problems in your code, so the hook: + +- **skips clang-tidy on your machine** and passes. The message says how to generate the database; + pre-commit shows it with `pre-commit run -v` or `verbose: true` on the hook. +- **fails in CI**, when the `CI` environment variable is set (GitHub Actions, GitLab CI and most CI + services set it), so a broken setup does not pass unnoticed. + +The hook runs clang-tidy anyway when `args` tell it how to compile: `-p`, `--extra-arg` or +`--extra-arg-before`. + +To disable auto-detection and always run clang-tidy without a database (e.g. in a monorepo where +auto-detect might pick the wrong database, or for code that needs no compiler flags): ```yaml - id: clang-tidy @@ -264,6 +306,11 @@ or `-j`: > overwrites a shared `--export-fixes` file and fixes to the same header can collide. Add > `require_serial: true` to the hook to run it once for all files. +With LLVM 21 and older, clang-tidy enables the `clang-analyzer-*` checks by default unless your +`Checks` start with `-*`, and they take most of its time: in our measurements on +[fmt](https://github.com/fmtlib/fmt), more than 90%. LLVM 22 removed them from the defaults. If +clang-tidy is slow and you do not need them, add `-clang-analyzer-*` to `Checks` in `.clang-tidy`. + Alternatively, if you want to run the hooks manually on only the changed files, you can use the following command: ```bash @@ -278,7 +325,7 @@ This approach ensures that only modified files are checked, further speeding up > Use `-v` or `--verbose` in `args` to enable verbose output. > For `clang-format`, it shows the list of processed files. > For `clang-tidy`, it prints which `compile_commands.json` is being used (when auto-detected or explicitly set) -> and keeps clang-tidy's full output, without `-quiet`. +> and which lines are reported, and keeps clang-tidy's full output, without `-quiet`. > pre-commit shows this output only when the hook fails; add `verbose: true` to the hook to see it on every run. ```yaml diff --git a/cpp_linter_hooks/changed_lines.py b/cpp_linter_hooks/changed_lines.py new file mode 100644 index 0000000..1b7e9e1 --- /dev/null +++ b/cpp_linter_hooks/changed_lines.py @@ -0,0 +1,110 @@ +"""Find the lines that a commit or push changes, using git.""" + +import codecs +import os +import re +import subprocess +from pathlib import PurePath + +# "@@ -12,3 +14,2 @@": the new side starts at line 14 and spans 2 lines. +HUNK_HEADER = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@") + +LineRanges = list[list[int]] + + +def diff_range() -> list[str]: + """Return the git diff arguments for the changes being checked. + + pre-commit and prek set PRE_COMMIT_FROM_REF and PRE_COMMIT_TO_REF for + ``run --from-ref ... --to-ref ...`` and for the pre-push stage. Otherwise + the hook checks a commit, so the changes are the staged ones. + """ + from_ref = os.environ.get("PRE_COMMIT_FROM_REF") + to_ref = os.environ.get("PRE_COMMIT_TO_REF") + if from_ref and to_ref: + return [f"{from_ref}...{to_ref}"] + return ["--cached"] + + +def _git_diff(args: list[str]) -> str | None: + """Run ``git diff`` and return its output, or None if git fails.""" + command = [ + "git", + "-c", + "core.quotePath=false", + "diff", + "--relative", + "--no-color", + "--no-ext-diff", + *args, + ] + try: + sp = subprocess.run(command, capture_output=True, check=False) + except OSError: + return None + if sp.returncode != 0: + return None + return sp.stdout.decode("utf-8", errors="surrogateescape") + + +def normalize(path: str) -> str: + """Return a path in the form git prints it, relative and with "/".""" + # PurePath also drops "./" segments. + return PurePath(path).as_posix() + + +def _unquote_path(path: str) -> str: + """Undo git's C-style quoting of unusual file names.""" + if len(path) >= 2 and path[0] == path[-1] == '"': + raw = codecs.escape_decode(path[1:-1].encode("utf-8"))[0] + return raw.decode("utf-8", errors="surrogateescape") + return path + + +def _parse_added_lines(diff: str) -> dict[str, LineRanges]: + """Return the added line ranges of each file in a ``git diff -U0`` output.""" + ranges: dict[str, LineRanges] = {} + current: LineRanges | None = None + for line in diff.splitlines(): + if line.startswith("+++ "): + name = _unquote_path(line[4:].rstrip("\t")) + if name == "/dev/null": + current = None + else: + current = ranges.setdefault(name.removeprefix("b/"), []) + continue + match = HUNK_HEADER.match(line) + if match is None or current is None: + continue + start = int(match.group(1)) + count = 1 if match.group(2) is None else int(match.group(2)) + if count > 0: + current.append([start, start + count - 1]) + return ranges + + +def changed_lines(files: list[str]) -> dict[str, LineRanges] | None: + """Return the added line ranges of each file, keyed by the given path. + + Returns None when the changed lines are unknown and every line should be + checked: outside a git repository, or when a file has no changes, which + means the hook runs on files that were not changed (``--all-files`` or + ``--files``). A file that is changed but has no added lines, for example a + pure rename, maps to an empty list. + """ + if not files: + return None + revisions = diff_range() + names = _git_diff(["--name-only", "-z", *revisions]) + if names is None: + return None + changed = set(names.split("\0")) + normalized = {path: normalize(path) for path in files} + if any(name not in changed for name in normalized.values()): + return None + + diff = _git_diff(["-U0", "--src-prefix=a/", "--dst-prefix=b/", *revisions]) + if diff is None: + return None + added = _parse_added_lines(diff) + return {path: added.get(name, []) for path, name in normalized.items()} diff --git a/cpp_linter_hooks/clang_tidy.py b/cpp_linter_hooks/clang_tidy.py index f8cc2b5..84a9084 100644 --- a/cpp_linter_hooks/clang_tidy.py +++ b/cpp_linter_hooks/clang_tidy.py @@ -1,5 +1,7 @@ """Pre-commit hook wrapper for clang-tidy.""" +import json +import os import re import subprocess import sys @@ -7,16 +9,31 @@ from concurrent.futures import ThreadPoolExecutor from pathlib import Path +from cpp_linter_hooks.changed_lines import ( + LineRanges, + changed_lines, + diff_range, + normalize, +) from cpp_linter_hooks.util import resolve_install_with_diagnostics -COMPILE_DB_SEARCH_DIRS = ["build", "out", "cmake-build-debug", "_build"] +COMPILE_DB_SEARCH_DIRS = ["build", "out", "cmake-build-debug", "_build", "builddir"] +# Searched after COMPILE_DB_SEARCH_DIRS. When a pattern matches several +# directories, the most recently written compile_commands.json wins. +COMPILE_DB_SEARCH_GLOBS = ["cmake-build-*", "build-*", "build/*", "out/build/*"] +# Files clang-tidy finds by itself in a source file's directory or its parents. +COMPILATION_DATABASE_FILES = ("compile_commands.json", "compile_flags.txt") SOURCE_FILE_SUFFIXES = { ".c", ".cc", + ".ccm", ".cp", ".cpp", + ".cppm", ".cxx", + ".cxxm", ".c++", + ".c++m", ".cu", ".cuh", ".h", @@ -24,9 +41,11 @@ ".hpp", ".hxx", ".h++", + ".ino", ".ipp", ".inl", ".ixx", + ".mm", ".tpp", ".txx", } @@ -34,7 +53,8 @@ Generate compile_commands.json with one of: CMake: cmake -S . -B build -DCMAKE_EXPORT_COMPILE_COMMANDS=ON Meson: meson setup builddir -Then run clang-tidy with --compile-commands=build or --compile-commands=builddir.""" +The hook finds it in build/ and builddir/. For another directory, pass +--compile-commands=.""" MSVC_HINT = """\ Windows/MSVC clang-tidy hints: @@ -142,6 +162,16 @@ def _positive_int(value: str) -> int: "-v", "--verbose", action="store_true", help="Enable verbose output" ) parser.add_argument("--fix", action="store_true", help="Apply fixes in place (-fix)") +parser.add_argument( + "--all-lines", + action="store_true", + dest="all_lines", + help=( + "Report problems on every line of the checked files. By default only " + "lines added or changed by the commit (or by the pushed commits) are " + "reported" + ), +) def _find_compile_commands() -> str | None: @@ -149,15 +179,85 @@ def _find_compile_commands() -> str | None: for d in COMPILE_DB_SEARCH_DIRS: if (Path(d) / "compile_commands.json").exists(): return d + for pattern in COMPILE_DB_SEARCH_GLOBS: + databases = list(Path().glob(f"{pattern}/compile_commands.json")) + if databases: + newest = max(databases, key=lambda db: db.stat().st_mtime) + return newest.parent.as_posix() return None -def _compile_commands_not_found_message(path: str | None = None) -> str: - """Build a user-facing message for missing compile_commands.json files.""" - if path is None: - return "No compile_commands.json was found in common build directories.\n\n" + ( - COMPILE_COMMANDS_HINT - ) +def _has_compilation_database(source_files: list[str]) -> bool: + """Return whether clang-tidy finds a compilation database by itself. + + Without -p, clang-tidy looks for compile_commands.json or compile_flags.txt + in each source file's directory and then in its parents. + """ + seen: set[Path] = set() + for source_file in source_files: + directory = Path(source_file).absolute().parent + for candidate in (directory, *directory.parents): + if candidate in seen: + break + seen.add(candidate) + if any((candidate / name).is_file() for name in COMPILATION_DATABASE_FILES): + return True + return False + + +def _has_option(args: list[str], *names: str) -> bool: + """Return whether args set one of the given options, in any spelling.""" + return any( + arg == name or arg.startswith(f"{name}=") for arg in args for name in names + ) + + +def _gives_compile_flags(args: list[str]) -> bool: + """Return whether args tell clang-tidy how to compile without a database.""" + return "--" in args or _has_option( + args, + "-p", + "--extra-arg", + "-extra-arg", + "--extra-arg-before", + "-extra-arg-before", + ) + + +def _is_ci() -> bool: + """Return whether the hook runs in CI, as the CI environment variable says.""" + return os.environ.get("CI", "").strip().lower() not in ("", "0", "false", "no") + + +def _missing_compile_db_result() -> tuple[int, str]: + """Build the result for a run that has no compilation database. + + Locally the hook skips clang-tidy, because without the compiler flags it + reports missing headers instead of problems in the code. In CI the hook + fails, so a broken setup does not pass unnoticed. + """ + searched = ", ".join( + f"{d}/" for d in COMPILE_DB_SEARCH_DIRS + COMPILE_DB_SEARCH_GLOBS + ) + details = ( + "No compile_commands.json was found. The hook looked in " + f"{searched} and clang-tidy looks next to each file and in its parent " + "directories (compile_commands.json or compile_flags.txt).\n\n" + f"{COMPILE_COMMANDS_HINT}\n\n" + "To run clang-tidy without a compilation database, add " + "--no-compile-commands." + ) + if _is_ci(): + return 1, f"clang-tidy needs a compilation database.\n{details}" + return 0, ( + "Skipped clang-tidy: it needs a compilation database to know how each " + "file is compiled. In CI (CI=true) this fails the hook instead.\n" + f"{details}" + ) + + +def _compile_commands_not_found_message(path: str) -> str: + """Build the message for a --compile-commands directory without a database.""" return ( f"--compile-commands: no compile_commands.json in '{path}'.\n\n" f"{COMPILE_COMMANDS_HINT}" @@ -175,7 +275,7 @@ def _resolve_compile_db( return None, None # Covers both "-p ./build" (two tokens) and "-p=./build" (one token) - has_p = any(a == "-p" or a.startswith("-p=") for a in other_args) + has_p = _has_option(other_args, "-p") if hook_args.compile_commands: if has_p: @@ -355,6 +455,20 @@ def _repair_check_args(args: list[str]) -> tuple[list[str], list[str]]: return repaired, notes +def _line_filter_option(lines: dict[str, LineRanges]) -> str: + """Build clang-tidy's --line-filter option for the given line ranges. + + clang-tidy matches each name against the end of the path it reports, and + drops diagnostics in files that are not listed. + """ + entries = [] + for path, ranges in lines.items(): + name = normalize(path) + names = dict.fromkeys((name, name.replace("/", os.sep))) + entries.extend({"name": n, "lines": ranges} for n in names) + return "--line-filter=" + json.dumps(entries, separators=(",", ":")) + + def _append_notes(output: str, notes: list[str]) -> str: """Append configuration notes after clang-tidy's output.""" if not notes: @@ -404,31 +518,58 @@ def run_file(source_file: str) -> tuple[int, str]: def run_clang_tidy(args=None) -> tuple[int, str]: """Run clang-tidy with hook-specific argument handling.""" hook_args, other_args = parser.parse_known_args(args) - tool_path, version_error = resolve_install_with_diagnostics( - "clang-tidy", hook_args.version, hook_args.verbose - ) - if version_error is not None: - return 1, version_error - # Run the executable that was resolved, not whatever comes first on PATH. - executable = str(tool_path) if tool_path else "clang-tidy" compile_db_path, error = _resolve_compile_db(hook_args, other_args) if error is not None: return error + clang_tidy_args, source_files = _split_source_files(other_args) + clang_tidy_args, notes = _repair_check_args(clang_tidy_args) + + if not hook_args.all_lines and not _has_option( + clang_tidy_args, "--line-filter", "-line-filter" + ): + lines = changed_lines(source_files) + if lines is not None: + source_files = [f for f in source_files if lines[f]] + if hook_args.verbose: + print( + "Reporting only lines changed in git diff " + f"{' '.join(diff_range())}; use --all-lines for every line", + file=sys.stderr, + ) + if not source_files: + return 0, _append_notes("", notes) + clang_tidy_args.append( + _line_filter_option({f: lines[f] for f in source_files}) + ) + if compile_db_path: if hook_args.verbose: print( f"Using compile_commands.json from: {compile_db_path}", file=sys.stderr ) - other_args = ["-p", compile_db_path] + other_args - elif hook_args.verbose and not hook_args.no_compile_commands: - has_p = any(a == "-p" or a.startswith("-p=") for a in other_args) - if not has_p: - print(_compile_commands_not_found_message(), file=sys.stderr) + clang_tidy_args = ["-p", compile_db_path] + clang_tidy_args + elif ( + not hook_args.no_compile_commands + and not _gives_compile_flags(clang_tidy_args) + and not _has_compilation_database(source_files) + ): + retval, message = _missing_compile_db_result() + if retval == 0: + # A passing hook's output is hidden; pre-commit shows stderr with + # `verbose: true` or `pre-commit run -v`. + print(message, file=sys.stderr) + return 0, _append_notes("", notes) + return retval, _append_notes(message, notes) - clang_tidy_args, source_files = _split_source_files(other_args) - clang_tidy_args, notes = _repair_check_args(clang_tidy_args) + tool_path, version_error = resolve_install_with_diagnostics( + "clang-tidy", hook_args.version, hook_args.verbose + ) + if version_error is not None: + return 1, version_error + # Run the executable that was resolved, not whatever comes first on PATH. + executable = str(tool_path) if tool_path else "clang-tidy" # -quiet hides clang-tidy's statistics about suppressed warnings; --verbose # keeps clang-tidy's full output. diff --git a/examples/README.md b/examples/README.md index 50f058d..88d8a4f 100644 --- a/examples/README.md +++ b/examples/README.md @@ -17,10 +17,10 @@ ### Meson ```bash -meson setup build # auto-detect works with build/ +meson setup builddir # builddir/ and build/ are auto-detected ``` -Or point explicitly: `args: [--compile-commands=builddir]`. +For another directory, point explicitly: `args: [--compile-commands=mydir]`. ### clang-tidy + compile_commands.json @@ -59,5 +59,10 @@ jobs: - uses: actions/checkout@v7 - uses: actions/setup-python@v7 with: { python-version: '3.11' } + # clang-tidy needs the compilation database in CI too: without it, the + # hook fails when CI is set. + - run: cmake -B build -DCMAKE_EXPORT_COMPILE_COMMANDS=ON - uses: pre-commit/action@v3.0.1 ``` + +`pre-commit/action` runs `pre-commit run --all-files`, so clang-tidy checks every line there. diff --git a/examples/cmake/README.md b/examples/cmake/README.md index 33ee95a..b929f56 100644 --- a/examples/cmake/README.md +++ b/examples/cmake/README.md @@ -31,5 +31,7 @@ Then `cmake -B build` and `pre-commit install`. Done. --- `compile_commands.json` is auto-detected from `build/`, `out/`, `cmake-build-debug/`, -or `_build/`. If your build dir has a different name, pass it explicitly: -`args: [--compile-commands=mybuild, ...]`. +`_build/`, `builddir/`, `cmake-build-*/`, `build-*/`, `build/*/` or `out/build/*/`. If your +build dir has a different name, pass it explicitly: +`args: [--compile-commands=mybuild, ...]`. Without a database, the clang-tidy hook skips +clang-tidy on your machine and fails in CI. diff --git a/examples/meson/README.md b/examples/meson/README.md index 3887ef3..51e4d97 100644 --- a/examples/meson/README.md +++ b/examples/meson/README.md @@ -14,7 +14,7 @@ repos: files: ^(src|include)/.*\.(cpp|cc|cxx|h|hpp)$ - id: clang-tidy - args: [--version=21, --compile-commands=builddir] + args: [--version=21] files: ^(src|include)/.*\.(cpp|cc|cxx)$ ``` @@ -33,10 +33,9 @@ This gives `clang-tidy` the compiler flags and include paths for each source fil No separate compilation step is required to generate the database. If the project needs generated headers, build those first with `meson compile -C builddir`. -`--compile-commands` takes the **directory** containing `compile_commands.json`, -not the file path. `builddir/` is not one of the auto-detected directories, so the -explicit argument above is required. If you use `meson setup build` instead, -`build/` is auto-detected and the argument can be omitted. +The hook auto-detects `builddir/` and `build/`. For another directory, pass +`--compile-commands=`: it takes the **directory** containing +`compile_commands.json`, not the file path. To regenerate the database after changing build options, run `meson setup --reconfigure builddir` before the hooks. diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..76b55d3 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,39 @@ +"""Shared fixtures for the hook tests.""" + +import pytest + +from cpp_linter_hooks import clang_tidy + +REAL_CHANGED_LINES = clang_tidy.changed_lines +REAL_HAS_COMPILATION_DATABASE = clang_tidy._has_compilation_database + + +@pytest.fixture(autouse=True) +def hook_environment(monkeypatch): + """Make clang-tidy's environment checks predictable in every test. + + The tests run in CI, inside this repository's git checkout, so by default: + - the CI and pre-commit range variables are removed, + - every line counts as changed, so no --line-filter is added, + - a compilation database is always found next to the source files. + + Tests of these checks use the fixtures below or set the variables. + """ + for name in ("CI", "PRE_COMMIT_FROM_REF", "PRE_COMMIT_TO_REF"): + monkeypatch.delenv(name, raising=False) + monkeypatch.setattr(clang_tidy, "changed_lines", lambda files: None) + monkeypatch.setattr(clang_tidy, "_has_compilation_database", lambda files: True) + + +@pytest.fixture +def real_changed_lines(monkeypatch): + """Use git to find the changed lines, as the hook does.""" + monkeypatch.setattr(clang_tidy, "changed_lines", REAL_CHANGED_LINES) + + +@pytest.fixture +def real_compile_db_check(monkeypatch): + """Look for a compilation database next to the files, as the hook does.""" + monkeypatch.setattr( + clang_tidy, "_has_compilation_database", REAL_HAS_COMPILATION_DATABASE + ) diff --git a/tests/test_changed_lines.py b/tests/test_changed_lines.py new file mode 100644 index 0000000..700fe13 --- /dev/null +++ b/tests/test_changed_lines.py @@ -0,0 +1,217 @@ +import subprocess +from unittest.mock import patch + +import pytest + +from cpp_linter_hooks.changed_lines import ( + _parse_added_lines, + changed_lines, + diff_range, + normalize, +) + +ORIGINAL = "".join(f"line {n}\n" for n in range(1, 11)) + + +def git(repo, *args): + """Run git in repo with a fixed identity and return its stdout.""" + return subprocess.run( + [ + "git", + "-c", + "user.name=Test", + "-c", + "user.email=test@example.com", + "-c", + "commit.gpgsign=false", + *args, + ], + cwd=repo, + check=True, + capture_output=True, + text=True, + ).stdout + + +@pytest.fixture +def repo(tmp_path, monkeypatch): + """A git repository with one commit that adds a.cpp, used as the cwd.""" + git(tmp_path, "init", "-q") + (tmp_path / "a.cpp").write_text(ORIGINAL) + git(tmp_path, "add", "a.cpp") + git(tmp_path, "commit", "-q", "-m", "init") + monkeypatch.chdir(tmp_path) + return tmp_path + + +def edit(path, replacements): + """Replace whole lines (1-based numbers) in a file.""" + lines = path.read_text().splitlines(keepends=True) + for number, text in replacements.items(): + lines[number - 1] = text + path.write_text("".join(lines)) + + +def test_staged_changes_give_the_added_lines(repo): + edit(repo / "a.cpp", {3: "changed 3\n", 7: "changed 7\n", 8: "changed 8\n"}) + git(repo, "add", "a.cpp") + + assert changed_lines(["a.cpp"]) == {"a.cpp": [[3, 3], [7, 8]]} + + +def test_unstaged_changes_are_ignored(repo): + edit(repo / "a.cpp", {2: "staged\n"}) + git(repo, "add", "a.cpp") + edit(repo / "a.cpp", {9: "not staged\n"}) + + assert changed_lines(["a.cpp"]) == {"a.cpp": [[2, 2]]} + + +def test_new_file_counts_every_line(repo): + (repo / "b.cpp").write_text("int x;\nint y;\n") + git(repo, "add", "b.cpp") + + assert changed_lines(["b.cpp"]) == {"b.cpp": [[1, 2]]} + + +def test_deleted_lines_leave_nothing_to_report(repo): + edit(repo / "a.cpp", {4: ""}) + git(repo, "add", "a.cpp") + + assert changed_lines(["a.cpp"]) == {"a.cpp": []} + + +def test_pure_rename_leaves_nothing_to_report(repo): + git(repo, "mv", "a.cpp", "renamed.cpp") + + assert changed_lines(["renamed.cpp"]) == {"renamed.cpp": []} + + +def test_paths_are_returned_as_given(repo): + (repo / "src").mkdir() + (repo / "src" / "b.cpp").write_text("int x;\n") + git(repo, "add", "src/b.cpp") + + assert changed_lines(["./src/b.cpp"]) == {"./src/b.cpp": [[1, 1]]} + + +def test_unusual_file_names(repo): + name = "dir with space/naïve.cpp" + (repo / "dir with space").mkdir() + (repo / name).write_text("int x;\n") + git(repo, "add", name) + + assert changed_lines([name]) == {name: [[1, 1]]} + + +def test_unchanged_file_means_every_line_is_checked(repo): + """pre-commit run --all-files passes files without changes.""" + (repo / "b.cpp").write_text("int x;\n") + git(repo, "add", "b.cpp") + + assert changed_lines(["a.cpp", "b.cpp"]) is None + + +def test_runs_from_a_subdirectory(repo, monkeypatch): + (repo / "src").mkdir() + (repo / "src" / "b.cpp").write_text("int x;\n") + git(repo, "add", "src/b.cpp") + monkeypatch.chdir(repo / "src") + + assert changed_lines(["b.cpp"]) == {"b.cpp": [[1, 1]]} + + +def test_first_commit(tmp_path, monkeypatch): + git(tmp_path, "init", "-q") + (tmp_path / "a.cpp").write_text("int x;\nint y;\n") + git(tmp_path, "add", "a.cpp") + monkeypatch.chdir(tmp_path) + + assert changed_lines(["a.cpp"]) == {"a.cpp": [[1, 2]]} + + +def test_pushed_range_from_pre_commit_variables(repo, monkeypatch): + base = git(repo, "rev-parse", "HEAD").strip() + edit(repo / "a.cpp", {5: "pushed 5\n"}) + git(repo, "commit", "-q", "-am", "second") + head = git(repo, "rev-parse", "HEAD").strip() + edit(repo / "a.cpp", {1: "staged, not pushed\n"}) + git(repo, "add", "a.cpp") + monkeypatch.setenv("PRE_COMMIT_FROM_REF", base) + monkeypatch.setenv("PRE_COMMIT_TO_REF", head) + + assert changed_lines(["a.cpp"]) == {"a.cpp": [[5, 5]]} + + +def test_outside_a_git_repository(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("GIT_CEILING_DIRECTORIES", str(tmp_path.parent)) + (tmp_path / "a.cpp").write_text("int x;\n") + + assert changed_lines(["a.cpp"]) is None + + +def test_without_git(repo): + with patch( + "cpp_linter_hooks.changed_lines.subprocess.run", + side_effect=FileNotFoundError("git"), + ): + assert changed_lines(["a.cpp"]) is None + + +def test_hunk_diff_failure(repo): + edit(repo / "a.cpp", {2: "changed\n"}) + git(repo, "add", "a.cpp") + real_run = subprocess.run + + def fail_hunks(command, **kwargs): + if "-U0" in command: + return subprocess.CompletedProcess(command, 128, b"", b"fatal") + return real_run(command, **kwargs) + + with patch("cpp_linter_hooks.changed_lines.subprocess.run", fail_hunks): + assert changed_lines(["a.cpp"]) is None + + +def test_no_files(): + assert changed_lines([]) is None + + +def test_diff_range(monkeypatch): + assert diff_range() == ["--cached"] + monkeypatch.setenv("PRE_COMMIT_FROM_REF", "origin/main") + assert diff_range() == ["--cached"] + monkeypatch.setenv("PRE_COMMIT_TO_REF", "HEAD") + assert diff_range() == ["origin/main...HEAD"] + + +@pytest.mark.parametrize( + ("path", "expected"), + (("a.cpp", "a.cpp"), ("./src/a.cpp", "src/a.cpp"), ("././a.cpp", "a.cpp")), +) +def test_normalize(path, expected): + assert normalize(path) == expected + + +def test_parse_added_lines(): + lines = [ + "diff --git a/a.cpp b/a.cpp", + "--- a/a.cpp", + "+++ b/a.cpp", + "@@ -3 +3 @@", + "@@ -10,0 +11,4 @@ int main() {", + "@@ -20,2 +23,0 @@", + "diff --git a/gone.cpp b/gone.cpp", + "--- a/gone.cpp", + "+++ /dev/null", + "@@ -1,2 +0,0 @@", + "diff --git a/b.cpp b/b.cpp", + "--- a/b.cpp", + '+++ "b/sp\\303\\251cial \\"q\\".cpp"', + "@@ -0,0 +1,2 @@", + ] + + assert _parse_added_lines("\n".join(lines)) == { + "a.cpp": [[3, 3], [11, 14]], + 'spécial "q".cpp': [[1, 2]], + } diff --git a/tests/test_clang_tidy.py b/tests/test_clang_tidy.py index 0666b37..b375e8e 100644 --- a/tests/test_clang_tidy.py +++ b/tests/test_clang_tidy.py @@ -1,3 +1,4 @@ +import os import subprocess import sys import time @@ -6,6 +7,7 @@ import pytest +from cpp_linter_hooks import clang_tidy from cpp_linter_hooks.clang_tidy import ( COMPILE_COMMANDS_HINT, COMPILE_DB_SEARCH_DIRS, @@ -13,6 +15,9 @@ _append_guidance, _combine_outputs, _exec_clang_tidy, + _find_compile_commands, + _has_compilation_database, + _line_filter_option, _looks_like_compile_db_error, _looks_like_msvc_error, _positive_int, @@ -283,7 +288,7 @@ def test_verbose_prints_compile_db_path(tmp_path, monkeypatch, capsys): def test_verbose_prints_compile_db_generation_hint_when_not_found( - tmp_path, monkeypatch, capsys + tmp_path, monkeypatch, capsys, real_compile_db_check ): monkeypatch.chdir(tmp_path) with ( @@ -1079,3 +1084,315 @@ def test_run_uses_the_resolved_executable(mock_resolve): assert commands[0] == [str(tool), "-quiet", "a.cpp"] assert sorted(command[-1] for command in commands[1:]) == ["a.cpp", "b.cpp"] assert all(command[0] == str(tool) for command in commands) + + +# --- only changed lines are reported --- + + +def test_line_filter_option(monkeypatch): + monkeypatch.setattr(clang_tidy.os, "sep", "/") + option = _line_filter_option({"./src/a.cpp": [[3, 3], [7, 8]], "b.h": [[1, 1]]}) + + assert option == ( + '--line-filter=[{"name":"src/a.cpp","lines":[[3,3],[7,8]]},' + '{"name":"b.h","lines":[[1,1]]}]' + ) + + +def test_line_filter_option_lists_windows_separators(monkeypatch): + monkeypatch.setattr(clang_tidy.os, "sep", "\\") + + option = _line_filter_option({"src/a.cpp": [[1, 2]]}) + + assert option == ( + '--line-filter=[{"name":"src/a.cpp","lines":[[1,2]]},' + '{"name":"src\\\\a.cpp","lines":[[1,2]]}]' + ) + + +def test_run_filters_to_changed_lines(monkeypatch, mock_exec): + monkeypatch.setattr( + clang_tidy, "changed_lines", lambda files: {"a.cpp": [[2, 4]], "b.cpp": []} + ) + + run_clang_tidy(["a.cpp", "b.cpp"]) + + mock_exec.assert_called_once_with( + [ + "clang-tidy", + "-quiet", + '--line-filter=[{"name":"a.cpp","lines":[[2,4]]}]', + "a.cpp", + ] + ) + + +def test_run_skips_clang_tidy_without_added_lines(monkeypatch, mock_exec): + monkeypatch.setattr(clang_tidy, "changed_lines", lambda files: {"a.cpp": []}) + + assert run_clang_tidy(["a.cpp"]) == (0, "") + mock_exec.assert_not_called() + + +def test_run_without_added_lines_keeps_notes(monkeypatch, mock_exec): + monkeypatch.setattr(clang_tidy, "changed_lines", lambda files: {"a.cpp": []}) + + ret, output = run_clang_tidy(["--checks=.clang-tidy", "a.cpp"]) + + assert ret == 0 + assert "--checks=.clang-tidy has no effect" in output + + +@pytest.mark.parametrize("args", (["--all-lines"], ["--line-filter=[]"])) +def test_run_checks_all_lines_when_asked(monkeypatch, mock_exec, args): + monkeypatch.setattr( + clang_tidy, "changed_lines", lambda files: pytest.fail("not wanted") + ) + + run_clang_tidy(args + ["a.cpp"]) + + mock_exec.assert_called_once() + assert not any( + arg.startswith("--line-filter=[{") for arg in mock_exec.call_args.args[0] + ) + + +def test_verbose_says_which_lines_are_checked(monkeypatch, mock_exec, capsys): + monkeypatch.setattr(clang_tidy, "changed_lines", lambda files: {"a.cpp": [[1, 1]]}) + + run_clang_tidy(["--verbose", "a.cpp"]) + + assert ( + "Reporting only lines changed in git diff --cached; use --all-lines" + in capsys.readouterr().err + ) + + +def test_line_filter_reaches_each_parallel_run(monkeypatch, mock_exec): + monkeypatch.setattr( + clang_tidy, + "changed_lines", + lambda files: {"a.cpp": [[1, 1]], "b.cpp": [[2, 2]]}, + ) + + run_clang_tidy(["--jobs=2", "a.cpp", "b.cpp"]) + + line_filter = ( + '--line-filter=[{"name":"a.cpp","lines":[[1,1]]},' + '{"name":"b.cpp","lines":[[2,2]]}]' + ) + commands = sorted(call.args[0] for call in mock_exec.call_args_list) + assert commands == [ + ["clang-tidy", "-quiet", line_filter, "a.cpp"], + ["clang-tidy", "-quiet", line_filter, "b.cpp"], + ] + + +def _git(repo, *args): + subprocess.run( + [ + "git", + "-c", + "user.name=Test", + "-c", + "user.email=test@example.com", + "-c", + "commit.gpgsign=false", + *args, + ], + cwd=repo, + check=True, + capture_output=True, + ) + + +@pytest.fixture +def braces_repo(tmp_path, monkeypatch, real_changed_lines, real_compile_db_check): + """A git repo whose committed braces.cpp has two old brace warnings. + + The staged change adds a third one on line 6. compile_flags.txt is the + compilation database; the file has no #include, so the result does not + depend on system headers being found. + """ + source = tmp_path / "braces.cpp" + source.write_text( + "int f(int x) {\n" + " if (x)\n" + " return 1;\n" + " if (x > 1)\n" + " return 2;\n" + " return 0;\n" + "}\n" + ) + (tmp_path / "compile_flags.txt").write_text("-std=c++17\n") + _git(tmp_path, "init", "-q") + _git(tmp_path, "add", ".") + _git(tmp_path, "commit", "-q", "-m", "init") + lines = source.read_text().splitlines(keepends=True) + lines[5] = " if (x > 2)\n return 3;\n return 0;\n" + source.write_text("".join(lines)) + _git(tmp_path, "add", "braces.cpp") + monkeypatch.chdir(tmp_path) + return source + + +CHECK_BRACES = ["--checks=-*,readability-braces-around-statements"] + + +def test_real_tool_reports_only_the_changed_lines(braces_repo): + ret, output = run_clang_tidy(CHECK_BRACES + ["braces.cpp"]) + + assert ret == 1 + assert "braces.cpp:6:" in output + assert "braces.cpp:2:" not in output + assert "braces.cpp:4:" not in output + + +def test_real_tool_reports_every_line_with_all_lines(braces_repo): + ret, output = run_clang_tidy(CHECK_BRACES + ["--all-lines", "braces.cpp"]) + + assert ret == 1 + assert "braces.cpp:2:" in output + assert "braces.cpp:4:" in output + assert "braces.cpp:6:" in output + + +def test_real_tool_fixes_only_the_changed_lines(braces_repo): + run_clang_tidy(CHECK_BRACES + ["--fix", "braces.cpp"]) + + fixed = braces_repo.read_text() + assert "if (x > 2) {" in fixed + assert "if (x) {" not in fixed + assert "if (x > 1) {" not in fixed + + +# --- what happens without a compilation database --- + + +@pytest.fixture +def no_compile_db(tmp_path, monkeypatch, real_compile_db_check): + """A directory without any compilation database, used as the cwd.""" + monkeypatch.chdir(tmp_path) + return tmp_path + + +def test_missing_compile_db_skips_locally(no_compile_db, mock_exec, capsys): + assert run_clang_tidy(["a.cpp"]) == (0, "") + + mock_exec.assert_not_called() + stderr = capsys.readouterr().err + assert "Skipped clang-tidy" in stderr + assert "cmake -S . -B build -DCMAKE_EXPORT_COMPILE_COMMANDS=ON" in stderr + assert "--no-compile-commands" in stderr + + +def test_missing_compile_db_skips_before_installing(no_compile_db, mock_resolve): + run_clang_tidy(["a.cpp"]) + + mock_resolve.assert_not_called() + + +@pytest.mark.parametrize("value", ("true", "1", "True", "yes")) +def test_missing_compile_db_fails_in_ci(no_compile_db, mock_exec, monkeypatch, value): + monkeypatch.setenv("CI", value) + + ret, output = run_clang_tidy(["--checks=.clang-tidy", "a.cpp"]) + + assert ret == 1 + assert output.startswith("clang-tidy needs a compilation database.") + assert "--no-compile-commands" in output + assert "--checks=.clang-tidy has no effect" in output + mock_exec.assert_not_called() + + +@pytest.mark.parametrize("value", ("", "false", "0", "no")) +def test_ci_variable_set_to_false(no_compile_db, mock_exec, monkeypatch, value): + monkeypatch.setenv("CI", value) + + assert run_clang_tidy(["a.cpp"]) == (0, "") + + +@pytest.mark.parametrize( + "args", + ( + ["--no-compile-commands"], + ["-p", "elsewhere"], + ["-p=elsewhere"], + ["--extra-arg=-std=c++20"], + ["-extra-arg=-std=c++20"], + ["--extra-arg-before=--driver-mode=cl"], + ["--extra-arg", "-std=c++20"], + ), +) +def test_missing_compile_db_runs_when_flags_are_given(no_compile_db, mock_exec, args): + run_clang_tidy(args + ["a.cpp"]) + + mock_exec.assert_called_once() + + +@pytest.mark.parametrize("name", ("compile_commands.json", "compile_flags.txt")) +def test_database_found_in_a_parent_directory(tmp_path, name): + (tmp_path / name).write_text("") + (tmp_path / "src" / "deep").mkdir(parents=True) + + assert _has_compilation_database([str(tmp_path / "src" / "deep" / "a.cpp")]) + + +def test_database_next_to_one_of_the_files(tmp_path): + (tmp_path / "with").mkdir() + (tmp_path / "with" / "compile_flags.txt").write_text("") + + assert _has_compilation_database( + [str(tmp_path / "a.cpp"), str(tmp_path / "with" / "b.cpp")] + ) + + +def test_no_database_in_any_parent(no_compile_db): + (no_compile_db / "src").mkdir() + + assert not _has_compilation_database(["a.cpp", "b.cpp", "src/c.cpp"]) + + +def test_testing_dir_uses_its_compile_flags(): + assert _has_compilation_database(["testing/main.c"]) + + +@pytest.mark.parametrize( + "db_dir", ("cmake-build-release", "build-gcc", "build/debug", "out/build/x64-Debug") +) +def test_compile_commands_found_by_pattern(tmp_path, monkeypatch, db_dir): + monkeypatch.chdir(tmp_path) + (tmp_path / db_dir).mkdir(parents=True) + (tmp_path / db_dir / "compile_commands.json").write_text("[]") + + assert _find_compile_commands() == db_dir + + +def test_newest_compile_commands_wins_within_a_pattern(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + for index, preset in enumerate(("release", "debug", "asan")): + db = tmp_path / "build" / preset / "compile_commands.json" + db.parent.mkdir(parents=True) + db.write_text("[]") + os.utime(db, (1_000_000 + index, 1_000_000 + index)) + + assert _find_compile_commands() == "build/asan" + + +def test_fixed_directories_win_over_patterns(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + for db_dir in ("cmake-build-release", "builddir"): + (tmp_path / db_dir).mkdir() + (tmp_path / db_dir / "compile_commands.json").write_text("[]") + + assert _find_compile_commands() == "builddir" + + +@pytest.mark.parametrize( + "name", ("a.mm", "a.cppm", "a.ccm", "a.cxxm", "a.c++m", "sketch.ino") +) +def test_split_source_files_knows_pre_commit_cpp_types(name): + assert _split_source_files(["-quiet", "a.cpp", name]) == ( + ["-quiet"], + ["a.cpp", name], + ) From 21507e686dda72f7bc7d5bf18b592ed1ce33967c Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Thu, 22 Oct 2026 11:39:13 +0000 Subject: [PATCH 2/8] refactor: name the compile_commands.json literal once --- cpp_linter_hooks/clang_tidy.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cpp_linter_hooks/clang_tidy.py b/cpp_linter_hooks/clang_tidy.py index 84a9084..2dbb44f 100644 --- a/cpp_linter_hooks/clang_tidy.py +++ b/cpp_linter_hooks/clang_tidy.py @@ -22,7 +22,8 @@ # directories, the most recently written compile_commands.json wins. COMPILE_DB_SEARCH_GLOBS = ["cmake-build-*", "build-*", "build/*", "out/build/*"] # Files clang-tidy finds by itself in a source file's directory or its parents. -COMPILATION_DATABASE_FILES = ("compile_commands.json", "compile_flags.txt") +COMPILE_COMMANDS_JSON = "compile_commands.json" +COMPILATION_DATABASE_FILES = (COMPILE_COMMANDS_JSON, "compile_flags.txt") SOURCE_FILE_SUFFIXES = { ".c", ".cc", @@ -177,10 +178,10 @@ def _positive_int(value: str) -> int: def _find_compile_commands() -> str | None: """Return the first common directory containing compile_commands.json.""" for d in COMPILE_DB_SEARCH_DIRS: - if (Path(d) / "compile_commands.json").exists(): + if (Path(d) / COMPILE_COMMANDS_JSON).exists(): return d for pattern in COMPILE_DB_SEARCH_GLOBS: - databases = list(Path().glob(f"{pattern}/compile_commands.json")) + databases = list(Path().glob(f"{pattern}/{COMPILE_COMMANDS_JSON}")) if databases: newest = max(databases, key=lambda db: db.stat().st_mtime) return newest.parent.as_posix() @@ -285,7 +286,7 @@ def _resolve_compile_db( ) return None, None p = Path(hook_args.compile_commands) - if not p.is_dir() or not (p / "compile_commands.json").exists(): + if not p.is_dir() or not (p / COMPILE_COMMANDS_JSON).exists(): return None, ( 1, _compile_commands_not_found_message(hook_args.compile_commands), @@ -301,7 +302,7 @@ def _resolve_compile_db( def _looks_like_compile_db_error(output: str) -> bool: """Return whether clang-tidy output indicates a compile database problem.""" lower_output = output.lower() - compile_db_error = "compile_commands.json" in lower_output and any( + compile_db_error = COMPILE_COMMANDS_JSON in lower_output and any( pattern in lower_output for pattern in ( "not found", From d160f1d91fc084b8867920a783fdbfbcfddffc18 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Thu, 22 Oct 2026 11:53:00 +0000 Subject: [PATCH 3/8] perf: find compile_commands.json with os.scandir; time out a stuck git diff --- cpp_linter_hooks/changed_lines.py | 4 ++-- cpp_linter_hooks/clang_tidy.py | 34 ++++++++++++++++++++++++++----- tests/test_changed_lines.py | 10 ++++----- tests/test_clang_tidy.py | 18 ++++++++++++++++ 4 files changed, 54 insertions(+), 12 deletions(-) diff --git a/cpp_linter_hooks/changed_lines.py b/cpp_linter_hooks/changed_lines.py index 1b7e9e1..fad29a1 100644 --- a/cpp_linter_hooks/changed_lines.py +++ b/cpp_linter_hooks/changed_lines.py @@ -39,8 +39,8 @@ def _git_diff(args: list[str]) -> str | None: *args, ] try: - sp = subprocess.run(command, capture_output=True, check=False) - except OSError: + sp = subprocess.run(command, capture_output=True, check=False, timeout=60) + except (OSError, subprocess.TimeoutExpired): return None if sp.returncode != 0: return None diff --git a/cpp_linter_hooks/clang_tidy.py b/cpp_linter_hooks/clang_tidy.py index 2dbb44f..f3f6843 100644 --- a/cpp_linter_hooks/clang_tidy.py +++ b/cpp_linter_hooks/clang_tidy.py @@ -175,16 +175,40 @@ def _positive_int(value: str) -> int: ) +def _subdirectories(parent: str) -> list[str]: + """Return the names of the directories in parent, or [] if it is missing.""" + try: + with os.scandir(parent) as entries: + return [entry.name for entry in entries if entry.is_dir()] + except OSError: + return [] + + def _find_compile_commands() -> str | None: """Return the first common directory containing compile_commands.json.""" + # os.path and os.scandir instead of pathlib: this runs on every commit. for d in COMPILE_DB_SEARCH_DIRS: - if (Path(d) / COMPILE_COMMANDS_JSON).exists(): + if os.path.isfile(os.path.join(d, COMPILE_COMMANDS_JSON)): return d + listings: dict[str, list[str]] = {} for pattern in COMPILE_DB_SEARCH_GLOBS: - databases = list(Path().glob(f"{pattern}/{COMPILE_COMMANDS_JSON}")) - if databases: - newest = max(databases, key=lambda db: db.stat().st_mtime) - return newest.parent.as_posix() + parent, _, name = pattern.rpartition("/") + prefix = name.removesuffix("*") + if parent not in listings: + listings[parent] = _subdirectories(parent or ".") + mtimes = {} + for subdirectory in listings[parent]: + if not subdirectory.startswith(prefix): + continue + directory = f"{parent}/{subdirectory}" if parent else subdirectory + try: + mtimes[directory] = os.stat( + os.path.join(directory, COMPILE_COMMANDS_JSON) + ).st_mtime + except OSError: + continue + if mtimes: + return max(mtimes, key=mtimes.__getitem__) return None diff --git a/tests/test_changed_lines.py b/tests/test_changed_lines.py index 700fe13..a940414 100644 --- a/tests/test_changed_lines.py +++ b/tests/test_changed_lines.py @@ -151,11 +151,11 @@ def test_outside_a_git_repository(tmp_path, monkeypatch): assert changed_lines(["a.cpp"]) is None -def test_without_git(repo): - with patch( - "cpp_linter_hooks.changed_lines.subprocess.run", - side_effect=FileNotFoundError("git"), - ): +@pytest.mark.parametrize( + "error", (FileNotFoundError("git"), subprocess.TimeoutExpired("git", 60)) +) +def test_without_git(repo, error): + with patch("cpp_linter_hooks.changed_lines.subprocess.run", side_effect=error): assert changed_lines(["a.cpp"]) is None diff --git a/tests/test_clang_tidy.py b/tests/test_clang_tidy.py index b375e8e..dea7a9d 100644 --- a/tests/test_clang_tidy.py +++ b/tests/test_clang_tidy.py @@ -1379,6 +1379,24 @@ def test_newest_compile_commands_wins_within_a_pattern(tmp_path, monkeypatch): assert _find_compile_commands() == "build/asan" +def test_pattern_skips_directories_without_a_database(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / "build-empty").mkdir() + (tmp_path / "build-gcc").mkdir() + (tmp_path / "build-gcc" / "compile_commands.json").write_text("[]") + (tmp_path / "build-file").write_text("") + + assert _find_compile_commands() == "build-gcc" + + +def test_no_compile_commands_anywhere(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / "build").mkdir() + (tmp_path / "build" / "CMakeFiles").mkdir() + + assert _find_compile_commands() is None + + def test_fixed_directories_win_over_patterns(tmp_path, monkeypatch): monkeypatch.chdir(tmp_path) for db_dir in ("cmake-build-release", "builddir"): From 4b04e8cf70572966367a98af14d736058296b8c3 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Thu, 22 Oct 2026 11:54:42 +0000 Subject: [PATCH 4/8] refactor: split the compile_commands.json pattern search --- cpp_linter_hooks/clang_tidy.py | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/cpp_linter_hooks/clang_tidy.py b/cpp_linter_hooks/clang_tidy.py index f3f6843..d2b9c28 100644 --- a/cpp_linter_hooks/clang_tidy.py +++ b/cpp_linter_hooks/clang_tidy.py @@ -184,6 +184,22 @@ def _subdirectories(parent: str) -> list[str]: return [] +def _newest_database(parent: str, prefix: str, names: list[str]) -> str | None: + """Return the prefix-matching directory with the newest compile_commands.json.""" + newest, newest_mtime = None, 0.0 + for name in names: + if not name.startswith(prefix): + continue + directory = f"{parent}/{name}" if parent else name + try: + mtime = os.stat(os.path.join(directory, COMPILE_COMMANDS_JSON)).st_mtime + except OSError: + continue + if newest is None or mtime > newest_mtime: + newest, newest_mtime = directory, mtime + return newest + + def _find_compile_commands() -> str | None: """Return the first common directory containing compile_commands.json.""" # os.path and os.scandir instead of pathlib: this runs on every commit. @@ -193,22 +209,11 @@ def _find_compile_commands() -> str | None: listings: dict[str, list[str]] = {} for pattern in COMPILE_DB_SEARCH_GLOBS: parent, _, name = pattern.rpartition("/") - prefix = name.removesuffix("*") if parent not in listings: listings[parent] = _subdirectories(parent or ".") - mtimes = {} - for subdirectory in listings[parent]: - if not subdirectory.startswith(prefix): - continue - directory = f"{parent}/{subdirectory}" if parent else subdirectory - try: - mtimes[directory] = os.stat( - os.path.join(directory, COMPILE_COMMANDS_JSON) - ).st_mtime - except OSError: - continue - if mtimes: - return max(mtimes, key=mtimes.__getitem__) + found = _newest_database(parent, name.removesuffix("*"), listings[parent]) + if found: + return found return None From dbccc24c13dc198058fb445888cc2cc295bb6754 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:04:43 +0000 Subject: [PATCH 5/8] fix: keep line filters exact and short for large commits - clang-tidy uses the first --line-filter name that ends the reported path, so list longer names first: "a.cpp" no longer takes the lines of "data.cpp" and hides its new warnings. - Give each parallel run only its own file's filter, and split a serial run into several commands when the filter would make the command line too long (Windows: 32,767 characters, Linux: 128 KiB per argument). - Report any OSError from starting clang-tidy as a failed hook instead of a traceback. --- cpp_linter_hooks/clang_tidy.py | 100 ++++++++++++++++++++++++++++----- tests/test_clang_tidy.py | 97 ++++++++++++++++++++++++++++++-- 2 files changed, 178 insertions(+), 19 deletions(-) diff --git a/cpp_linter_hooks/clang_tidy.py b/cpp_linter_hooks/clang_tidy.py index d2b9c28..39b7ca4 100644 --- a/cpp_linter_hooks/clang_tidy.py +++ b/cpp_linter_hooks/clang_tidy.py @@ -24,6 +24,9 @@ # Files clang-tidy finds by itself in a source file's directory or its parents. COMPILE_COMMANDS_JSON = "compile_commands.json" COMPILATION_DATABASE_FILES = (COMPILE_COMMANDS_JSON, "compile_flags.txt") +# Windows limits a whole command line to 32,767 characters and Linux limits one +# argument to 128 KiB. Commands that carry a --line-filter stay below this. +COMMAND_LINE_BUDGET = 30_000 if os.name == "nt" else 100_000 SOURCE_FILE_SUFFIXES = { ".c", ".cc", @@ -395,7 +398,8 @@ def _exec_clang_tidy(command) -> tuple[int, str]: 1 if sp.returncode != 0 or "warning:" in output or "error:" in output else 0 ) return retval, output - except FileNotFoundError as e: + except OSError as e: + # For example a missing executable, or a command line that is too long. return 1, str(e) @@ -488,17 +492,55 @@ def _repair_check_args(args: list[str]) -> tuple[list[str], list[str]]: def _line_filter_option(lines: dict[str, LineRanges]) -> str: """Build clang-tidy's --line-filter option for the given line ranges. - clang-tidy matches each name against the end of the path it reports, and - drops diagnostics in files that are not listed. + clang-tidy matches each name against the end of the path it reports, uses + the first entry that matches, and drops diagnostics in files that are not + listed. Longer names come first, so "a.cpp" cannot take the lines of + "data.cpp". """ entries = [] for path, ranges in lines.items(): name = normalize(path) names = dict.fromkeys((name, name.replace("/", os.sep))) entries.extend({"name": n, "lines": ranges} for n in names) + entries.sort(key=lambda entry: len(entry["name"]), reverse=True) return "--line-filter=" + json.dumps(entries, separators=(",", ":")) +def _command_line_length(arg: str) -> int: + """Return about how much of the command line an argument takes. + + Windows quotes an argument with spaces and escapes each double quote. + """ + return len(arg) + arg.count('"') + 3 + + +def _line_filter_batches( + command_prefix: list[str], source_files: list[str], lines: dict[str, LineRanges] +) -> list[list[str]]: + """Split the files into commands that each carry a --line-filter for them. + + The filter of a large commit can be longer than the operating system + allows for one command, so the files are split into several commands. + """ + budget = COMMAND_LINE_BUDGET - sum(map(_command_line_length, command_prefix)) + batches: list[list[str]] = [] + batch: list[str] = [] + used = 0 + for source_file in source_files: + cost = _command_line_length( + _line_filter_option({source_file: lines[source_file]}) + ) + _command_line_length(source_file) + if batch and used + cost > budget: + batches.append(batch) + batch, used = [], 0 + batch.append(source_file) + used += cost + batches.append(batch) + return [ + [_line_filter_option({f: lines[f] for f in batch}), *batch] for batch in batches + ] + + def _append_notes(output: str, notes: list[str]) -> str: """Append configuration notes after clang-tidy's output.""" if not notes: @@ -529,20 +571,51 @@ def _combine_outputs(results: list[tuple[int, str]]) -> str: return "\n".join(output.rstrip("\n") for _, output in results if output) +def _combine_results(results: list[tuple[int, str]]) -> tuple[int, str]: + """Combine the results of several clang-tidy runs into one.""" + retval = 1 if any(retval != 0 for retval, _ in results) else 0 + return retval, _combine_outputs(results) + + +def _exec_serial_clang_tidy( + command_prefix: list[str], + source_files: list[str], + lines: dict[str, LineRanges] | None, +) -> tuple[int, str]: + """Run clang-tidy over all source files, in one command when possible.""" + if lines is None: + return _exec_clang_tidy(command_prefix + source_files) + # Several commands only for very large commits; they run one after another, + # so -fix stays safe. + return _combine_results( + [ + _exec_clang_tidy(command_prefix + batch) + for batch in _line_filter_batches(command_prefix, source_files, lines) + ] + ) + + def _exec_parallel_clang_tidy( - command_prefix: list[str], source_files: list[str], jobs: int + command_prefix: list[str], + source_files: list[str], + jobs: int, + lines: dict[str, LineRanges] | None = None, ) -> tuple[int, str]: """Run clang-tidy over source files in parallel and combine the results.""" def run_file(source_file: str) -> tuple[int, str]: - """Run clang-tidy for a single source file.""" - return _exec_clang_tidy(command_prefix + [source_file]) + """Run clang-tidy for a single source file, with its own line filter.""" + line_filter = ( + [] + if lines is None + else [_line_filter_option({source_file: lines[source_file]})] + ) + return _exec_clang_tidy(command_prefix + line_filter + [source_file]) with ThreadPoolExecutor(max_workers=min(jobs, len(source_files))) as executor: results = list(executor.map(run_file, source_files)) - retval = 1 if any(retval != 0 for retval, _ in results) else 0 - return retval, _combine_outputs(results) + return _combine_results(results) def run_clang_tidy(args=None) -> tuple[int, str]: @@ -556,12 +629,14 @@ def run_clang_tidy(args=None) -> tuple[int, str]: clang_tidy_args, source_files = _split_source_files(other_args) clang_tidy_args, notes = _repair_check_args(clang_tidy_args) + lines = None if not hook_args.all_lines and not _has_option( clang_tidy_args, "--line-filter", "-line-filter" ): lines = changed_lines(source_files) if lines is not None: source_files = [f for f in source_files if lines[f]] + lines = {f: lines[f] for f in source_files} if hook_args.verbose: print( "Reporting only lines changed in git diff " @@ -570,9 +645,6 @@ def run_clang_tidy(args=None) -> tuple[int, str]: ) if not source_files: return 0, _append_notes("", notes) - clang_tidy_args.append( - _line_filter_option({f: lines[f] for f in source_files}) - ) if compile_db_path: if hook_args.verbose: @@ -626,10 +698,12 @@ def run_clang_tidy(args=None) -> tuple[int, str]: if hook_args.jobs > 1 and len(source_files) > 1 and not unsafe_parallel: retval, output = _exec_parallel_clang_tidy( - [executable] + clang_tidy_args, source_files, hook_args.jobs + [executable] + clang_tidy_args, source_files, hook_args.jobs, lines ) else: - retval, output = _exec_clang_tidy([executable] + clang_tidy_args + source_files) + retval, output = _exec_serial_clang_tidy( + [executable] + clang_tidy_args, source_files, lines + ) if not hook_args.verbose: output = _strip_noise(output) diff --git a/tests/test_clang_tidy.py b/tests/test_clang_tidy.py index dea7a9d..7a432f7 100644 --- a/tests/test_clang_tidy.py +++ b/tests/test_clang_tidy.py @@ -1,3 +1,4 @@ +import json import os import subprocess import sys @@ -1177,17 +1178,80 @@ def test_line_filter_reaches_each_parallel_run(monkeypatch, mock_exec): run_clang_tidy(["--jobs=2", "a.cpp", "b.cpp"]) - line_filter = ( - '--line-filter=[{"name":"a.cpp","lines":[[1,1]]},' - '{"name":"b.cpp","lines":[[2,2]]}]' - ) commands = sorted(call.args[0] for call in mock_exec.call_args_list) assert commands == [ - ["clang-tidy", "-quiet", line_filter, "a.cpp"], - ["clang-tidy", "-quiet", line_filter, "b.cpp"], + [ + "clang-tidy", + "-quiet", + '--line-filter=[{"name":"a.cpp","lines":[[1,1]]}]', + "a.cpp", + ], + [ + "clang-tidy", + "-quiet", + '--line-filter=[{"name":"b.cpp","lines":[[2,2]]}]', + "b.cpp", + ], ] +def test_line_filter_lists_longer_names_first(monkeypatch): + # clang-tidy uses the first name that ends the reported path, and + # "data.cpp" ends with "a.cpp". + monkeypatch.setattr(clang_tidy.os, "sep", "/") + + option = _line_filter_option({"a.cpp": [[5, 5]], "data.cpp": [[1, 1]]}) + + assert option == ( + '--line-filter=[{"name":"data.cpp","lines":[[1,1]]},' + '{"name":"a.cpp","lines":[[5,5]]}]' + ) + + +def test_large_commit_is_split_into_short_commands(monkeypatch, mock_exec): + files = [f"src/module/file_{i}.cpp" for i in range(40)] + monkeypatch.setattr(clang_tidy, "COMMAND_LINE_BUDGET", 1000) + monkeypatch.setattr( + clang_tidy, "changed_lines", lambda paths: {f: [[1, 2]] for f in paths} + ) + + run_clang_tidy(files) + + commands = [call.args[0] for call in mock_exec.call_args_list] + assert len(commands) > 1 + assert [f for command in commands for f in command[3:]] == files + for command in commands: + assert sum(len(arg) + arg.count('"') + 3 for arg in command) <= 1000 + assert command[:2] == ["clang-tidy", "-quiet"] + names = {entry["name"] for entry in json.loads(command[2].split("=", 1)[1])} + assert {f.replace("/", os.sep) for f in command[3:]} <= names + + +def test_split_commands_combine_their_results(monkeypatch, mock_exec): + monkeypatch.setattr(clang_tidy, "COMMAND_LINE_BUDGET", 0) + monkeypatch.setattr( + clang_tidy, "changed_lines", lambda paths: {f: [[1, 1]] for f in paths} + ) + mock_exec.side_effect = [(0, ""), (1, "b.cpp:1:1: warning: x")] + + ret, output = run_clang_tidy(["a.cpp", "b.cpp"]) + + assert mock_exec.call_count == 2 + assert ret == 1 + assert output == "b.cpp:1:1: warning: x" + + +def test_command_that_cannot_start_fails_the_hook(): + with patch( + "cpp_linter_hooks.clang_tidy.subprocess.run", + side_effect=OSError(7, "Argument list too long"), + ): + ret, output = clang_tidy._exec_clang_tidy(["clang-tidy", "a.cpp"]) + + assert ret == 1 + assert "Argument list too long" in output + + def _git(repo, *args): subprocess.run( [ @@ -1266,6 +1330,27 @@ def test_real_tool_fixes_only_the_changed_lines(braces_repo): assert "if (x > 1) {" not in fixed +def test_real_tool_keeps_the_lines_of_files_with_similar_names(braces_repo, tmp_path): + # "data.cpp" ends with "a.cpp": its new warning on line 2 must not be + # filtered with the lines of a.cpp. + (tmp_path / "a.cpp").write_text("int a() {\n return 0;\n}\n") + (tmp_path / "data.cpp").write_text("int d(int x) {\n return 0;\n}\n") + _git(tmp_path, "add", ".") + _git(tmp_path, "commit", "-q", "-m", "more") + (tmp_path / "a.cpp").write_text( + "int a() {\n return 0;\n}\nint b() { return 1; }\n" + ) + (tmp_path / "data.cpp").write_text( + "int d(int x) {\n if (x)\n return 1;\n return 0;\n}\n" + ) + _git(tmp_path, "add", ".") + + ret, output = run_clang_tidy(CHECK_BRACES + ["a.cpp", "data.cpp"]) + + assert ret == 1 + assert "data.cpp:2:" in output + + # --- what happens without a compilation database --- From 928083a9aadf387804b5bfee13c3f86323a597ae Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:16:51 +0000 Subject: [PATCH 6/8] fix: check every line when the file on disk is not the diffed version The staged (or pushed) line numbers only fit the file clang-tidy reads when the two match. pre-commit hides unstaged changes during a commit, but a direct run, --files, or a push with uncommitted changes can differ: then check every line instead of filtering with lines that do not fit. --- AGENTS.md | 2 +- README.md | 4 +++- cpp_linter_hooks/changed_lines.py | 16 +++++++++++++--- tests/test_changed_lines.py | 30 +++++++++++++++++++++++++++--- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fdf5eb8..0e7e7ab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -46,7 +46,7 @@ bash testing/run.sh # Integration tests (full pre-commit pipeline) - **Version resolution**: A wheel already installed in the hook environment (found by its `-.dist-info` directory in site-packages, e.g. from `additional_dependencies`, with its executable in the environment's scripts directory) is used without network access when it matches `--version` (exact or prefix) or when no `--version` is given. Otherwise the version is resolved dynamically from PyPI at runtime. No hardcoded version lists. Prefix match supported (`--version=20` → `20.1.8`). - **Return values**: All hooks return `tuple[int, str]` — `(retval, output)`, where `retval` is the tool's exit code (`0` on success, non-zero on failure; `--dry-run` auto-adds `--Werror` so formatting issues fail). - **clang-tidy compile DB**: Auto-detected from `build/`, `out/`, `cmake-build-debug/`, `_build/`, `builddir/`, then `cmake-build-*/`, `build-*/`, `build/*/`, `out/build/*/` (newest wins). Override with `--compile-commands=`, disable with `--no-compile-commands`. If none is found, no `-p`/`--extra-arg*` is given and clang-tidy finds no `compile_commands.json`/`compile_flags.txt` next to the files or in their parents, the hook skips clang-tidy (retval 0, message on stderr), or fails when `CI` is set. -- **clang-tidy changed lines**: By default only diagnostics on added lines are reported, via clang-tidy's `--line-filter` (so `-fix` is limited too). Files without added lines are not checked. If any passed file has no changes (`--all-files`), or git fails, every line is checked. `--all-lines` or a user `--line-filter` turns this off. Tests: `tests/conftest.py` disables this and the missing-database check by default; use the `real_changed_lines` / `real_compile_db_check` fixtures to test them. +- **clang-tidy changed lines**: By default only diagnostics on added lines are reported, via clang-tidy's `--line-filter` (so `-fix` is limited too). Files without added lines are not checked. If any passed file has no changes (`--all-files`), differs on disk from the diffed version (index or `PRE_COMMIT_TO_REF`), or git fails, every line is checked. `--all-lines` or a user `--line-filter` turns this off. Tests: `tests/conftest.py` disables this and the missing-database check by default; use the `real_changed_lines` / `real_compile_db_check` fixtures to test them. - **clang-tidy parallel**: `--jobs=N` runs per-file in parallel. Forced serial when `--export-fixes`, `-fix`, or `-fix-errors` is used. - **clang-tidy output**: Without `--verbose`, the hook adds `-quiet` (unless the user set it) and strips status lines (`N warnings generated.`, `Suppressed N warnings ...`, `Use -header-filter=...`, `[1/N] Processing file ...`). With `--verbose`, output is unchanged. - **clang-tidy check lists**: `--checks=` / `--warnings-as-errors=` lists that YAML split at commas are joined again, quotes inside the value are removed, and `--checks=` (e.g. `.clang-tidy`) gets a note. Notes are appended to the output, which pre-commit shows when the hook fails. diff --git a/README.md b/README.md index bbe9ccc..badab3e 100644 --- a/README.md +++ b/README.md @@ -178,7 +178,9 @@ lines. Compiler errors are always reported. Files with no added lines, such as a not checked. The hook checks every line when the files it gets have no changes, for example with -`pre-commit run --all-files` on a clean working tree, or outside a git repository. To always check +`pre-commit run --all-files` on a clean working tree, when a file on disk differs from the +version the lines come from (unstaged changes in a direct run, or uncommitted changes on +`pre-push`), or outside a git repository. To always check every line, for example in a manual stage, add `--all-lines`: ```yaml diff --git a/cpp_linter_hooks/changed_lines.py b/cpp_linter_hooks/changed_lines.py index fad29a1..3b07e03 100644 --- a/cpp_linter_hooks/changed_lines.py +++ b/cpp_linter_hooks/changed_lines.py @@ -87,10 +87,12 @@ def changed_lines(files: list[str]) -> dict[str, LineRanges] | None: """Return the added line ranges of each file, keyed by the given path. Returns None when the changed lines are unknown and every line should be - checked: outside a git repository, or when a file has no changes, which + checked: outside a git repository, when a file has no changes, which means the hook runs on files that were not changed (``--all-files`` or - ``--files``). A file that is changed but has no added lines, for example a - pure rename, maps to an empty list. + ``--files``), or when a file on disk differs from the version the lines + come from, so the line numbers would not match what clang-tidy reads. A + file that is changed but has no added lines, for example a pure rename, + maps to an empty list. """ if not files: return None @@ -102,6 +104,14 @@ def changed_lines(files: list[str]) -> dict[str, LineRanges] | None: normalized = {path: normalize(path) for path in files} if any(name not in changed for name in normalized.values()): return None + # pre-commit hides unstaged changes during a commit, so the files on disk + # match the index. A direct run, --files or a push with uncommitted + # changes can leave them different from the diffed version. + diffed_version = ( + [] if revisions == ["--cached"] else [os.environ["PRE_COMMIT_TO_REF"]] + ) + if _git_diff(["--quiet", *diffed_version, "--", *files]) is None: + return None diff = _git_diff(["-U0", "--src-prefix=a/", "--dst-prefix=b/", *revisions]) if diff is None: diff --git a/tests/test_changed_lines.py b/tests/test_changed_lines.py index a940414..a30e1ae 100644 --- a/tests/test_changed_lines.py +++ b/tests/test_changed_lines.py @@ -59,14 +59,26 @@ def test_staged_changes_give_the_added_lines(repo): assert changed_lines(["a.cpp"]) == {"a.cpp": [[3, 3], [7, 8]]} -def test_unstaged_changes_are_ignored(repo): +def test_unstaged_changes_in_other_files_are_ignored(repo): + (repo / "b.cpp").write_text("int x;\n") + git(repo, "add", "b.cpp") + git(repo, "commit", "-q", "-m", "b") edit(repo / "a.cpp", {2: "staged\n"}) git(repo, "add", "a.cpp") - edit(repo / "a.cpp", {9: "not staged\n"}) + (repo / "b.cpp").write_text("int y;\n") assert changed_lines(["a.cpp"]) == {"a.cpp": [[2, 2]]} +def test_unstaged_changes_mean_every_line_is_checked(repo): + """The staged line numbers do not match the file that clang-tidy reads.""" + edit(repo / "a.cpp", {2: "staged\n"}) + git(repo, "add", "a.cpp") + (repo / "a.cpp").write_text("not staged\n" + (repo / "a.cpp").read_text()) + + assert changed_lines(["a.cpp"]) is None + + def test_new_file_counts_every_line(repo): (repo / "b.cpp").write_text("int x;\nint y;\n") git(repo, "add", "b.cpp") @@ -131,6 +143,18 @@ def test_first_commit(tmp_path, monkeypatch): def test_pushed_range_from_pre_commit_variables(repo, monkeypatch): + base = git(repo, "rev-parse", "HEAD").strip() + edit(repo / "a.cpp", {5: "pushed 5\n"}) + git(repo, "commit", "-q", "-am", "second") + head = git(repo, "rev-parse", "HEAD").strip() + monkeypatch.setenv("PRE_COMMIT_FROM_REF", base) + monkeypatch.setenv("PRE_COMMIT_TO_REF", head) + + assert changed_lines(["a.cpp"]) == {"a.cpp": [[5, 5]]} + + +def test_pushed_range_with_uncommitted_changes(repo, monkeypatch): + """The file on disk is not the pushed version, so its lines differ.""" base = git(repo, "rev-parse", "HEAD").strip() edit(repo / "a.cpp", {5: "pushed 5\n"}) git(repo, "commit", "-q", "-am", "second") @@ -140,7 +164,7 @@ def test_pushed_range_from_pre_commit_variables(repo, monkeypatch): monkeypatch.setenv("PRE_COMMIT_FROM_REF", base) monkeypatch.setenv("PRE_COMMIT_TO_REF", head) - assert changed_lines(["a.cpp"]) == {"a.cpp": [[5, 5]]} + assert changed_lines(["a.cpp"]) is None def test_outside_a_git_repository(tmp_path, monkeypatch): From be0384acf6005526389a8f6be768a0bc0bf25516 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Thu, 22 Oct 2026 16:13:43 +0000 Subject: [PATCH 7/8] fix: do not read an added "++ " line as a diff file header An added line "++ i;" shows as "+++ i;" in the diff, which the parser took for the next file's header, so later hunks of the file were lost. Skip each hunk's lines by their counts. Also say in the docs that --fix applies the fixes of the reported problems, which can touch a nearby unchanged line. --- AGENTS.md | 2 +- README.md | 5 +++-- cpp_linter_hooks/changed_lines.py | 23 +++++++++++++++++------ tests/test_changed_lines.py | 21 +++++++++++++++++++++ 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0e7e7ab..e5552dc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -46,7 +46,7 @@ bash testing/run.sh # Integration tests (full pre-commit pipeline) - **Version resolution**: A wheel already installed in the hook environment (found by its `-.dist-info` directory in site-packages, e.g. from `additional_dependencies`, with its executable in the environment's scripts directory) is used without network access when it matches `--version` (exact or prefix) or when no `--version` is given. Otherwise the version is resolved dynamically from PyPI at runtime. No hardcoded version lists. Prefix match supported (`--version=20` → `20.1.8`). - **Return values**: All hooks return `tuple[int, str]` — `(retval, output)`, where `retval` is the tool's exit code (`0` on success, non-zero on failure; `--dry-run` auto-adds `--Werror` so formatting issues fail). - **clang-tidy compile DB**: Auto-detected from `build/`, `out/`, `cmake-build-debug/`, `_build/`, `builddir/`, then `cmake-build-*/`, `build-*/`, `build/*/`, `out/build/*/` (newest wins). Override with `--compile-commands=`, disable with `--no-compile-commands`. If none is found, no `-p`/`--extra-arg*` is given and clang-tidy finds no `compile_commands.json`/`compile_flags.txt` next to the files or in their parents, the hook skips clang-tidy (retval 0, message on stderr), or fails when `CI` is set. -- **clang-tidy changed lines**: By default only diagnostics on added lines are reported, via clang-tidy's `--line-filter` (so `-fix` is limited too). Files without added lines are not checked. If any passed file has no changes (`--all-files`), differs on disk from the diffed version (index or `PRE_COMMIT_TO_REF`), or git fails, every line is checked. `--all-lines` or a user `--line-filter` turns this off. Tests: `tests/conftest.py` disables this and the missing-database check by default; use the `real_changed_lines` / `real_compile_db_check` fixtures to test them. +- **clang-tidy changed lines**: By default only diagnostics on added lines are reported, via clang-tidy's `--line-filter`, so `-fix` applies only the fixes of those diagnostics (a fix may still edit an unchanged line nearby). Files without added lines are not checked. If any passed file has no changes (`--all-files`), differs on disk from the diffed version (index or `PRE_COMMIT_TO_REF`), or git fails, every line is checked. `--all-lines` or a user `--line-filter` turns this off. Tests: `tests/conftest.py` disables this and the missing-database check by default; use the `real_changed_lines` / `real_compile_db_check` fixtures to test them. - **clang-tidy parallel**: `--jobs=N` runs per-file in parallel. Forced serial when `--export-fixes`, `-fix`, or `-fix-errors` is used. - **clang-tidy output**: Without `--verbose`, the hook adds `-quiet` (unless the user set it) and strips status lines (`N warnings generated.`, `Suppressed N warnings ...`, `Use -header-filter=...`, `[1/N] Processing file ...`). With `--verbose`, output is unchanged. - **clang-tidy check lists**: `--checks=` / `--warnings-as-errors=` lists that YAML split at commas are joined again, quotes inside the value are removed, and `--checks=` (e.g. `.clang-tidy`) gets a note. Notes are appended to the output, which pre-commit shows when the hook fails. diff --git a/README.md b/README.md index badab3e..788b59e 100644 --- a/README.md +++ b/README.md @@ -173,8 +173,9 @@ warnings elsewhere in a file do not block your commit. It reads the changes with - With `pre-commit run --from-ref A --to-ref B` and in the `pre-push` stage: the changes between the two commits. -The hook passes these lines to clang-tidy's `--line-filter`, so `--fix` also changes only those -lines. Compiler errors are always reported. Files with no added lines, such as a pure rename, are +The hook passes these lines to clang-tidy's `--line-filter`, so `--fix` applies only the fixes +for problems reported on those lines (a fix can still touch a nearby line, for example to add a +closing brace). Compiler errors are always reported. Files with no added lines, such as a pure rename, are not checked. The hook checks every line when the files it gets have no changes, for example with diff --git a/cpp_linter_hooks/changed_lines.py b/cpp_linter_hooks/changed_lines.py index 3b07e03..b13145e 100644 --- a/cpp_linter_hooks/changed_lines.py +++ b/cpp_linter_hooks/changed_lines.py @@ -6,8 +6,9 @@ import subprocess from pathlib import PurePath -# "@@ -12,3 +14,2 @@": the new side starts at line 14 and spans 2 lines. -HUNK_HEADER = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@") +# "@@ -12,3 +14,2 @@": the old side spans 3 lines; the new side starts at line +# 14 and spans 2 lines. A missing count means 1. +HUNK_HEADER = re.compile(r"^@@ -\d+(?:,(\d+))? \+(\d+)(?:,(\d+))? @@") LineRanges = list[list[int]] @@ -65,7 +66,15 @@ def _parse_added_lines(diff: str) -> dict[str, LineRanges]: """Return the added line ranges of each file in a ``git diff -U0`` output.""" ranges: dict[str, LineRanges] = {} current: LineRanges | None = None + # Lines of the current hunk not read yet. They are skipped, because an + # added line such as "++ i;" looks like a "+++ " file header. + hunk_lines = 0 for line in diff.splitlines(): + if hunk_lines: + # "\ No newline at end of file" is not one of the hunk's lines. + if line[:1] in ("+", "-", " "): + hunk_lines -= 1 + continue if line.startswith("+++ "): name = _unquote_path(line[4:].rstrip("\t")) if name == "/dev/null": @@ -74,11 +83,13 @@ def _parse_added_lines(diff: str) -> dict[str, LineRanges]: current = ranges.setdefault(name.removeprefix("b/"), []) continue match = HUNK_HEADER.match(line) - if match is None or current is None: + if match is None: continue - start = int(match.group(1)) - count = 1 if match.group(2) is None else int(match.group(2)) - if count > 0: + old_count = 1 if match.group(1) is None else int(match.group(1)) + start = int(match.group(2)) + count = 1 if match.group(3) is None else int(match.group(3)) + hunk_lines = old_count + count + if count > 0 and current is not None: current.append([start, start + count - 1]) return ranges diff --git a/tests/test_changed_lines.py b/tests/test_changed_lines.py index a30e1ae..0347482 100644 --- a/tests/test_changed_lines.py +++ b/tests/test_changed_lines.py @@ -217,22 +217,43 @@ def test_normalize(path, expected): assert normalize(path) == expected +def test_added_line_that_looks_like_a_file_header(repo): + """An added "++ i;" shows as "+++ i;" in the diff.""" + edit(repo / "a.cpp", {2: "line 2\n++ i;\n", 6: "changed 6\n"}) + git(repo, "add", "a.cpp") + + assert changed_lines(["a.cpp"]) == {"a.cpp": [[3, 3], [7, 7]]} + + def test_parse_added_lines(): lines = [ "diff --git a/a.cpp b/a.cpp", "--- a/a.cpp", "+++ b/a.cpp", "@@ -3 +3 @@", + "-old", + "+new", "@@ -10,0 +11,4 @@ int main() {", + "+++ i;", + "+--- j;", + "+", + "+x", + "\\ No newline at end of file", "@@ -20,2 +23,0 @@", + "-a", + "-b", "diff --git a/gone.cpp b/gone.cpp", "--- a/gone.cpp", "+++ /dev/null", "@@ -1,2 +0,0 @@", + "--- x;", + "-y", "diff --git a/b.cpp b/b.cpp", "--- a/b.cpp", '+++ "b/sp\\303\\251cial \\"q\\".cpp"', "@@ -0,0 +1,2 @@", + "+a", + "+b", ] assert _parse_added_lines("\n".join(lines)) == { From e206344d17629e0706de17e6455640e83f6649cb Mon Sep 17 00:00:00 2001 From: Xianpeng Shen <3353385+shenxianpeng@users.noreply.github.com> Date: Thu, 22 Oct 2026 16:16:06 +0000 Subject: [PATCH 8/8] refactor: move hunk skipping out of the diff parser Keeps _parse_added_lines simple (SonarCloud cognitive complexity) and covers a diff that ends inside a hunk. --- cpp_linter_hooks/changed_lines.py | 47 ++++++++++++++++++++----------- tests/test_changed_lines.py | 6 ++++ 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/cpp_linter_hooks/changed_lines.py b/cpp_linter_hooks/changed_lines.py index b13145e..4aff497 100644 --- a/cpp_linter_hooks/changed_lines.py +++ b/cpp_linter_hooks/changed_lines.py @@ -4,6 +4,7 @@ import os import re import subprocess +from collections.abc import Iterator from pathlib import PurePath # "@@ -12,3 +14,2 @@": the old side spans 3 lines; the new side starts at line @@ -62,35 +63,47 @@ def _unquote_path(path: str) -> str: return path +def _count(group: str | None) -> int: + """Return a hunk header's line count, which is 1 when git leaves it out.""" + return 1 if group is None else int(group) + + +def _skip_hunk_body(lines: Iterator[str], count: int) -> None: + """Consume the next count lines of a hunk. + + They are skipped, not parsed, because an added line such as "++ i;" + looks like a "+++ " file header. "\\ No newline at end of file" is not + one of the hunk's lines. + """ + while count > 0: + line = next(lines, None) + if line is None: + return + if line[:1] in ("+", "-", " "): + count -= 1 + + def _parse_added_lines(diff: str) -> dict[str, LineRanges]: """Return the added line ranges of each file in a ``git diff -U0`` output.""" ranges: dict[str, LineRanges] = {} current: LineRanges | None = None - # Lines of the current hunk not read yet. They are skipped, because an - # added line such as "++ i;" looks like a "+++ " file header. - hunk_lines = 0 - for line in diff.splitlines(): - if hunk_lines: - # "\ No newline at end of file" is not one of the hunk's lines. - if line[:1] in ("+", "-", " "): - hunk_lines -= 1 - continue + lines = iter(diff.splitlines()) + for line in lines: if line.startswith("+++ "): name = _unquote_path(line[4:].rstrip("\t")) - if name == "/dev/null": - current = None - else: - current = ranges.setdefault(name.removeprefix("b/"), []) + current = ( + None + if name == "/dev/null" + else ranges.setdefault(name.removeprefix("b/"), []) + ) continue match = HUNK_HEADER.match(line) if match is None: continue - old_count = 1 if match.group(1) is None else int(match.group(1)) - start = int(match.group(2)) - count = 1 if match.group(3) is None else int(match.group(3)) - hunk_lines = old_count + count + start, count = int(match.group(2)), _count(match.group(3)) if count > 0 and current is not None: current.append([start, start + count - 1]) + _skip_hunk_body(lines, _count(match.group(1)) + count) return ranges diff --git a/tests/test_changed_lines.py b/tests/test_changed_lines.py index 0347482..c445c94 100644 --- a/tests/test_changed_lines.py +++ b/tests/test_changed_lines.py @@ -225,6 +225,12 @@ def test_added_line_that_looks_like_a_file_header(repo): assert changed_lines(["a.cpp"]) == {"a.cpp": [[3, 3], [7, 7]]} +def test_parse_added_lines_of_a_cut_off_diff(): + diff = "+++ b/a.cpp\n@@ -0,0 +1,3 @@\n+a" + + assert _parse_added_lines(diff) == {"a.cpp": [[1, 3]]} + + def test_parse_added_lines(): lines = [ "diff --git a/a.cpp b/a.cpp",