[clang-tools-extra] [clang-tidy] rewrite matchers in modernize-use-starts-ends-with (PR #112101)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: Julian Schmidt (5chmidti)

<details>
<summary>Changes</summary>

Rewrite the AST matchers for slightly more composability and
performance. Furthermore, check that the `starts_with` and `ends_with`
functions return a `bool`.
There is one behavioral change, in that the methods of a class (and
transitive classes) are searched once for a matching
`starts_with`/`ends_with` function, picking the first it can find.
Previously, the matchers would try to find `starts_with`, then
`startsWith`, and finally, `startswith`. Now, the first of the three that
is encountered will be the matched method.


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


2 Files Affected:

- (modified) clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp (+25-34) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp (+2-2) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
index 5eb3267adb0799..a1376aa8fa5e6e 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
@@ -9,8 +9,10 @@
 #include "UseStartsEndsWithCheck.h"
 
 #include "../utils/ASTUtils.h"
-#include "../utils/OptionsUtils.h"
+#include "../utils/Matchers.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/ArrayRef.h"
 
 #include <string>
 
@@ -82,34 +84,25 @@ UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name,
 void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
   const auto ZeroLiteral = integerLiteral(equals(0));
 
-  const auto HasStartsWithMethodWithName = [](const std::string &Name) {
-    return hasMethod(
-        cxxMethodDecl(hasName(Name), isConst(), parameterCountIs(1))
-            .bind("starts_with_fun"));
-  };
-  const auto HasStartsWithMethod =
-      anyOf(HasStartsWithMethodWithName("starts_with"),
-            HasStartsWithMethodWithName("startsWith"),
-            HasStartsWithMethodWithName("startswith"));
-  const auto OnClassWithStartsWithFunction =
-      on(hasType(hasCanonicalType(hasDeclaration(cxxRecordDecl(
-          anyOf(HasStartsWithMethod,
-                hasAnyBase(hasType(hasCanonicalType(
-                    hasDeclaration(cxxRecordDecl(HasStartsWithMethod)))))))))));
-
-  const auto HasEndsWithMethodWithName = [](const std::string &Name) {
-    return hasMethod(
-        cxxMethodDecl(hasName(Name), isConst(), parameterCountIs(1))
-            .bind("ends_with_fun"));
-  };
-  const auto HasEndsWithMethod = anyOf(HasEndsWithMethodWithName("ends_with"),
-                                       HasEndsWithMethodWithName("endsWith"),
-                                       HasEndsWithMethodWithName("endswith"));
+  const auto ClassTypeWithMethod =
+      [](const StringRef MethodBoundName,
+         const llvm::ArrayRef<StringRef> &Methods) {
+        const auto Method =
+            cxxMethodDecl(isConst(), parameterCountIs(1),
+                          returns(booleanType()), hasAnyName(Methods))
+                .bind(MethodBoundName);
+        return qualType(hasCanonicalType(hasDeclaration(cxxRecordDecl(
+            anyOf(hasMethod(Method),
+                  hasAnyBase(hasType(hasCanonicalType(
+                      hasDeclaration(cxxRecordDecl(hasMethod(Method)))))))))));
+      };
+
+  const auto OnClassWithStartsWithFunction = on(hasType(ClassTypeWithMethod(
+      "starts_with_fun", {"starts_with", "startsWith", "startswith"})));
+
   const auto OnClassWithEndsWithFunction =
-      on(expr(hasType(hasCanonicalType(hasDeclaration(cxxRecordDecl(
-                  anyOf(HasEndsWithMethod,
-                        hasAnyBase(hasType(hasCanonicalType(hasDeclaration(
-                            cxxRecordDecl(HasEndsWithMethod)))))))))))
+      on(expr(hasType(ClassTypeWithMethod(
+                  "ends_with_fun", {"ends_with", "endsWith", "endswith"})))
              .bind("haystack"));
 
   // Case 1: X.find(Y) [!=]= 0 -> starts_with.
@@ -145,7 +138,7 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
   // All cases comparing to 0.
   Finder->addMatcher(
       binaryOperator(
-          hasAnyOperatorName("==", "!="),
+          matchers::isEqualityOperator(),
           hasOperands(cxxMemberCallExpr(anyOf(FindExpr, RFindExpr, CompareExpr,
                                               CompareEndsWithExpr))
                           .bind("find_expr"),
@@ -156,7 +149,7 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
   // Case 5: X.rfind(Y) [!=]= LEN(X) - LEN(Y) -> ends_with.
   Finder->addMatcher(
       binaryOperator(
-          hasAnyOperatorName("==", "!="),
+          matchers::isEqualityOperator(),
           hasOperands(
               cxxMemberCallExpr(
                   anyOf(
@@ -190,9 +183,8 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
   const CXXMethodDecl *ReplacementFunction =
       StartsWithFunction ? StartsWithFunction : EndsWithFunction;
 
-  if (ComparisonExpr->getBeginLoc().isMacroID()) {
+  if (ComparisonExpr->getBeginLoc().isMacroID())
     return;
-  }
 
   const bool Neg = ComparisonExpr->getOpcode() == BO_NE;
 
@@ -220,9 +212,8 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
       (ReplacementFunction->getName() + "(").str());
 
   // Add possible negation '!'.
-  if (Neg) {
+  if (Neg)
     Diagnostic << FixItHint::CreateInsertion(FindExpr->getBeginLoc(), "!");
-  }
 }
 
 } // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp
index 798af260a3b66c..9365aeac068ee2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp
@@ -150,8 +150,8 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss,
   // CHECK-FIXES: puv.starts_with("a");
 
   puvf.find("a") == 0;
-  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
-  // CHECK-FIXES: puvf.starts_with("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use startsWith
+  // CHECK-FIXES: puvf.startsWith("a");
 
   // Here, the subclass has startsWith, the superclass has starts_with.
   // We prefer the version from the subclass.

``````````

</details>


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