[clang-tools-extra] [clang-tidy][readability-container-contains] Use hasOperands when possible (PR #109178)

[Nicolas van Kempen via cfe-commits]

https://github.com/nicovank created https://github.com/llvm/llvm-project/pull/109178


Simplifies two cases and matches two new cases previously missing, `container.end() [!=]= container.find(...)`.

I had split this change from #107521 for easier review.


>From a8b8ce089e9eeadf1b8b424e426f7372d0a97662 Mon Sep 17 00:00:00 2001
From: Nicolas van Kempen <nvankemp at gmail.com>
Date: Wed, 18 Sep 2024 14:59:51 -0400
Subject: [PATCH] [clang-tidy][readability-container-contains] Use hasOperands
 when possible

Simplifies two cases and matches two new cases previously missing, `container.end() [!=]= container.find(...)`.

I had split this change from #107521 for easier review.
---
 .../readability/ContainerContainsCheck.cpp    | 14 +++-------
 clang-tools-extra/docs/ReleaseNotes.rst       |  3 ++-
 .../readability/container-contains.cpp        | 27 +++++++++++++++++++
 3 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
index 698231d777d2d4..05d4c87bc73cef 100644
--- a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
@@ -62,10 +62,7 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
                          .bind("positiveComparison"),
                      this);
   AddSimpleMatcher(
-      binaryOperator(hasLHS(CountCall), hasOperatorName("!="), hasRHS(Literal0))
-          .bind("positiveComparison"));
-  AddSimpleMatcher(
-      binaryOperator(hasLHS(Literal0), hasOperatorName("!="), hasRHS(CountCall))
+      binaryOperator(hasOperatorName("!="), hasOperands(CountCall, Literal0))
           .bind("positiveComparison"));
   AddSimpleMatcher(
       binaryOperator(hasLHS(CountCall), hasOperatorName(">"), hasRHS(Literal0))
@@ -82,10 +79,7 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
 
   // Find inverted membership tests which use `count()`.
   AddSimpleMatcher(
-      binaryOperator(hasLHS(CountCall), hasOperatorName("=="), hasRHS(Literal0))
-          .bind("negativeComparison"));
-  AddSimpleMatcher(
-      binaryOperator(hasLHS(Literal0), hasOperatorName("=="), hasRHS(CountCall))
+      binaryOperator(hasOperatorName("=="), hasOperands(CountCall, Literal0))
           .bind("negativeComparison"));
   AddSimpleMatcher(
       binaryOperator(hasLHS(CountCall), hasOperatorName("<="), hasRHS(Literal0))
@@ -102,10 +96,10 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
 
   // Find membership tests based on `find() == end()`.
   AddSimpleMatcher(
-      binaryOperator(hasLHS(FindCall), hasOperatorName("!="), hasRHS(EndCall))
+      binaryOperator(hasOperatorName("!="), hasOperands(FindCall, EndCall))
           .bind("positiveComparison"));
   AddSimpleMatcher(
-      binaryOperator(hasLHS(FindCall), hasOperatorName("=="), hasRHS(EndCall))
+      binaryOperator(hasOperatorName("=="), hasOperands(FindCall, EndCall))
           .bind("negativeComparison"));
 }
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 82a761bd7f40ab..c125223adcdf64 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -167,7 +167,8 @@ Changes in existing checks
 
 - Improved :doc:`readability-container-contains
   <clang-tidy/checks/readability/container-contains>` check to let it work on
-  any class that has a ``contains`` method.
+  any class that has a ``contains`` method. Also now match previously missing
+  cases with uncommon operand ordering.
 
 - Improved :doc:`readability-implicit-bool-conversion
   <clang-tidy/checks/readability/implicit-bool-conversion>` check
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 906515b075d4de..9a9b233e07229b 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
@@ -426,3 +426,30 @@ void testBox() {
   // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
   // CHECK-FIXES: if (Set.contains(0)) {};
 }
+
+void testOperandPermutations(std::map<int, int>& Map) {
+  if (Map.count(0) != 0) {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: if (Map.contains(0)) {};
+  if (0 != Map.count(0)) {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: if (Map.contains(0)) {};
+  if (Map.count(0) == 0) {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: if (!Map.contains(0)) {};
+  if (0 == Map.count(0)) {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: if (!Map.contains(0)) {};
+  if (Map.find(0) != Map.end()) {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: if (Map.contains(0)) {};
+  if (Map.end() != Map.find(0)) {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: if (Map.contains(0)) {};
+  if (Map.find(0) == Map.end()) {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: if (!Map.contains(0)) {};
+  if (Map.end() == Map.find(0)) {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: if (!Map.contains(0)) {};
+}