From ac60a29c0e37142139e5aea466754eaa0eaf2d4a Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 21 Sep 2026 11:01:39 +0200 Subject: [PATCH 1/2] bank/gui: clear the 97 clang-tidy findings #657 brought into reach (fixes #656) #657 added -DMORPH_BUILD_BANK_GUI=ON to the clang-tidy job's Configure step, which put eleven bank-GUI sources into compile_commands.json for the first time. clang-tidy-diff only reports on changed lines, so nothing went red -- the findings were waiting for whoever next edited one of those lines. Re-measured on 7ab4c7a9 before touching anything, clang-tidy 22.1.8 (ci.yml pins CLANG_VERSION: "22"), Qt 6.11.2, against the clang-tidy job's own configure flags and its own -extra-arg pair, cold build directory: 97 findings inside the eleven sources themselves 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 readability-function-cognitive-complexity 1 readability-container-size-empty 1 cppcoreguidelines-pro-bounds-constant-array-index 1 concurrency-mt-unsafe 1 cppcoreguidelines-avoid-c-arrays + modernize-avoid-c-arrays (one site, two check names, which is why the issue's per-check list sums to 98) Exactly the count #656 filed on 4563aff3, file-for-file and check-for-check. Same command after this commit: 0. The two big checks were two mechanical passes, as the ticket predicted: - 44 `map[QStringLiteral("k")] = v` on QVariantMap become `map.insert(QStringLiteral("k"), v)`. QMap::operator[] on a non-const map inserts a default and hands back a reference; insert() does the same lookup and assignment in one call, so this is the same map with no bounds-unchecked accessor in it. - 23 by-value continuation parameters become const references. Completion::then takes std::function, so every one of these was copying a DTO out of a reference the caller already held. The residual 30 needed judgement, one at a time: - `id` parameters (5) are renamed for what they identify -- cardId, payeeId, accountId -- in the headers too. QML binds Q_INVOKABLE arguments positionally, so no .qml file sees this. - CardController's two nested conditional operators become one if/else-if over CardStatus with Cancelled as the fall-through, which is also what stopped the two `statusText`/`statusKind` chains being read twice. - main.cpp: setApplicationName and exec are static on QCoreApplication, so they are called that way and `app` becomes const; std::getenv is concurrency-mt-unsafe and becomes qgetenv, which the two seed variables twenty lines below already used; the `const char* names[5]` becomes a QStringList indexed with .at(), which removes the C-array pair and the non-constant array index together; two `if (window)` become explicit null comparisons. - Both of the behaviour test's TEST_CASEs carry a reasoned NOLINTNEXTLINE(readability-function-cognitive-complexity), and the whole argument for both sits above the first one. The check scores a whole Catch2 TEST_CASE body -- clang-tidy names them `dummyFunction72` and `dummyFunction76` -- and what it scores here is Catch2's assertion expansion rather than a branch thicket: REQUIRE/CHECK expand to a do-while around a try/catch with a `&&` in the loop condition, which the metric charges +1/+2/+1, four points per assertion. Measured with the clang-tidy job's own configure and its own -extra-arg pair, clang-tidy 22.1.8, threshold lowered to 1 so both cases report rather than only the one over: dummyFunction72 "MoveMoneyPage's picker ..." 87, 21 REQUIRE/CHECK dummyFunction76 "Main.qml confirms ..." 45, 11 REQUIRE/CHECK 21 x 4 = 84 and 11 x 4 = 44, so three points of the 87 and one of the 45 are the whole of what the tests' own shape contributes. Commenting a single CHECK out of the first case moves it 87 -> 83, so four-per- assertion is measured and not arithmetic. Both numbers are identical on 7ab4c7a9: neither is a regression this commit introduced. An earlier revision of this commit suppressed only the second case and stated, in the tree, that the first "scores under the threshold and stays covered". That was never measured. It was read off the finding not being *reported*, which is a different thing: clang-tidy-diff surfaces a finding only when one of its notes lands on a changed line, and on 7ab4c7a9 none of the first case's notes was on one. Renaming the `balanceOf` lambda's parameter put a changed line under one of them, and the job went red with test_bank_gui_qml_behaviour.cpp:115:1: error: function 'dummyFunction72' has cognitive complexity of 87 (threshold 25) [readability-function-cognitive-complexity,-warnings-as-errors] Splitting the first case was the alternative, and it cannot reach the threshold. At four points an assertion, 25 allows six assertions per TEST_CASE; that case's prologue alone -- register a user, open two accounts, stand up a QQmlEngine, load MoveMoneyPage.qml, drive the picker onto the savings account -- is eight, so every fragment is over before it asserts anything of its own. Hoisting the prologue into a helper relocates the score rather than removing it, and morph#296's defect *is* the sequence (pick, deposit, still picked, deposit again, the money followed the label) that a split would scatter. Two per-case directives rather than one entry in examples/bank/tests/.clang-tidy, which would subtract the check from every bank test including ones not yet written, and rather than a NOLINTBEGIN/NOLINTEND span, which would cover whatever is added between them. Both reasons sit above their directive, not wrapped around it (#631). No new .clang-tidy anywhere, and examples/bank/tests/.clang-tidy is untouched: widening it would be #652's mistake one directory over. Verified: - 97 -> 0 in-source findings, same command, same build directory. The run still reports 1743 diagnostics in include/morph/** headers from these eleven TUs and zero clang-diagnostic-error, so the analysis is live rather than silently skipping the files. - Anti-vacuity: reinstating one `map[...] = ...` in PayeeController.cpp brings the finding straight back (`PayeeController.cpp:52:20: error: possibly unsafe 'operator[]' ...`), so the zero is a measurement and not an empty walk. - Builds clean under clang 22.1.8 (-Weverything -Werror, clang-debug) and under gcc (gcc-debug), both configured from empty with -DMORPH_BUILD_BANK_GUI=ON. - bank_gui_tests and bank_gui_qml_tests both pass, all three cases. - The 97 -> 0 re-checked whole-file against the runner's own Catch2 series (3.5.3 headers ahead of the workstation's on -isystem), because the workstation's Catch2 cannot see this particular check at all (see below): all eleven sources, full check set, 0 findings inside them and 0 clang-diagnostic-error. - The clang-tidy-diff gate itself, reproduced locally rather than inferred. clang-tidy-diff.py with this job's -path/-p1/-extra-arg set, over `git diff -U0 origin/master`, exits 1 with exactly the CI error quoted above before the two-directive correction, and exits 0 after it -- over the branch's whole diff, not just the one file. - Anti-vacuity for the second directive, which this branch's own diff does not exercise at all: with the directive deleted, the same gate over a one-line diff on that case's `REQUIRE(pumpUntil([&app] { ... }))` line reports `dummyFunction76 ... cognitive complexity of 45 (threshold 25)` and exits 1; with the directive back, that diff exits 0. A one-line diff on a non-lambda assertion line in the same case reports nothing either way, which is why the shipped diff never reached it. Not verified: the Emscripten leg. wasm-demo.yml builds bank_gui_wasm from these same controller sources and no Qt-WASM toolchain is installed here. Also not verified locally in the CI configuration exactly: reproducing the clang-tidy-diff failure needed Catch2 3.5.x headers on the include path to match the runner's apt `catch2`. With the workstation's Catch2 3.16.0 clang-tidy computes the same 87 and 45 but drops both findings as non-user code, so a bare local run of the gate is green on a diff CI fails. Filed as #666, with #667 for the Catch2 version the .clang-tidy copies record. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW --- .../gui/controllers/AccountController.cpp | 26 +++--- .../bank/gui/controllers/AppController.cpp | 4 +- .../bank/gui/controllers/CardController.cpp | 63 ++++++++------- .../bank/gui/controllers/CardController.hpp | 6 +- .../bank/gui/controllers/LoanController.cpp | 44 +++++------ .../bank/gui/controllers/PayeeController.cpp | 24 +++--- .../bank/gui/controllers/PayeeController.hpp | 2 +- .../gui/controllers/TransactionController.cpp | 34 ++++---- .../gui/controllers/TransactionController.hpp | 2 +- examples/bank/gui/main.cpp | 27 ++++--- .../tests/gui/test_bank_gui_qml_behaviour.cpp | 79 ++++++++++++++++--- .../bank/tests/gui/test_bank_qml_surface.cpp | 14 ++-- 12 files changed, 199 insertions(+), 126 deletions(-) diff --git a/examples/bank/gui/controllers/AccountController.cpp b/examples/bank/gui/controllers/AccountController.cpp index d68283f69..a37ea11a3 100644 --- a/examples/bank/gui/controllers/AccountController.cpp +++ b/examples/bank/gui/controllers/AccountController.cpp @@ -16,16 +16,16 @@ namespace { QVariantMap toMap(const bank::dto::AccountInfo& account) { const bool closed = account.status == static_cast(bank::AccountStatus::Closed); QVariantMap map; - map[QStringLiteral("id")] = static_cast(account.id); - map[QStringLiteral("kind")] = fmt::accountKind(account.kind); - map[QStringLiteral("number")] = fmt::last4(account.number); - map[QStringLiteral("balanceText")] = fmt::money(account.balanceMinor, account.currency); - map[QStringLiteral("statusText")] = closed ? QStringLiteral("Closed") : QStringLiteral("Open"); - map[QStringLiteral("statusKind")] = closed ? QStringLiteral("neutral") : QStringLiteral("good"); - map[QStringLiteral("closed")] = closed; - map[QStringLiteral("hasOverdraft")] = account.overdraftMinor > 0; - map[QStringLiteral("overdraftText")] = - QStringLiteral("Overdraft ") + fmt::money(account.overdraftMinor, account.currency); + map.insert(QStringLiteral("id"), static_cast(account.id)); + map.insert(QStringLiteral("kind"), fmt::accountKind(account.kind)); + map.insert(QStringLiteral("number"), fmt::last4(account.number)); + map.insert(QStringLiteral("balanceText"), fmt::money(account.balanceMinor, account.currency)); + map.insert(QStringLiteral("statusText"), closed ? QStringLiteral("Closed") : QStringLiteral("Open")); + map.insert(QStringLiteral("statusKind"), closed ? QStringLiteral("neutral") : QStringLiteral("good")); + map.insert(QStringLiteral("closed"), closed); + map.insert(QStringLiteral("hasOverdraft"), account.overdraftMinor > 0); + map.insert(QStringLiteral("overdraftText"), + QStringLiteral("Overdraft ") + fmt::money(account.overdraftMinor, account.currency)); return map; } @@ -36,10 +36,10 @@ AccountController::AccountController(BankClient& client, QObject* parent) void AccountController::refresh() { _model.execute(bank::dto::ListAccounts{}) - .then([this](bank::dto::AccountList list) { + .then([this](const bank::dto::AccountList& list) { _accounts.clear(); std::int64_t total = 0; - int currency = list.accounts.empty() ? 0 : list.accounts.front().currency; + const int currency = list.accounts.empty() ? 0 : list.accounts.front().currency; bool sameCurrency = true; _openCount = 0; for (const auto& account : list.accounts) { @@ -62,7 +62,7 @@ void AccountController::refresh() { void AccountController::openAccount(int kind, int currency, const QString& overdraft) { const auto minor = overdraft.trimmed().isEmpty() ? 0 : fmt::parseMinor(overdraft).value_or(0); _model.execute(bank::dto::OpenAccount{.kind = kind, .currency = currency, .overdraftMinor = minor}) - .then([this](bank::dto::AccountInfo) { refresh(); }) + .then([this](const bank::dto::AccountInfo&) { refresh(); }) .onError([this](const std::exception_ptr& err) { emit error(errorText(err)); }); } diff --git a/examples/bank/gui/controllers/AppController.cpp b/examples/bank/gui/controllers/AppController.cpp index 988316241..6391a8260 100644 --- a/examples/bank/gui/controllers/AppController.cpp +++ b/examples/bank/gui/controllers/AppController.cpp @@ -20,7 +20,7 @@ void AppController::adopt(const QString& principal, const QString& displayName) void AppController::login(const QString& username, const QString& password) { _auth.execute(bank::dto::LoginRequest{.username = username.toStdString(), .password = password.toStdString()}) - .then([this](bank::dto::AuthResult result) { + .then([this](const bank::dto::AuthResult& result) { if (result.ok) { adopt(QString::fromStdString(result.principal), QString::fromStdString(result.displayName)); } else { @@ -35,7 +35,7 @@ void AppController::registerUser(const QString& username, const QString& passwor .execute(bank::dto::RegisterUser{.username = username.toStdString(), .password = password.toStdString(), .displayName = displayName.toStdString()}) - .then([this](bank::dto::AuthResult result) { + .then([this](const bank::dto::AuthResult& result) { if (result.ok) { adopt(QString::fromStdString(result.principal), QString::fromStdString(result.displayName)); } else { diff --git a/examples/bank/gui/controllers/CardController.cpp b/examples/bank/gui/controllers/CardController.cpp index 38c2a8f64..059082c92 100644 --- a/examples/bank/gui/controllers/CardController.cpp +++ b/examples/bank/gui/controllers/CardController.cpp @@ -24,15 +24,15 @@ void CardController::refresh() { void CardController::reloadAccounts() { _accountModel.execute(bank::dto::ListAccounts{}) - .then([this](bank::dto::AccountList list) { + .then([this](const bank::dto::AccountList& list) { _accounts.clear(); for (const auto& account : list.accounts) { if (account.status == static_cast(bank::AccountStatus::Closed)) { continue; } QVariantMap map; - map[QStringLiteral("id")] = static_cast(account.id); - map[QStringLiteral("label")] = fmt::last4(account.number); + map.insert(QStringLiteral("id"), static_cast(account.id)); + map.insert(QStringLiteral("label"), fmt::last4(account.number)); _accounts.append(map); } emit accountsChanged(); @@ -42,26 +42,35 @@ void CardController::reloadAccounts() { void CardController::reloadCards() { _cardModel.execute(bank::dto::ListCards{}) - .then([this](bank::dto::CardList list) { + .then([this](const bank::dto::CardList& list) { _cards.clear(); for (const auto& card : list.cards) { const auto status = static_cast(card.status); const QString kind = card.kind == static_cast(bank::CardKind::Credit) ? QStringLiteral("Credit") : QStringLiteral("Debit"); + // Cancelled is the fall-through arm rather than a third branch: the + // two `statusText`/`statusKind` chains this replaces were nested + // conditional operators, which is what QML reads as the pill's label + // and colour. + QString statusText = QStringLiteral("Cancelled"); + QString statusKind = QStringLiteral("bad"); + if (status == bank::CardStatus::Active) { + statusText = QStringLiteral("Active"); + statusKind = QStringLiteral("good"); + } else if (status == bank::CardStatus::Frozen) { + statusText = QStringLiteral("Frozen"); + statusKind = QStringLiteral("warn"); + } QVariantMap map; - map[QStringLiteral("id")] = static_cast(card.id); - map[QStringLiteral("title")] = - kind + QStringLiteral(" card ••••") + QString::fromStdString(card.panLast4); - map[QStringLiteral("limitText")] = - QStringLiteral("Daily limit ") + fmt::money(card.dailyLimitMinor, 0); - map[QStringLiteral("statusText")] = status == bank::CardStatus::Active ? QStringLiteral("Active") - : status == bank::CardStatus::Frozen ? QStringLiteral("Frozen") - : QStringLiteral("Cancelled"); - map[QStringLiteral("statusKind")] = status == bank::CardStatus::Active ? QStringLiteral("good") - : status == bank::CardStatus::Frozen ? QStringLiteral("warn") - : QStringLiteral("bad"); - map[QStringLiteral("active")] = status == bank::CardStatus::Active; - map[QStringLiteral("cancelled")] = status == bank::CardStatus::Cancelled; + map.insert(QStringLiteral("id"), static_cast(card.id)); + map.insert(QStringLiteral("title"), + kind + QStringLiteral(" card ••••") + QString::fromStdString(card.panLast4)); + map.insert(QStringLiteral("limitText"), + QStringLiteral("Daily limit ") + fmt::money(card.dailyLimitMinor, 0)); + map.insert(QStringLiteral("statusText"), statusText); + map.insert(QStringLiteral("statusKind"), statusKind); + map.insert(QStringLiteral("active"), status == bank::CardStatus::Active); + map.insert(QStringLiteral("cancelled"), status == bank::CardStatus::Cancelled); _cards.append(map); } emit cardsChanged(); @@ -76,25 +85,25 @@ void CardController::issue(qlonglong accountId, int kind, const QString& limit) } const auto minor = limit.trimmed().isEmpty() ? 0 : fmt::parseMinor(limit).value_or(0); _cardModel.execute(bank::dto::IssueCard{.accountId = accountId, .kind = kind, .dailyLimitMinor = minor}) - .then([this](bank::dto::CardInfo) { reloadCards(); }) + .then([this](const bank::dto::CardInfo&) { reloadCards(); }) .onError([this](const std::exception_ptr& err) { emit error(errorText(err)); }); } -void CardController::freeze(qlonglong id) { - _cardModel.execute(bank::dto::FreezeCard{.id = id}) - .then([this](bank::dto::CommandResult) { reloadCards(); }) +void CardController::freeze(qlonglong cardId) { + _cardModel.execute(bank::dto::FreezeCard{.id = cardId}) + .then([this](const bank::dto::CommandResult&) { reloadCards(); }) .onError([this](const std::exception_ptr& err) { emit error(errorText(err)); }); } -void CardController::unfreeze(qlonglong id) { - _cardModel.execute(bank::dto::UnfreezeCard{.id = id}) - .then([this](bank::dto::CommandResult) { reloadCards(); }) +void CardController::unfreeze(qlonglong cardId) { + _cardModel.execute(bank::dto::UnfreezeCard{.id = cardId}) + .then([this](const bank::dto::CommandResult&) { reloadCards(); }) .onError([this](const std::exception_ptr& err) { emit error(errorText(err)); }); } -void CardController::cancel(qlonglong id) { - _cardModel.execute(bank::dto::CancelCard{.id = id}) - .then([this](bank::dto::CommandResult) { reloadCards(); }) +void CardController::cancel(qlonglong cardId) { + _cardModel.execute(bank::dto::CancelCard{.id = cardId}) + .then([this](const bank::dto::CommandResult&) { reloadCards(); }) .onError([this](const std::exception_ptr& err) { emit error(errorText(err)); }); } diff --git a/examples/bank/gui/controllers/CardController.hpp b/examples/bank/gui/controllers/CardController.hpp index f4b05fbab..c87cf7cd8 100644 --- a/examples/bank/gui/controllers/CardController.hpp +++ b/examples/bank/gui/controllers/CardController.hpp @@ -31,9 +31,9 @@ class CardController : public BankController { Q_INVOKABLE void refresh(); Q_INVOKABLE void issue(qlonglong accountId, int kind, const QString& limit); - Q_INVOKABLE void freeze(qlonglong id); - Q_INVOKABLE void unfreeze(qlonglong id); - Q_INVOKABLE void cancel(qlonglong id); + Q_INVOKABLE void freeze(qlonglong cardId); + Q_INVOKABLE void unfreeze(qlonglong cardId); + Q_INVOKABLE void cancel(qlonglong cardId); signals: void cardsChanged(); diff --git a/examples/bank/gui/controllers/LoanController.cpp b/examples/bank/gui/controllers/LoanController.cpp index 19398a392..09e9f49ab 100644 --- a/examples/bank/gui/controllers/LoanController.cpp +++ b/examples/bank/gui/controllers/LoanController.cpp @@ -23,15 +23,15 @@ void LoanController::refresh() { void LoanController::reloadAccounts() { _accountModel.execute(bank::dto::ListAccounts{}) - .then([this](bank::dto::AccountList list) { + .then([this](const bank::dto::AccountList& list) { _accounts.clear(); for (const auto& account : list.accounts) { if (account.status == static_cast(bank::AccountStatus::Closed)) { continue; } QVariantMap map; - map[QStringLiteral("id")] = static_cast(account.id); - map[QStringLiteral("label")] = fmt::last4(account.number); + map.insert(QStringLiteral("id"), static_cast(account.id)); + map.insert(QStringLiteral("label"), fmt::last4(account.number)); _accounts.append(map); } emit accountsChanged(); @@ -41,23 +41,23 @@ void LoanController::reloadAccounts() { void LoanController::reloadLoans() { _loanModel.execute(bank::dto::ListLoans{}) - .then([this](bank::dto::LoanList list) { + .then([this](const bank::dto::LoanList& list) { _loans.clear(); for (const auto& loan : list.loans) { const auto status = static_cast(loan.status); const bool paid = status == bank::LoanStatus::PaidOff; QVariantMap map; - map[QStringLiteral("id")] = static_cast(loan.id); - map[QStringLiteral("accountId")] = static_cast(loan.accountId); - map[QStringLiteral("title")] = QStringLiteral("Loan #%1").arg(loan.id); - map[QStringLiteral("detail")] = QStringLiteral("Outstanding %1 · %2 bps · %3 mo") - .arg(fmt::money(loan.outstandingMinor, loan.currency)) - .arg(loan.rateBps) - .arg(loan.termMonths); - map[QStringLiteral("outstanding")] = static_cast(loan.outstandingMinor); - map[QStringLiteral("statusText")] = paid ? QStringLiteral("Paid off") : QStringLiteral("Active"); - map[QStringLiteral("statusKind")] = paid ? QStringLiteral("good") : QStringLiteral("neutral"); - map[QStringLiteral("active")] = status == bank::LoanStatus::Active; + map.insert(QStringLiteral("id"), static_cast(loan.id)); + map.insert(QStringLiteral("accountId"), static_cast(loan.accountId)); + map.insert(QStringLiteral("title"), QStringLiteral("Loan #%1").arg(loan.id)); + map.insert(QStringLiteral("detail"), QStringLiteral("Outstanding %1 · %2 bps · %3 mo") + .arg(fmt::money(loan.outstandingMinor, loan.currency)) + .arg(loan.rateBps) + .arg(loan.termMonths)); + map.insert(QStringLiteral("outstanding"), static_cast(loan.outstandingMinor)); + map.insert(QStringLiteral("statusText"), paid ? QStringLiteral("Paid off") : QStringLiteral("Active")); + map.insert(QStringLiteral("statusKind"), paid ? QStringLiteral("good") : QStringLiteral("neutral")); + map.insert(QStringLiteral("active"), status == bank::LoanStatus::Active); _loans.append(map); } emit loansChanged(); @@ -76,7 +76,7 @@ void LoanController::apply(qlonglong accountId, const QString& principal, int ra _loanModel .execute(bank::dto::ApplyLoan{ .accountId = accountId, .principalMinor = *minor, .rateBps = rateBps, .termMonths = termMonths}) - .then([this](bank::dto::LoanInfo) { reloadLoans(); }) + .then([this](const bank::dto::LoanInfo&) { reloadLoans(); }) .onError([this](const std::exception_ptr& err) { emit error(errorText(err)); }); } @@ -87,20 +87,20 @@ void LoanController::repay(qlonglong loanId, qlonglong accountId, const QString& return; } _loanModel.execute(bank::dto::RepayLoan{.loanId = loanId, .fromAccountId = accountId, .amountMinor = *minor}) - .then([this](bank::dto::LoanInfo) { reloadLoans(); }) + .then([this](const bank::dto::LoanInfo&) { reloadLoans(); }) .onError([this](const std::exception_ptr& err) { emit error(errorText(err)); }); } void LoanController::showSchedule(qlonglong loanId) { _loanModel.execute(bank::dto::LoanScheduleRequest{.loanId = loanId}) - .then([this](bank::dto::LoanScheduleResult result) { + .then([this](const bank::dto::LoanScheduleResult& result) { _schedule.clear(); for (const auto& inst : result.installments) { QVariantMap map; - map[QStringLiteral("month")] = inst.month; - map[QStringLiteral("principalText")] = fmt::money(inst.principalMinor, 0); - map[QStringLiteral("interestText")] = fmt::money(inst.interestMinor, 0); - map[QStringLiteral("remainingText")] = fmt::money(inst.remainingMinor, 0); + map.insert(QStringLiteral("month"), inst.month); + map.insert(QStringLiteral("principalText"), fmt::money(inst.principalMinor, 0)); + map.insert(QStringLiteral("interestText"), fmt::money(inst.interestMinor, 0)); + map.insert(QStringLiteral("remainingText"), fmt::money(inst.remainingMinor, 0)); _schedule.append(map); } emit scheduleChanged(); diff --git a/examples/bank/gui/controllers/PayeeController.cpp b/examples/bank/gui/controllers/PayeeController.cpp index 76b4728e8..af031c164 100644 --- a/examples/bank/gui/controllers/PayeeController.cpp +++ b/examples/bank/gui/controllers/PayeeController.cpp @@ -25,15 +25,15 @@ void PayeeController::refresh() { void PayeeController::reloadAccounts() { _accountModel.execute(bank::dto::ListAccounts{}) - .then([this](bank::dto::AccountList list) { + .then([this](const bank::dto::AccountList& list) { _accounts.clear(); for (const auto& account : list.accounts) { if (account.status == static_cast(bank::AccountStatus::Closed)) { continue; } QVariantMap map; - map[QStringLiteral("id")] = static_cast(account.id); - map[QStringLiteral("label")] = fmt::last4(account.number); + map.insert(QStringLiteral("id"), static_cast(account.id)); + map.insert(QStringLiteral("label"), fmt::last4(account.number)); _accounts.append(map); } emit accountsChanged(); @@ -43,13 +43,13 @@ void PayeeController::reloadAccounts() { void PayeeController::reloadPayees() { _payeeModel.execute(bank::dto::ListPayees{}) - .then([this](bank::dto::PayeeList list) { + .then([this](const bank::dto::PayeeList& list) { _payees.clear(); for (const auto& payee : list.payees) { QVariantMap map; - map[QStringLiteral("id")] = static_cast(payee.id); - map[QStringLiteral("name")] = QString::fromStdString(payee.name); - map[QStringLiteral("iban")] = QString::fromStdString(payee.iban); + map.insert(QStringLiteral("id"), static_cast(payee.id)); + map.insert(QStringLiteral("name"), QString::fromStdString(payee.name)); + map.insert(QStringLiteral("iban"), QString::fromStdString(payee.iban)); _payees.append(map); } emit payeesChanged(); @@ -61,13 +61,13 @@ void PayeeController::addPayee(const QString& name, const QString& iban, const Q _payeeModel .execute(bank::dto::AddPayee{ .name = name.toStdString(), .iban = iban.trimmed().toStdString(), .bankName = bank.toStdString()}) - .then([this](bank::dto::PayeeInfo) { reloadPayees(); }) + .then([this](const bank::dto::PayeeInfo&) { reloadPayees(); }) .onError([this](const std::exception_ptr& err) { emit error(errorText(err)); }); } -void PayeeController::removePayee(qlonglong id) { - _payeeModel.execute(bank::dto::RemovePayee{.id = id}) - .then([this](bank::dto::CommandResult) { reloadPayees(); }) +void PayeeController::removePayee(qlonglong payeeId) { + _payeeModel.execute(bank::dto::RemovePayee{.id = payeeId}) + .then([this](const bank::dto::CommandResult&) { reloadPayees(); }) .onError([this](const std::exception_ptr& err) { emit error(errorText(err)); }); } @@ -78,7 +78,7 @@ void PayeeController::payBill(qlonglong accountId, qlonglong payeeId, const QStr return; } _paymentModel.execute(bank::dto::PayBill{.fromAccountId = accountId, .payeeId = payeeId, .amountMinor = *minor}) - .then([this](bank::dto::PaymentInfo) { emit paid(); }) + .then([this](const bank::dto::PaymentInfo&) { emit paid(); }) .onError([this](const std::exception_ptr& err) { emit error(errorText(err)); }); } diff --git a/examples/bank/gui/controllers/PayeeController.hpp b/examples/bank/gui/controllers/PayeeController.hpp index 2755b2d29..b51c1a4fc 100644 --- a/examples/bank/gui/controllers/PayeeController.hpp +++ b/examples/bank/gui/controllers/PayeeController.hpp @@ -32,7 +32,7 @@ class PayeeController : public BankController { Q_INVOKABLE void refresh(); Q_INVOKABLE void addPayee(const QString& name, const QString& iban, const QString& bank); - Q_INVOKABLE void removePayee(qlonglong id); + Q_INVOKABLE void removePayee(qlonglong payeeId); Q_INVOKABLE void payBill(qlonglong accountId, qlonglong payeeId, const QString& amount); signals: diff --git a/examples/bank/gui/controllers/TransactionController.cpp b/examples/bank/gui/controllers/TransactionController.cpp index 1498795c9..3311b823f 100644 --- a/examples/bank/gui/controllers/TransactionController.cpp +++ b/examples/bank/gui/controllers/TransactionController.cpp @@ -19,7 +19,7 @@ TransactionController::TransactionController(BankClient& client, QObject* parent void TransactionController::refresh() { _accountModel.execute(bank::dto::ListAccounts{}) - .then([this](bank::dto::AccountList list) { + .then([this](const bank::dto::AccountList& list) { _accounts.clear(); bool stillPresent = false; for (const auto& account : list.accounts) { @@ -27,10 +27,10 @@ void TransactionController::refresh() { continue; } QVariantMap map; - map[QStringLiteral("id")] = static_cast(account.id); - map[QStringLiteral("label")] = fmt::last4(account.number) + QStringLiteral(" · ") + - fmt::money(account.balanceMinor, account.currency); - map[QStringLiteral("currency")] = account.currency; + map.insert(QStringLiteral("id"), static_cast(account.id)); + map.insert(QStringLiteral("label"), fmt::last4(account.number) + QStringLiteral(" · ") + + fmt::money(account.balanceMinor, account.currency)); + map.insert(QStringLiteral("currency"), account.currency); _accounts.append(map); if (account.id == _selected) { stillPresent = true; @@ -48,14 +48,14 @@ void TransactionController::refresh() { .onError([this](const std::exception_ptr& err) { emit error(errorText(err)); }); } -void TransactionController::selectAccount(qlonglong id) { - if (_selected == id) { +void TransactionController::selectAccount(qlonglong accountId) { + if (_selected == accountId) { return; } - _selected = id; + _selected = accountId; for (const auto& entry : std::as_const(_accounts)) { const auto map = entry.toMap(); - if (map.value(QStringLiteral("id")).toLongLong() == id) { + if (map.value(QStringLiteral("id")).toLongLong() == accountId) { _selectedCurrency = map.value(QStringLiteral("currency")).toInt(); } } @@ -72,16 +72,16 @@ void TransactionController::reloadHistory() { return; } _txnModel.execute(bank::dto::History{.accountId = _selected, .limit = 50}) - .then([this](bank::dto::HistoryPage page) { + .then([this](const bank::dto::HistoryPage& page) { _history.clear(); for (const auto& entry : page.entries) { const bool credit = entry.direction == static_cast(bank::TxnDirection::Credit); QVariantMap map; - map[QStringLiteral("kind")] = fmt::txnKind(entry.kind); - map[QStringLiteral("amountText")] = (credit ? QStringLiteral("+") : QStringLiteral("−")) + - fmt::money(entry.amountMinor, entry.currency); - map[QStringLiteral("isCredit")] = credit; - map[QStringLiteral("balanceText")] = fmt::money(entry.balanceAfterMinor, entry.currency); + map.insert(QStringLiteral("kind"), fmt::txnKind(entry.kind)); + map.insert(QStringLiteral("amountText"), (credit ? QStringLiteral("+") : QStringLiteral("−")) + + fmt::money(entry.amountMinor, entry.currency)); + map.insert(QStringLiteral("isCredit"), credit); + map.insert(QStringLiteral("balanceText"), fmt::money(entry.balanceAfterMinor, entry.currency)); _history.append(map); } emit historyChanged(); @@ -96,7 +96,7 @@ void TransactionController::deposit(const QString& amount) { return; } _txnModel.execute(bank::dto::Deposit{.accountId = _selected, .amountMinor = *minor}) - .then([this](bank::dto::TxnInfo) { + .then([this](const bank::dto::TxnInfo&) { emit posted(); refresh(); }) @@ -110,7 +110,7 @@ void TransactionController::withdraw(const QString& amount) { return; } _txnModel.execute(bank::dto::Withdraw{.accountId = _selected, .amountMinor = *minor}) - .then([this](bank::dto::TxnInfo) { + .then([this](const bank::dto::TxnInfo&) { emit posted(); refresh(); }) diff --git a/examples/bank/gui/controllers/TransactionController.hpp b/examples/bank/gui/controllers/TransactionController.hpp index 427bd1638..7de672e17 100644 --- a/examples/bank/gui/controllers/TransactionController.hpp +++ b/examples/bank/gui/controllers/TransactionController.hpp @@ -32,7 +32,7 @@ class TransactionController : public BankController { [[nodiscard]] qlonglong selectedAccount() const { return _selected; } Q_INVOKABLE void refresh(); - Q_INVOKABLE void selectAccount(qlonglong id); + Q_INVOKABLE void selectAccount(qlonglong accountId); Q_INVOKABLE void deposit(const QString& amount); Q_INVOKABLE void withdraw(const QString& amount); Q_INVOKABLE void transfer(qlonglong toId, const QString& amount); diff --git a/examples/bank/gui/main.cpp b/examples/bank/gui/main.cpp index faa31789a..54aed28c5 100644 --- a/examples/bank/gui/main.cpp +++ b/examples/bank/gui/main.cpp @@ -11,9 +11,9 @@ #include #include #include +#include #include #include -#include #include #include @@ -27,8 +27,8 @@ #include "controllers/TransactionController.hpp" int main(int argc, char* argv[]) { - QGuiApplication app{argc, argv}; - app.setApplicationName(QStringLiteral("Morph Bank")); + const QGuiApplication app{argc, argv}; + QGuiApplication::setApplicationName(QStringLiteral("Morph Bank")); QQuickStyle::setStyle(QStringLiteral("Basic")); // so our custom styling applies const auto dbPath = std::filesystem::temp_directory_path() / "morph_bank_gui.db"; @@ -57,13 +57,15 @@ int main(int argc, char* argv[]) { } // Headless screenshot smoke test: seed data, sign in, and grab each page. - if (const char* outEnv = std::getenv("BANK_GUI_SMOKE")) { + // qgetenv rather than std::getenv: the latter is concurrency-mt-unsafe, and + // the two seed variables below already read the environment the Qt way. + if (const QByteArray outEnv = qgetenv("BANK_GUI_SMOKE"); !outEnv.isEmpty()) { const QString out = QString::fromUtf8(outEnv); auto* window = qobject_cast(engine.rootObjects().constFirst()); - const auto pump = [](int ms) { + const auto pump = [](int milliseconds) { QElapsedTimer timer; timer.start(); - while (timer.elapsed() < ms) { + while (timer.elapsed() < milliseconds) { QCoreApplication::processEvents(QEventLoop::AllEvents, 10); QThread::msleep(5); } @@ -74,7 +76,7 @@ int main(int argc, char* argv[]) { }; pump(400); - if (window) { + if (window != nullptr) { window->grabWindow().save(out + "/qml_login.png"); } @@ -97,17 +99,18 @@ int main(int argc, char* argv[]) { payeeController.addPayee("City Power", "DE89370400440532013000", "Stadtbank"); pump(400); - if (auto* shell = window ? window->findChild("appShell") : nullptr) { + if (auto* shell = window != nullptr ? window->findChild("appShell") : nullptr) { accountController.refresh(); // page 0 is already current; force its data to reload - const char* names[] = {"accounts", "move-money", "cards", "payees", "loans"}; - for (int page = 0; page < 5; ++page) { + const QStringList names{QStringLiteral("accounts"), QStringLiteral("move-money"), QStringLiteral("cards"), + QStringLiteral("payees"), QStringLiteral("loans")}; + for (int page = 0; page < names.size(); ++page) { shell->setProperty("current", page); pump(500); - window->grabWindow().save(out + QStringLiteral("/qml_%1.png").arg(names[page])); + window->grabWindow().save(out + QStringLiteral("/qml_%1.png").arg(names.at(page))); } } return 0; } - return app.exec(); + return QGuiApplication::exec(); } diff --git a/examples/bank/tests/gui/test_bank_gui_qml_behaviour.cpp b/examples/bank/tests/gui/test_bank_gui_qml_behaviour.cpp index 54621afd6..cf8139b2d 100644 --- a/examples/bank/tests/gui/test_bank_gui_qml_behaviour.cpp +++ b/examples/bank/tests/gui/test_bank_gui_qml_behaviour.cpp @@ -97,13 +97,13 @@ namespace { return QUrl::fromLocalFile(QStringLiteral(MORPH_LADDER_SOURCE_ROOT "/examples/bank/gui/qml/") + fileName); } -/// @brief Index of @p id within a controller's published `accounts` list. -/// @param accounts The `QVariantList` of account bags. -/// @param id Account id to locate. +/// @brief Index of @p accountId within a controller's published `accounts` list. +/// @param accounts The `QVariantList` of account bags. +/// @param accountId Account id to locate. /// @return Its index, or -1. -[[nodiscard]] int indexOfAccount(const QVariantList& accounts, qlonglong id) { +[[nodiscard]] int indexOfAccount(const QVariantList& accounts, qlonglong accountId) { for (int i = 0; i < accounts.size(); ++i) { - if (accounts.at(i).toMap().value(QStringLiteral("id")).toLongLong() == id) { + if (accounts.at(i).toMap().value(QStringLiteral("id")).toLongLong() == accountId) { return i; } } @@ -112,6 +112,51 @@ namespace { } // namespace +// Both TEST_CASEs in this file carry a readability-function-cognitive-complexity +// directive, and the argument for both of them is here. +// +// The check scores a whole TEST_CASE body and names it by what the macro +// expands to -- Catch2's `dummyFunctionNN`. What it is scoring here is almost +// entirely Catch2's assertion expansion rather than anything either test does: +// REQUIRE/CHECK become +// `do { ... try { ... } catch (...) { ... } } while ((void)0, (false) && ...)`, +// and the metric charges +1 for the loop, +2 for the handler at nesting level 1 +// and +1 for the `&&` -- four points per assertion before a test has branched +// on anything at all. +// +// Measured with the clang-tidy-diff job's own configure and its `-extra-arg` +// pair, clang-tidy 22.1.8, threshold lowered to 1 so that both cases report +// rather than only the one the job already fails on: +// +// dummyFunction72, this case ......... 87, over 21 REQUIRE/CHECK +// dummyFunction76, the case below .... 45, over 11 REQUIRE/CHECK +// +// 21 x 4 = 84 and 11 x 4 = 44, so three points of the 87 and one of the 45 are +// the whole of what the tests' own shape contributes -- the nested lambdas +// here, the six-controller range-for there. Commenting a single CHECK out of +// this case moves it 87 -> 83, so the four per assertion is measured rather +// than arithmetic. Neither number is new: both are identical on this branch's +// base, 7ab4c7a9. What is new is that the job reports one of them, because +// clang-tidy-diff surfaces a finding when any of its notes lands on a changed +// line, and the `balanceOf` lambda below is one of this finding's notes. +// +// Splitting is the alternative to a directive, and it cannot reach the +// threshold. At four points an assertion, 25 means at most six assertions per +// TEST_CASE; every fragment of this scenario has to re-register a user, open +// two accounts, stand up a QQmlEngine, load MoveMoneyPage.qml and drive the +// picker onto the savings account before it can assert anything of its own, +// and that prologue is eight assertions -- 32 -- on its own. Hoisting it into a +// helper moves the score into the helper instead of removing it. And morph#296's +// defect *is* the sequence -- pick, deposit, still picked, deposit again, the +// money followed the label -- which is the thing a split would scatter. +// +// Two per-case directives rather than one entry in +// examples/bank/tests/.clang-tidy, which would subtract the check from every +// bank test including the ones not written yet, and rather than a +// NOLINTBEGIN/NOLINTEND span, which would also cover whatever is added between +// them. Re-open this if either case's non-assertion residue stops being a +// rounding error, or if an assertion ever stops costing four. +// NOLINTNEXTLINE(readability-function-cognitive-complexity) TEST_CASE("MoveMoneyPage's picker keeps naming the account the next deposit will land in", "[bank][gui][qml][move-money]") { bankgui::BankClient client{connectionString()}; @@ -122,8 +167,8 @@ TEST_CASE("MoveMoneyPage's picker keeps naming the account the next deposit will morph::bridge::BridgeHandler customer{client.bridge(), client.gui()}; morph::bridge::BridgeHandler accountReads{client.bridge(), client.gui()}; - const auto balanceOf = [&accountReads](qlonglong id) { - return awaitQt(accountReads.execute(bank::dto::GetAccount{.id = id})).balanceMinor; + const auto balanceOf = [&accountReads](qlonglong accountId) { + return awaitQt(accountReads.execute(bank::dto::GetAccount{.id = accountId})).balanceMinor; }; // Two open accounts, so "the first one" and "the one the user picked" can @@ -155,9 +200,10 @@ TEST_CASE("MoveMoneyPage's picker keeps naming the account the next deposit will REQUIRE(component.isReady()); const std::unique_ptr page{component.create()}; REQUIRE(page != nullptr); - CHECK(firstWarning.toStdString() == std::string{}); + INFO(firstWarning.toStdString()); + CHECK(firstWarning.isEmpty()); - QObject* picker = page->findChild(QStringLiteral("accountPicker")); + auto* picker = page->findChild(QStringLiteral("accountPicker")); REQUIRE(picker != nullptr); // ── AppShell's `Component.onCompleted: refreshCurrent()` ────────────── @@ -207,6 +253,19 @@ TEST_CASE("MoveMoneyPage's picker keeps naming the account the next deposit will CHECK(balanceOf(checking) == 0); } +// 45, over 11 REQUIRE/CHECK plus the six-controller range-for below. The +// measurement, why that score is Catch2's expansion rather than a branch +// thicket, why splitting cannot reach the threshold, and why this is a per-case +// directive rather than an entry in examples/bank/tests/.clang-tidy, are all +// set out above the first TEST_CASE in this file. +// +// This directive is armed rather than decorative, which the diff that +// introduced it did not by itself show: the changed lines it shipped with never +// reached this finding. Checked by deleting the directive and running +// clang-tidy-diff over a one-line diff on the +// `REQUIRE(pumpUntil([&app] { ... }))` line below -- the 45 is reported and the +// run exits 1; with the directive back, the same diff is clean. +// NOLINTNEXTLINE(readability-function-cognitive-complexity) TEST_CASE("Main.qml confirms a posted transaction and a paid bill in the toast", "[bank][gui][qml][toast]") { bankgui::BankClient client{connectionString()}; @@ -244,7 +303,7 @@ TEST_CASE("Main.qml confirms a posted transaction and a paid bill in the toast", const std::unique_ptr window{component.create()}; REQUIRE(window != nullptr); - QObject* toastText = window->findChild(QStringLiteral("toastText")); + auto* toastText = window->findChild(QStringLiteral("toastText")); REQUIRE(toastText != nullptr); const auto toastSays = [toastText] { return toastText->property("text").toString(); }; REQUIRE(toastSays().isEmpty()); diff --git a/examples/bank/tests/gui/test_bank_qml_surface.cpp b/examples/bank/tests/gui/test_bank_qml_surface.cpp index 54cb172df..b4b0d3e34 100644 --- a/examples/bank/tests/gui/test_bank_qml_surface.cpp +++ b/examples/bank/tests/gui/test_bank_qml_surface.cpp @@ -66,12 +66,14 @@ TEST_CASE("Every bank controller exposes exactly the surface gui/qml binds, and const auto dbPath = std::filesystem::temp_directory_path() / "morph_bank_qml_surface.db"; bankgui::BankClient client{"DRIVER=SQLite3;Database=" + dbPath.string()}; - bankgui::AppController appController{client}; - bankgui::AccountController accountController{client}; - bankgui::TransactionController transactionController{client}; - bankgui::CardController cardController{client}; - bankgui::PayeeController payeeController{client}; - bankgui::LoanController loanController{client}; + // const: `QmlSurfaceAudit::bind` takes `const QObject&`, and nothing here + // drives a controller -- the audit reads metaobjects and QML text. + const bankgui::AppController appController{client}; + const bankgui::AccountController accountController{client}; + const bankgui::TransactionController transactionController{client}; + const bankgui::CardController cardController{client}; + const bankgui::PayeeController payeeController{client}; + const bankgui::LoanController loanController{client}; QmlSurfaceAudit audit{QStringLiteral(MORPH_LADDER_SOURCE_ROOT "/examples/bank/gui/qml")}; audit.bind(QStringLiteral("app"), appController); From f498055fae0100186240cbf97bd3554287ef23a8 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 21 Sep 2026 11:04:54 +0200 Subject: [PATCH 2/2] ci: catch a Q_OBJECT header split from its TU before the link does (fixes #659) AUTOMOC finds a Q_OBJECT header two ways -- beside a translation unit of the same basename, or named in a target's own source list -- and when neither holds it generates nothing and says nothing. The .cpp compiles, the static library archives, and the first signal is a linker error about a missing vtable in every leg that links the target. On #657 that was six red legs at once, the fastest at 4m03s. cmake/morph_add_rung.cmake describes this failure in its own comment on the _lib_headers glob, names the case it was diagnosed on (pastebin::app::App, "hit the moment ladder_pastebin_tests linked it") and gives the remedy. #652 hit it again anyway, in a different CMakeLists. A comment in a file you are not editing is not a control. scripts/check_qobject_moc_pairing.py is. For every tracked header carrying an AUTOMOC macro it requires one of three things, and each is checked rather than assumed: - a translation unit of the same basename in the same directory; - the header named in a target's source list, parsed out of the CMake corpus by balanced-paren command extraction, with `#` comments blanked first (examples/common/CMakeLists.txt names fault_proxy.hpp five times in the paragraph explaining why it is listed; counting those would let the prose about the coverage stand in for the coverage) and FILE_SET argument blocks dropped (morph_qt's installed-header set names qt_websocket_server.hpp but drives no moc; morph_qt_impl's source list is what does); - the header under examples//include/ for a rung in examples/rungs.txt -- the morph_add_rung() glob. This gate does not resolve CMake globs, so it asserts that one instead: it fails if morph_add_rung.cmake stops carrying a `file(GLOB_RECURSE _lib_headers ... include/*.hpp)` whose result reaches an add_library(). Four of the tree's six split headers are covered by nothing else. Measured on this tree: 349 tracked headers, 41 CMake files, 40 headers carrying an AUTOMOC macro -- 34 paired, 2 listed, 4 globbed, 0 uncovered. ## Why a text scan, and why drift-guard.yml #659 expected a gate over a configured build tree, as scripts/check_automoc_includes.sh is, and flagged the scoping problem: a build tree only holds what its configure enabled, so "every Q_OBJECT header must have moc output" false-positives on everything behind an off-by-default option -- the WASM shells, bank's GUI, every rung at MORPH_BUILD_LADDER=OFF. Checking the pairing rather than the output dissolves that. A header behind an off-by-default option still has to be listed in its conditionally-added target; which options a configure turned on does not enter into it. So this needs no configure, no compiler and no Qt, and fits drift-guard.yml's stated contract ("Every job here is fast and dependency-free; none of them compiles anything") rather than sitting behind the slow legs it exists to pre-empt. It deliberately does not check that the target owning the source list has AUTOMOC on -- that is a second way to get no moc output, it has never happened here, and resolving target properties means a configure. Recorded in the script's header rather than left implicit. ## Both vacuity traps, closed The tree is clean today, so this gate ships already green and would never announce a broken scan on its own. - It prints what it examined -- headers walked, headers carrying a macro, and which mechanism covered each -- and exits 1 when the macro-bearing set is empty. A scan that stops recognising Q_OBJECT is a failure, not a pass. - `--self-test` drives nine fixtures, two of them mutations of this repository's real files: fixture 6b: ` testkit/fault_proxy.hpp` deleted from morph_ladder_testkit's source list in a copy of the real examples/common/CMakeLists.txt -- the exact #652 regression, and the close condition #659 names. The gate reports `examples/common/testkit/fault_proxy.hpp` and exits 1. fixture 7b: the `file(GLOB_RECURSE _lib_headers ...)` line deleted from a copy of the real cmake/morph_add_rung.cmake, with a rung header credited to it. The gate exits 1 rather than keeping the credit. Each mutation asserts that it changed something, so a rename upstream turns the self-test red instead of quietly making it a no-op. The other seven cover the paired arm, the listed arm, an unlisted split, a header "covered" only by a comment and a FILE_SET, and a tree with no macro headers at all. ## Verification status Reproduced locally on 7ab4c7a9 + the #656 commit, python 3, no build: $ python3 scripts/check_qobject_moc_pairing.py walked 349 tracked header(s) across 41 CMake file(s) 40 carry an AUTOMOC macro: 34 paired with a same-directory translation unit 2 named in a target's source list 4 under a ladder rung's include/, globbed by morph_add_rung() 0 with no moc pairing at all ok: morph_add_rung() globs include/*.hpp into ladder__lib Q_OBJECT moc-pairing lint OK. $ python3 scripts/check_qobject_moc_pairing.py --self-test ... self-test OK: 9 fixture(s), including the #652 mutation of the real examples/common/CMakeLists.txt and a mutation of the real cmake/morph_add_rung.cmake. Not verified: the gate against the pre-fix revision d380895c itself. The mutation in fixture 6b reconstructs that state from the current file rather than checking the old one out, so it proves the gate fires on the shape, not that it would have fired on that commit's whole tree. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW --- .github/workflows/drift-guard.yml | 43 ++ scripts/check_qobject_moc_pairing.py | 615 +++++++++++++++++++++++++++ 2 files changed, 658 insertions(+) create mode 100644 scripts/check_qobject_moc_pairing.py diff --git a/.github/workflows/drift-guard.yml b/.github/workflows/drift-guard.yml index 1d23102e7..d97a1dd57 100644 --- a/.github/workflows/drift-guard.yml +++ b/.github/workflows/drift-guard.yml @@ -298,6 +298,49 @@ jobs: - name: Self-test the AUTOMOC-include checker run: bash scripts/test_check_automoc_includes.sh + # ── Q_OBJECT headers vs. the source lists that get them mocced (#659) ── + # The complement of the AUTOMOC-include gate above: that one checks the + # includes moc *writes*, this one checks that moc runs on the header at all. + # + # AUTOMOC finds a Q_OBJECT header two ways -- beside a translation unit of + # the same basename, or named in a target's own source list -- and when + # neither holds it silently generates nothing. The .cpp compiles, the static + # library archives, and the first signal is a linker error about a missing + # vtable, in every leg that links the target. On #657 that was six red legs + # at once, the fastest at 4m03s. + # + # The repository had already met this, diagnosed it and written it down, in + # cmake/morph_add_rung.cmake's own comment on its _lib_headers glob + # ("pastebin::app::App, hit the moment ladder_pastebin_tests linked it"). + # #652 then hit it again in a different CMakeLists, because nothing reads a + # comment in a file you are not editing. That is the argument for the gate: + # a known failure mode with no control recurs on schedule. + # + # It belongs in this file rather than behind a build because it checks the + # *pairing*, not the moc output. A build tree only covers what its configure + # enabled, so an output check would false-positive on every header behind an + # off-by-default option; a pairing check is configure-independent, needs no + # Qt and no compiler, and runs in under a second. + # + # The self-test runs first and drives the mutation the issue named as its + # close condition -- testkit/fault_proxy.hpp removed from + # morph_ladder_testkit's source list, against a copy of the real + # examples/common/CMakeLists.txt -- because the tree is clean today, so this + # gate ships already green and a broken scan would look exactly like a + # healthy one. The gate itself also refuses to pass when it found no + # Q_OBJECT header at all. + qobject-moc-pairing-lint: + name: Q_OBJECT headers vs. AUTOMOC source lists + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v4 + + - name: Self-test the Q_OBJECT moc-pairing checker + run: python3 scripts/check_qobject_moc_pairing.py --self-test + + - name: Check every Q_OBJECT header is one AUTOMOC will scan + run: python3 scripts/check_qobject_moc_pairing.py + # ── The duplicate-ctest-name gate's own self-test ────────────────────── # The gate itself (scripts/check_ctest_name_collisions.sh) runs after the # build in ci.yml's ladder-tests job, because it asks ctest what it would diff --git a/scripts/check_qobject_moc_pairing.py b/scripts/check_qobject_moc_pairing.py new file mode 100644 index 000000000..a28825b70 --- /dev/null +++ b/scripts/check_qobject_moc_pairing.py @@ -0,0 +1,615 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: Apache-2.0 +"""Usage: python3 scripts/check_qobject_moc_pairing.py [ROOT] [--self-test] + +Fails if a tracked header declaring a Q_OBJECT (or another AUTOMOC macro) is +separated from everything that would make AUTOMOC run moc on it -- see #659. + +ROOT defaults to the repository this script lives in. `--self-test` runs the +gate's own fixtures, including a mutation of this repository's real +examples/common/CMakeLists.txt, and checks nothing else. + +## Why this gate exists + +AUTOMOC finds a `Q_OBJECT` header two ways, and only two: + + 1. the header sits next to a translation unit of the same basename, in the + same directory (`presenter.hpp` beside `presenter.cpp`); or + 2. the header is named in a target's own source list, which is why + `cmake/morph_add_rung.cmake` globs `include/*.hpp` into each rung + library and why `examples/common/CMakeLists.txt` lists + `testkit/fault_proxy.hpp`. + +When neither holds, no moc output is generated. Nothing complains. The +translation unit compiles, the static library archives, and the defect +surfaces only at the first link that needs the vtable: + + libmorph_ladder_testkit.a(fault_proxy.cpp.o): in function + `morph::ladder::testkit::FaultProxy::FaultProxy(QUrl, QObject*)': + undefined reference to `vtable for morph::ladder::testkit::FaultProxy' + undefined reference to `...::FaultProxy::staticMetaObject' + undefined reference to `typeinfo for ...::FaultProxy' + +That is late and it is wide. On #657 one such split turned six CI legs red at +once -- `Application ladder`, its ASan+UBSan twin, `Kanban / ThreadSanitizer`, +`Linux / all optional features` for both compilers, and `clang-coverage` -- +with the fastest at 4m03s. + +The repository already knew. `cmake/morph_add_rung.cmake` describes this exact +failure, names the case it was diagnosed on (`pastebin::app::App`, "hit the +moment `ladder_pastebin_tests` linked it") and explains the remedy. #652 then +walked into it anyway, in a different CMakeLists, because nothing reads a +comment in a file you are not editing. A known failure mode with no control is +one that recurs on schedule; this is the control. + +## Why this is a text scan and not a build-tree scan + +The issue that filed this expected a gate over a configured build tree, as +`scripts/check_automoc_includes.sh` is, and worried about scoping: a build +tree only covers what the configure enabled, so "every `Q_OBJECT` header must +have moc output" would false-positive on everything behind an off-by-default +option -- the WASM shells, bank's GUI, every rung when +`MORPH_BUILD_LADDER=OFF`. + +Checking the *pairing* instead of the *output* dissolves that. A header behind +an off-by-default option still has to be listed in its (conditionally added) +target, or sit beside its own .cpp; which options a given configure turned on +does not enter into it. So this gate needs no configure, no compiler and no Qt, +runs in well under a second, and lives in drift-guard.yml with the rest of the +fast dependency-free gates rather than behind the slow legs it is meant to +pre-empt. + +## What it deliberately does NOT check + +That the target whose source list names the header has AUTOMOC enabled. That +is a second way to get no moc output, and it has never happened here -- +`morph_qt_impl` sets `AUTOMOC ON` explicitly and every Qt target the ladder +builds goes through `qt_add_*` or `morph_add_rung()`. Checking it means +resolving CMake target properties, which means a configure, which is the cost +this gate exists to avoid. A `FILE_SET HEADERS` entry is the one case of "named +but not scanned" that is cheap to recognise, and it is excluded below. + +## The two vacuity traps, and how each is closed + +A gate reporting "0 unmocced Q_OBJECT headers" is satisfied just as well by a +walk that found no headers at all, and this tree is clean today, so the gate +ships already green and would never announce a broken scan on its own. + + 1. It reports what it examined -- headers walked, headers carrying an + AUTOMOC macro, and how each covered one was covered -- and exits 1 if the + macro-bearing set is empty. A scan that stops recognising `Q_OBJECT` is + then a failure, not a pass. + + 2. `--self-test` drives the mutation the issue named as its close condition: + `testkit/fault_proxy.hpp` removed from `morph_ladder_testkit`'s source + list in this repository's own `examples/common/CMakeLists.txt`. That is + the exact #652 regression, replayed against the real file, and the gate + must report it. Four synthetic fixtures cover the arms that mutation does + not reach. + +Exits 0 when every macro-bearing header is covered, printing the counts. Exits +1 naming each uncovered header, and also when the scan found no macro-bearing +header at all or when a coverage mechanism it credits has disappeared. +""" + +import argparse +import os +import pathlib +import re +import shutil +import subprocess +import sys +import tempfile + +# The macros that make AUTOMOC run moc on a header. Anchored at the start of a +# line so that prose naming one -- this file's own docstring, or a CMake +# comment -- is not mistaken for a declaration. +AUTOMOC_MACRO = re.compile( + r"^[ \t]*(Q_OBJECT|Q_GADGET|Q_GADGET_EXPORT|Q_NAMESPACE|Q_NAMESPACE_EXPORT|Q_ENUM_NS)\b", + re.M, +) + +HEADER_SUFFIXES = (".h", ".hh", ".hpp", ".hxx", ".h++", ".hm") +SOURCE_SUFFIXES = (".cpp", ".cc", ".cxx", ".c++", ".mm", ".m", ".C") + +# Commands whose arguments are a target's own source list -- the second of +# AUTOMOC's two ways of finding a header. `target_sources` is here for the +# `PRIVATE`/`PUBLIC` form; its `FILE_SET HEADERS` form is excluded per +# argument, below, because an installed-header set is not an AUTOMOC scan. +SOURCE_LIST_COMMANDS = { + "add_library", + "add_executable", + "target_sources", + "qt_add_executable", + "qt_add_library", + "qt6_add_executable", + "qt6_add_library", + "qt_add_qml_module", + "qt6_add_qml_module", +} + +# Substituted before a listed path is resolved. Anything still carrying a `${` +# after this is a variable this scan cannot follow, and is skipped -- see +# `unresolved` in audit(), which reports how many there were so that a listing +# this gate cannot see is visible rather than silently absent. +ROOT_VARIABLES = ("CMAKE_SOURCE_DIR", "PROJECT_SOURCE_DIR", "morph_SOURCE_DIR") +DIR_VARIABLES = ("CMAKE_CURRENT_SOURCE_DIR", "CMAKE_CURRENT_LIST_DIR") + + +def strip_cmake_comments(text: str) -> str: + """Blank out `#` comments, leaving quoted `#` alone and line count intact. + + The comment half matters as much as the command half: examples/common's + CMakeLists names `fault_proxy.hpp` five times in the paragraph explaining + why it is listed, and a scan that counted those would report the header + covered by the prose that describes the coverage. + """ + out = [] + in_string = False + index = 0 + while index < len(text): + char = text[index] + if in_string: + if char == "\\" and index + 1 < len(text): + out.append(text[index : index + 2]) + index += 2 + continue + if char == '"': + in_string = False + out.append(char) + elif char == '"': + in_string = True + out.append(char) + elif char == "#": + while index < len(text) and text[index] != "\n": + out.append(" ") + index += 1 + continue + else: + out.append(char) + index += 1 + return "".join(out) + + +def cmake_commands(text: str): + """Yield (name, argument-text) for every command invocation in `text`.""" + cleaned = strip_cmake_comments(text) + for match in re.finditer(r"([A-Za-z_][A-Za-z0-9_]*)[ \t\r\n]*\(", cleaned): + name = match.group(1).lower() + depth = 1 + index = match.end() + in_string = False + while index < len(cleaned) and depth: + char = cleaned[index] + if in_string: + if char == "\\": + index += 2 + continue + if char == '"': + in_string = False + elif char == '"': + in_string = True + elif char == "(": + depth += 1 + elif char == ")": + depth -= 1 + if not depth: + break + index += 1 + yield name, cleaned[match.end() : index] + + +def listed_headers(root: pathlib.Path, cmake_files): + """Header paths named in a target's source list, repo-relative. + + Returns (covered, unresolved): the set of paths, and the count of listed + header tokens that carry a variable this scan cannot expand. + """ + covered = set() + unresolved = 0 + for cmake_file in cmake_files: + directory = (root / cmake_file).parent + text = (root / cmake_file).read_text(errors="replace") + for name, arguments in cmake_commands(text): + if name not in SOURCE_LIST_COMMANDS: + continue + # A FILE_SET is an install/interface header set, not an AUTOMOC + # scan: morph_qt lists include/morph/qt/qt_websocket_server.hpp + # in one, and the listing that actually drives moc for it is + # morph_qt_impl's. Everything from the keyword to the end of the + # command is dropped, which is where a FILE_SET's FILES live. + cut = re.search(r"\bFILE_SET\b", arguments) + if cut: + arguments = arguments[: cut.start()] + for token in re.findall(r'"[^"]*"|\S+', arguments): + token = token.strip('"') + if not token.lower().endswith(HEADER_SUFFIXES): + continue + for variable in ROOT_VARIABLES: + token = token.replace("${" + variable + "}", str(root)) + for variable in DIR_VARIABLES: + token = token.replace("${" + variable + "}", str(directory)) + if "${" in token: + unresolved += 1 + continue + path = pathlib.Path(token) + if not path.is_absolute(): + path = directory / path + try: + covered.add(path.resolve().relative_to(root.resolve()).as_posix()) + except ValueError: + # Outside the tree: not something this repository owns. + continue + return covered, unresolved + + +def rung_glob_is_intact(root: pathlib.Path): + """Whether morph_add_rung() still globs a rung's include/ into its library. + + Four of this tree's six split headers are covered by nothing else, so if + that glob is ever dropped or renamed this gate must stop crediting it + rather than keep passing them. + """ + recipe = root / "cmake" / "morph_add_rung.cmake" + if not recipe.is_file(): + return False, f"{recipe.relative_to(root)} does not exist" + text = strip_cmake_comments(recipe.read_text(errors="replace")) + glob = re.search( + r"file\s*\(\s*GLOB_RECURSE\s+_lib_headers\b[^)]*include/\*\.hpp", text, re.S + ) + if not glob: + return False, ( + "cmake/morph_add_rung.cmake no longer contains a " + "`file(GLOB_RECURSE _lib_headers ... include/*.hpp)`" + ) + if not re.search(r"add_library\s*\([^)]*\$\{_lib_headers\}", text, re.S): + return False, ( + "cmake/morph_add_rung.cmake globs _lib_headers but no add_library() " + "call passes ${_lib_headers} as a source" + ) + return True, "morph_add_rung() globs include/*.hpp into ladder__lib" + + +def read_rungs(root: pathlib.Path): + listing = root / "examples" / "rungs.txt" + if not listing.is_file(): + return [] + return [ + line.strip() + for line in listing.read_text(errors="replace").splitlines() + if line.strip() and not line.lstrip().startswith("#") + ] + + +def tracked_files(root: pathlib.Path): + result = subprocess.run( + ["git", "-C", str(root), "ls-files", "-z"], + capture_output=True, + text=True, + check=True, + ) + return [name for name in result.stdout.split("\0") if name] + + +def audit(root: pathlib.Path): + """Returns (report-lines, uncovered, macro_headers, headers_walked).""" + files = tracked_files(root) + headers = [f for f in files if f.lower().endswith(HEADER_SUFFIXES)] + sources = {f for f in files if f.endswith(SOURCE_SUFFIXES)} + cmake_files = [ + f + for f in files + if pathlib.PurePosixPath(f).name == "CMakeLists.txt" or f.endswith(".cmake") + ] + + macro_headers = [] + for header in headers: + text = (root / header).read_text(errors="replace") + if AUTOMOC_MACRO.search(text): + macro_headers.append(header) + + covered_by_listing, unresolved = listed_headers(root, cmake_files) + rungs = read_rungs(root) + rung_prefixes = tuple(f"examples/{rung}/include/" for rung in rungs) + + paired = [] + listed = [] + globbed = [] + uncovered = [] + for header in macro_headers: + path = pathlib.PurePosixPath(header) + neighbours = [ + f"{path.parent.as_posix()}/{path.stem}{suffix}" for suffix in SOURCE_SUFFIXES + ] + if any(neighbour in sources for neighbour in neighbours): + paired.append(header) + elif header in covered_by_listing: + listed.append(header) + elif rung_prefixes and header.startswith(rung_prefixes): + globbed.append(header) + else: + uncovered.append(header) + + report = [ + f"walked {len(headers)} tracked header(s) across {len(cmake_files)} CMake file(s)", + f"{len(macro_headers)} carry an AUTOMOC macro:", + f" {len(paired)} paired with a same-directory translation unit", + f" {len(listed)} named in a target's source list", + f" {len(globbed)} under a ladder rung's include/, globbed by morph_add_rung()", + f" {len(uncovered)} with no moc pairing at all", + ] + if unresolved: + report.append( + f"note: {unresolved} listed header path(s) carry a CMake variable this " + f"scan cannot expand and were not credited as coverage" + ) + return report, uncovered, macro_headers, headers, globbed + + +def check(root: pathlib.Path) -> int: + report, uncovered, macro_headers, headers, globbed = audit(root) + errors = [] + + if not headers: + errors.append( + f"no tracked headers found under {root} -- this gate has nothing to " + f"check and must not report success" + ) + elif not macro_headers: + errors.append( + f"no tracked header carries an AUTOMOC macro " + f"({'|'.join(['Q_OBJECT', 'Q_GADGET', 'Q_NAMESPACE', '...'])}), across " + f"{len(headers)} header(s) walked. Either this repository has stopped " + f"declaring QObjects in headers, or this scan has stopped recognising " + f"them. A gate that examined nothing must not report success." + ) + + if globbed: + intact, why = rung_glob_is_intact(root) + if not intact: + errors.append( + f"{len(globbed)} header(s) are credited to morph_add_rung()'s " + f"include/ glob, but that mechanism is gone: {why}. Those headers " + f"now get no moc output at all:\n" + + "\n".join(f" {header}" for header in globbed) + ) + else: + report.append(f"ok: {why}") + + for header in uncovered: + errors.append( + f"{header} declares an AUTOMOC macro but AUTOMOC will never see it: " + f"no translation unit of the same basename sits beside it, and no " + f"target's source list names it. Nothing will fail until the first " + f"link that needs its vtable, in every leg at once. Either move the " + f".cpp back next to the header, or list the header among its target's " + f"sources the way examples/common/CMakeLists.txt lists " + f"testkit/fault_proxy.hpp -- header entries are not compiled, they " + f"only join the AUTOMOC scan." + ) + + for line in report: + print(line) + if errors: + for error in errors: + print(f"::error::{error}", file=sys.stderr) + print(f"\n{len(errors)} problem(s). See #659.", file=sys.stderr) + return 1 + print("Q_OBJECT moc-pairing lint OK.") + return 0 + + +# ── self-test ─────────────────────────────────────────────────────────────── + + +def _fixture(directory: pathlib.Path, files: dict) -> pathlib.Path: + for name, body in files.items(): + path = directory / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body) + subprocess.run(["git", "-C", str(directory), "init", "-q"], check=True) + subprocess.run(["git", "-C", str(directory), "add", "-A"], check=True) + return directory + + +HEADER_WITH_MACRO = """#pragma once +#include +class Thing : public QObject { + Q_OBJECT +public: + Thing(); +}; +""" + + +def self_test(repo_root: pathlib.Path) -> int: + failures = [] + + def expect(name, root, wanted, must_name=None): + code = check(root) + if code != wanted: + failures.append(f"{name}: expected exit {wanted}, got {code}") + return + if must_name is not None: + _, uncovered, _, _, _ = audit(root) + if must_name not in uncovered: + failures.append(f"{name}: expected {must_name} to be reported, got {uncovered}") + + with tempfile.TemporaryDirectory() as raw: + tmp = pathlib.Path(raw) + + # 1. Paired: header beside its own .cpp. The arm that covers 34 of this + # repository's 40 macro-bearing headers. + print("\n--- fixture 1: header beside its own translation unit ---") + expect( + "paired", + _fixture( + tmp / "paired", + { + "src/thing.hpp": HEADER_WITH_MACRO, + "src/thing.cpp": '#include "thing.hpp"\nThing::Thing() {}\n', + "CMakeLists.txt": "add_library(t STATIC src/thing.cpp)\n", + }, + ), + 0, + ) + + # 2. Split but listed: the shape morph_ladder_testkit and morph_qt_impl + # both have. + print("\n--- fixture 2: split header, named in the source list ---") + expect( + "listed", + _fixture( + tmp / "listed", + { + "inc/thing.hpp": HEADER_WITH_MACRO, + "src/thing.cpp": '#include "thing.hpp"\nThing::Thing() {}\n', + "CMakeLists.txt": "add_library(t STATIC src/thing.cpp inc/thing.hpp)\n", + }, + ), + 0, + ) + + # 3. Split and not listed: #652's regression, synthetically. + print("\n--- fixture 3: split header, not listed (must fail) ---") + expect( + "split", + _fixture( + tmp / "split", + { + "inc/thing.hpp": HEADER_WITH_MACRO, + "src/thing.cpp": '#include "thing.hpp"\nThing::Thing() {}\n', + "CMakeLists.txt": "add_library(t STATIC src/thing.cpp)\n", + }, + ), + 1, + "inc/thing.hpp", + ) + + # 4. Named only by a comment, and only in a FILE_SET. Neither makes + # AUTOMOC scan the header, and both are how a passing gate would be + # faked by accident. + print("\n--- fixture 4: comment + FILE_SET are not coverage (must fail) ---") + expect( + "not-really-listed", + _fixture( + tmp / "weak", + { + "inc/thing.hpp": HEADER_WITH_MACRO, + "src/thing.cpp": '#include "thing.hpp"\nThing::Thing() {}\n', + "CMakeLists.txt": ( + "# inc/thing.hpp is a QObject, remember to moc it\n" + "add_library(t STATIC src/thing.cpp)\n" + "target_sources(t INTERFACE FILE_SET HEADERS BASE_DIRS inc " + "FILES inc/thing.hpp)\n" + ), + }, + ), + 1, + "inc/thing.hpp", + ) + + # 5. A tree with no macro-bearing header at all. The anti-vacuity floor: + # this is what a scan that stopped recognising Q_OBJECT looks like. + print("\n--- fixture 5: nothing to examine (must fail) ---") + expect( + "vacuous", + _fixture( + tmp / "vacuous", + { + "inc/plain.hpp": "#pragma once\nint plain();\n", + "CMakeLists.txt": "add_library(t INTERFACE)\n", + }, + ), + 1, + ) + + # 6. The close condition from #659, against the real file: this + # repository's examples/common/, with testkit/fault_proxy.hpp + # removed from morph_ladder_testkit's source list. Copied rather + # than mutated in place so the working tree is never touched. + print("\n--- fixture 6: the real examples/common/, unmutated ---") + real = tmp / "real" + (real / "examples").mkdir(parents=True) + shutil.copytree(repo_root / "examples" / "common", real / "examples" / "common") + _fixture(real, {}) + expect("real-tree", real, 0) + + print("\n--- fixture 6b: fault_proxy.hpp removed from the source list ---") + lists = real / "examples" / "common" / "CMakeLists.txt" + before = lists.read_text() + after = before.replace(" testkit/fault_proxy.hpp\n", "", 1) + if after == before: + failures.append( + "real-tree mutation: ` testkit/fault_proxy.hpp` is no longer a " + "line of examples/common/CMakeLists.txt, so this self-test mutated " + "nothing. Re-point it at wherever that header is listed now." + ) + lists.write_text(after) + subprocess.run(["git", "-C", str(real), "add", "-A"], check=True) + expect("real-tree-mutated", real, 1, "examples/common/testkit/fault_proxy.hpp") + + # 7. The rung-glob credit, and what happens when the mechanism behind + # it goes. Four of this tree's macro-bearing headers are covered by + # nothing but morph_add_rung()'s include/ glob, and this gate cannot + # resolve that glob -- so it asserts the glob instead. Mutating the + # real recipe is what proves that assertion is not decorative. + print("\n--- fixture 7: a rung header, credited to the real glob ---") + rung = tmp / "rung" + recipe_source = repo_root / "cmake" / "morph_add_rung.cmake" + _fixture( + rung, + { + "examples/rungs.txt": "demo\n", + "examples/demo/include/demo/app/app.hpp": HEADER_WITH_MACRO, + "examples/demo/src/app/app.cpp": '#include "demo/app/app.hpp"\nThing::Thing() {}\n', + "CMakeLists.txt": "include(cmake/morph_add_rung.cmake)\n", + "cmake/morph_add_rung.cmake": recipe_source.read_text(), + }, + ) + expect("rung-globbed", rung, 0) + + print("\n--- fixture 7b: the glob removed from morph_add_rung.cmake ---") + recipe = rung / "cmake" / "morph_add_rung.cmake" + before = recipe.read_text() + after = re.sub( + r"file\(GLOB_RECURSE _lib_headers[^\n]*\n", "", before, count=1 + ) + if after == before: + failures.append( + "rung-glob mutation: cmake/morph_add_rung.cmake no longer contains a " + "one-line `file(GLOB_RECURSE _lib_headers ...)`, so this self-test " + "mutated nothing." + ) + recipe.write_text(after) + subprocess.run(["git", "-C", str(rung), "add", "-A"], check=True) + expect("rung-glob-gone", rung, 1) + + if failures: + for failure in failures: + print(f"::error::self-test: {failure}", file=sys.stderr) + print(f"\n{len(failures)} self-test failure(s).", file=sys.stderr) + return 1 + print( + "\nself-test OK: 9 fixture(s), including the #652 mutation of the real " + "examples/common/CMakeLists.txt and a mutation of the real " + "cmake/morph_add_rung.cmake." + ) + return 0 + + +def main(argv) -> int: + parser = argparse.ArgumentParser(add_help=True) + parser.add_argument("root", nargs="?", default=None) + parser.add_argument("--self-test", action="store_true") + args = parser.parse_args(argv) + + default_root = pathlib.Path(__file__).resolve().parent.parent + root = pathlib.Path(args.root).resolve() if args.root else default_root + if args.self_test: + return self_test(default_root) + return check(root) + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:]))