[clang-tools-extra] 3696c70 - [clang-tidy] Add `readability-container-contains` check
via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 24 03:57:33 PST 2022
Author: Adrian Vogelsgesang
Date: 2022-01-24T12:57:18+01:00
New Revision: 3696c70e67d9b9e54307ef25077bae7a6f76636e
URL: https://github.com/llvm/llvm-project/commit/3696c70e67d9b9e54307ef25077bae7a6f76636e
DIFF: https://github.com/llvm/llvm-project/commit/3696c70e67d9b9e54307ef25077bae7a6f76636e.diff
LOG: [clang-tidy] Add `readability-container-contains` check
This commit introduces a new check `readability-container-contains` which finds
usages of `container.count()` and `container.find() != container.end()` and
instead recommends the `container.contains()` method introduced in C++20.
For containers which permit multiple entries per key (`multimap`, `multiset`,
...), `contains` is more efficient than `count` because `count` has to do
unnecessary additional work.
While this this performance difference does not exist for containers with only
a single entry per key (`map`, `unordered_map`, ...), `contains` still conveys
the intent better.
Reviewed By: xazax.hun, whisperity
Differential Revision: http://reviews.llvm.org/D112646
Added:
clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h
clang-tools-extra/docs/clang-tidy/checks/readability-container-contains.rst
clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
Modified:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 22ce8f62751ec..ea09b2193eb7c 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -7,6 +7,7 @@ add_clang_library(clangTidyReadabilityModule
AvoidConstParamsInDecls.cpp
BracesAroundStatementsCheck.cpp
ConstReturnTypeCheck.cpp
+ ContainerContainsCheck.cpp
ContainerDataPointerCheck.cpp
ContainerSizeEmptyCheck.cpp
ConvertMemberFunctionsToStatic.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
new file mode 100644
index 0000000000000..7a20480fb501c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
@@ -0,0 +1,144 @@
+//===--- ContainerContainsCheck.cpp - clang-tidy --------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ContainerContainsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
+ const auto SupportedContainers = hasType(
+ hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(
+ hasAnyName("::std::set", "::std::unordered_set", "::std::map",
+ "::std::unordered_map", "::std::multiset",
+ "::std::unordered_multiset", "::std::multimap",
+ "::std::unordered_multimap"))))));
+
+ const auto CountCall =
+ cxxMemberCallExpr(on(SupportedContainers),
+ callee(cxxMethodDecl(hasName("count"))),
+ argumentCountIs(1))
+ .bind("call");
+
+ const auto FindCall =
+ cxxMemberCallExpr(on(SupportedContainers),
+ callee(cxxMethodDecl(hasName("find"))),
+ argumentCountIs(1))
+ .bind("call");
+
+ const auto EndCall = cxxMemberCallExpr(on(SupportedContainers),
+ callee(cxxMethodDecl(hasName("end"))),
+ argumentCountIs(0));
+
+ const auto Literal0 = integerLiteral(equals(0));
+ const auto Literal1 = integerLiteral(equals(1));
+
+ auto AddSimpleMatcher = [&](auto Matcher) {
+ Finder->addMatcher(
+ traverse(TK_IgnoreUnlessSpelledInSource, std::move(Matcher)), this);
+ };
+
+ // Find membership tests which use `count()`.
+ Finder->addMatcher(implicitCastExpr(hasImplicitDestinationType(booleanType()),
+ hasSourceExpression(CountCall))
+ .bind("positiveComparison"),
+ this);
+ AddSimpleMatcher(
+ binaryOperator(hasLHS(CountCall), hasOperatorName("!="), hasRHS(Literal0))
+ .bind("positiveComparison"));
+ AddSimpleMatcher(
+ binaryOperator(hasLHS(Literal0), hasOperatorName("!="), hasRHS(CountCall))
+ .bind("positiveComparison"));
+ AddSimpleMatcher(
+ binaryOperator(hasLHS(CountCall), hasOperatorName(">"), hasRHS(Literal0))
+ .bind("positiveComparison"));
+ AddSimpleMatcher(
+ binaryOperator(hasLHS(Literal0), hasOperatorName("<"), hasRHS(CountCall))
+ .bind("positiveComparison"));
+ AddSimpleMatcher(
+ binaryOperator(hasLHS(CountCall), hasOperatorName(">="), hasRHS(Literal1))
+ .bind("positiveComparison"));
+ AddSimpleMatcher(
+ binaryOperator(hasLHS(Literal1), hasOperatorName("<="), hasRHS(CountCall))
+ .bind("positiveComparison"));
+
+ // Find inverted membership tests which use `count()`.
+ AddSimpleMatcher(
+ binaryOperator(hasLHS(CountCall), hasOperatorName("=="), hasRHS(Literal0))
+ .bind("negativeComparison"));
+ AddSimpleMatcher(
+ binaryOperator(hasLHS(Literal0), hasOperatorName("=="), hasRHS(CountCall))
+ .bind("negativeComparison"));
+ AddSimpleMatcher(
+ binaryOperator(hasLHS(CountCall), hasOperatorName("<="), hasRHS(Literal0))
+ .bind("negativeComparison"));
+ AddSimpleMatcher(
+ binaryOperator(hasLHS(Literal0), hasOperatorName(">="), hasRHS(CountCall))
+ .bind("negativeComparison"));
+ AddSimpleMatcher(
+ binaryOperator(hasLHS(CountCall), hasOperatorName("<"), hasRHS(Literal1))
+ .bind("negativeComparison"));
+ AddSimpleMatcher(
+ binaryOperator(hasLHS(Literal1), hasOperatorName(">"), hasRHS(CountCall))
+ .bind("negativeComparison"));
+
+ // Find membership tests based on `find() == end()`.
+ AddSimpleMatcher(
+ binaryOperator(hasLHS(FindCall), hasOperatorName("!="), hasRHS(EndCall))
+ .bind("positiveComparison"));
+ AddSimpleMatcher(
+ binaryOperator(hasLHS(FindCall), hasOperatorName("=="), hasRHS(EndCall))
+ .bind("negativeComparison"));
+}
+
+void ContainerContainsCheck::check(const MatchFinder::MatchResult &Result) {
+ // Extract the information about the match
+ const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call");
+ const auto *PositiveComparison =
+ Result.Nodes.getNodeAs<Expr>("positiveComparison");
+ const auto *NegativeComparison =
+ Result.Nodes.getNodeAs<Expr>("negativeComparison");
+ assert(
+ !PositiveComparison ||
+ !NegativeComparison &&
+ "only one of PositiveComparison or NegativeComparison should be set");
+ bool Negated = NegativeComparison != nullptr;
+ const auto *Comparison = Negated ? NegativeComparison : PositiveComparison;
+
+ // Diagnose the issue.
+ auto Diag =
+ diag(Call->getExprLoc(), "use 'contains' to check for membership");
+
+ // Don't fix it if it's in a macro invocation. Leave fixing it to the user.
+ SourceLocation FuncCallLoc = Comparison->getEndLoc();
+ if (!FuncCallLoc.isValid() || FuncCallLoc.isMacroID())
+ return;
+
+ // Create the fix it.
+ const auto *Member = cast<MemberExpr>(Call->getCallee());
+ Diag << FixItHint::CreateReplacement(
+ Member->getMemberNameInfo().getSourceRange(), "contains");
+ SourceLocation ComparisonBegin = Comparison->getSourceRange().getBegin();
+ SourceLocation ComparisonEnd = Comparison->getSourceRange().getEnd();
+ SourceLocation CallBegin = Call->getSourceRange().getBegin();
+ SourceLocation CallEnd = Call->getSourceRange().getEnd();
+ Diag << FixItHint::CreateReplacement(
+ CharSourceRange::getCharRange(ComparisonBegin, CallBegin),
+ Negated ? "!" : "");
+ Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
+ CallEnd.getLocWithOffset(1), ComparisonEnd.getLocWithOffset(1)));
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h
new file mode 100644
index 0000000000000..0c2705d437797
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h
@@ -0,0 +1,40 @@
+//===--- ContainerContainsCheck.h - clang-tidy ------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINERCONTAINSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINERCONTAINSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Finds usages of `container.count()` and `find() == end()` which should be
+/// replaced by a call to the `container.contains()` method introduced in C++20.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-container-contains.html
+class ContainerContainsCheck : public ClangTidyCheck {
+public:
+ ContainerContainsCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) final;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) final;
+
+protected:
+ bool isLanguageVersionSupported(const LangOptions &LO) const final {
+ return LO.CPlusPlus20;
+ }
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINERCONTAINSCHECK_H
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index b0493d43ff318..6bbef6b7fa07c 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -12,6 +12,7 @@
#include "AvoidConstParamsInDecls.h"
#include "BracesAroundStatementsCheck.h"
#include "ConstReturnTypeCheck.h"
+#include "ContainerContainsCheck.h"
#include "ContainerDataPointerCheck.h"
#include "ContainerSizeEmptyCheck.h"
#include "ConvertMemberFunctionsToStatic.h"
@@ -64,6 +65,8 @@ class ReadabilityModule : public ClangTidyModule {
"readability-braces-around-statements");
CheckFactories.registerCheck<ConstReturnTypeCheck>(
"readability-const-return-type");
+ CheckFactories.registerCheck<ContainerContainsCheck>(
+ "readability-container-contains");
CheckFactories.registerCheck<ContainerDataPointerCheck>(
"readability-container-data-pointer");
CheckFactories.registerCheck<ContainerSizeEmptyCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 060af42521552..ba0e530b7fec4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -118,6 +118,12 @@ New checks
Reports identifier with unicode right-to-left characters.
+- New :doc:`readability-container-contains
+ <clang-tidy/checks/readability-container-contains>` check.
+
+ Finds usages of ``container.count()`` and ``container.find() == container.end()`` which should
+ be replaced by a call to the ``container.contains()`` method introduced in C++20.
+
- New :doc:`readability-container-data-pointer
<clang-tidy/checks/readability-container-data-pointer>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 5878345bdfcfd..fcf661a406959 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -290,6 +290,7 @@ Clang-Tidy Checks
`readability-avoid-const-params-in-decls <readability-avoid-const-params-in-decls.html>`_, "Yes"
`readability-braces-around-statements <readability-braces-around-statements.html>`_, "Yes"
`readability-const-return-type <readability-const-return-type.html>`_, "Yes"
+ `readability-container-contains <readability-container-contains.html>`_, "Yes"
`readability-container-data-pointer <readability-container-data-pointer.html>`_, "Yes"
`readability-container-size-empty <readability-container-size-empty.html>`_, "Yes"
`readability-convert-member-functions-to-static <readability-convert-member-functions-to-static.html>`_, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-container-contains.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-container-contains.rst
new file mode 100644
index 0000000000000..07d1e352d3b1b
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability-container-contains.rst
@@ -0,0 +1,25 @@
+.. title:: clang-tidy - readability-container-contains
+
+readability-container-contains
+==============================
+
+Finds usages of ``container.count()`` and ``container.find() == container.end()`` which should be replaced by a call to the ``container.contains()`` method introduced in C++ 20.
+
+Whether an element is contained inside a container should be checked with ``contains`` instead of ``count``/``find`` because ``contains`` conveys the intent more clearly. Furthermore, for containers which permit multiple entries per key (``multimap``, ``multiset``, ...), ``contains`` is more efficient than ``count`` because ``count`` has to do unnecessary additional work.
+
+Examples:
+
+=========================================== ==============================
+Initial expression Result
+------------------------------------------- ------------------------------
+``myMap.find(x) == myMap.end()`` ``!myMap.contains(x)``
+``myMap.find(x) != myMap.end()`` ``myMap.contains(x)``
+``if (myMap.count(x))`` ``if (myMap.contains(x))``
+``bool exists = myMap.count(x)`` ``bool exists = myMap.contains(x)``
+``bool exists = myMap.count(x) > 0`` ``bool exists = myMap.contains(x)``
+``bool exists = myMap.count(x) >= 1`` ``bool exists = myMap.contains(x)``
+``bool missing = myMap.count(x) == 0`` ``bool missing = !myMap.contains(x)``
+=========================================== ==============================
+
+This check applies to ``std::set``, ``std::unordered_set``, ``std::map``, ``std::unordered_map`` and the corresponding multi-key variants.
+It is only active for C++20 and later, as the ``contains`` method was only added in C++20.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
new file mode 100644
index 0000000000000..c4ea1e27e63e6
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
@@ -0,0 +1,230 @@
+// RUN: %check_clang_tidy -std=c++20 %s readability-container-contains %t
+
+// Some *very* simplified versions of `map` etc.
+namespace std {
+
+template <class Key, class T>
+struct map {
+ unsigned count(const Key &K) const;
+ bool contains(const Key &K) const;
+ void *find(const Key &K);
+ void *end();
+};
+
+template <class Key>
+struct set {
+ unsigned count(const Key &K) const;
+ bool contains(const Key &K) const;
+};
+
+template <class Key>
+struct unordered_set {
+ unsigned count(const Key &K) const;
+ bool contains(const Key &K) const;
+};
+
+template <class Key, class T>
+struct multimap {
+ unsigned count(const Key &K) const;
+ bool contains(const Key &K) const;
+};
+
+} // namespace std
+
+// Check that we detect various common ways to check for membership
+int testDifferentCheckTypes(std::map<int, int> &MyMap) {
+ if (MyMap.count(0))
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: if (MyMap.contains(0))
+ return 1;
+ bool C1 = MyMap.count(1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: bool C1 = MyMap.contains(1);
+ auto C2 = static_cast<bool>(MyMap.count(1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C2 = static_cast<bool>(MyMap.contains(1));
+ auto C3 = MyMap.count(2) != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C3 = MyMap.contains(2);
+ auto C4 = MyMap.count(3) > 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C4 = MyMap.contains(3);
+ auto C5 = MyMap.count(4) >= 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C5 = MyMap.contains(4);
+ auto C6 = MyMap.find(5) != MyMap.end();
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C6 = MyMap.contains(5);
+ return C1 + C2 + C3 + C4 + C5 + C6;
+}
+
+// Check that we detect various common ways to check for non-membership
+int testNegativeChecks(std::map<int, int> &MyMap) {
+ bool C1 = !MyMap.count(-1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: bool C1 = !MyMap.contains(-1);
+ auto C2 = MyMap.count(-2) == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C2 = !MyMap.contains(-2);
+ auto C3 = MyMap.count(-3) <= 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C3 = !MyMap.contains(-3);
+ auto C4 = MyMap.count(-4) < 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C4 = !MyMap.contains(-4);
+ auto C5 = MyMap.find(-5) == MyMap.end();
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C5 = !MyMap.contains(-5);
+ return C1 + C2 + C3 + C4 + C5;
+}
+
+// Check for various types
+int testDifferentTypes(std::map<int, int> &M, std::unordered_set<int> &US, std::set<int> &S, std::multimap<int, int> &MM) {
+ bool C1 = M.count(1001);
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: bool C1 = M.contains(1001);
+ bool C2 = US.count(1002);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: bool C2 = US.contains(1002);
+ bool C3 = S.count(1003);
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: bool C3 = S.contains(1003);
+ bool C4 = MM.count(1004);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: bool C4 = MM.contains(1004);
+ return C1 + C2 + C3 + C4;
+}
+
+// The check detects all kind of `const`, reference, rvalue-reference and value types.
+int testQualifiedTypes(std::map<int, int> ValueM, std::map<int, int> &RefM, const std::map<int, int> &ConstRefM, std::map<int, int> &&RValueM) {
+ bool C1 = ValueM.count(2001);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: bool C1 = ValueM.contains(2001);
+ bool C2 = RefM.count(2002);
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: bool C2 = RefM.contains(2002);
+ bool C3 = ConstRefM.count(2003);
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: bool C3 = ConstRefM.contains(2003);
+ bool C4 = RValueM.count(2004);
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: bool C4 = RValueM.contains(2004);
+ return C1 + C2 + C3 + C4;
+}
+
+// This is effectively a membership check, as the result is implicitly casted
+// to `bool`.
+bool returnContains(std::map<int, int> &M) {
+ return M.count(42);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: return M.contains(42);
+}
+
+// This returns the actual count and should not be rewritten
+int actualCount(std::multimap<int, int> &M) {
+ return M.count(21);
+ // NO-WARNING.
+ // CHECK-FIXES: return M.count(21);
+}
+
+// Check that we are not confused by aliases
+namespace s2 = std;
+using MyMapT = s2::map<int, int>;
+int typeAliases(MyMapT &MyMap) {
+ bool C1 = MyMap.count(99);
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: bool C1 = MyMap.contains(99);
+ return C1;
+}
+
+// Check that the tests also trigger for a local variable and not only for
+// function arguments.
+bool localVar() {
+ using namespace std;
+ map<int, int> LocalM;
+ return LocalM.count(42);
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: return LocalM.contains(42);
+}
+
+// Check various usages of an actual `count` which isn't rewritten
+int nonRewrittenCount(std::multimap<int, int> &MyMap) {
+ // This is an actual test if we have at least 2 usages. Shouldn't be rewritten.
+ bool C1 = MyMap.count(1) >= 2;
+ // NO-WARNING.
+ // CHECK-FIXES: bool C1 = MyMap.count(1) >= 2;
+
+ // "< 0" makes little sense and is always `false`. Still, let's ensure we
+ // don't accidentally rewrite it to 'contains'.
+ bool C2 = MyMap.count(2) < 0;
+ // NO-WARNING.
+ // CHECK-FIXES: bool C2 = MyMap.count(2) < 0;
+
+ // The `count` is used in some more complicated formula.
+ bool C3 = MyMap.count(1) + MyMap.count(2) * 2 + MyMap.count(3) / 3 >= 20;
+ // NO-WARNING.
+ // CHECK-FIXES: bool C3 = MyMap.count(1) + MyMap.count(2) * 2 + MyMap.count(3) / 3 >= 20;
+
+ // This could theoretically be rewritten into a 'contains' after removig the
+ // `4` on both sides of the comparison. For the time being, we don't detect
+ // this case.
+ bool C4 = MyMap.count(1) + 4 > 4;
+ // NO-WARNING.
+ // CHECK-FIXES: bool C4 = MyMap.count(1) + 4 > 4;
+
+ return C1 + C2 + C3 + C4;
+}
+
+// We don't want to rewrite if the `contains` call is from a macro expansion
+int testMacroExpansion(std::unordered_set<int> &MySet) {
+#define COUNT_ONES(SET) SET.count(1)
+ // Rewriting the macro would break the code
+ // CHECK-FIXES: #define COUNT_ONES(SET) SET.count(1)
+ // We still want to warn the user even if we don't offer a fixit
+ if (COUNT_ONES(MySet)) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-MESSAGES: note: expanded from macro 'COUNT_ONES'
+ return COUNT_ONES(MySet);
+ }
+#undef COUNT_ONES
+#define COUNT_ONES count(1)
+ // Rewriting the macro would break the code
+ // CHECK-FIXES: #define COUNT_ONES count(1)
+ // We still want to warn the user even if we don't offer a fixit
+ if (MySet.COUNT_ONES) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-MESSAGES: note: expanded from macro 'COUNT_ONES'
+ return MySet.COUNT_ONES;
+ }
+#undef COUNT_ONES
+#define MY_SET MySet
+ // CHECK-FIXES: #define MY_SET MySet
+ // We still want to rewrite one of the two calls to `count`
+ if (MY_SET.count(1)) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: if (MY_SET.contains(1)) {
+ return MY_SET.count(1);
+ }
+#undef MY_SET
+ return 0;
+}
+
+// The following map has the same interface like `std::map`.
+template <class Key, class T>
+struct CustomMap {
+ unsigned count(const Key &K) const;
+ bool contains(const Key &K) const;
+ void *find(const Key &K);
+ void *end();
+};
+
+// The clang-tidy check is currently hard-coded against the `std::` containers
+// and hence won't annotate the following instance. We might change this in the
+// future and also detect the following case.
+void *testDifferentCheckTypes(CustomMap<int, int> &MyMap) {
+ if (MyMap.count(0))
+ // NO-WARNING.
+ // CHECK-FIXES: if (MyMap.count(0))
+ return nullptr;
+ return MyMap.find(2);
+}
More information about the cfe-commits
mailing list