From 47a235b7be9bc37a4ff1cda9b9604abd48c8caea Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Mon, 16 Mar 2026 21:19:37 +0000 Subject: [PATCH] chore: Enable clang-tidy `switch-missing-default-case` check (#6461) --- .clang-tidy | 2 +- include/xrpl/protocol/STTx.h | 4 +-- src/libxrpl/basics/base64.cpp | 1 + src/libxrpl/protocol/STTx.cpp | 8 ++--- src/libxrpl/server/JSONRPCUtil.cpp | 1 + .../tx/transactors/system/LedgerStateFix.cpp | 21 ++++++------- src/test/nodestore/Timing_test.cpp | 1 + src/xrpld/app/misc/NetworkOPs.cpp | 3 ++ src/xrpld/app/misc/detail/Transaction.cpp | 22 +++++++------- src/xrpld/app/paths/Pathfinder.cpp | 1 + src/xrpld/overlay/detail/Message.cpp | 2 ++ src/xrpld/overlay/detail/OverlayImpl.cpp | 30 +++++++++---------- 12 files changed, 51 insertions(+), 45 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 761c8012b1..6997ac2b6f 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -60,6 +60,7 @@ Checks: "-*, bugprone-suspicious-string-compare, bugprone-suspicious-stringview-data-usage, bugprone-swapped-arguments, + bugprone-switch-missing-default-case, bugprone-terminating-continue, bugprone-throw-keyword-missing, bugprone-too-small-loop-variable, @@ -100,7 +101,6 @@ Checks: "-*, # checks that have some issues that need to be resolved: # # bugprone-move-forwarding-reference, -# bugprone-switch-missing-default-case, # bugprone-use-after-move, # # cppcoreguidelines-misleading-capture-default-by-value, diff --git a/include/xrpl/protocol/STTx.h b/include/xrpl/protocol/STTx.h index 72bd38d5d6..f00f288008 100644 --- a/include/xrpl/protocol/STTx.h +++ b/include/xrpl/protocol/STTx.h @@ -15,7 +15,7 @@ namespace xrpl { -enum TxnSql : char { +enum class TxnSql : char { txnSqlNew = 'N', txnSqlConflict = 'C', txnSqlHeld = 'H', @@ -122,7 +122,7 @@ public: getMetaSQL( Serializer rawTxn, std::uint32_t inLedger, - char status, + TxnSql status, std::string const& escapedMetaData) const; std::vector const& diff --git a/src/libxrpl/basics/base64.cpp b/src/libxrpl/basics/base64.cpp index 76e924f42e..90bbc71359 100644 --- a/src/libxrpl/basics/base64.cpp +++ b/src/libxrpl/basics/base64.cpp @@ -116,6 +116,7 @@ encode(void* dest, void const* src, std::size_t len) in += 3; } + // NOLINTNEXTLINE(bugprone-switch-missing-default-case) switch (len % 3) { case 2: diff --git a/src/libxrpl/protocol/STTx.cpp b/src/libxrpl/protocol/STTx.cpp index 098ca1a400..6f201ba066 100644 --- a/src/libxrpl/protocol/STTx.cpp +++ b/src/libxrpl/protocol/STTx.cpp @@ -351,7 +351,7 @@ STTx::getMetaSQL(std::uint32_t inLedger, std::string const& escapedMetaData) con { Serializer s; add(s); - return getMetaSQL(s, inLedger, txnSqlValidated, escapedMetaData); + return getMetaSQL(s, inLedger, TxnSql::txnSqlValidated, escapedMetaData); } // VFALCO This could be a free function elsewhere @@ -359,7 +359,7 @@ std::string STTx::getMetaSQL( Serializer rawTxn, std::uint32_t inLedger, - char status, + TxnSql status, std::string const& escapedMetaData) const { static boost::format bfTrans("('%s', '%s', '%s', '%d', '%d', '%c', %s, %s)"); @@ -370,8 +370,8 @@ STTx::getMetaSQL( return str( boost::format(bfTrans) % to_string(getTransactionID()) % format->getName() % - toBase58(getAccountID(sfAccount)) % getFieldU32(sfSequence) % inLedger % status % rTxn % - escapedMetaData); + toBase58(getAccountID(sfAccount)) % getFieldU32(sfSequence) % inLedger % + safe_cast(status) % rTxn % escapedMetaData); } static Expected diff --git a/src/libxrpl/server/JSONRPCUtil.cpp b/src/libxrpl/server/JSONRPCUtil.cpp index d32a579d8d..6cf09327f2 100644 --- a/src/libxrpl/server/JSONRPCUtil.cpp +++ b/src/libxrpl/server/JSONRPCUtil.cpp @@ -68,6 +68,7 @@ HTTPReply(int nStatus, std::string const& content, Json::Output const& output, b return; } + // NOLINTNEXTLINE(bugprone-switch-missing-default-case) switch (nStatus) { case 200: diff --git a/src/libxrpl/tx/transactors/system/LedgerStateFix.cpp b/src/libxrpl/tx/transactors/system/LedgerStateFix.cpp index da04d60142..0ce0720ba0 100644 --- a/src/libxrpl/tx/transactors/system/LedgerStateFix.cpp +++ b/src/libxrpl/tx/transactors/system/LedgerStateFix.cpp @@ -35,15 +35,13 @@ LedgerStateFix::calculateBaseFee(ReadView const& view, STTx const& tx) TER LedgerStateFix::preclaim(PreclaimContext const& ctx) { - switch (ctx.tx[sfLedgerFixType]) + if (ctx.tx[sfLedgerFixType] == FixType::nfTokenPageLink) { - case FixType::nfTokenPageLink: { - AccountID const owner{ctx.tx[sfOwner]}; - if (!ctx.view.read(keylet::account(owner))) - return tecOBJECT_NOT_FOUND; + AccountID const owner{ctx.tx[sfOwner]}; + if (!ctx.view.read(keylet::account(owner))) + return tecOBJECT_NOT_FOUND; - return tesSUCCESS; - } + return tesSUCCESS; } // preflight is supposed to verify that only valid FixTypes get to preclaim. @@ -53,13 +51,12 @@ LedgerStateFix::preclaim(PreclaimContext const& ctx) TER LedgerStateFix::doApply() { - switch (ctx_.tx[sfLedgerFixType]) + if (ctx_.tx[sfLedgerFixType] == FixType::nfTokenPageLink) { - case FixType::nfTokenPageLink: - if (!nft::repairNFTokenDirectoryLinks(view(), ctx_.tx[sfOwner])) - return tecFAILED_PROCESSING; + if (!nft::repairNFTokenDirectoryLinks(view(), ctx_.tx[sfOwner])) + return tecFAILED_PROCESSING; - return tesSUCCESS; + return tesSUCCESS; } // preflight is supposed to verify that only valid FixTypes get to doApply. diff --git a/src/test/nodestore/Timing_test.cpp b/src/test/nodestore/Timing_test.cpp index b537e3abb7..e4ecd80654 100644 --- a/src/test/nodestore/Timing_test.cpp +++ b/src/test/nodestore/Timing_test.cpp @@ -550,6 +550,7 @@ public: p[1] = 1 - p[0]; for (int q = 0; q < 2; ++q) { + // NOLINTNEXTLINE(bugprone-switch-missing-default-case) switch (p[q]) { case 0: { diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index b8663a76fb..fd3caa4dab 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -2535,6 +2535,9 @@ NetworkOPsImp::getServerInfo(bool human, bool admin, bool counters) if (admin) { + // Note: By default the node size is "tiny". When parsing it's an error if the final + // NODE_SIZE is over 4 so below code should be safe. + // NOLINTNEXTLINE(bugprone-switch-missing-default-case) switch (registry_.app().config().NODE_SIZE) { case 0: diff --git a/src/xrpld/app/misc/detail/Transaction.cpp b/src/xrpld/app/misc/detail/Transaction.cpp index fae26873fb..35b49504d5 100644 --- a/src/xrpld/app/misc/detail/Transaction.cpp +++ b/src/xrpld/app/misc/detail/Transaction.cpp @@ -53,26 +53,26 @@ Transaction::setStatus( TransStatus Transaction::sqlTransactionStatus(boost::optional const& status) { - char const c = (status) ? (*status)[0] : safe_cast(txnSqlUnknown); + auto const c = (status) ? safe_cast((*status)[0]) : TxnSql::txnSqlUnknown; - switch (c) + switch (static_cast(c)) { - case txnSqlNew: + case TxnSql::txnSqlNew: return NEW; - case txnSqlConflict: + case TxnSql::txnSqlConflict: return CONFLICTED; - case txnSqlHeld: + case TxnSql::txnSqlHeld: return HELD; - case txnSqlValidated: + case TxnSql::txnSqlValidated: return COMMITTED; - case txnSqlIncluded: + case TxnSql::txnSqlIncluded: return INCLUDED; + default: + XRPL_ASSERT( + c == TxnSql::txnSqlUnknown, + "xrpl::Transaction::sqlTransactionStatus : unknown transaction status"); } - XRPL_ASSERT( - c == txnSqlUnknown, - "xrpl::Transaction::sqlTransactionStatus : unknown transaction " - "status"); return INVALID; } diff --git a/src/xrpld/app/paths/Pathfinder.cpp b/src/xrpld/app/paths/Pathfinder.cpp index 254db35ea5..14bfe2b4fc 100644 --- a/src/xrpld/app/paths/Pathfinder.cpp +++ b/src/xrpld/app/paths/Pathfinder.cpp @@ -1158,6 +1158,7 @@ makePath(char const* string) while (true) { + // NOLINTNEXTLINE(bugprone-switch-missing-default-case) switch (*string++) { case 's': // source diff --git a/src/xrpld/overlay/detail/Message.cpp b/src/xrpld/overlay/detail/Message.cpp index 510bc24fe2..672ce995ca 100644 --- a/src/xrpld/overlay/detail/Message.cpp +++ b/src/xrpld/overlay/detail/Message.cpp @@ -58,6 +58,8 @@ Message::compress() bool const compressible = [&] { if (messageBytes <= 70) return false; + + // NOLINTNEXTLINE(bugprone-switch-missing-default-case) switch (type) { case protocol::mtMANIFESTS: diff --git a/src/xrpld/overlay/detail/OverlayImpl.cpp b/src/xrpld/overlay/detail/OverlayImpl.cpp index 4379f1b265..f3280c4923 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.cpp +++ b/src/xrpld/overlay/detail/OverlayImpl.cpp @@ -907,9 +907,9 @@ OverlayImpl::processHealth(http_request_type const& req, Handoff& handoff) std::string server_state = info[jss::server_state].asString(); auto load_factor = info[jss::load_factor_server].asDouble() / info[jss::load_base].asDouble(); - enum { healthy, warning, critical }; - int health = healthy; - auto set_health = [&health](int state) { + enum class HealthState { healthy, warning, critical }; + auto health = HealthState::healthy; + auto set_health = [&health](HealthState state) { if (health < state) health = state; }; @@ -919,24 +919,24 @@ OverlayImpl::processHealth(http_request_type const& req, Handoff& handoff) { msg.body()[jss::info][jss::validated_ledger] = last_validated_ledger_age; if (last_validated_ledger_age < 20) - set_health(warning); + set_health(HealthState::warning); else - set_health(critical); + set_health(HealthState::critical); } if (amendment_blocked) { msg.body()[jss::info][jss::amendment_blocked] = true; - set_health(critical); + set_health(HealthState::critical); } if (number_peers <= 7) { msg.body()[jss::info][jss::peers] = number_peers; if (number_peers != 0) - set_health(warning); + set_health(HealthState::warning); else - set_health(critical); + set_health(HealthState::critical); } if (!(server_state == "full" || server_state == "validating" || server_state == "proposing")) @@ -944,30 +944,30 @@ OverlayImpl::processHealth(http_request_type const& req, Handoff& handoff) msg.body()[jss::info][jss::server_state] = server_state; if (server_state == "syncing" || server_state == "tracking" || server_state == "connected") { - set_health(warning); + set_health(HealthState::warning); } else - set_health(critical); + set_health(HealthState::critical); } if (load_factor > 100) { msg.body()[jss::info][jss::load_factor] = load_factor; if (load_factor < 1000) - set_health(warning); + set_health(HealthState::warning); else - set_health(critical); + set_health(HealthState::critical); } switch (health) { - case healthy: + case HealthState::healthy: msg.result(boost::beast::http::status::ok); break; - case warning: + case HealthState::warning: msg.result(boost::beast::http::status::service_unavailable); break; - case critical: + case HealthState::critical: msg.result(boost::beast::http::status::internal_server_error); break; }