You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Un-analyzable files should not pass silently or crash the run #551
Follow-up to #550. While testing rc.25 I hit two related cases where cpp-linter can't actually analyze a file:
1. clang-format exits non-zero (bad config). Fixed the destructive part in #550: an invalid --style, or a .clang-format using a key the installed clang-format version rejects, now logs a warning and skips the file (before, it diffed the empty stdout and proposed deleting the whole file). But the file now counts as clean — checks-failed stays 0, the pre-commit hook passes, and a PR review can approve. A project that bumps its clang version without updating/verifying .clang-format would get green CI even though clang-format never ran.
2. Non-UTF-8 source file.run_clang_format reads the source with fs::read_to_string, so a single latin-1/GBK file (still common in older C/C++) returns stream did not contain valid UTF-8 and aborts the whole run (exit 1) — every other file loses its feedback. (The clang-tidy path reads bytes, so it's unaffected.)
Both come down to one design question: when cpp-linter cannot analyze a file, what should happen?
Surface it as a failure: e.g. count it into checks-failed so the step / pre-commit fails and the output reflects it, while still processing the other files and not generating a patch for the failed one.
For case 2 specifically: either skip+warn, or diff on raw bytes so non-UTF-8 files can still be formatted. (A lossy from_utf8_lossy isn't safe here — it would corrupt a --fix patch.)
Our lean (Xianpeng and me): case 1 shouldn't pass silently — a broken formatter config ought to fail CI. For case 2 I'd start by just fixing the crash (don't abort the whole run) and handle it the same way as case 1 for consistency; byte-accurate diffing of non-UTF-8 files is a bigger change in the shared diff code and could follow later if there's demand.
@2bndy5 which surface do you prefer? Happy to send a PR once the direction is settled.
I'm more interested in byte-accurate diffing. If that is a multi-task goal, then we can just stack PRs (I've been really liking that recently).
My thinking
Config error (all attempts fail)
If it is a config error, then no attempts to run clang-format should exit 0. In that case we should crash. I heard from a user (in cpp-linter repo issue) that they expected clang-format to crash cpp-linter on non-zero exit code.
UTF8 or other problem (only certain files fail)
If clang-format fails for only certain files, we should treat this as a log warning and add it to the clang-format-checks-failed output. I hope the byte-accurate approach reduces the surface area for this case.
When there is only 1 file to be formatted, and it exits non-zero: crash cpp-linter.
Activity
2bndy5 commentedon Nov 6, 2026
I'm more interested in byte-accurate diffing. If that is a multi-task goal, then we can just stack PRs (I've been really liking that recently).
My thinking
Config error (all attempts fail)
If it is a config error, then no attempts to run clang-format should exit 0. In that case we should crash. I heard from a user (in cpp-linter repo issue) that they expected clang-format to crash cpp-linter on non-zero exit code.
UTF8 or other problem (only certain files fail)
If clang-format fails for only certain files, we should treat this as a log warning and add it to the clang-format-checks-failed output. I hope the byte-accurate approach reduces the surface area for this case.
When there is only 1 file to be formatted, and it exits non-zero: crash cpp-linter.