← Watch feed
refactor: enable `readability-container-contains` clang-tidy rule
Public commit record
97/100 · Strong
What the developer wrote
Authored by Lőrinc
refactor: enable `readability-container-contains` clang-tidy rule
Replace the last few instances of `.count() != 0` and `.count() == 0` and `.count()` 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.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope✓ Provides detailed explanatory context✓ Explains rationale or failure mode
What changed, and why it matters
This commit is a code cleanup that replaces old-style container lookups like `.count()` with the newer, clearer `.contains()` method introduced in C++20. It also turns on a linting rule to keep future code consistent. There are no functional changes and no security impact.
Recommended action
No security action required. Treat as ordinary code-quality/maintenance review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors several call sites across Bitcoin Core to use std::set/map/unordered_set::contains() instead of .count() != 0, .count() == 0, or bare .count() used as a boolean. It enables the readability-container-contains clang-tidy check in src/.clang-tidy to enforce the pattern going forward. The changes are behavior-preserving: contains() is semantically equivalent to the previous .count() checks for the standard containers involved. No logic, validation rules, or trust boundaries are altered.
Changed components
src/.clang-tidysrc/qt/transactiondesc.cppsrc/rpc/mining.cppsrc/test/transaction_tests.cppsrc/wallet/test/fuzz/scriptpubkeyman.cppInspect captured patch +11 / −10
diff --git a/src/.clang-tidy b/src/.clang-tidy
index da53a588..01153649 100644
--- 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
index 93d31ee6..918d0af9 100644
--- 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
index e710f590..e6c03639 100644
--- 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
index 6274e368..89fc6a11 100644
--- 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/wallet/test/fuzz/scriptpubkeyman.cpp b/src/wallet/test/fuzz/scriptpubkeyman.cpp
index 3edf2265..ea1431a7 100644
--- 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));
}
},
[&] {
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.