Skip to content

chore: add clang-tidy pre-commit hook - #571

Merged
egolearner merged 2 commits into
alibaba:mainfrom
Mordris:chore/clang-tidy-pre-commit
Sep 23, 2026
Merged

egolearner merged 2 commits into
alibaba:mainfrom
Mordris:chore/clang-tidy-pre-commit

Conversation

@Mordris

@Mordris Mordris commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Closes #294.

What changed

  • Add a local clang-tidy hook to .pre-commit-config.yaml so the CI clang-tidy check can be run locally before pushing.
  • Add scripts/run_clang_tidy.py, which the hook calls. It mirrors .github/workflows/clang_tidy.yml: only the changed C/C++ files that appear in build/compile_commands.json are analysed, using the repo .clang-tidy config and --warnings-as-errors=*.
  • Files not in the compilation database — and anything under thirdparty/, build/, or the generated gen/ dirs — are skipped, matching the workflow's ignore list.
  • When clang-tidy or the compilation database is unavailable the hook skips with a hint instead of blocking the commit, so contributors without a configured build are not blocked (CI stays the enforcing gate). Set ZVEC_CLANG_TIDY_STRICT=1 to make missing tooling an error; CLANG_TIDY / BUILD_DIR override the binary and build directory.

How to use

cmake -S . -B build -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
pre-commit install

The hook then runs on the C/C++ files you change; commit as usual.

Validation

  • pre-commit validate-config passes; scripts/run_clang_tidy.py passes the repo ruff lint and format checks.
  • Verified against a real local build (cmake -S . -B build -G Ninja -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DBUILD_TOOLS=ON; compilation database with ~1.6k entries; clang-tidy 18.1.1), driven through pre-commit run clang-tidy:
    • clean upstream sources (e.g. files under src/ailego/) pass;
    • a modernize-use-nullptr violation temporarily added to a tracked source file makes the hook fail (exit 1) with the diagnostic, and passes again once reverted;
    • files under thirdparty/ and generated gen/ dirs, and files absent from the compilation database, are skipped;
    • with no build/compile_commands.json the hook skips without blocking the commit (and fails under ZVEC_CLANG_TIDY_STRICT=1).

Mordris and others added 2 commits September 23, 2026 10:42
Run the same clang-tidy static analysis as CI locally before pushing.
scripts/run_clang_tidy.py analyses only the changed C/C++ files present in
build/compile_commands.json, using the repo .clang-tidy config and
--warnings-as-errors=*, matching .github/workflows/clang_tidy.yml.

When clang-tidy or the compilation database is unavailable the hook skips
with a hint instead of blocking the commit, so a configured build is not
required to commit; set ZVEC_CLANG_TIDY_STRICT=1 to make missing tooling
an error.
Move the clang-tidy hook to the pre-push stage so it does not slow down
every commit, and pin clang-tidy to 18.1.8 via a pre-commit managed
virtualenv to match the CI workflow.

Locate compile_commands.json flexibly: the repo root wins outright,
otherwise the configured/well-known build directories are tried, falling
back to a shallow scan of the working tree up to two levels deep.
@egolearner
egolearner force-pushed the chore/clang-tidy-pre-commit branch from d904284 to c673bc6 Compare September 23, 2026 02:56
@egolearner

Copy link
Copy Markdown
Collaborator

Thanks a lot for this contribution, @Mordris — solid foundation for wiring clang-tidy into the local workflow! 🙏

I've pushed a follow-up commit on top of your work to align it with the direction we'd like to take here

  1. Runs at the pre-push stage instead of on every commit, so day-to-day commits stay fast while pushes still get checked.
  2. Pins clang-tidy==18.1.8 via pre-commit's managed virtualenv (language: python + additional_dependencies) to match the version used in the CI clang_tidy workflow.
  3. Flexible compile_commands.json discovery: repo root wins outright, otherwise $BUILD_DIR / the well-known build, build.release, build.debug dirs are tried, falling back to a shallow scan of the working tree (up to two levels).

Comment thread .pre-commit-config.yaml
@egolearner
egolearner merged commit e9c4803 into alibaba:main Sep 23, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

clang-tidy: add pre-commit check

4 participants