[clang-tools-extra] [clang-tidy] support string::contains (PR #110351)

[Tommy Chen via cfe-commits]

https://github.com/dl8sd11 created https://github.com/llvm/llvm-project/pull/110351

Starting from c++23, we can replace `std::string::find() == std::string::npos` with `std::string.contains()` . #109327

Currently, this is WIP because there are two limitations:

1. False positive: SubStr type is `const std::string&` which does not match `std::string::contains(basic_string_view)` type.

```
std::string SubStr;
  if (Str.find(SubStr) != std::string::npos) {};
```

2. Currently, the fixit for `std::string::find("test", 0)` is incorrect. 

>From b98e9a4d50d74c298096d2bd2d70ff4c0ef5c4a4 Mon Sep 17 00:00:00 2001
From: dl8sd11 <gcchen at google.com>
Date: Sat, 28 Sep 2024 07:37:50 +0000
Subject: [PATCH] [clang-tidy] support string::contains

---
 .../readability/ContainerContainsCheck.cpp    | 18 +++++--
 .../readability/container-contains.cpp        | 49 +++++++++++++++++++
 2 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
index 05d4c87bc73cef..a5e5960eaa6a3c 100644
--- a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
@@ -9,6 +9,7 @@
 #include "ContainerContainsCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 
 using namespace clang::ast_matchers;
 
@@ -32,7 +33,8 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
 
   const auto FindCall =
       cxxMemberCallExpr(
-          argumentCountIs(1),
+          anyOf(argumentCountIs(1),
+                allOf(argumentCountIs(2), hasArgument(1, cxxDefaultArgExpr()))),
           callee(cxxMethodDecl(
               hasName("find"),
               hasParameter(0, hasType(hasUnqualifiedDesugaredType(
@@ -51,6 +53,12 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
   const auto Literal0 = integerLiteral(equals(0));
   const auto Literal1 = integerLiteral(equals(1));
 
+  const auto StringLikeClass = cxxRecordDecl(
+      hasAnyName("::std::basic_string", "::std::basic_string_view",
+                 "::absl::string_view"));
+  const auto StringNpos = declRefExpr(
+      to(varDecl(hasName("npos"), hasDeclContext(StringLikeClass))));
+
   auto AddSimpleMatcher = [&](auto Matcher) {
     Finder->addMatcher(
         traverse(TK_IgnoreUnlessSpelledInSource, std::move(Matcher)), this);
@@ -94,12 +102,14 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
       binaryOperator(hasLHS(Literal1), hasOperatorName(">"), hasRHS(CountCall))
           .bind("negativeComparison"));
 
-  // Find membership tests based on `find() == end()`.
+  // Find membership tests based on `find() == end() or find() == npos`.
   AddSimpleMatcher(
-      binaryOperator(hasOperatorName("!="), hasOperands(FindCall, EndCall))
+      binaryOperator(hasOperatorName("!="),
+                     hasOperands(FindCall, anyOf(EndCall, StringNpos)))
           .bind("positiveComparison"));
   AddSimpleMatcher(
-      binaryOperator(hasOperatorName("=="), hasOperands(FindCall, EndCall))
+      binaryOperator(hasOperatorName("=="),
+                     hasOperands(FindCall, anyOf(EndCall, StringNpos)))
           .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..69cc5c88479040 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
@@ -29,6 +29,43 @@ struct multimap {
   bool contains(const Key &K) const;
 };
 
+using size_t = decltype(sizeof(int));
+
+// Lightweight standin for std::string_view.
+template <typename C>
+class basic_string_view {
+public:
+  basic_string_view();
+  basic_string_view(const basic_string_view &);
+  basic_string_view(const C *);
+  ~basic_string_view();
+  int find(basic_string_view s, int pos = 0);
+  int find(const C *s, int pos = 0);
+  int find(const C *s, int pos, int n);
+  int find(char c, int pos = 0);
+  static constexpr size_t npos = -1;
+};
+typedef basic_string_view<char> string_view;
+
+// Lightweight standin for std::string.
+template <typename C>
+class basic_string {
+public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const C *);
+  ~basic_string();
+  int find(basic_string s, int pos = 0);
+  int find(const C *s, int pos = 0);
+  int find(const C *s, int pos, int n);
+  int find(char c, int pos = 0);
+  bool contains(const C *s) const;
+  bool contains(C s) const;
+  bool contains(basic_string_view<C> s) const;
+  static constexpr size_t npos = -1;
+};
+typedef basic_string<char> string;
+
 } // namespace std
 
 // Check that we detect various common ways to check for membership
@@ -453,3 +490,15 @@ void testOperandPermutations(std::map<int, int>& Map) {
   // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
   // CHECK-FIXES: if (!Map.contains(0)) {};
 }
+
+void testStringNops(std::string Str, std::string SubStr) {
+  if (Str.find("test") == std::string::npos) {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: if (!Str.contains("test")) {};
+
+  if (Str.find('c') != std::string::npos) {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: if (Str.contains('c')) {};
+  
+  if (Str.find(SubStr) != std::string::npos) {};
+}