[clang-tools-extra] [clang-tidy] Enhance modernize-use-starts-ends-with to handle substr patterns (PR #116033)

Nicolas van Kempen via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 18 10:56:19 PST 2024


================
@@ -183,40 +209,44 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *EndsWithFunction =
       Result.Nodes.getNodeAs<CXXMethodDecl>("ends_with_fun");
   assert(bool(StartsWithFunction) != bool(EndsWithFunction));
+
   const CXXMethodDecl *ReplacementFunction =
       StartsWithFunction ? StartsWithFunction : EndsWithFunction;
 
   if (ComparisonExpr->getBeginLoc().isMacroID())
----------------
nicovank wrote:

I just meant that the set of tests in the existing function has stuff that shouldn't match at the bottom/end of the function.

Are you sure? I pulled and tested on my end, only three seem to break, I think it is fine not to cover them in this check.

<details>

<summary>Diff, this passes <code>ninja check-clang-extra-clang-tidy-checkers-modernize</code> for me.</summary>

```diff
--- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
@@ -214,7 +214,8 @@
   const CXXMethodDecl *ReplacementFunction =
       StartsWithFunction ? StartsWithFunction : EndsWithFunction;
 
-  if (ComparisonExpr->getBeginLoc().isMacroID())
+  if (ComparisonExpr->getBeginLoc().isMacroID() ||
+      FindExpr->getBeginLoc().isMacroID())
     return;
 
   const bool Neg = isNegativeComparison(ComparisonExpr);
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
--- 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
@@ -91,16 +91,6 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
   // CHECK-FIXES: !s.starts_with("a");
 
-  #define STR(x) std::string(x)
-  0 == STR(s).find("a");
-  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
-  // CHECK-FIXES: STR(s).starts_with("a");
-
-  #define STRING s
-  if (0 == STRING.find("ala")) { /* do something */}
-  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
-  // CHECK-FIXES: if (STRING.starts_with("ala"))
-
   #define FIND find
   s.FIND("a") == 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
@@ -265,6 +255,12 @@
 
   s.compare(0, 1, "ab") == 0;
   s.rfind(suffix, 1) == s.size() - suffix.size();
+
+  #define STR(x) std::string(x)
+  0 == STR(s).find("a");
+
+  #define STRING s
+  if (0 == STRING.find("ala")) { /* do something */}
 }
 
 void test_substr() {
@@ -311,8 +307,5 @@
 
     #define STR() str
     SUBSTR(STR(), 0, 6) == "prefix";
-
     "prefix" == SUBSTR(STR(), 0, 6);
-    // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of substr() == [modernize-use-starts-ends-with]
-
 }
```

</details>

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


More information about the cfe-commits mailing list