[clang-tools-extra] [clang-tidy][readability-container-contains] Fix matching of non-binaryOperator cases (PR #110386)

via cfe-commits cfe-commits at lists.llvm.org
Sat Sep 28 14:53:36 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Nicolas van Kempen (nicovank)

<details>
<summary>Changes</summary>


Fix #<!-- -->79437.

It works with non-mock `std::map`:
```
# All cases detailed in #<!-- -->79437.
> cat tmp.cpp
#include <map>
bool a(std::map<int, int> &m, int key) { return m.find(key) != m.end(); }
bool b(std::map<int, int> &m, int key) { return m.count(key) > 0; }
bool c(std::map<int, int> &m, int key) { return m.find(key) == m.end(); }
bool d(std::map<int, int> &m, int key) { return m.count(key) == 0; }
bool e(std::map<int, int> *m, int key) { return m->find(key) != m->end(); }
bool f(std::map<int, int> *m, int key) { return m->find(key) == m->end(); }
bool g(std::map<int, int> *m, int key) { return m->count(key) > 0; }
bool h(std::map<int, int> *m, int key) { return m->count(key) == 0; }

> ./build/bin/clang-tidy -checks="-*,readability-container-contains" tmp.cpp -- -std=c++20
8 warnings generated.
tmp.cpp:2:51: warning: use 'contains' to check for membership [readability-container-contains]
    2 | bool a(std::map<int, int> &m, int key) { return m.find(key) != m.end(); }
      |                                                   ^~~~      ~~~~~~~~~~
      |                                                   contains
tmp.cpp:3:51: warning: use 'contains' to check for membership [readability-container-contains]
    3 | bool b(std::map<int, int> &m, int key) { return m.count(key) > 0; }
      |                                                   ^~~~~      ~~~
      |                                                   contains
tmp.cpp:4:51: warning: use 'contains' to check for membership [readability-container-contains]
    4 | bool c(std::map<int, int> &m, int key) { return m.find(key) == m.end(); }
      |                                                   ^~~~      ~~~~~~~~~~
      |                                                 ! contains
tmp.cpp:5:51: warning: use 'contains' to check for membership [readability-container-contains]
    5 | bool d(std::map<int, int> &m, int key) { return m.count(key) == 0; }
      |                                                   ^~~~~      ~~~~
      |                                                 ! contains
tmp.cpp:6:52: warning: use 'contains' to check for membership [readability-container-contains]
    6 | bool e(std::map<int, int> *m, int key) { return m->find(key) != m->end(); }
      |                                                    ^~~~      ~~~~~~~~~~~
      |                                                    contains
tmp.cpp:7:52: warning: use 'contains' to check for membership [readability-container-contains]
    7 | bool f(std::map<int, int> *m, int key) { return m->find(key) == m->end(); }
      |                                                    ^~~~      ~~~~~~~~~~~
      |                                                 !  contains
tmp.cpp:8:52: warning: use 'contains' to check for membership [readability-container-contains]
    8 | bool g(std::map<int, int> *m, int key) { return m->count(key) > 0; }
      |                                                    ^~~~~      ~~~
      |                                                    contains
tmp.cpp:9:52: warning: use 'contains' to check for membership [readability-container-contains]
    9 | bool h(std::map<int, int> *m, int key) { return m->count(key) == 0; }
      |                                                    ^~~~~      ~~~~
      |                                                 !  contains
```


---
Full diff: https://github.com/llvm/llvm-project/pull/110386.diff


2 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp (+20-20) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp (+8-3) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
index 05d4c87bc73cef..fb68c7d334b7f8 100644
--- a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
@@ -62,44 +62,44 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
                          .bind("positiveComparison"),
                      this);
   AddSimpleMatcher(
-      binaryOperator(hasOperatorName("!="), hasOperands(CountCall, Literal0))
+      binaryOperation(hasOperatorName("!="), hasOperands(CountCall, Literal0))
           .bind("positiveComparison"));
   AddSimpleMatcher(
-      binaryOperator(hasLHS(CountCall), hasOperatorName(">"), hasRHS(Literal0))
+      binaryOperation(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))
+      binaryOperation(hasLHS(Literal0), hasOperatorName("<"), hasRHS(CountCall))
           .bind("positiveComparison"));
+  AddSimpleMatcher(binaryOperation(hasLHS(CountCall), hasOperatorName(">="),
+                                   hasRHS(Literal1))
+                       .bind("positiveComparison"));
+  AddSimpleMatcher(binaryOperation(hasLHS(Literal1), hasOperatorName("<="),
+                                   hasRHS(CountCall))
+                       .bind("positiveComparison"));
 
   // Find inverted membership tests which use `count()`.
   AddSimpleMatcher(
-      binaryOperator(hasOperatorName("=="), hasOperands(CountCall, Literal0))
-          .bind("negativeComparison"));
-  AddSimpleMatcher(
-      binaryOperator(hasLHS(CountCall), hasOperatorName("<="), hasRHS(Literal0))
-          .bind("negativeComparison"));
-  AddSimpleMatcher(
-      binaryOperator(hasLHS(Literal0), hasOperatorName(">="), hasRHS(CountCall))
+      binaryOperation(hasOperatorName("=="), hasOperands(CountCall, Literal0))
           .bind("negativeComparison"));
+  AddSimpleMatcher(binaryOperation(hasLHS(CountCall), hasOperatorName("<="),
+                                   hasRHS(Literal0))
+                       .bind("negativeComparison"));
+  AddSimpleMatcher(binaryOperation(hasLHS(Literal0), hasOperatorName(">="),
+                                   hasRHS(CountCall))
+                       .bind("negativeComparison"));
   AddSimpleMatcher(
-      binaryOperator(hasLHS(CountCall), hasOperatorName("<"), hasRHS(Literal1))
+      binaryOperation(hasLHS(CountCall), hasOperatorName("<"), hasRHS(Literal1))
           .bind("negativeComparison"));
   AddSimpleMatcher(
-      binaryOperator(hasLHS(Literal1), hasOperatorName(">"), hasRHS(CountCall))
+      binaryOperation(hasLHS(Literal1), hasOperatorName(">"), hasRHS(CountCall))
           .bind("negativeComparison"));
 
   // Find membership tests based on `find() == end()`.
   AddSimpleMatcher(
-      binaryOperator(hasOperatorName("!="), hasOperands(FindCall, EndCall))
+      binaryOperation(hasOperatorName("!="), hasOperands(FindCall, EndCall))
           .bind("positiveComparison"));
   AddSimpleMatcher(
-      binaryOperator(hasOperatorName("=="), hasOperands(FindCall, EndCall))
+      binaryOperation(hasOperatorName("=="), hasOperands(FindCall, EndCall))
           .bind("negativeComparison"));
 }
 
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
index 9a9b233e07229b..f345b3e7768a8a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp
@@ -1,14 +1,19 @@
-// RUN: %check_clang_tidy -std=c++20-or-later %s readability-container-contains %t
+// RUN: %check_clang_tidy -std=c++11-or-later %s readability-container-contains %t
 
 // Some *very* simplified versions of `map` etc.
 namespace std {
 
 template <class Key, class T>
 struct map {
+  struct iterator {
+    bool operator==(const iterator &Other) const;
+    bool operator!=(const iterator &Other) const;
+  };
+
   unsigned count(const Key &K) const;
   bool contains(const Key &K) const;
-  void *find(const Key &K);
-  void *end();
+  iterator find(const Key &K);
+  iterator end();
 };
 
 template <class Key>

``````````

</details>


https://github.com/llvm/llvm-project/pull/110386


More information about the cfe-commits mailing list