Skip to content

Fix #3: Add project validator CI - #4

Open
leftibot wants to merge 4 commits into
cpp-best-practices:mainfrom
leftibot:fix/issue-3-add-project-validator-ci
Open

leftibot wants to merge 4 commits into
cpp-best-practices:mainfrom
leftibot:fix/issue-3-add-project-validator-ci

Conversation

@leftibot

@leftibot leftibot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Automated fix by @leftibot.

What changed

Fix #3: Run cpp-best-practices/project_validator in CI
The project was never checked against the project_validator standards. A new
project-validator CI job checks out the validator's main branch, sets up
Python 3.14 with PyYAML, and runs it with --build against this repository.
A unit test asserts the workflow keeps that job, pinned to the main branch.
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com

Files

 .github/workflows/ci.yml  | 22 ++++++++++++++++++
 tests/test_ci_workflow.py | 57 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

Closes #3

Triggered by @lefticus.

The project was never checked against the project_validator standards. A new
project-validator CI job checks out the validator's main branch, sets up
Python 3.14 with PyYAML, and runs it with --build against this repository.
A unit test asserts the workflow keeps that job, pinned to the main branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lefticus

lefticus commented Nov 6, 2026

Copy link
Copy Markdown
Member

@leftibot address issues from project_validator by enhancing the CI

@leftibot

leftibot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor Author

✅ leftibot finished addressing the review feedback.

leftibot and others added 3 commits November 6, 2026 14:05
…lyzers

project_validator found no C++ ever compiled: no warnings, hardening,
sanitizers, language-extension setting or static analysis. The test cases
are now built as an object library with C++20 (extensions off),
-Wall -Wextra -Wpedantic -Wshadow -Wconversion -Wsign-conversion and
hardening flags, with options for sanitizers, coverage, clang-tidy and
cppcheck. This also guarantees every test case is valid C++, so a check
cannot pass on code the compiler rejects.

A cpp-best-practices .clang-format and a strict .clang-tidy are added;
checks/.clang-tidy turns off the clang-tidy checks that duplicate the
custom checks the test cases deliberately trigger. The minimum Python
is raised to 3.10, as 3.9 is end-of-life and mypy no longer checks it.

Requested by @lefticus in PR cpp-best-practices#4 review.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds ruff (format and lint, 120 columns as the code already used) and
strict mypy configuration, with a stub for the cppcheck addon module, and
applies the resulting formatting and annotations.

Requested by @lefticus in PR cpp-best-practices#4 review.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
check-tests now builds Debug and Release, with and without sanitizers,
on Linux gcc (with cppcheck), Linux clang (with clang-tidy) and macOS
AppleClang, across Python 3.10, 3.12 and 3.14, with warnings as errors.
New jobs report Python and C++ branch coverage, check formatting, lint,
type-check, limit function length, detect copy-paste, lint the workflows
(actionlint, zizmor), audit Python tools and scan for secrets. CodeQL and
OpenSSF Scorecard workflows, Dependabot for actions, a .gitignore, a
SECURITY.md and README badges are added, and every action is pinned to a
commit. Unit tests guard the build matrix and the action pinning.

The README, .gitignore and CI changes were explicitly requested here,
overriding the default rule against touching them.

Requested by @lefticus in PR cpp-best-practices#4 review.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@leftibot

leftibot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Addressed review feedback from @lefticus. New commits:

a6192f3 Address review: expand CI to satisfy project_validator
dec92f4 Address review: format, lint and type-check the Python tools
943d63b Address review: compile check test cases with strict warnings and analyzers

@github-advanced-security

Copy link
Copy Markdown

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

@lefticus

lefticus commented Nov 6, 2026

Copy link
Copy Markdown
Member

@leftibot since the C++ in this project is all meant as examples, does it make sense to run full analysis on it? Can that be ignored by the project_validator and can we simplify the C++ side of the CI since this is meant to be used for just testing the C++?

@leftibot

leftibot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor Author

✅ leftibot finished looking into this — see reply below.

