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)

[Yaraslaut]

Two tickets, one commit each.

---

#646 — the 84 findings #647 made reachable

#647 taught the clang-tidy job to build ladder_common_tests_autogen and
morph_forms_qml_tests_autogen, so examples/common/testkit/test_qml_surface.cpp
and src/qt/forms/tests/tst_main.cpp parse and are analysed for the first
time. Nothing was broken — clang-tidy-diff reports only changed lines — but
the first PR to touch one of those lines would have inherited findings that
were not its own.

Re-measured, not quoted

The ticket carried the #647 lane's figure from c55ea5b7, and the triage
comment explicitly declined to repeat it as fact. Re-derived here on
0067b5bf with the job's own configure flags and clang-tidy 22.1.8 (CI's
pinned major), both AUTOMOC targets built first:

file	findings	breakdown
test_qml_surface.cpp	81	73 misc-const-correctness, 3 readability-convert-member-functions-to-static, 2 readability-inconsistent-declaration-parameter-name, 2 readability-identifier-length, 1 bugprone-easily-swappable-parameters
tst_main.cpp	3	misc-use-internal-linkage, readability-redundant-access-specifiers, readability-convert-member-functions-to-static

That reproduces the reported 81 / 73 / 3 exactly. The ticket is not an
artefact of the pre-#647 parse failure; it stands.

77 of 84 fixed, 7 suppressed with an argument

Fixed:

• 73 misc-const-correctness — local QTemporaryDir and fixture-bridge
  declarations never mutated. Applied with clang-tidy --fix, then rewritten
  from the tool's east-const output to the west-const spelling the rest of the
  file uses (.clang-format sets no QualifierAlignment, so the formatter
  would have left QTemporaryDir const dir; standing).
• 2 readability-identifier-length — id → rowId, ok → okay. Safe
  because QmlSurfaceAudit reads QMetaMethod::name() and parameterCount()
  and never a parameter name (examples/common/testkit/qml_surface.cpp:145-158);
  the QML fixture text is untouched.
• 1 misc-use-internal-linkage — MorphFormsQmlTestSetup moves into an
  anonymous namespace. QUICK_TEST_MAIN_WITH_SETUP expands in the same TU
  (quicktest.h constructs QuickTestSetupClass setup; and passes &setup),
  so internal linkage is enough.
• 1 readability-redundant-access-specifiers — the explicitly defaulted
  default constructor and its public: are removed. Q_OBJECT ends in
  private:, so public Q_SLOTS: following it directly is not redundant; the
  finding existed only because of the public: section above it.

Suppressed, one reasoned NOLINTNEXTLINE per site, reason above the
directive so #627/#631's wrapped-reason trap cannot apply. No NOLINT sweep,
no new or edited .clang-tidy — which is what #632 and
scripts/check_tidy_suppression_scope.sh are about:

• 4 readability-convert-member-functions-to-static on two Q_PROPERTY
  readers, a Q_INVOKABLE, and a Qt Quick Test setup slot. The reason given
  is shape, not legality, because legality was measured and the honest
  answer is that the static form works: with the three fixture methods made
  static, moc emits _t->title() exactly as for a member function,
  registers the same property and invokable, and the TU compiles. It is
  declined because no bridge these fixtures stand in for has a static property
  reader, and a Qt slot is a member function by definition — the same argument
  tests/.clang-tidy already records for this check over its stub execute
  methods.
• 2 readability-inconsistent-declaration-parameter-name on the two
  signals. moc's generated definitions name the parameters positionally —
  SurfaceFixtureBridge::listed(const QVariantList & _t1, bool _t2) in
  test_qml_surface.moc:155 — so no edit to the declarations can remove the
  mismatch. Leaving them unnamed would trade it for
  readability-named-parameter and take the Doxygen with it. These findings
  reach a first-party file only because the classes are declared in a .cpp,
  which is what puts moc's output inside the translation unit.
• 1 bugprone-easily-swappable-parameters on the file-local writeQml
  helper's two adjacent const QString&, across 40 call sites that all pass a
  QStringLiteral("….qml") first and a generated body second.

Verification — the specific trap this leg had

A clean clang-tidy run proves nothing unless the TUs were analysed. Four
independent checks:

1. Both files are in the compile database. This configure yields 703
   entries, 695 in-workspace, 270 under examples/ — above core/ladder: retire IBackend's four async twins and move the examples' prose onto the structural surface (fixes #570, fixes #571) #649's 600 / 200
   floors, and matching the figures in ci.yml's own comment. Both target
   files resolve inside it, so core/ladder: retire IBackend's four async twins and move the examples' prose onto the structural surface (fixes #570, fixes #571) #649's filter does not drop them.
2. Mutation probe. With every NOLINTNEXTLINE line stripped and nothing
   else changed, clang-tidy exits 1 and reports exactly the 6 + 1 suppressed
   findings again:

   test_qml_surface.cpp:63:27: method 'title' can be made static [readability-convert-member-functions-to-static,-warnings-as-errors]
   test_qml_surface.cpp:69:23: method 'depth' can be made static [...]
   test_qml_surface.cpp:78:22: method 'open' can be made static [...]
   test_qml_surface.cpp:94:10: function '(anonymous namespace)::SurfaceFixtureBridge::listed' has a definition with different parameter names [...]
   test_qml_surface.cpp:130:10: function '(anonymous namespace)::BaseFixtureBridge::failed' has a definition with different parameter names [...]
   test_qml_surface.cpp:156:44: 2 adjacent parameters of 'writeQml' of similar type ('const QString &') are easily swapped by mistake [...]
   tst_main.cpp:54:10: method 'qmlEngineAvailable' can be made static [...]
   

   Every directive is load-bearing, and the files really are being read. The
   findings that were fixed rather than suppressed do not come back, which
   is the other half of the same probe.
3. clang-tidy-diff.py over this branch's own diff against origin/master,
   with the job's flags: 2 TUs analysed, exit 0.
4. The tests still pass. ladder_common_tests "[qml-surface]" — 182
   assertions in 36 test cases. morph_forms_qml_tests -input src/qt/forms/tests
   (the command add_test registers) — 292 passed, 0 failed, the two
   corpus-reading QML suites among them, which is what proves the setup slot
   still runs after the anonymous-namespace move rather than silently
   injecting nothing.

Also run green: clang-format 22.1.8 --dry-run -Werror,
check_nolint_directives.sh (171 directives), check_bidi_controls.py,
check_tidy_suppression_scope.sh, check_automoc_includes.sh,
check_catch_test_names.sh.

---

#643 — a stale name in a docstring

scripts/check_workflow_job_banners.py's ## Scope paragraph named
suppression-guard.yml, deleted by #635. It was the only surviving reference
in the tree.

The name is deleted rather than swapped for another example, against the
ticket's suggestion, because there is no sixth current example to put in its
place: enumerating the tree with the checker's own BANNER_RE / JOB_KEY_RE
gives exactly five bannerless workflows, all single-job — docs.yml,
mutation.yml, spec-sync.yml, wasm-demo.yml, wasm-ladder.yml — which is
precisely "docs.yml, mutation.yml, spec-sync.yml and the two wasm workflows".
With suppression-guard.yml removed the sentence is exact, not short.

A second paragraph then says the list is an illustration with a shelf life,
that nothing reads it, and that a run's own
no section banners, not in the banner style lines are the current list — so
the next workflow deletion dates one sentence instead of producing a third
round of this. Gate behaviour is unchanged either way: the skip is recomputed
per file.

Verified: python3 scripts/check_workflow_job_banners.py . (24 banners) and
bash scripts/test_check_workflow_job_banners.sh both pass, as do
check_bidi_controls.py and check_workflow_option_coverage.py.

---

Filed, not folded

#652 — examples/common/testkit/.clang-tidy says "Checks suppressed
inside this test directory only" and "a hand-written a < b < c in the code
under test is still reported", but that directory also holds the
morph_ladder_testkit library (fault_proxy.cpp, qml_surface.cpp), so the
suppression governs those two TUs as well. Reproduced with a probe line, with
both runs' output in the issue. Narrow in practice — -Wparentheses catches
that exact shape anyway and only one check is subtracted — but it is a false
claim in a file whose value is the accuracy of its claims, the same class as
#632. Not folded here; this PR is about two unrelated files.

Review notes

Reasoned inline rather than through /code-review, as instructed.

• The one claim everything else rests on: clang-tidy exits 0 on both files
  because they were analysed and are clean, not because the invocation missed
  them. Check 2 above is what carries it — strip the directives and the
  exact seven findings return, at the expected lines and checks.
• Risk concentrated in two places. The anonymous-namespace move in
  tst_main.cpp could have broken Qt Quick Test's setup-hook lookup silently
  (it would have injected empty corpora, and the QML side fails loudly on
  that, which is why 292/0 is the evidence and not just a green build). The
  73 consts are tool-generated; they were reviewed as a diff and the suite
  exercises them.
• const SurfaceFixtureBridge bridge; binds to QmlSurfaceAudit::bind(const QString&, const QObject&), so the audit's view of the fixture is unchanged.
• No file held by another lane is touched.

Not verified

CI. Reporting at push time as instructed, without waiting.

Fixes #646
Fixes #643

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[Yaraslaut]

Runner verification

My brief was wrong on #643 and the lane checked rather than complying. I told it that replacing suppression-guard.yml with another example would keep the sentence useful while deleting the name would leave "and the two wasm workflows" with a wrong count. There is no sixth example — the checker's own output on current master:

ok: .github/workflows/docs.yml: no section banners, not in the banner style
ok: .github/workflows/mutation.yml: no section banners, not in the banner style
ok: .github/workflows/spec-sync.yml: no section banners, not in the banner style
ok: .github/workflows/wasm-demo.yml: no section banners, not in the banner style
ok: .github/workflows/wasm-ladder.yml: no section banners, not in the banner style

Exactly five, so "docs.yml, mutation.yml, spec-sync.yml and the two wasm workflows" is exact once the stale name is gone. Deleting was right. The added paragraph is better than either option I offered — it marks the list as an illustration with a shelf life and points the reader at the run's own no section banners output as the live list, which is the only form of that sentence that cannot rot again.

Diff shape matches the report exactly: three files, no .clang-tidy touched, seven NOLINTNEXTLINE added — no sweep, and #632's gated area untouched.

The east-const rewrite checks out and the reason for it is sound. .clang-format sets no QualifierAlignment, so clang-tidy --fix's spelling would have persisted unformatted. In the diff: 77 west-const added lines, 0 east-const. The rewrite was actually carried out, consistently, rather than claimed.

What I could not verify: the branch's load-bearing claim — that clang-tidy exits 0 on both files because they were analysed and are clean, not because the invocation missed them. That rests on the lane's mutation probe (strip every NOLINTNEXTLINE, get exactly the 6 + 1 findings back at the expected lines, with the fixed ones not returning), and reproducing it needs the job's full Qt + ladder configure, which I did not run. The probe is the right shape — it distinguishes "clean" from "not looked at", which is the only thing that matters here — and it is corroborated by both files being in compile_commands.json and by the 292/0 QML run proving the setup slot still runs after the namespace move. Accepting it on that basis, stated rather than glossed.

The best thing in this report is a reason the lane refused to use. It probed whether moc rejects a static Q_PROPERTY reader or Q_INVOKABLE — which would have made four convert-member-functions-to-static suppressions trivially justified — and found it false: the static form compiles and moc emits _t->title() and registers the same property. It then argued those four on shape instead, and recorded the measurement that killed the convenient reason. A suppression resting on a false premise is worse than no suppression, because it reads as verified.

#652 is the same class as #632 and worth its own look: examples/common/testkit/.clang-tidy says its suppressions apply "inside this test directory only", but that directory also holds the morph_ladder_testkit library, so they govern fault_proxy.cpp and qml_surface.cpp too. A suppression file that misdescribes its own reach is exactly what #632 was about. Filed, not folded.

Not merged: CI incomplete at hand-off. Next sweep counts the checks.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!