[clang-tidy][readability-container-contains] Extend to any class with contains

This check will now work out of the box with other containers that have a
`contains` method, such as `folly::F14` or Abseil containers.

It will also work with strings, which are basically just weird containers.
`std::string` and `std::string_view` will have a `contains` method starting with
C++23. `llvm::StringRef` and `folly::StringPiece` are examples of existing
implementations with a `contains` method.

Author: nicovank

clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp

@@ -15,26 +15,26 @@ using namespace clang::ast_matchers;
 namespace clang::tidy::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 HasContainsMethod = hasMethod(
+      cxxMethodDecl(isConst(), returns(booleanType()), hasName("contains")));
+  const auto ContainerWithContains = hasType(
+      hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(anyOf(
+          HasContainsMethod, hasAnyBase(hasType(hasCanonicalType(hasDeclaration(
+                                 cxxRecordDecl(HasContainsMethod)))))))))));
 
   const auto CountCall =
-      cxxMemberCallExpr(on(SupportedContainers),
+      cxxMemberCallExpr(on(ContainerWithContains),
                         callee(cxxMethodDecl(hasName("count"))),
                         argumentCountIs(1))
           .bind("call");
 
   const auto FindCall =
-      cxxMemberCallExpr(on(SupportedContainers),
+      cxxMemberCallExpr(on(ContainerWithContains),
                         callee(cxxMethodDecl(hasName("find"))),
                         argumentCountIs(1))
           .bind("call");
 
-  const auto EndCall = cxxMemberCallExpr(on(SupportedContainers),
+  const auto EndCall = cxxMemberCallExpr(on(ContainerWithContains),
                                          callee(cxxMethodDecl(hasName("end"))),
                                          argumentCountIs(0));
 

clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h

@@ -24,10 +24,8 @@ class ContainerContainsCheck : public ClangTidyCheck {
       : 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;
+    return LO.CPlusPlus;
   }
 };
 

clang-tools-extra/docs/ReleaseNotes.rst

@@ -116,7 +116,11 @@ Changes in existing checks
   <clang-tidy/checks/modernize/use-std-print>` check to support replacing
   member function calls too.
 
-- Improved :doc:`readablility-implicit-bool-conversion
+- Improved :doc:`readability-container-contains
+  <clang-tidy/checks/readability/container-contains>` check
+  to let it work on any class that has a ``contains`` method.
+
+- Improved :doc:`readability-implicit-bool-conversion
   <clang-tidy/checks/readability/implicit-bool-conversion>` check
   by adding the option `UseUpperCaseLiteralSuffix` to select the
   case of the literal suffix in fixes.

clang-tools-extra/docs/clang-tidy/checks/readability/container-contains.rst

@@ -3,23 +3,31 @@
 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.
+Finds usages of ``container.count()`` and
+``container.find() == container.end()`` which should be replaced by a call to
+the ``container.contains()`` method.
 
-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.
+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)``
-===========================================  ==============================
+=========================================  ==============================
+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.
+This check will apply to any class that has a ``contains`` method, notably
+including ``std::set``, ``std::unordered_set``, ``std::map``, and
+``std::unordered_map`` as of C++20, and ``std::string`` and ``std::string_view``
+as of C++23.

clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp

@@ -240,7 +240,7 @@ int testMacroExpansion(std::unordered_set<int> &MySet) {
   return 0;
 }
 
-// The following map has the same interface like `std::map`.
+// The following map has the same interface as `std::map`.
 template <class Key, class T>
 struct CustomMap {
   unsigned count(const Key &K) const;
@@ -249,13 +249,30 @@ struct CustomMap {
   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);
+void testDifferentCheckTypes(CustomMap<int, int> &MyMap) {
+  if (MyMap.count(0)) {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: if (MyMap.contains(0)) {};
+
+  MyMap.find(0) != MyMap.end();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: MyMap.contains(0);
+}
+
+struct MySubmap : public CustomMap<int, int> {};
+
+void testSubclass(MySubmap& MyMap) {
+  if (MyMap.count(0)) {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: if (MyMap.contains(0)) {};
+}
+
+using UsingMap = CustomMap<int, int>;
+struct MySubmap2 : public UsingMap {};
+using UsingMap2 = MySubmap2;
+
+void testUsing(UsingMap2& MyMap) {
+  if (MyMap.count(0)) {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: if (MyMap.contains(0)) {};
 }