Author: Julian Schmidt Date: 2024-10-18T09:07:21+02:00 New Revision: 18b50189a749a39d1ac61a72af1d103f68fefc6b
URL: https://github.com/llvm/llvm-project/commit/18b50189a749a39d1ac61a72af1d103f68fefc6b DIFF: https://github.com/llvm/llvm-project/commit/18b50189a749a39d1ac61a72af1d103f68fefc6b.diff LOG: [clang-tidy] rewrite matchers in modernize-use-starts-ends-with (#112101) Rewrite the AST matchers for slightly more composability. 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. --------- Co-authored-by: Nicolas van Kempen <nvank...@gmail.com> Added: Modified: clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index 5eb3267adb0799..1231f954298adc 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -9,7 +9,8 @@ #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 <string> @@ -82,60 +83,53 @@ 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 ClassTypeWithMethod = [](const StringRef MethodBoundName, + const auto... Methods) { + return cxxRecordDecl(anyOf( + hasMethod(cxxMethodDecl(isConst(), parameterCountIs(1), + returns(booleanType()), hasAnyName(Methods)) + .bind(MethodBoundName))...)); }; - 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 OnClassWithEndsWithFunction = - on(expr(hasType(hasCanonicalType(hasDeclaration(cxxRecordDecl( - anyOf(HasEndsWithMethod, - hasAnyBase(hasType(hasCanonicalType(hasDeclaration( - cxxRecordDecl(HasEndsWithMethod))))))))))) - .bind("haystack")); + 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("-"), @@ -145,7 +139,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 +150,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( @@ -166,8 +160,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("-"), @@ -190,9 +186,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 +215,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..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"); @@ -153,12 +148,6 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss, // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with // CHECK-FIXES: puvf.starts_with("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"); - s.compare(0, 1, "a") == 0; // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of compare() == 0 // CHECK-FIXES: s.starts_with("a"); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
