refactor: enable readability-container-contains clang-tidy rule

l0rinc · l0rinc · commit a8a0a0ff7730 · 2025-12-18T15:45:28.000+01:00
Replace the last few instances of `.count() != 0` and `.count() == 0` patterns with the more expressive C++20 `.contains()` method: * `std::set<std::string>` in `getblocktemplate` RPC; * `std::map<std::string, ...>` in `transaction_tests`; * other bare `std::unordered_set` and `std::map` count calls. With no remaining violations, enable the `readability-container-contains` clang-tidy check to prevent future regressions. Also fixes #34101 by reverting boost::multi_index::contains calls available in Boost 1.74.0 only
diff --git a/src/.clang-tidy b/src/.clang-tidy
@@ -25,6 +25,7 @@ performance-*,
 -performance-noexcept-move-constructor,
 -performance-unnecessary-value-param,
 readability-const-return-type,
+readability-container-contains,
 readability-redundant-declaration,
 readability-redundant-string-init,
 '
diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp
@@ -124,7 +124,7 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall
     {
         strHTML += "<b>" + tr("Source") + ":</b> " + tr("Generated") + "<br>";
     }
-    else if (wtx.value_map.count("from") && !wtx.value_map["from"].empty())
+    else if (wtx.value_map.contains("from") && !wtx.value_map["from"].empty())
     {
         // Online transaction
         strHTML += "<b>" + tr("From") + ":</b> " + GUIUtil::HtmlEscape(wtx.value_map["from"]) + "<br>";
@@ -157,7 +157,7 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall
     //
     // To
     //
-    if (wtx.value_map.count("to") && !wtx.value_map["to"].empty())
+    if (wtx.value_map.contains("to") && !wtx.value_map["to"].empty())
     {
         // Online transaction
         std::string strAddress = wtx.value_map["to"];
@@ -212,7 +212,7 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall
                 if (toSelf && all_from_me)
                     continue;
 
-                if (!wtx.value_map.count("to") || wtx.value_map["to"].empty())
+                if (!wtx.value_map.contains("to") || wtx.value_map["to"].empty())
                 {
                     // Offline transaction
                     CTxDestination address;
@@ -273,9 +273,9 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall
     //
     // Message
     //
-    if (wtx.value_map.count("message") && !wtx.value_map["message"].empty())
+    if (wtx.value_map.contains("message") && !wtx.value_map["message"].empty())
         strHTML += "<br><b>" + tr("Message") + ":</b><br>" + GUIUtil::HtmlEscape(wtx.value_map["message"], true) + "<br>";
-    if (wtx.value_map.count("comment") && !wtx.value_map["comment"].empty())
+    if (wtx.value_map.contains("comment") && !wtx.value_map["comment"].empty())
         strHTML += "<br><b>" + tr("Comment") + ":</b><br>" + GUIUtil::HtmlEscape(wtx.value_map["comment"], true) + "<br>";
 
     strHTML += "<b>" + tr("Transaction ID") + ":</b> " + rec->getTxHash() + "<br>";
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
@@ -846,12 +846,12 @@ static RPCHelpMan getblocktemplate()
     const Consensus::Params& consensusParams = chainman.GetParams().GetConsensus();
 
     // GBT must be called with 'signet' set in the rules for signet chains
-    if (consensusParams.signet_blocks && setClientRules.count("signet") != 1) {
+    if (consensusParams.signet_blocks && !setClientRules.contains("signet")) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "getblocktemplate must be called with the signet rule set (call with {\"rules\": [\"segwit\", \"signet\"]})");
     }
 
     // GBT must be called with 'segwit' set in the rules
-    if (setClientRules.count("segwit") != 1) {
+    if (!setClientRules.contains("segwit")) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "getblocktemplate must be called with the segwit rule set (call with {\"rules\": [\"segwit\"]})");
     }
 
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
@@ -60,7 +60,7 @@ script_verify_flags ParseScriptFlags(std::string strFlags)
     std::vector<std::string> words = SplitString(strFlags, ',');
     for (const std::string& word : words)
     {
-        if (!mapFlagNames.count(word)) {
+        if (!mapFlagNames.contains(word)) {
             BOOST_ERROR("Bad test: unknown verification flag '" << word << "'");
             continue;
         }
@@ -90,7 +90,7 @@ bool CheckTxScripts(const CTransaction& tx, const std::map<COutPoint, CScript>&
     ScriptError err = expect_valid ? SCRIPT_ERR_UNKNOWN_ERROR : SCRIPT_ERR_OK;
     for (unsigned int i = 0; i < tx.vin.size() && tx_valid; ++i) {
         const CTxIn input = tx.vin[i];
-        const CAmount amount = map_prevout_values.count(input.prevout) ? map_prevout_values.at(input.prevout) : 0;
+        const CAmount amount = map_prevout_values.contains(input.prevout) ? map_prevout_values.at(input.prevout) : 0;
         try {
             tx_valid = VerifyScript(input.scriptSig, map_prevout_scriptPubKeys.at(input.prevout),
                 &input.scriptWitness, flags, TransactionSignatureChecker(&tx, i, amount, txdata, MissingDataBehavior::ASSERT_FAIL), &err);
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
@@ -988,7 +988,7 @@ util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool::
 CTxMemPool::ChangeSet::TxHandle CTxMemPool::ChangeSet::StageAddition(const CTransactionRef& tx, const CAmount fee, int64_t time, unsigned int entry_height, uint64_t entry_sequence, bool spends_coinbase, int64_t sigops_cost, LockPoints lp)
 {
     LOCK(m_pool->cs);
-    Assume(!m_to_add.contains(tx->GetHash()));
+    Assume(m_to_add.find(tx->GetHash()) == m_to_add.end());
     Assume(!m_dependencies_processed);
 
     // We need to process dependencies after adding a new transaction.
diff --git a/src/txrequest.cpp b/src/txrequest.cpp
@@ -580,7 +580,7 @@ class TxRequestTracker::Impl {
         // Bail out if we already have a CANDIDATE_BEST announcement for this (txhash, peer) combination. The case
         // where there is a non-CANDIDATE_BEST announcement already will be caught by the uniqueness property of the
         // ByPeer index when we try to emplace the new object below.
-        if (m_index.get<ByPeer>().contains(ByPeerView{peer, true, gtxid.ToUint256()})) return;
+        if (m_index.get<ByPeer>().count(ByPeerView{peer, true, gtxid.ToUint256()})) return;
 
         // Try creating the announcement with CANDIDATE_DELAYED state (which will fail due to the uniqueness
         // of the ByPeer index if a non-CANDIDATE_BEST announcement already exists with the same txhash and peer).
diff --git a/src/wallet/test/fuzz/scriptpubkeyman.cpp b/src/wallet/test/fuzz/scriptpubkeyman.cpp
@@ -125,7 +125,7 @@ FUZZ_TARGET(scriptpubkeyman, .init = initialize_spkm)
             [&] {
                 const CScript script{ConsumeScript(fuzzed_data_provider)};
                 if (spk_manager->IsMine(script)) {
-                    assert(spk_manager->GetScriptPubKeys().count(script));
+                    assert(spk_manager->GetScriptPubKeys().contains(script));
                 }
             },
             [&] {

Original file line number	Diff line number	Diff line change
`@@ -25,6 +25,7 @@ performance-*,`
`25`	`25`	`-performance-noexcept-move-constructor,`
`26`	`26`	`-performance-unnecessary-value-param,`
`27`	`27`	`readability-const-return-type,`
	`28`	`+readability-container-contains,`
`28`	`29`	`readability-redundant-declaration,`
`29`	`30`	`readability-redundant-string-init,`
`30`	`31`	`'`
Original file line number	Diff line number	Diff line change
`@@ -846,12 +846,12 @@ static RPCHelpMan getblocktemplate()`
`846`	`846`	`const Consensus::Params& consensusParams = chainman.GetParams().GetConsensus();`
`847`	`847`
`848`	`848`	`// GBT must be called with 'signet' set in the rules for signet chains`
`849`		`- if (consensusParams.signet_blocks && setClientRules.count("signet") != 1) {`
	`849`	`+ if (consensusParams.signet_blocks && !setClientRules.contains("signet")) {`
`850`	`850`	`throw JSONRPCError(RPC_INVALID_PARAMETER, "getblocktemplate must be called with the signet rule set (call with {\"rules\": [\"segwit\", \"signet\"]})");`
`851`	`851`	`}`
`852`	`852`
`853`	`853`	`// GBT must be called with 'segwit' set in the rules`
`854`		`- if (setClientRules.count("segwit") != 1) {`
	`854`	`+ if (!setClientRules.contains("segwit")) {`
`855`	`855`	`throw JSONRPCError(RPC_INVALID_PARAMETER, "getblocktemplate must be called with the segwit rule set (call with {\"rules\": [\"segwit\"]})");`
`856`	`856`	`}`
`857`	`857`
Original file line number	Diff line number	Diff line change
`@@ -988,7 +988,7 @@ util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool::`
`988`	`988`	`CTxMemPool::ChangeSet::TxHandle CTxMemPool::ChangeSet::StageAddition(const CTransactionRef& tx, const CAmount fee, int64_t time, unsigned int entry_height, uint64_t entry_sequence, bool spends_coinbase, int64_t sigops_cost, LockPoints lp)`
`989`	`989`	`{`
`990`	`990`	`LOCK(m_pool->cs);`
`991`		`- Assume(!m_to_add.contains(tx->GetHash()));`
	`991`	`+ Assume(m_to_add.find(tx->GetHash()) == m_to_add.end());`
`992`	`992`	`Assume(!m_dependencies_processed);`
`993`	`993`
`994`	`994`	`// We need to process dependencies after adding a new transaction.`
Original file line number	Diff line number	Diff line change
`@@ -125,7 +125,7 @@ FUZZ_TARGET(scriptpubkeyman, .init = initialize_spkm)`
`125`	`125`	`[&] {`
`126`	`126`	`const CScript script{ConsumeScript(fuzzed_data_provider)};`
`127`	`127`	`if (spk_manager->IsMine(script)) {`
`128`		`- assert(spk_manager->GetScriptPubKeys().count(script));`
	`128`	`+ assert(spk_manager->GetScriptPubKeys().contains(script));`
`129`	`129`	`}`
`130`	`130`	`},`
`131`	`131`	`[&] {`