Skip to content

Un-analyzable files should not pass silently or crash the run #551

Open

Description

@shenxianpeng

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?

  • Skip + warn (what fix: skip a file when clang-format fails to run #550 does for case 1): safe, never corrupts, but passes silently.
  • 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.

Activity

  1. 2bndy5 commented on Nov 6, 2026

    @2bndy5
    Collaborator

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions