[shenxianpeng]

Problem

clang-format advice is derived by running clang-format <file> and diffing its stdout against the file's current contents. But the exit status was ignored — stderr was only logged at debug level — and the (empty) stdout of a failed run was diffed anyway.

When clang-format exits non-zero, its stdout is empty, so the diff reports the entire file as needing reformatting. The consequences:

• The report flags every line of the file.
• With --fix / --gen-patch-path, the generated patch deletes the whole file's contents.
• In PR-review mode, a suggestion to delete the file's content is posted.

Two realistic triggers:

1. An invalid --style value (eg. a typo like --style=googel).
2. --style=file with a .clang-format that uses a key the installed clang-format version does not recognise (a common version mismatch — clang-format exits with error: unknown key ...).

Reproduction (before this change):

mkdir t && cd t && git init -q
printf '#include <cstdio>\nint main() {\n  printf("hi");\n  return 0;\n}\n' > a.cpp
git add . && git -c user.name=t -c user.email=t@t commit -qm init
cpp-linter -l false --tidy-checks='-*' --style=googel -F
# => "does not conform ... (lines 1,2,3,4,5)" and an auto-fix.patch that deletes a.cpp

The v1 (Python) linter does not have this issue: it reads advice from --output-replacements-xml, so a failed run yields empty XML and therefore no advice.

Fix

After running clang-format, check output.status.success(). On failure, log clang-format's stderr at warning level (so the real cause is visible, not hidden in debug output) and skip the file with no format advice — matching the behaviour when clang-format reports no fixes. On success, stderr is still logged at debug as before.

Tests

• nonzero_exit_skips_file_with_warning: a fake clang-format that exits non-zero with an error on stderr now produces one warning log and empty format advice (no whole-file deletion).
• errors_reported_in_stderr updated to cover the success-with-stderr path (stderr still logged at debug on a successful run).

cargo nextest run (clang_format + run tests), cargo clippy --all-features --all-targets, and cargo fmt --check all pass.

Summary by CodeRabbit

• Bug Fixes
  • Failed formatting runs are now reported as failures, with diagnostic details included in a warning. Format suggestions are no longer generated from unsuccessful runs.

[coderabbitai]

Walkthrough

run_clang_format now returns without diffing output or generating a patch when clang-format exits unsuccessfully. Tests cover this behavior and confirm that successful runs with stderr retain the existing debug-log path.

Changes: clang-format Failure Handling

Layer / File(s)	Summary
Exit status handling and tests cpp-linter/src/clang_tools/clang_format.rs	A nonzero exit logs a warning with stderr, sets empty format advice, and returns early. Tests cover this behavior and successful runs that write to stderr.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: Low · up to 6a17b

A file whose clang-format run fails is now skipped with a warning instead of getting a destructive whole-file patch. That file counts as clean in the final status, so a persistent formatter configuration failure could let checks pass. Owners should be aware of this, but it does not need to block the merge.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title "fix: skip a file when clang-format fails to run" matches the main change. The raw summary says run_clang_format now treats any nonzero clang-format exit as a failed run. It logs a warning w…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title "fix: skip a file when clang-format fails to run" matches the main change. The raw summary says run_clang_format now treats any nonzero clang-format exit as a failed run. It logs a warning with stderr, sets empty format advice, and stops before diffing output. The PR objectives describe the same behavior: a nonzero exit skips format advice for that file with a warning. The title is specific and uses the conventional commit prefix. It names the behavior change, not the implementation detail.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[coderabbitai]

Additional comments: 1

cpp-linter/src/clang_tools/clang_format.rs-107-108: 1

107-108: Major · Data Integrity & Integration · Heavy lift

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Keep failed formatting checks distinct from clean files.

If clang-format rejects the configuration, this returns success with empty advice. post_feedback then reports zero failed formatting checks, and the pre-commit hook can pass without checking the files. A configured PR review can also approve them. (raw.githubusercontent.com)

Track tool failures separately from formatting advice and propagate them to the final status. Continue processing other files without generating a patch for the failed file.

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI (base), Organization UI (inherited)
• Review profile: CHILL
• Plan: Advanced
• Run ID: 21062955-1d75-4e4b-9695-bc1e599ff6b7

Commits

Reviewing files that changed from the base of the PR and between 8bf0fa8 and 6a17b53.

Files selected for processing: 1

• cpp-linter/src/clang_tools/clang_format.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.89%. Comparing base (389dde6) to head (6a17b53).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #550   +/-   ##
=======================================
  Coverage   99.89%   99.89%           
=======================================
  Files          29       29           
  Lines        7631     7664   +33     
=======================================
+ Hits         7623     7656   +33     
  Misses          8        8           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[2bndy5]

Good find. This could've been triggered silently when a project updates the clang version without updating/verifying their clang-format config.