Skip to content

S24.11: Extend tidy / cppcheck / IWYU CI to cover platform + BDD code #459

Description

@DavidCozens

Context

Discovered during S28.03 (#457) review on 2026-05-26: the four CI
analyzer jobs — analyze-tidy, analyze-cppcheck, analyze-iwyu,
analyze-format — all run on the base cpputest:sha-… or
cpputest-clang:sha-… container images. Those images have none of
the platform env vars set (FREERTOS_KERNEL_PATH, LWIP_PATH,
MBEDTLS_DIR, FATFS_PATH), so the top-level CMakeLists.txt gates
skip every platform tree and its corresponding test tree:

  • Platform/FreeRtos/, Platform/PlusTcp/, Platform/LwipRaw/,
    Platform/MbedTls/, Platform/FatFs/
  • Tests/FreeRtos/, Tests/Lwip/, Tests/MbedTls/, Tests/FatFs/
  • Bdd/Targets/FreeRtos/ (and forthcoming Bdd/Targets/FreeRtosLwip/
    from S28.06)

analyze-format is the only one that escapes the gap — it uses find

  • clang-format directly without going through CMake.

The only freertos-aware CI job (build-freertos-host-tdd-plustcp) runs
the debug preset; tidy / cppcheck / IWYU are not enabled in that lane.

Net effect: tidy / cppcheck / IWYU have zero CI coverage on all
FreeRTOS / lwIP / mbedTLS / FatFs platform code and on the BDD target
sources. Pre-existing issues in this code (e.g. cppcoreguidelines-macro-to-enum
on Tests/FreeRtos/CmsdkUartFake.c) sit there green on the project
board with no signal.

Parent epic: #254

Scope

Extend the three CMake-driven analyzer jobs to cover the currently-skipped
trees. Each analyzer gets one new CI job per container that hosts the
relevant env vars; the existing base-image jobs stay as they are
(Core / Posix / Winsock / OpenSsl coverage).

CI matrix expansion

Existing Add
analyze-tidy (base) analyze-tidy-freertos-plustcp, analyze-tidy-freertos-lwip (S28.06), analyze-tidy-mbedtls
analyze-cppcheck (base) analyze-cppcheck-freertos-plustcp, analyze-cppcheck-freertos-lwip (S28.06), analyze-cppcheck-mbedtls
analyze-iwyu (base) analyze-iwyu-freertos-plustcp, analyze-iwyu-freertos-lwip (S28.06), analyze-iwyu-mbedtls

Each new job uses the matching container image (cpputest-freertos:sha-…
for FreeRTOS / lwIP / mbedTLS, since that image ships all three
toolchains today). All become required status checks in branch
protection; the summary job's needs: array grows accordingly.

Code work — fix where possible, suppress where not

Once the analyzers run, they will surface findings on the
previously-uncovered code. For each finding:

  1. Fix it if the rule is honoured project-wide and the platform code
    should comply (e.g. static-prefix consistency, MISRA single-exit,
    include-hygiene).
  2. Suppress it with a NOLINT bracket / cppcheck-suppress /
    misra_suppressions.txt entry, with the why in a comment, when:
    • The rule conflicts with how the upstream API has to be used
      (e.g. #define register offsets in CmsdkUartFake.c per the
      hardware register map; FreeRTOS / lwIP / mbedTLS macros required
      by the upstream API surface).
    • The rule's intent is stylistic and the platform code has a stronger
      domain reason to deviate (e.g. bugprone-easily-swappable-parameters
      on Fake_Write32(uintptr_t address, uint32_t value) — those types
      are semantically distinct in MMIO).

Each suppression carries a one-line -- reason at the call site (same
convention used in the existing codebase for cppcheck/NOLINT brackets).

Relaxed rules vs. tested code

Platform and BDD code may run a relaxed analyzer rule set compared
to Core. The Core bar (full MISRA-leaning style, no magic numbers,
single exit, etc.) reflects a stronger contract than what we ask of
target-glue or hardware-shim layers. Specific relaxations are decided
during refinement when the actual finding pattern is visible — likely
candidates:

  • cppcoreguidelines-macro-usage — already NOLINT'd in
    Tests/Support/FreeRtosFakes/portmacro.h etc., extend that pattern
    to platform fakes and BDD target code that must define macros for
    upstream APIs.
  • bugprone-easily-swappable-parameters — relaxed for register-map
    / MMIO functions where the types carry semantic distinction the
    rule can't see.
  • modernize-macro-to-enum — relaxed for hardware register offsets
    where the macro form aligns with the datasheet and a future
    reviewer benefits from grep'ing for DATA_OFFSET against vendor
    docs.

However, relaxing analyzer rules does not relax the test-coverage
bar.
Every platform-class production file already has a host-side
test executable that exercises it (PlusTcpAddressTest, MbedTlsStreamTest,
LwipRawAddressTest, etc.). New work under this story confirms that
coverage is present — if a relaxation lands on a file whose behaviour
isn't currently driven by a test, add the test first or scope it out
explicitly in this story's deliverables.

Suppression discipline

  • Each suppression must carry a -- reason at the suppression site
    explaining why this code can't / shouldn't comply.
  • Each new entry in misra_suppressions.txt follows the existing
    deviation-id header convention and is cross-referenced in
    docs/misra-deviations.md if it's a per-class pattern.
  • A blanket --suppress=toomanyconfigs (or similar) for an analyzer
    preset is acceptable if the alternative is per-file noise across
    every upstream-header-pulling TU — document the call in the relevant
    CMake comment block + DEVLOG.

Out of scope

  • Adding a separate analyze-* lane for the cross-compile job
    (build-freertos-target-plustcp and forthcoming -lwip). Cross-build
    analyzer support is a bigger lift and arguably less valuable —
    separate story if needed.
  • MbedTlsIntegration / OpenSslIntegration / BDD-runtime test
    executables
    that have legitimate analyzer-disable per
    Tests/MbedTlsIntegration/CMakeLists.txt reasoning (third-party
    macro indirection that cppcheck can't model). Leave their per-target
    disables as they are.
  • Rule-set tuning for Core code. Core's analyzer rules stay where
    they are; this story only adds relaxations for the previously-uncovered
    trees.
  • Restructuring the CMake to make analyzer-coverage scale better
    long-term — see project_cmake_restructure_post_e28 memory, deferred
    to its own epic.

Done when

  • All three analyzer jobs (analyze-tidy, analyze-cppcheck,
    analyze-iwyu) have at least one freertos-aware sibling running on
    cpputest-freertos:sha-…, plus mbedTLS coverage as appropriate.
  • New jobs are required status checks on branch protection.
  • Summary panel includes the new jobs.
  • Zero analyzer findings on the now-covered trees — either fixed in
    the code, suppressed with a documented -- reason, or covered by a
    blanket preset-level suppression with a comment + DEVLOG entry.
  • Tests/FreeRtos/CmsdkUartFake.c specifically goes from "would fail
    tidy if a job saw it" to "passes the new tidy lane".
  • DEVLOG entry summarising the relaxations adopted and the fix vs.
    suppress split.

Metadata

Metadata

Assignees

No one assigned

    Labels

    storyStory issue

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions