84 clang-tidy findings become reachable in the two AUTOMOC self-including TUs once #624's fix lands

[Yaraslaut]

What

examples/common/testkit/test_qml_surface.cpp and src/qt/forms/tests/tst_main.cpp self-include their generated .moc. Until #624, the clang-tidy job could not parse either translation unit at all — it died at the include with clang-diagnostic-error — so no clang-tidy check has ever run over either file in CI.

#624's fix generates the mocs, which makes both TUs analysable. clang-tidy-diff only reports on changed lines, so nothing breaks today. But the findings that were invisible are now live: the first PR that touches one of these lines will fail the clang-tidy job on a finding that has been sitting there the whole time, and will look like the PR's fault.

Verification status

Reproduced. Measured on the #624 branch (4b34bfd5), with this job's own configure, the mocs generated, clang-tidy 22.1.8, running the full file rather than a diff — so this is the whole-file count, not what any one PR would hit:

=== src/qt/forms/tests/tst_main.cpp  (clang-tidy exit=1)
    findings in this file: 3
      1 [readability-redundant-access-specifiers]
      1 [readability-convert-member-functions-to-static]
      1 [misc-use-internal-linkage]
=== examples/common/testkit/test_qml_surface.cpp  (clang-tidy exit=1)
    findings in this file: 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]

A sample, verbatim:

src/qt/forms/tests/tst_main.cpp:35:7: error: class 'MorphFormsQmlTestSetup' can be moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage,-warnings-as-errors]
src/qt/forms/tests/tst_main.cpp:41:1: error: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers,-warnings-as-errors]

examples/common/testkit/.clang-tidy is already in force for the 81-finding file, and tests/.clang-tidy does not apply to either path (src/qt/forms/tests/ and examples/common/testkit/ are outside tests/), which is also why #638's 134-TU sweep never saw them.

Not verified: whether any of the 84 are real defects rather than style. The 73 misc-const-correctness hits are very likely mechanical. Nothing here was measured on a hosted runner.

Why this is filed rather than fixed in #624's PR

examples/common/testkit/test_qml_surface.cpp is held by a concurrent lane, and 81 mechanical edits to a test-surface file are a different change from a CI step. Folding them in would also have hidden the interesting part: the count is the measure of how long the gate was blind.

What would change the verdict

Close this when both files are clean under their own .clang-tidy, or when the specific checks are argued as inapplicable in the nearest .clang-tidy with the reason written down — per #632/#638, a suppression whose reach is not recorded is the bug, not the fix. Re-open if the count grows, which would mean the files are accumulating findings faster than the gate can now report them.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[Yaraslaut]

Triage: valid — structural premise confirmed; the count is the filer's, not re-measured

The premise holds on 0067b5bf. #647 landed, so the clang-tidy job now builds ladder_common_tests_autogen and morph_forms_qml_tests_autogen and both TUs parse. Both files are in the compile database — #647's own assertion step depends on that, and #649's filter (also landed) only drops sources absent from it, so neither is filtered out. They are analysed now, where before they produced only clang-diagnostic-error.

The 84 figure is the filer's measurement and I have not reproduced it — that needs the job's full Qt + ladder configure plus a clang-tidy sweep. Per this repository's own rule I am labelling it rather than repeating it as fact: 81 in test_qml_surface.cpp (73 of them misc-const-correctness) and 3 in tst_main.cpp, measured by the #647 lane at c55ea5b7, before #649 landed. Master has moved twice since. The count is almost certainly still the right order of magnitude — nothing in #649 touches those files — but the exact number should be re-derived by whoever takes this, not quoted from here.

That matters less than usual for this ticket, because the number does not drive the design: the work is "read the findings and dispose of each", and the first step is enumerating them anyway.

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

The real content of this ticket

Nothing is broken today — clang-tidy-diff reports only changed lines, so the debt is invisible until someone edits one of those lines. The cost is that the first PR to touch either file inherits findings that are not its own, which is a bad experience and an incentive to avoid touching the files.

For whoever takes it:

• 73 of 81 being one check (misc-const-correctness) is the interesting shape. That is likely one mechanical pass, not 73 decisions. Do that first and re-count; the residual is the part that needs judgement.
• Do not reach for a blanket suppression. A NOLINT sweep or a tests/.clang-tidy entry would re-create exactly what tests/.clang-tidy's suppressions also silence include/morph headers, so a header gate driven by test TUs under-reports #632 was about — and tests/.clang-tidy's suppressions also silence include/morph headers, so a header gate driven by test TUs under-reports #632's check_tidy_suppression_scope.sh now gates that area.
• Treat any finding that looks like a real defect as a separate issue, filed not folded.

What would change the verdict

Close as invalid if a re-measurement on current master reports substantially nothing — i.e. if the findings were an artefact of the pre-#647 parse failure rather than real analysis output.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW