97 unreported clang-tidy findings in the eleven bank-GUI sources morph#651 brings into reach

[Yaraslaut]

What

PR for #651 adds -DMORPH_BUILD_BANK_GUI=ON to the clang-tidy job's
Configure step, which brings eleven previously-absent sources into
compile_commands.json and so into clang-tidy-diff's reach. Those eleven
sources carry 97 clang-tidy findings that no gate has ever reported,
because no configure that clang-tidy reads has ever built them.

Nothing goes red today: clang-tidy-diff.py only reports diagnostics on
changed lines. The findings surface one pull request at a time, on whoever
next edits one of these files on a line that already carries one. This issue
exists so that person finds a ticket rather than a mystery.

This is #646's shape exactly, one directory over: #647 made two AUTOMOC
self-including TUs parse, 84 findings appeared in them, and clearing those was
its own ticket (#646, merged as #653) rather than folded into the change that
revealed them.

Measurement

Reproduced, on 4563aff3, clang-tidy 22.1.8 (the version ci.yml pins as
CLANG_VERSION: "22"), Qt 6.11.2, against the clang-tidy job's own configure
flags plus -DMORPH_BUILD_BANK_GUI=ON, cold build directory:

$ cmake --preset clang-debug -DMORPH_BUILD_NET=ON -DMORPH_BUILD_QT=ON \
    -DMORPH_BUILD_FORMS_QML=ON -DMORPH_BUILD_OFFLINE_SQLITE=ON \
    -DMORPH_BUILD_LOAD_TESTS=ON -DMORPH_BUILD_HMAC_EXAMPLES=ON \
    -DMORPH_BUILD_FUZZERS=ON -DMORPH_BUILD_LADDER=ON \
    -DMORPH_BUILD_BANK_EXAMPLE=ON -DMORPH_BUILD_BANK_GUI=ON \
    -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++

$ clang-tidy -p build/clang-debug --extra-arg=-std=c++23 \
      --extra-arg=-Wno-missing-include-dirs --quiet <each of the eleven>

Whole-file, not changed-lines-only: that is the latent total, and the number
any given pull request pays is a subset of it.

Findings located inside the analysed source itself (diagnostics landing in
include/morph/** headers are excluded -- those headers are already analysed
from 700-odd other TUs in the same database and are not new exposure):

source	findings
examples/bank/gui/controllers/CardController.cpp	20
examples/bank/gui/controllers/LoanController.cpp	19
examples/bank/gui/controllers/TransactionController.cpp	12
examples/bank/gui/controllers/AccountController.cpp	11
examples/bank/gui/controllers/PayeeController.cpp	11
examples/bank/gui/main.cpp	10
examples/bank/tests/gui/test_bank_gui_qml_behaviour.cpp	6
examples/bank/tests/gui/test_bank_qml_surface.cpp	6
examples/bank/gui/controllers/AppController.cpp	2
examples/bank/gui/BankClient.cpp	0
examples/bank/gui/controllers/BankController.cpp	0
total	97

By check:

    44  cppcoreguidelines-pro-bounds-avoid-unchecked-container-access
    23  performance-unnecessary-value-param
     9  misc-const-correctness
     8  readability-identifier-length
     2  readability-static-accessed-through-instance
     2  readability-implicit-bool-conversion
     2  readability-avoid-nested-conditional-operator
     2  modernize-use-auto
     1  concurrency-mt-unsafe
     1  cppcoreguidelines-avoid-c-arrays
     1  modernize-avoid-c-arrays
     1  cppcoreguidelines-pro-bounds-constant-array-index
     1  readability-container-size-empty
     1  readability-function-cognitive-complexity

Real output, the first six from CardController.cpp:

examples/bank/gui/controllers/CardController.cpp:27:45: error: the parameter 'list' of type 'bank::dto::AccountList' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param,-warnings-as-errors]
examples/bank/gui/controllers/CardController.cpp:34:20: error: possibly unsafe 'operator[]', consider bounds-safe alternatives [cppcoreguidelines-pro-bounds-avoid-unchecked-container-access,-warnings-as-errors]
examples/bank/gui/controllers/CardController.cpp:35:20: error: possibly unsafe 'operator[]', consider bounds-safe alternatives [cppcoreguidelines-pro-bounds-avoid-unchecked-container-access,-warnings-as-errors]
examples/bank/gui/controllers/CardController.cpp:45:42: error: the parameter 'list' of type 'bank::dto::CardList' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param,-warnings-as-errors]
examples/bank/gui/controllers/CardController.cpp:52:20: error: possibly unsafe 'operator[]', consider bounds-safe alternatives [cppcoreguidelines-pro-bounds-avoid-unchecked-container-access,-warnings-as-errors]
examples/bank/gui/controllers/CardController.cpp:53:20: error: possibly unsafe 'operator[]', consider bounds-safe alternatives [cppcoreguidelines-pro-bounds-avoid-unchecked-container-access,-warnings-as-errors]

Note the diagnostic tail: findings end [<check>,-warnings-as-errors], not
[<check>], because the root .clang-tidy carries WarningsAsErrors: "*". A
grep of the form \[[a-z0-9-]+\] matches none of them.

Why this is not folded into #651's change

Two reasons, and the second is the load-bearing one.

1. AGENTS.md: "Do not fold an unrelated finding into the current change." The
   flag being absent and the code under it not being clean are two facts, not
   one.
2. Clearing 97 findings across ten Qt GUI sources -- 44 of them
   operator[] -> .at() on DTO lists, 23 by-value parameters -- is a change
   that has to be built to be trustworthy, and the clang-tidy job
   deliberately never builds. It belongs with a leg that compiles bank_gui
   (linux-all-features), not with a one-line flag addition.

Verification status

• Reproduced: the 97 findings, the per-file and per-check split, on the
  revision and toolchain named above.
• Reproduced: that the eleven files are absent from the database without
  the flag (63 of 473 tracked sources absent) and present with it (52 of 473).
• Inferred, not measured: that CI's clang 22 on ubuntu-24.04 produces the
  same 97. Local clang-tidy is 22.1.8 and ci.yml pins major 22, but the
  runner's patch level and its Qt 6 version (aqtinstall QT_VERSION, not the
  6.11.2 used here) differ, and several of these checks are sensitive to what
  the Qt headers declare. Expect the number to move by a few either way.
• Not attempted: any fix. Nothing here has been changed.

What would change the verdict

Close this when the eleven sources report zero findings under the
clang-tidy job's configure -- verified by re-running the command above and
pasting the output, not by a green CI run, since a green clang-tidy-diff leg
on an unrelated PR proves only that those lines were not touched.

Re-open if the count rises: that would mean new bank-GUI code is being written
against a gate that still is not reporting on it.

Related

• 60 of 470 tracked C/C++ sources are outside clang-tidy-diff's reach, and 11 of them only because one CMake flag is missing #651 -- the flag that brings these files into reach.
• 84 clang-tidy findings become reachable in the two AUTOMOC self-including TUs once #624's fix lands #646 / ci: let clang-tidy-diff parse the two AUTOMOC self-including sources, and name option-coverage by its job id (fixes #624, fixes #637) #647 / testkit/qt/ci: clear the clang-tidy debt in the two AUTOMOC self-including TUs, and drop a deleted workflow from a docstring (fixes #646, fixes #643) #653 -- the same sequence for the two AUTOMOC TUs.
• core/ladder: retire IBackend's four async twins and move the examples' prose onto the structural surface (fixes #570, fixes #571) #649 -- the filter that made these eleven silent rather than fatal.

[Yaraslaut]

Triage: valid — the 11 sources are confirmed; the 97 is the filer's count, unreproduced

On f24e225a:

$ git ls-files 'examples/bank/gui/*.cpp' 'examples/bank/tests/gui/*.cpp'
examples/bank/gui/BankClient.cpp
examples/bank/gui/controllers/{Account,App,Bank,Card,Loan,Payee,Transaction}Controller.cpp
examples/bank/gui/main.cpp
examples/bank/tests/gui/test_bank_gui_qml_behaviour.cpp
examples/bank/tests/gui/test_bank_qml_surface.cpp
count: 11

and the clang-tidy job still names no MORPH_BUILD_BANK_GUI, so all eleven are outside its reach today. The set and the cause are exactly as filed.

The 97 is a measurement I have not reproduced — it needs a configure with the GUI enabled plus a clang-tidy sweep, and the filer notes several of those checks read Qt headers, with local Qt 6.11.2 against CI's aqtinstall version. Treat the per-file and per-check breakdown as a well-evidenced estimate, not a contract. It is good enough to plan with: 44 cppcoreguidelines-pro-bounds-avoid-unchecked-container-access and 23 performance-unnecessary-value-param being two-thirds of the total is the shape that matters, and that will not flip.

triage: valid, enhancement, area: ci, area: ladder.

Sequencing, which is the real content here

This ticket only bites once #651's flag lands (PR #657, currently open and red for an unrelated AUTOMOC problem). Until then nothing analyses these files and nothing goes red. After it lands, the first PR that edits one of these eleven on a line carrying a finding inherits it.

Two consequences worth stating:

• This is not urgent, and it is also not safe to forget. The window between the flag landing and someone editing a bank-GUI file is exactly the interval in which this ticket is invisible.
• Do not make it a precondition of ci/testkit: three gates that did not cover what they appeared to (fixes #651, fixes #652, fixes #655) #657. The flag is worth landing on its own — it closes a real coverage hole — and gating it on clearing 97 findings would be the tail wagging the dog. 84 clang-tidy findings become reachable in the two AUTOMOC self-including TUs once #624's fix lands #646 set that precedent two merges ago and it worked.

For whoever takes it

• Re-measure first. The count is from 4563aff3-era; master has moved twice.
• 44 + 23 of one and another check is mechanical, not 67 decisions. Do those passes, re-count, and treat the residual as the part needing judgement. That is how testkit/qt/ci: clear the clang-tidy debt in the two AUTOMOC self-including TUs, and drop a deleted workflow from a docstring (fixes #646, fixes #643) #653 handled its 84.
• No blanket suppression — scripts/check_tidy_suppression_scope.sh gates that area, and a .clang-tidy under examples/bank/ would be the examples/common/testkit/.clang-tidy also governs the testkit *library* sources, contradicting its own prose #652 mistake repeated one directory over.
• Anything that looks like a real defect rather than style gets its own issue.

What would change the verdict

Close as invalid if a re-measurement after the flag lands reports substantially nothing — which would mean the findings were an artefact of the filer's local Qt rather than real analysis output.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW