https://github.com/5chmidti updated
https://github.com/llvm/llvm-project/pull/112101
>From cd6de859501e8c0102aa1c1e57341ecc35266ed1 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schm...@gmail.com>
Date: Sat, 12 Oct 2024 18:07:38 +0200
Subject: [PATCH 1/3] [clang-tidy] rewrite matchers in
modernize-use-starts-ends-with
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.
---
.../modernize/UseStartsEndsWithCheck.cpp | 59 ++++++++-----------
.../modernize/use-starts-ends-with.cpp | 4 +-
2 files changed, 27 insertions(+), 36 deletions(-)
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.
>From 7261cb4c1b286f6c421226b0581da3195a168bc1 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schm...@gmail.com>
Date: Sun, 13 Oct 2024 01:01:58 +0200
Subject: [PATCH 2/3] address comments
---
.../modernize/UseStartsEndsWithCheck.cpp | 64 ++++++++++---------
.../modernize/use-starts-ends-with.cpp | 17 +----
2 files changed, 37 insertions(+), 44 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
index a1376aa8fa5e6e..4cc588bd7bc825 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
@@ -84,51 +84,53 @@ UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef
Name,
void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
const auto ZeroLiteral = integerLiteral(equals(0));
- 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(ClassTypeWithMethod(
- "ends_with_fun", {"ends_with", "endsWith", "endswith"})))
- .bind("haystack"));
+ const auto ClassTypeWithMethod = [](const StringRef MethodBoundName,
+ const auto... Methods) {
+ return cxxRecordDecl(anyOf(
+ hasMethod(cxxMethodDecl(isConst(), parameterCountIs(1),
+ returns(booleanType()), hasAnyName(Methods))
+ .bind(MethodBoundName))...));
+ };
+
+ const auto OnClassWithStartsWithFunction =
+ ClassTypeWithMethod("starts_with_fun", "starts_with", "startsWith",
+ "startswith", "StartsWith");
+
+ const auto OnClassWithEndsWithFunction = ClassTypeWithMethod(
+ "ends_with_fun", "ends_with", "endsWith", "endswith", "EndsWith");
// Case 1: X.find(Y) [!=]= 0 -> starts_with.
const auto FindExpr = cxxMemberCallExpr(
anyOf(argumentCountIs(1), hasArgument(1, ZeroLiteral)),
- callee(cxxMethodDecl(hasName("find")).bind("find_fun")),
- OnClassWithStartsWithFunction, hasArgument(0, expr().bind("needle")));
+ callee(
+ cxxMethodDecl(hasName("find"),
ofClass(OnClassWithStartsWithFunction))
+ .bind("find_fun")),
+ hasArgument(0, expr().bind("needle")));
// Case 2: X.rfind(Y, 0) [!=]= 0 -> starts_with.
const auto RFindExpr = cxxMemberCallExpr(
hasArgument(1, ZeroLiteral),
- callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")),
- OnClassWithStartsWithFunction, hasArgument(0, expr().bind("needle")));
+ callee(cxxMethodDecl(hasName("rfind"),
+ ofClass(OnClassWithStartsWithFunction))
+ .bind("find_fun")),
+ hasArgument(0, expr().bind("needle")));
// Case 3: X.compare(0, LEN(Y), Y) [!=]= 0 -> starts_with.
const auto CompareExpr = cxxMemberCallExpr(
argumentCountIs(3), hasArgument(0, ZeroLiteral),
- callee(cxxMethodDecl(hasName("compare")).bind("find_fun")),
- OnClassWithStartsWithFunction, hasArgument(2, expr().bind("needle")),
+ callee(cxxMethodDecl(hasName("compare"),
+ ofClass(OnClassWithStartsWithFunction))
+ .bind("find_fun")),
+ hasArgument(2, expr().bind("needle")),
hasArgument(1, lengthExprForStringNode("needle")));
// Case 4: X.compare(LEN(X) - LEN(Y), LEN(Y), Y) [!=]= 0 -> ends_with.
const auto CompareEndsWithExpr = cxxMemberCallExpr(
argumentCountIs(3),
- callee(cxxMethodDecl(hasName("compare")).bind("find_fun")),
- OnClassWithEndsWithFunction, hasArgument(2, expr().bind("needle")),
+ callee(cxxMethodDecl(hasName("compare"),
+ ofClass(OnClassWithEndsWithFunction))
+ .bind("find_fun")),
+ on(expr().bind("haystack")), hasArgument(2, expr().bind("needle")),
hasArgument(1, lengthExprForStringNode("needle")),
hasArgument(0,
binaryOperator(hasOperatorName("-"),
@@ -159,8 +161,10 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder
*Finder) {
1,
anyOf(declRefExpr(to(varDecl(hasName("npos")))),
memberExpr(member(hasName("npos"))))))),
- callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")),
- OnClassWithEndsWithFunction,
+ callee(cxxMethodDecl(hasName("rfind"),
+ ofClass(OnClassWithEndsWithFunction))
+ .bind("find_fun")),
+ on(expr().bind("haystack")),
hasArgument(0, expr().bind("needle")))
.bind("find_expr"),
binaryOperator(hasOperatorName("-"),
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 9365aeac068ee2..91477241e82e54 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
@@ -32,14 +32,9 @@ struct prefer_underscore_version_flip {
size_t find(const char *s, size_t pos = 0) const;
};
-struct prefer_underscore_version_inherit : public string_like {
- bool startsWith(const char *s) const;
-};
-
void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string
sss,
string_like sl, string_like_camel slc, prefer_underscore_version puv,
- prefer_underscore_version_flip puvf,
- prefer_underscore_version_inherit puvi) {
+ prefer_underscore_version_flip puvf) {
s.find("a") == 0;
// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead
of find() == 0
// CHECK-FIXES: s.starts_with("a");
@@ -150,14 +145,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 startsWith
- // CHECK-FIXES: puvf.startsWith("a");
-
- // Here, the subclass has startsWith, the superclass has starts_with.
- // We prefer the version from the subclass.
- puvi.find("a") == 0;
- // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use startsWith
- // CHECK-FIXES: puvi.startsWith("a");
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: puvf.starts_with("a");
s.compare(0, 1, "a") == 0;
// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead
of compare() == 0
>From f00e083b6caf7bc55ab39aa388fdcdd5f80c55de Mon Sep 17 00:00:00 2001
From: Julian Schmidt <schmidt.julia...@gmail.com>
Date: Sun, 13 Oct 2024 11:06:59 +0200
Subject: [PATCH 3/3] Update
clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
Co-authored-by: Nicolas van Kempen <nvank...@gmail.com>
---
.../clang-tidy/modernize/UseStartsEndsWithCheck.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
index 4cc588bd7bc825..1231f954298adc 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
@@ -12,7 +12,6 @@
#include "../utils/Matchers.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Lex/Lexer.h"
-#include "llvm/ADT/ArrayRef.h"
#include <string>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
