diff --git a/AGENTS.md b/AGENTS.md index c84bc81..e5552dc 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` 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 3dfeaad..788b59e 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,34 @@ 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` 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 +`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 + - 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 +216,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 +240,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 +309,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 +328,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..4aff497 --- /dev/null +++ b/cpp_linter_hooks/changed_lines.py @@ -0,0 +1,144 @@ +"""Find the lines that a commit or push changes, using git.""" + +import codecs +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 +# 14 and spans 2 lines. A missing count means 1. +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, timeout=60) + except (OSError, subprocess.TimeoutExpired): + 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 _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 = iter(diff.splitlines()) + for line in lines: + if line.startswith("+++ "): + name = _unquote_path(line[4:].rstrip("\t")) + current = ( + None + if name == "/dev/null" + else ranges.setdefault(name.removeprefix("b/"), []) + ) + continue + match = HUNK_HEADER.match(line) + if match is None: + continue + 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 + + +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, when a file has no changes, which + means the hook runs on files that were not changed (``--all-files`` or + ``--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 + 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 + # 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: + 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..39b7ca4 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,35 @@ 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. +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", + ".ccm", ".cp", ".cpp", + ".cppm", ".cxx", + ".cxxm", ".c++", + ".c++m", ".cu", ".cuh", ".h", @@ -24,9 +45,11 @@ ".hpp", ".hxx", ".h++", + ".ino", ".ipp", ".inl", ".ixx", + ".mm", ".tpp", ".txx", } @@ -34,7 +57,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,22 +166,131 @@ 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 _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 _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. 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: + parent, _, name = pattern.rpartition("/") + if parent not in listings: + listings[parent] = _subdirectories(parent or ".") + found = _newest_database(parent, name.removesuffix("*"), listings[parent]) + if found: + return found 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 +308,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: @@ -185,7 +318,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), @@ -201,7 +334,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", @@ -265,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) @@ -355,6 +489,58 @@ 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, 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: @@ -385,50 +571,107 @@ 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]: """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) + + 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 " + f"{' '.join(diff_range())}; use --all-lines for every line", + file=sys.stderr, + ) + if not source_files: + return 0, _append_notes("", notes) + 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. @@ -455,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/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..c445c94 --- /dev/null +++ b/tests/test_changed_lines.py @@ -0,0 +1,268 @@ +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_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") + (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") + + 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() + 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") + 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"]) is None + + +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 + + +@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 + + +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_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_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", + "--- 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)) == { + "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..7a432f7 100644 --- a/tests/test_clang_tidy.py +++ b/tests/test_clang_tidy.py @@ -1,3 +1,5 @@ +import json +import os import subprocess import sys import time @@ -6,6 +8,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 +16,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 +289,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 +1085,417 @@ 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"]) + + commands = sorted(call.args[0] for call in mock_exec.call_args_list) + assert commands == [ + [ + "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( + [ + "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 + + +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 --- + + +@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_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"): + (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], + )