From 79e7796e8e0b591a47cd8deb5346a7a794d30854 Mon Sep 17 00:00:00 2001 From: leftibot Date: Mon, 2 Nov 2026 23:45:16 -0700 Subject: [PATCH 1/4] 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 --- .github/workflows/ci.yml | 22 +++++++++++++++ tests/test_ci_workflow.py | 57 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 tests/test_ci_workflow.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 525c069..362046c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,3 +21,25 @@ jobs: - name: Test run: ctest --test-dir build --output-on-failure + + project-validator: + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v4 + + - uses: actions/checkout@v4 + with: + repository: cpp-best-practices/project_validator + ref: main + path: project_validator + + - uses: actions/setup-python@v5 + with: + python-version: "3.14" + + - name: Install validator dependencies + run: python -m pip install "PyYAML>=6.0" + + - name: Validate project + working-directory: project_validator + run: python -m project_validator --build --exclude project_validator ${{ github.workspace }} diff --git a/tests/test_ci_workflow.py b/tests/test_ci_workflow.py new file mode 100644 index 0000000..d45c380 --- /dev/null +++ b/tests/test_ci_workflow.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python3 +"""Unit tests for .github/workflows/ci.yml.""" + +from __future__ import annotations + +import re +import unittest +from pathlib import Path + +WORKFLOW = Path(__file__).resolve().parent.parent / ".github" / "workflows" / "ci.yml" +VALIDATOR_JOB = "project-validator" +JOB_HEADER = re.compile(r"^ (?P[A-Za-z0-9_-]+):\s*$") + + +def workflow_jobs(text: str) -> dict[str, str]: + """Each job under `jobs:`, keyed by name, with its indented body.""" + jobs: dict[str, list[str]] = {} + current: list[str] | None = None + in_jobs = False + for line in text.splitlines(): + if not line.startswith(" ") and line.strip(): + in_jobs = line.rstrip() == "jobs:" + current = None + continue + header = JOB_HEADER.match(line) if in_jobs else None + if header is not None: + current = jobs.setdefault(header.group("name"), []) + elif current is not None: + current.append(line) + return {name: "\n".join(body) for name, body in jobs.items()} + + +class ProjectValidatorJobTest(unittest.TestCase): + def setUp(self) -> None: + self.jobs = workflow_jobs(WORKFLOW.read_text(encoding="utf-8")) + + def validator_job(self) -> str: + self.assertIn(VALIDATOR_JOB, self.jobs, f"{WORKFLOW} has no {VALIDATOR_JOB} job") + return self.jobs[VALIDATOR_JOB] + + def test_checks_out_the_validator_main_branch(self) -> None: + job = self.validator_job() + self.assertRegex(job, re.compile(r"^\s+repository:\s*cpp-best-practices/project_validator\s*$", re.M)) + self.assertRegex(job, re.compile(r"^\s+ref:\s*main\s*$", re.M)) + + def test_runs_the_validator_on_this_project(self) -> None: + self.assertRegex(self.validator_job(), r"-m project_validator\b.*\$\{\{\s*github\.workspace\s*\}\}") + + +class WorkflowJobsTest(unittest.TestCase): + def test_splits_jobs_by_name(self) -> None: + text = "name: ci\njobs:\n first:\n runs-on: a\n second:\n runs-on: b\nother:\n third:\n" + self.assertEqual(workflow_jobs(text), {"first": " runs-on: a", "second": " runs-on: b"}) + + +if __name__ == "__main__": + unittest.main() From 943d63b7e56d3c33d9f3ef653813a0a2ee012d74 Mon Sep 17 00:00:00 2001 From: leftibot Date: Fri, 6 Nov 2026 14:05:12 -0700 Subject: [PATCH 2/4] Address review: compile check test cases with strict warnings and analyzers 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 #4 review. Co-Authored-By: Claude Opus 5.5 --- .clang-format | 98 +++++++++++++++++++ .clang-tidy | 17 ++++ CMakeLists.txt | 17 +++- checks/.clang-tidy | 13 +++ checks/README.md | 4 + .../tests/pass/named_casts.cpp | 5 +- cmake/compiler_options.cmake | 44 +++++++++ cmake/static_analyzers.cmake | 23 +++++ 8 files changed, 214 insertions(+), 7 deletions(-) create mode 100644 .clang-format create mode 100644 .clang-tidy create mode 100644 checks/.clang-tidy create mode 100644 cmake/compiler_options.cmake create mode 100644 cmake/static_analyzers.cmake diff --git a/.clang-format b/.clang-format new file mode 100644 index 0000000..ffd5aa9 --- /dev/null +++ b/.clang-format @@ -0,0 +1,98 @@ +AccessModifierOffset: -2 +AlignAfterOpenBracket: DontAlign +AlignConsecutiveAssignments: false +AlignConsecutiveDeclarations: false +AlignEscapedNewlines: Left +AlignOperands: true +AlignTrailingComments: false +AllowAllParametersOfDeclarationOnNextLine: false +AllowShortBlocksOnASingleLine: true +AllowShortCaseLabelsOnASingleLine: false +AllowShortFunctionsOnASingleLine: All +AllowShortIfStatementsOnASingleLine: true +AllowShortLoopsOnASingleLine: true +AlwaysBreakAfterDefinitionReturnType: None +AlwaysBreakAfterReturnType: None +AlwaysBreakBeforeMultilineStrings: true +AlwaysBreakTemplateDeclarations: false +BinPackArguments: false +BinPackParameters: false +BraceWrapping: + AfterClass: true + AfterControlStatement: false + AfterEnum: false + AfterFunction: true + AfterNamespace: false + AfterObjCDeclaration: false + AfterStruct: true + AfterUnion: false + BeforeCatch: false + BeforeElse: false + IndentBraces: false + SplitEmptyFunction: false + SplitEmptyNamespace: true + SplitEmptyRecord: true +BreakAfterJavaFieldAnnotations: true +BreakBeforeBinaryOperators: NonAssignment +BreakBeforeBraces: Custom +BreakBeforeInheritanceComma: true +BreakBeforeTernaryOperators: true +BreakConstructorInitializers: BeforeColon +BreakConstructorInitializersBeforeComma: false +BreakStringLiterals: true +ColumnLimit: 120 +CommentPragmas: '^ IWYU pragma:' +CompactNamespaces: false +ConstructorInitializerAllOnOneLineOrOnePerLine: false +ConstructorInitializerIndentWidth: 2 +ContinuationIndentWidth: 2 +Cpp11BracedListStyle: false +DerivePointerAlignment: false +DisableFormat: false +ExperimentalAutoDetectBinPacking: true +FixNamespaceComments: true +ForEachMacros: +- foreach +- Q_FOREACH +- BOOST_FOREACH +IncludeCategories: +- Priority: 2 + Regex: ^"(llvm|llvm-c|clang|clang-c)/ +- Priority: 3 + Regex: ^(<|"(gtest|gmock|isl|json)/) +- Priority: 1 + Regex: .* +IncludeIsMainRegex: (Test)?$ +IndentCaseLabels: false +IndentWidth: 2 +IndentWrappedFunctionNames: true +JavaScriptQuotes: Leave +JavaScriptWrapImports: true +KeepEmptyLinesAtTheStartOfBlocks: true +Language: Cpp +MacroBlockBegin: '' +MacroBlockEnd: '' +MaxEmptyLinesToKeep: 2 +NamespaceIndentation: Inner +ObjCBlockIndentWidth: 7 +ObjCSpaceAfterProperty: true +ObjCSpaceBeforeProtocolList: false +PointerAlignment: Right +ReflowComments: true +SortIncludes: true +SortUsingDeclarations: false +SpaceAfterCStyleCast: false +SpaceAfterTemplateKeyword: false +SpaceBeforeAssignmentOperators: true +SpaceBeforeParens: ControlStatements +SpaceInEmptyParentheses: false +SpacesBeforeTrailingComments: 0 +SpacesInAngles: false +SpacesInCStyleCastParentheses: false +SpacesInContainerLiterals: true +SpacesInParentheses: false +SpacesInSquareBrackets: false +Standard: c++20 +TabWidth: 8 +UseTab: Never + diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..45f261e --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,17 @@ +--- +Checks: >- + bugprone-*, + cert-*, + clang-analyzer-*, + concurrency-*, + cppcoreguidelines-*, + misc-*, + modernize-*, + performance-*, + portability-*, + readability-*, + -modernize-use-trailing-return-type, + -readability-identifier-length +WarningsAsErrors: '*' +CheckOptions: + readability-function-size.LineThreshold: 30 diff --git a/CMakeLists.txt b/CMakeLists.txt index 161499b..c2b92b1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,12 +1,17 @@ -cmake_minimum_required(VERSION 3.20) +cmake_minimum_required(VERSION 3.24) -project(cpp_extra_analysis LANGUAGES NONE) +project(cpp_extra_analysis LANGUAGES CXX) + +set(CMAKE_CXX_STANDARD 20) +set(CMAKE_CXX_STANDARD_REQUIRED ON) +set(CMAKE_CXX_EXTENSIONS OFF) option(CPP_EXTRA_ANALYSIS_REQUIRE_TOOLS "Fail configuration if an analysis tool is missing" OFF) include(CTest) +include(cmake/compiler_options.cmake) -find_package(Python3 3.9 REQUIRED COMPONENTS Interpreter) +find_package(Python3 3.10 REQUIRED COMPONENTS Interpreter) set(CLANG_QUERY_NAMES clang-query) set(CPPCHECK_NAMES cppcheck) @@ -27,6 +32,8 @@ foreach(tool CLANG_QUERY CPPCHECK) endif() endforeach() +include(cmake/static_analyzers.cmake) + set(HARNESS ${CMAKE_CURRENT_SOURCE_DIR}/tools/run_check_tests.py) set(HARNESS_ENVIRONMENT PYTHONDONTWRITEBYTECODE=1 @@ -41,6 +48,10 @@ file(GLOB_RECURSE CHECK_TEST_CASES CONFIGURE_DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/checks/*/*/tests/pass/*.cpp ${CMAKE_CURRENT_SOURCE_DIR}/checks/*/*/tests/fail/*.cpp) +# Every test case must be valid, warning-free C++, or a check could pass on code the compiler rejects. +add_library(check_test_cases OBJECT ${CHECK_TEST_CASES}) +target_link_libraries(check_test_cases PRIVATE cpp_extra_analysis_options) + foreach(test_case IN LISTS CHECK_TEST_CASES) file(RELATIVE_PATH test_name ${CMAKE_CURRENT_SOURCE_DIR}/checks ${test_case}) string(REPLACE "/tests/" "/" test_name ${test_name}) diff --git a/checks/.clang-tidy b/checks/.clang-tidy new file mode 100644 index 0000000..d3fcc77 --- /dev/null +++ b/checks/.clang-tidy @@ -0,0 +1,13 @@ +--- +# Test cases are free-standing snippets that deliberately contain what the +# custom checks report (C-style casts, std::endl) and their accepted +# alternatives, so the clang-tidy checks that would duplicate them are off. +InheritParentConfig: true +Checks: >- + -cppcoreguidelines-avoid-magic-numbers, + -cppcoreguidelines-pro-type-cstyle-cast, + -cppcoreguidelines-pro-type-reinterpret-cast, + -google-readability-casting, + -misc-use-internal-linkage, + -performance-avoid-endl, + -readability-magic-numbers diff --git a/checks/README.md b/checks/README.md index e25af0a..eefc13c 100644 --- a/checks/README.md +++ b/checks/README.md @@ -31,6 +31,10 @@ many of each as needed. * A `fail` case marks every line that must be reported with `// expect-finding`; the check must report exactly those lines. +Every case is also compiled as C++20 with the project's warnings, so it must be +valid, warning-free code; clang-tidy runs over it too, with the checks that +duplicate the custom checks turned off in `checks/.clang-tidy`. + ## Running ``` diff --git a/checks/clang-query/no-c-style-cast/tests/pass/named_casts.cpp b/checks/clang-query/no-c-style-cast/tests/pass/named_casts.cpp index 55faabf..e529129 100644 --- a/checks/clang-query/no-c-style-cast/tests/pass/named_casts.cpp +++ b/checks/clang-query/no-c-style-cast/tests/pass/named_casts.cpp @@ -2,7 +2,4 @@ [[nodiscard]] constexpr int truncate(const double value) { return static_cast(value); } -[[nodiscard]] std::uintptr_t address_of(const int &value) -{ - return reinterpret_cast(&value); -} +[[nodiscard]] std::uintptr_t address_of(const int &value) { return reinterpret_cast(&value); } diff --git a/cmake/compiler_options.cmake b/cmake/compiler_options.cmake new file mode 100644 index 0000000..dfb419d --- /dev/null +++ b/cmake/compiler_options.cmake @@ -0,0 +1,44 @@ +include_guard(GLOBAL) + +include(CheckCXXCompilerFlag) + +option(CPP_EXTRA_ANALYSIS_ENABLE_SANITIZERS "Build with AddressSanitizer and UndefinedBehaviorSanitizer" OFF) +option(CPP_EXTRA_ANALYSIS_ENABLE_COVERAGE "Instrument the build for branch coverage (GCC)" OFF) + +add_library(cpp_extra_analysis_options INTERFACE) + +function(cpp_extra_analysis_add_supported_flag flag) + string(MAKE_C_IDENTIFIER "CPP_EXTRA_ANALYSIS_HAS${flag}" supported) + check_cxx_compiler_flag(${flag} ${supported}) + if(${supported}) + target_compile_options(cpp_extra_analysis_options INTERFACE ${flag}) + endif() +endfunction() + +if(MSVC) + target_compile_options(cpp_extra_analysis_options INTERFACE /W4 /permissive-) + return() +endif() + +target_compile_options(cpp_extra_analysis_options + INTERFACE -Wall -Wextra -Wpedantic -Wshadow -Wconversion -Wsign-conversion) + +foreach(flag -fstack-protector-strong -fcf-protection -fstack-clash-protection) + cpp_extra_analysis_add_supported_flag(${flag}) +endforeach() + +target_compile_definitions(cpp_extra_analysis_options INTERFACE _GLIBCXX_ASSERTIONS) + +# _FORTIFY_SOURCE only works with optimisation, and toolchains may already define a lower level. +target_compile_options(cpp_extra_analysis_options + INTERFACE "$<$:-U_FORTIFY_SOURCE;-D_FORTIFY_SOURCE=3>") + +if(CPP_EXTRA_ANALYSIS_ENABLE_SANITIZERS) + target_compile_options(cpp_extra_analysis_options INTERFACE -fsanitize=address,undefined -fno-omit-frame-pointer) + target_link_options(cpp_extra_analysis_options INTERFACE -fsanitize=address,undefined) +endif() + +if(CPP_EXTRA_ANALYSIS_ENABLE_COVERAGE) + target_compile_options(cpp_extra_analysis_options INTERFACE --coverage) + target_link_options(cpp_extra_analysis_options INTERFACE --coverage) +endif() diff --git a/cmake/static_analyzers.cmake b/cmake/static_analyzers.cmake new file mode 100644 index 0000000..485c04a --- /dev/null +++ b/cmake/static_analyzers.cmake @@ -0,0 +1,23 @@ +include_guard(GLOBAL) + +option(CPP_EXTRA_ANALYSIS_ENABLE_CLANG_TIDY "Run clang-tidy on every C++ source as it compiles" OFF) +option(CPP_EXTRA_ANALYSIS_ENABLE_CPPCHECK "Run cppcheck on every C++ source as it compiles" OFF) + +if(CPP_EXTRA_ANALYSIS_ENABLE_CLANG_TIDY) + set(CLANG_TIDY_NAMES clang-tidy) + foreach(version RANGE 30 14 -1) + list(APPEND CLANG_TIDY_NAMES clang-tidy-${version}) + endforeach() + find_program(CLANG_TIDY_EXECUTABLE NAMES ${CLANG_TIDY_NAMES} REQUIRED) + set(CMAKE_CXX_CLANG_TIDY ${CLANG_TIDY_EXECUTABLE}) +endif() + +if(CPP_EXTRA_ANALYSIS_ENABLE_CPPCHECK) + find_program(CPPCHECK_EXECUTABLE NAMES cppcheck REQUIRED) + set(CMAKE_CXX_CPPCHECK + ${CPPCHECK_EXECUTABLE} + --enable=warning,style,performance,portability + --inline-suppr + --error-exitcode=2 + --std=c++20) +endif() From dec92f4e928482f11f835ea07ab5d57839f1fcf1 Mon Sep 17 00:00:00 2001 From: leftibot Date: Fri, 6 Nov 2026 14:05:12 -0700 Subject: [PATCH 3/4] Address review: format, lint and type-check the Python tools 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 #4 review. Co-Authored-By: Claude Opus 5.5 --- .../prefer-newline-over-endl.py | 6 +++-- mypy.ini | 4 +++ ruff.toml | 6 +++++ stubs/cppcheck.pyi | 9 +++++++ tools/run_check_tests.py | 26 +++++-------------- 5 files changed, 29 insertions(+), 22 deletions(-) create mode 100644 mypy.ini create mode 100644 ruff.toml create mode 100644 stubs/cppcheck.pyi diff --git a/checks/cppcheck/prefer-newline-over-endl/prefer-newline-over-endl.py b/checks/cppcheck/prefer-newline-over-endl/prefer-newline-over-endl.py index 0468338..5c5dc0d 100755 --- a/checks/cppcheck/prefer-newline-over-endl/prefer-newline-over-endl.py +++ b/checks/cppcheck/prefer-newline-over-endl/prefer-newline-over-endl.py @@ -1,10 +1,12 @@ #!/usr/bin/env python3 """Flags std::endl; stream '\\n' instead and flush explicitly when required.""" +from typing import Any + import cppcheck -def is_std_endl(token): +def is_std_endl(token: Any) -> bool: scope = token.previous return ( token.str == "endl" @@ -16,7 +18,7 @@ def is_std_endl(token): @cppcheck.checker -def stdEndl(cfg, data): +def stdEndl(cfg: Any, data: Any) -> None: for token in filter(is_std_endl, cfg.tokenlist): cppcheck.reportError( token, diff --git a/mypy.ini b/mypy.ini new file mode 100644 index 0000000..2a49e06 --- /dev/null +++ b/mypy.ini @@ -0,0 +1,4 @@ +[mypy] +strict = True +mypy_path = stubs +exclude = ^project_validator/ diff --git a/ruff.toml b/ruff.toml new file mode 100644 index 0000000..98a6e2c --- /dev/null +++ b/ruff.toml @@ -0,0 +1,6 @@ +line-length = 120 +target-version = "py310" +extend-exclude = ["project_validator"] + +[lint] +select = ["E", "W", "F", "I", "B", "UP", "SIM", "RUF", "PL"] diff --git a/stubs/cppcheck.pyi b/stubs/cppcheck.pyi new file mode 100644 index 0000000..7ea2d2e --- /dev/null +++ b/stubs/cppcheck.pyi @@ -0,0 +1,9 @@ +"""Type stubs for the `cppcheck` module that cppcheck provides to its addons at run time.""" + +from collections.abc import Callable +from typing import Any, TypeVar + +_Checker = TypeVar("_Checker", bound=Callable[[Any, Any], None]) + +def checker(f: _Checker) -> _Checker: ... +def reportError(location: Any, severity: str, message: str, errorId: str | None = None) -> None: ... diff --git a/tools/run_check_tests.py b/tools/run_check_tests.py index 7e4ab70..8377224 100755 --- a/tools/run_check_tests.py +++ b/tools/run_check_tests.py @@ -22,8 +22,8 @@ import shutil import subprocess import sys +from collections.abc import Callable, Iterable, Sequence from pathlib import Path -from typing import Callable, Iterable, Sequence REPO_ROOT = Path(__file__).resolve().parent.parent DEFAULT_CHECKS_DIR = REPO_ROOT / "checks" @@ -35,9 +35,7 @@ EXIT_FAILURE = 1 EXIT_SKIPPED = 77 -CLANG_QUERY_BINDING = re.compile( - r'^(?P.+?):(?P\d+):(?P\d+): note: "root" binds here$' -) +CLANG_QUERY_BINDING = re.compile(r'^(?P.+?):(?P\d+):(?P\d+): note: "root" binds here$') CPPCHECK_FINDING = re.compile( r"^(?P.+?):(?P\d+):(?P\d+): (?P\w+): (?P.*) \[(?P[^\]]+)\]$" ) @@ -75,11 +73,7 @@ class Outcome: def expected_finding_lines(source_text: str) -> frozenset[int]: - return frozenset( - number - for number, line in enumerate(source_text.splitlines(), start=1) - if EXPECT_MARKER in line - ) + return frozenset(number for number, line in enumerate(source_text.splitlines(), start=1) if EXPECT_MARKER in line) def is_same_file(reported: str, source: Path) -> bool: @@ -91,9 +85,7 @@ def is_same_file(reported: str, source: Path) -> bool: def parse_clang_query_output(output: str, source: Path) -> frozenset[int]: matches = (CLANG_QUERY_BINDING.match(line) for line in output.splitlines()) - return frozenset( - int(match["line"]) for match in matches if match and is_same_file(match["file"], source) - ) + return frozenset(int(match["line"]) for match in matches if match and is_same_file(match["file"], source)) def parse_cppcheck_output(output: str, source: Path, check_name: str) -> frozenset[int]: @@ -129,9 +121,7 @@ def run_tool(command: Sequence[str]) -> subprocess.CompletedProcess[str]: def run_clang_query(case: TestCase) -> frozenset[int]: executable = find_executable("CLANG_QUERY", "clang-query") - result = run_tool( - [executable, "-f", str(case.check_file), str(case.source), "--", f"-std={CXX_STANDARD}"] - ) + result = run_tool([executable, "-f", str(case.check_file), str(case.source), "--", f"-std={CXX_STANDARD}"]) if result.returncode != 0 or "error:" in result.stderr: raise HarnessError(f"clang-query failed ({result.returncode}):\n{result.stdout}{result.stderr}") return parse_clang_query_output(result.stdout, case.source) @@ -270,11 +260,7 @@ def parse_arguments(arguments: Sequence[str]) -> argparse.Namespace: def select_cases(options: argparse.Namespace) -> list[TestCase]: cases = [case_for_source(source) for source in options.cases] or discover_cases(options.checks_dir) - return [ - case - for case in cases - if options.tool in (None, case.tool) and options.check in (None, case.check_name) - ] + return [case for case in cases if options.tool in (None, case.tool) and options.check in (None, case.check_name)] def main(arguments: Sequence[str]) -> int: From a6192f32a66dda65c51dd69107d065b053dd46b4 Mon Sep 17 00:00:00 2001 From: leftibot Date: Fri, 6 Nov 2026 14:05:12 -0700 Subject: [PATCH 4/4] Address review: expand CI to satisfy project_validator 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 #4 review. Co-Authored-By: Claude Opus 5.5 --- .github/dependabot.yml | 8 ++ .github/workflows/ci.yml | 145 ++++++++++++++++++++++++++++++-- .github/workflows/codeql.yml | 46 ++++++++++ .github/workflows/scorecard.yml | 36 ++++++++ .gitignore | 10 +++ README.md | 4 + SECURITY.md | 7 ++ tests/test_ci_workflow.py | 30 ++++++- 8 files changed, 278 insertions(+), 8 deletions(-) create mode 100644 .github/dependabot.yml create mode 100644 .github/workflows/codeql.yml create mode 100644 .github/workflows/scorecard.yml create mode 100644 .gitignore create mode 100644 SECURITY.md diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..9cbb9b7 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,8 @@ +version: 2 +updates: + - package-ecosystem: github-actions + directory: / + schedule: + interval: weekly + cooldown: + default-days: 7 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 362046c..edebce4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,35 +5,168 @@ on: branches: [main] pull_request: +permissions: + contents: read + jobs: check-tests: + name: check-tests (${{ matrix.platform.os }}, ${{ matrix.platform.compiler }}, ${{ matrix.build_type }}, sanitizers ${{ matrix.sanitizers }}) + runs-on: ${{ matrix.platform.os }} + strategy: + fail-fast: false + matrix: + platform: + - { os: ubuntu-24.04, compiler: gcc, cxx: g++, python-version: "3.10", analyzers: -DCPP_EXTRA_ANALYSIS_ENABLE_CPPCHECK=ON } + - { os: ubuntu-24.04, compiler: clang, cxx: clang++, python-version: "3.14", analyzers: -DCPP_EXTRA_ANALYSIS_ENABLE_CLANG_TIDY=ON } + - { os: macos-15, compiler: appleclang, cxx: c++, python-version: "3.12", analyzers: "" } + build_type: [Debug, Release] + sanitizers: ["ON", "OFF"] + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0 + with: + python-version: ${{ matrix.platform.python-version }} + + - name: Install analysis tools (Linux) + if: runner.os == 'Linux' + run: | + sudo apt-get update + sudo apt-get install --yes --no-install-recommends clang-tools clang-tidy cppcheck + + - name: Install analysis tools (macOS) + if: runner.os == 'macOS' + run: | + brew install llvm cppcheck + echo "CMAKE_PROGRAM_PATH=$(brew --prefix llvm)/bin" >> "$GITHUB_ENV" + echo "SDKROOT=$(xcrun --show-sdk-path)" >> "$GITHUB_ENV" + + - name: Configure + env: + CXX: ${{ matrix.platform.cxx }} + BUILD_TYPE: ${{ matrix.build_type }} + SANITIZERS: ${{ matrix.sanitizers }} + ANALYZERS: ${{ matrix.platform.analyzers }} + run: > + cmake -S . -B build + -DCMAKE_BUILD_TYPE="$BUILD_TYPE" + -DCMAKE_COMPILE_WARNING_AS_ERROR=ON + -DCPP_EXTRA_ANALYSIS_ENABLE_SANITIZERS="$SANITIZERS" + -DCPP_EXTRA_ANALYSIS_REQUIRE_TOOLS=ON + ${ANALYZERS:+"$ANALYZERS"} + + - name: Build + run: cmake --build build --parallel + + - name: Test + run: ctest --test-dir build --output-on-failure + + coverage: runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - - name: Install analysis tools + - uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0 + with: + python-version: "3.14" + + - name: Install tools run: | sudo apt-get update sudo apt-get install --yes --no-install-recommends clang-tools cppcheck + python -m pip install coverage==7.17.2 gcovr==8.8 - name: Configure - run: cmake -S . -B build -DCPP_EXTRA_ANALYSIS_REQUIRE_TOOLS=ON + run: cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug -DCPP_EXTRA_ANALYSIS_ENABLE_COVERAGE=ON -DCPP_EXTRA_ANALYSIS_REQUIRE_TOOLS=ON + + - name: Build + run: cmake --build build --parallel - name: Test run: ctest --test-dir build --output-on-failure + - name: Python branch coverage + run: | + python -W error -m coverage run --branch --source=tools,tests -m unittest discover -s tests + python -m coverage report --show-missing + + - name: C++ branch coverage + run: gcovr --root . --filter checks/ --txt-metric branch build + + lint: + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0 + with: + python-version: "3.14" + + - name: Install tools + run: | + sudo apt-get update + sudo apt-get install --yes --no-install-recommends clang-format + python -m pip install ruff==0.17.4 mypy==2.4.1 lizard==1.25.0 actionlint-py==1.7.12.25 zizmor==1.32.0 pip-audit==2.10.1 + + - name: Check C++ formatting + run: git ls-files -z '*.cpp' '*.hpp' | xargs -0 clang-format --dry-run -Werror + + - name: Check Python formatting + run: ruff format --check . + + - name: Lint Python + run: ruff check . + + - name: Type check Python + run: mypy . + + - name: Limit function length + run: lizard --length 30 --warnings_only tools tests checks + + - name: Detect copy-paste + run: npx --yes jscpd@5.4.2 --exit-code 1 --ignore ".github/**" . + + - name: Lint GitHub Actions + run: | + actionlint + zizmor .github + + - name: Audit Python tools + run: pip-audit + + secret-scanning: + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + persist-credentials: false + + - uses: trufflesecurity/trufflehog@ea68ca03c865c5f324e86e849ced20ed3cf9dbbb # v3.99.8 + with: + extra_args: --results=verified,unknown + project-validator: runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - - uses: actions/checkout@v4 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: repository: cpp-best-practices/project_validator ref: main path: project_validator + persist-credentials: false - - uses: actions/setup-python@v5 + - uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0 with: python-version: "3.14" diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 0000000..d686cf7 --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,46 @@ +name: codeql + +on: + push: + branches: [main] + pull_request: + schedule: + - cron: "17 4 * * 1" + +permissions: + contents: read + +jobs: + analyze: + name: analyze (${{ matrix.language }}) + runs-on: ubuntu-24.04 + permissions: + contents: read + security-events: write + strategy: + fail-fast: false + matrix: + include: + - { language: actions, build-mode: none } + - { language: c-cpp, build-mode: manual } + - { language: python, build-mode: none } + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - uses: github/codeql-action/init@33d7b85da1a04171e211befe2c739303b9a4c066 # v4.39.3 + with: + languages: ${{ matrix.language }} + build-mode: ${{ matrix.build-mode }} + queries: security-and-quality + + - name: Build + if: matrix.build-mode == 'manual' + run: | + cmake -S . -B build + cmake --build build --parallel + + - uses: github/codeql-action/analyze@33d7b85da1a04171e211befe2c739303b9a4c066 # v4.39.3 + with: + category: /language:${{ matrix.language }} diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml new file mode 100644 index 0000000..9fd00ed --- /dev/null +++ b/.github/workflows/scorecard.yml @@ -0,0 +1,36 @@ +name: scorecard + +on: + push: + branches: [main] + schedule: + - cron: "41 5 * * 1" + +permissions: read-all + +jobs: + analysis: + runs-on: ubuntu-24.04 + permissions: + security-events: write + id-token: write + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - uses: ossf/scorecard-action@fa7117db83c52d787ae5a89f350999701505d78e # v2.5.0 + with: + results_file: results.sarif + results_format: sarif + publish_results: true + + - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: scorecard-results + path: results.sarif + retention-days: 5 + + - uses: github/codeql-action/upload-sarif@33d7b85da1a04171e211befe2c739303b9a4c066 # v4.39.3 + with: + sarif_file: results.sarif diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..fced9df --- /dev/null +++ b/.gitignore @@ -0,0 +1,10 @@ +/build/ +/build-*/ +/out/ +__pycache__/ +*.py[cod] +/.venv/ +/.coverage +/.mypy_cache/ +/.ruff_cache/ +/project_validator/ diff --git a/README.md b/README.md index 7e293d4..43e416a 100644 --- a/README.md +++ b/README.md @@ -1,2 +1,6 @@ # cpp_extra_analysis + +[![ci](https://github.com/cpp-best-practices/cpp_extra_analysis/actions/workflows/ci.yml/badge.svg)](https://github.com/cpp-best-practices/cpp_extra_analysis/actions/workflows/ci.yml) +[![OpenSSF Scorecard](https://api.scorecard.dev/projects/github.com/cpp-best-practices/cpp_extra_analysis/badge)](https://scorecard.dev/viewer/?uri=github.com/cpp-best-practices/cpp_extra_analysis) + Extra analysis scripts for static analysis tools that allow for plugin analysis diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..9ef4542 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,7 @@ +# Security Policy + +## Reporting a Vulnerability + +Please report vulnerabilities privately through GitHub's +[private vulnerability reporting](https://github.com/cpp-best-practices/cpp_extra_analysis/security/advisories/new) +rather than in a public issue. diff --git a/tests/test_ci_workflow.py b/tests/test_ci_workflow.py index d45c380..d50d490 100644 --- a/tests/test_ci_workflow.py +++ b/tests/test_ci_workflow.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Unit tests for .github/workflows/ci.yml.""" +"""Unit tests for the GitHub Actions workflows.""" from __future__ import annotations @@ -7,8 +7,12 @@ import unittest from pathlib import Path -WORKFLOW = Path(__file__).resolve().parent.parent / ".github" / "workflows" / "ci.yml" +WORKFLOWS_DIR = Path(__file__).resolve().parent.parent / ".github" / "workflows" +WORKFLOW = WORKFLOWS_DIR / "ci.yml" VALIDATOR_JOB = "project-validator" +CHECK_TESTS_JOB = "check-tests" +ACTION_REFERENCE = re.compile(r"^\s*(?:-\s*)?uses:\s*(?P[^@\s]+)@(?P\S+)", re.MULTILINE) +COMMIT_SHA = re.compile(r"[0-9a-f]{40}") JOB_HEADER = re.compile(r"^ (?P[A-Za-z0-9_-]+):\s*$") @@ -47,6 +51,28 @@ def test_runs_the_validator_on_this_project(self) -> None: self.assertRegex(self.validator_job(), r"-m project_validator\b.*\$\{\{\s*github\.workspace\s*\}\}") +class CheckTestsJobTest(unittest.TestCase): + def setUp(self) -> None: + jobs = workflow_jobs(WORKFLOW.read_text(encoding="utf-8")) + self.assertIn(CHECK_TESTS_JOB, jobs, f"{WORKFLOW} has no {CHECK_TESTS_JOB} job") + self.job = jobs[CHECK_TESTS_JOB] + + def test_builds_debug_and_release_with_and_without_sanitizers(self) -> None: + self.assertRegex(self.job, re.compile(r"^\s+build_type:\s*\[Debug, Release\]\s*$", re.MULTILINE)) + self.assertRegex(self.job, re.compile(r'^\s+sanitizers:\s*\["ON", "OFF"\]\s*$', re.MULTILINE)) + + def test_treats_warnings_as_errors(self) -> None: + self.assertIn("-DCMAKE_COMPILE_WARNING_AS_ERROR=ON", self.job) + + +class ActionPinningTest(unittest.TestCase): + def test_every_action_is_pinned_to_a_commit(self) -> None: + for workflow in sorted(WORKFLOWS_DIR.glob("*.yml")): + for reference in ACTION_REFERENCE.finditer(workflow.read_text(encoding="utf-8")): + with self.subTest(workflow=workflow.name, action=reference["action"]): + self.assertRegex(reference["ref"], COMMIT_SHA) + + class WorkflowJobsTest(unittest.TestCase): def test_splits_jobs_by_name(self) -> None: text = "name: ci\njobs:\n first:\n runs-on: a\n second:\n runs-on: b\nother:\n third:\n"