@leftibot

leftibot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Short answer: yes, it should be simplified. The validator can ignore the example C++, but it will still judge the repo as a C++ project. I ran project_validator (current main) locally against this repo to check.

Full C++ analysis doesn't fit these files

The .cpp files under checks/*/*/tests/{pass,fail}/ are test fixtures, not product code, and several analyses in this PR work against them:

  • clang-tidy/cppcheck on the fixtures. fail/ cases are meant to contain C-style casts and std::endl. To keep CI green, checks/.clang-tidy already turns off cppcoreguidelines-pro-type-cstyle-cast, google-readability-casting, performance-avoid-endl, and others. Those are the same rules our custom checks exist to test. Generic analyzers on the fixtures are just noise to suppress.
  • Debug/Release × sanitizers × hardening. check_test_cases is an OBJECT library that is never linked or run. Sanitizers, _FORTIFY_SOURCE, stack protector and the 3×2×2 = 12-job matrix in ci.yml add nothing for it.
  • C++ branch coverage. gcovr --filter checks/ measures code that never runs, so the number means nothing. Python branch coverage of tools/ and tests/ is the coverage that matters.
  • CodeQL c-cpp. It scans fixtures that are intentionally "bad" code.

What's worth keeping for the C++

  1. A compile check with strict warnings (-Wall -Wextra -Wpedantic -Wshadow -Wconversion -Werror) on GCC and Clang. A pass/ case that the compiler rejects or warns on would make a check look correct when it isn't, so this one is valuable. -fsyntax-only is enough; no build types, sanitizers or hardening needed.
  2. clang-format on the fixtures. They are examples people will read.
  3. An OS matrix only where tool versions differ (clang-query/cppcheck from apt vs. Homebrew LLVM). That's 3 jobs, not 12.

Everything else in CI should target the actual product: the Python harness and checks (ruff, mypy, Python branch coverage, lizard), the .query files, and the workflow linting.

Can project_validator ignore it?

Partly. The catch:

  • --exclude DIR only matches exact directories (no globs; Project._excludes in core.py compares against parent paths). So every fixture directory has to be listed, e.g. --exclude checks/clang-query/no-c-style-cast/tests --exclude checks/cppcheck/prefer-newline-over-endl/tests. Each new check adds another.
  • Even with every fixture excluded, --map still reports C/C++: ['build file CMakeLists.txt']. The root CMakeLists.txt alone makes it a C++ project, and that stays true with project(... LANGUAGES NONE) as on main. So ci-build-types, hardening, cxx-extensions, platform-matrix (C++ side) and C++ coverage all still apply.
  • --skip is global. Skipping platform-matrix, coverage or compiler-warnings would also turn them off for the Python side, which we do want judged.

Recommendation

  1. Fix it upstream in project_validator. A CMake project with project(... LANGUAGES NONE), or with no C/C++ sources left after --exclude, should not be judged as C/C++. Directory globs for --exclude (checks/**/tests) would also help. With that, this repo is judged as a Python project plus data, which is what it is.
  2. Simplify this PR in the meantime:
    • Go back to LANGUAGES NONE in the root project and remove cmake/static_analyzers.cmake and the hardening, sanitizer and coverage parts of cmake/compiler_options.cmake.
    • Keep a small compile-check step for the fixtures (a try_compile-style CTest, or a CI step running $CXX -std=c++20 -fsyntax-only -Wall … -Werror).
    • Remove checks/.clang-tidy, the C++ part of the coverage job, and c-cpp from codeql.yml.
    • Cut the check-tests matrix down to the 3 OS/compiler entries.
    • Run the validator with the fixture --excludes, and accept the C++-only failures until the upstream change lands.

The alternative is to drop CMake/CTest and drive the fixtures from Python (parametrized unittest), which would make this a pure Python project today. But per-case ctest output is nice, and the upstream fix is the better long-term answer for other "tooling about C++" repos too.

If this direction works for you, I can open the project_validator issue/PR and push the simplification here.

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.

Add project validator CI

3 participants