Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,8 @@ jobs:
verbose: true # optional (default = false)
- name: Test cpp-linter-hooks
run: bash testing/run.sh
- name: Test cpp-linter-hooks with prek
if: matrix.os == 'ubuntu-latest' && matrix.python-version == '3.13'
run: bash testing/run.sh
env:
RUNNER: prek
24 changes: 22 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
[![part of cpp-linter](https://img.shields.io/badge/part%20of-cpp--linter-ffc20a?labelColor=454a63)](https://cpp-linter.github.io/)

[pre-commit](https://pre-commit.com/) hooks that pip-install the clang-format and clang-tidy
version you pin, on every developer's machine.
version you pin, on every developer's machine. They also run with
[prek](https://prek.j178.dev/), which the tests cover too.

[Website](https://cpp-linter.github.io/) ·
[Get started](https://cpp-linter.github.io/getting-started/#before-every-commit) ·
Expand Down Expand Up @@ -43,7 +44,7 @@ This uses the newest clang-format and clang-tidy. To choose a version, add `--ve

### Clang tool version

The `rev` tag (e.g. `v1.8.1`) is the version of these hooks, not of clang-format or clang-tidy.
The `rev` tag is the version of these hooks, not of clang-format or clang-tidy.
Choose the clang tool version with `--version`:

```yaml
Expand Down Expand Up @@ -142,6 +143,24 @@ int main() {for (;;) break; printf("Hello world!\n");return 0;}
^
```

#### Format only changed lines

By default the hook formats whole files. In a code base that was never formatted, that turns a
one-line change into a large diff. Add `--lines-changed-only` to format only the lines that the
commit adds or changes:

```yaml
- id: clang-format
args: [--style=file, --lines-changed-only]
```

The hook reads the changes with `git diff`, in the same way as the clang-tidy hook (see
[Only changed lines](https://github.com/cpp-linter/cpp-linter-hooks#only-changed-lines)), and
passes them to clang-format's `--lines`. clang-format can still touch a nearby line, because it
formats whole statements. The hook formats every line when it cannot tell which lines changed,
for example with `pre-commit run --all-files` on a clean working tree, and when `args` already
has `--lines`.

### clang-tidy

clang-tidy 22 and newer enable no checks by themselves. Without a `.clang-tidy` file or
Expand Down Expand Up @@ -369,6 +388,7 @@ mirror of the clang-format wheel.
| Supports passing format style string | via `--style` | via `--style` |
| Verbose output | via `--verbose` | via `--verbose` |
| Dry-run mode | via `--dry-run` | via `--dry-run --Werror` |
| Format only changed lines | via `--lines-changed-only` | No |
| Auto-fix mode | via `--fix` (clang-tidy only) | No |
| Compilation database support | auto-detect or `--compile-commands` | No |

Expand Down
84 changes: 60 additions & 24 deletions cpp_linter_hooks/clang_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,16 @@
import sys
from argparse import ArgumentParser

from cpp_linter_hooks.util import file_argument, resolve_install_with_diagnostics
from cpp_linter_hooks.changed_lines import changed_lines
from cpp_linter_hooks.util import (
file_argument,
resolve_install_with_diagnostics,
split_source_files,
)

parser = ArgumentParser()
# No abbreviations: clang-format's own --lines must not be read as a short
# form of --lines-changed-only.
parser = ArgumentParser(allow_abbrev=False)
parser.add_argument(
"--version",
default=None,
Expand All @@ -18,6 +25,35 @@
parser.add_argument(
"-v", "--verbose", action="store_true", help="Enable verbose output"
)
parser.add_argument(
"--lines-changed-only",
action="store_true",
help=(
"Format only the lines that the commit or push changes; every line "
"is formatted when the changed lines are unknown, for example with "
"--all-files"
),
)


def _changed_lines_commands(command: list[str], args: list[str]) -> list[list[str]]:
"""Return one clang-format command per file, limited to its changed lines.

clang-format takes ``--lines`` for one file at a time. A file with no
added lines needs no run. When the changed lines are unknown, or the
arguments already name lines, the one command for all files is returned.
"""
options, files = split_source_files(args)
if any(a.lstrip("-").split("=")[0] == "lines" for a in options):
return [[*command, *args]]
lines = changed_lines(files)
if lines is None:
return [[*command, *args]]
return [
[*command, *options, *(f"--lines={a}:{b}" for a, b in lines[f]), f]
for f in files
if lines[f]
]


def run_clang_format(args=None) -> tuple[int, str]:
Expand All @@ -43,32 +79,32 @@
):
command.append("--Werror")

command.extend(other_args)
commands = (
_changed_lines_commands(command, other_args)
if hook_args.lines_changed_only
else [[*command, *other_args]]
)

retval, output = 0, ""
try:
# Run the clang-format command with captured output
sp = subprocess.run(
command,
capture_output=True,
encoding="utf-8",
# clang-format prints source lines, which may not be UTF-8.
errors="replace",
check=False,
)

# Combine stdout and stderr for complete output
output = (sp.stdout or "") + (sp.stderr or "")

retval = sp.returncode

# Print verbose information if requested
if hook_args.verbose:
_print_verbose_info(command, retval, output)

return retval, output

for command in commands:
sp = subprocess.run(
command,
capture_output=True,
encoding="utf-8",
# clang-format prints source lines, which may not be UTF-8.
errors="replace",
check=False,
)

Check failure on line 98 in cpp_linter_hooks/clang_format.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Command Argument Injection via faulty LLM-supplied CLI arguments in clang_format.run_clang_format()

See more on https://sonarcloud.io/project/issues?id=cpp-linter_cpp-linter-hooks&issues=AaHFiKnN_wxgQOln4JIX&open=AaHFiKnN_wxgQOln4JIX&pullRequest=327

Check failure on line 98 in cpp_linter_hooks/clang_format.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Command Injection via faulty LLM-supplied CLI arguments in clang_format.run_clang_format()

See more on https://sonarcloud.io/project/issues?id=cpp-linter_cpp-linter-hooks&issues=AaHFiKnN_wxgQOln4JIW&open=AaHFiKnN_wxgQOln4JIW&pullRequest=327
Comment thread
shenxianpeng marked this conversation as resolved.
Resolved
Comment thread
shenxianpeng marked this conversation as resolved.
Resolved
# Combine stdout and stderr for complete output
command_output = (sp.stdout or "") + (sp.stderr or "")
if hook_args.verbose:
_print_verbose_info(command, sp.returncode, command_output)
retval = retval or sp.returncode
output += command_output
except FileNotFoundError as e:
return 1, str(e)
return retval, output


def _print_verbose_info(command: list, retval: int, output: str) -> None:
Expand Down
60 changes: 6 additions & 54 deletions cpp_linter_hooks/clang_tidy.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@
diff_range,
normalize,
)
from cpp_linter_hooks.util import file_argument, resolve_install_with_diagnostics
from cpp_linter_hooks.util import (
file_argument,
resolve_install_with_diagnostics,
split_source_files,
)

COMPILE_DB_SEARCH_DIRS = ["build", "out", "cmake-build-debug", "_build", "builddir"]
# Searched after COMPILE_DB_SEARCH_DIRS. When a pattern matches several
Expand All @@ -27,33 +31,6 @@
# 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",
".hh",
".hpp",
".hxx",
".h++",
".ino",
".ipp",
".inl",
".ixx",
".m",
".mm",
".tpp",
".txx",
}
COMPILE_COMMANDS_HINT = """\
Generate compile_commands.json with one of:
CMake: cmake -S . -B build -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
Expand Down Expand Up @@ -590,31 +567,6 @@ def _append_notes(output: str, notes: list[str]) -> str:
return output.rstrip("\n") + separator + "\n".join(notes)


def _looks_like_source_file(path: str) -> bool:
"""Return whether a path has a recognized C or C++ source suffix."""
return Path(path).suffix.lower() in SOURCE_FILE_SUFFIXES


def _split_source_files(args: list[str]) -> tuple[list[str], list[str]]:
"""Split clang-tidy options from the source files pre-commit appends.

Going back from the end, an argument is a source file when it has a C or
C++ suffix, or when it is an existing file that does not follow an option,
which could take it as its value (``--export-fixes fixes.yaml``). The
second case covers suffixes added with ``types_or`` or ``files``.
"""
split_idx = len(args)
while split_idx > 0:
arg = args[split_idx - 1]
previous = args[split_idx - 2] if split_idx > 1 else ""
if not _looks_like_source_file(arg) and (
previous.startswith("-") or not os.path.isfile(arg)
):
break
split_idx -= 1
return args[:split_idx], args[split_idx:]


def _combine_outputs(results: list[tuple[int, str]]) -> str:
"""Join non-empty clang-tidy outputs from multiple executions."""
return "\n".join(output.rstrip("\n") for _, output in results if output)
Expand Down Expand Up @@ -685,7 +637,7 @@ def run_clang_tidy(args=None) -> tuple[int, str]:
if error is not None:
return error

clang_tidy_args, source_files = _split_source_files(other_args)
clang_tidy_args, source_files = split_source_files(other_args)
source_files = [file_argument(f) for f in source_files]
clang_tidy_args, notes = _repair_check_args(clang_tidy_args)
# clang-tidy reads everything after "--" as compiler arguments, so the
Expand Down
54 changes: 54 additions & 0 deletions cpp_linter_hooks/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,35 @@
LOG = logging.getLogger(__name__)


SOURCE_FILE_SUFFIXES = {
".c",
".cc",
".ccm",
".cp",
".cpp",
".cppm",
".cxx",
".cxxm",
".c++",
".c++m",
".cu",
".cuh",
".h",
".hh",
".hpp",
".hxx",
".h++",
".ino",
".ipp",
".inl",
".ixx",
".m",
".mm",
".tpp",
".txx",
}


def file_argument(arg: str) -> str:
"""Return a file name that a clang tool cannot read as an option.

Expand All @@ -27,6 +56,31 @@ def file_argument(arg: str) -> str:
return arg


def _looks_like_source_file(path: str) -> bool:
"""Return whether a path has a recognized C or C++ source suffix."""
return Path(path).suffix.lower() in SOURCE_FILE_SUFFIXES


def split_source_files(args: list[str]) -> tuple[list[str], list[str]]:
"""Split a clang tool's options from the source files pre-commit appends.

Going back from the end, an argument is a source file when it has a C or
C++ suffix, or when it is an existing file that does not follow an option,
which could take it as its value (``--export-fixes fixes.yaml``). The
second case covers suffixes added with ``types_or`` or ``files``.
"""
split_idx = len(args)
while split_idx > 0:
arg = args[split_idx - 1]
previous = args[split_idx - 2] if split_idx > 1 else ""
if not _looks_like_source_file(arg) and (
previous.startswith("-") or not os.path.isfile(arg)
):
break
split_idx -= 1
return args[:split_idx], args[split_idx:]


@lru_cache(maxsize=1)
def _env_paths() -> dict[str, str]:
"""Return this Python environment's install paths, which never change at runtime."""
Expand Down
9 changes: 6 additions & 3 deletions testing/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@

set -euo pipefail

# The tool that runs the hooks: pre-commit, or prek with RUNNER=prek.
RUNNER="${RUNNER:-pre-commit}"

PASS=0
FAIL=0

Expand All @@ -27,12 +30,12 @@ run_test() {
local files="$3"
local expect_nonzero="$4"

echo "---- $description ----"
uvx pre-commit clean
echo "---- $description ($RUNNER) ----"
uvx "$RUNNER" clean

local output
# shellcheck disable=SC2086 # word-splitting of $files is intentional
if output=$(uvx pre-commit run -c "$config" --files $files 2>&1); then
if output=$(uvx "$RUNNER" run -c "$config" --files $files 2>&1); then
local got_nonzero=false
else
local got_nonzero=true
Expand Down
Loading
Loading