https://github.com/hjanuschka updated https://github.com/llvm/llvm-project/pull/116033
>From adafcceb45853c3e8f797eba51d90e64654dafe6 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Wed, 13 Nov 2024 12:52:36 +0100 Subject: [PATCH 1/5] [clang-tidy] Add modernize-substr-to-starts-with check Adds a new check that finds calls to substr when its first argument is a zero-equivalent expression and can be replaced with starts_with() (introduced in C++20). This modernization improves code readability by making the intent clearer and can be more efficient as it avoids creating temporary strings. Converts patterns like: str.substr(0, 3) == "foo" -> str.starts_with("foo") str.substr(x-x, 3) == "foo" -> str.starts_with("foo") str.substr(zero, n) == prefix -> str.starts_with(prefix) "bar" == str.substr(i-i, 3) -> str.starts_with("bar") str.substr(0, n) != prefix -> !str.starts_with(prefix) The check: - Detects zero-equivalent expressions: * Direct zero literals (0) * Variables initialized to zero * Self-canceling expressions (x-x, i-i) - Only converts when length matches exactly for string literals - Supports both string literals and string variables - Handles both == and != operators --- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../modernize/ModernizeTidyModule.cpp | 3 + .../modernize/SubstrToStartsWithCheck.cpp | 104 ++++++++++++++++++ .../modernize/SubstrToStartsWithCheck.h | 30 +++++ .../modernize/substr-to-starts-with.rst | 35 ++++++ .../modernize/substr-to-starts-with.cpp | 42 +++++++ 6 files changed, 215 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/substr-to-starts-with.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/substr-to-starts-with.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index c919d49b42873a..ed1ba2ab62a90f 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -49,6 +49,7 @@ add_clang_library(clangTidyModernizeModule STATIC UseTransparentFunctorsCheck.cpp UseUncaughtExceptionsCheck.cpp UseUsingCheck.cpp + SubstrToStartsWithCheck.cpp LINK_LIBS clangTidy diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 18607593320635..7569211d2552ea 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -50,6 +50,7 @@ #include "UseTransparentFunctorsCheck.h" #include "UseUncaughtExceptionsCheck.h" #include "UseUsingCheck.h" +#include "SubstrToStartsWithCheck.h" using namespace clang::ast_matchers; @@ -122,6 +123,8 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<UseUncaughtExceptionsCheck>( "modernize-use-uncaught-exceptions"); CheckFactories.registerCheck<UseUsingCheck>("modernize-use-using"); + CheckFactories.registerCheck<SubstrToStartsWithCheck>( + "modernize-substr-to-starts-with"); } }; diff --git a/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp new file mode 100644 index 00000000000000..253e5ba3ca32ba --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp @@ -0,0 +1,104 @@ +//===--- SubstrToStartsWithCheck.cpp - clang-tidy ------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "SubstrToStartsWithCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +void SubstrToStartsWithCheck::registerMatchers(MatchFinder *Finder) { + auto isZeroExpr = expr(anyOf( + integerLiteral(equals(0)), + ignoringParenImpCasts(declRefExpr( + to(varDecl(hasInitializer(integerLiteral(equals(0))))))), + binaryOperator(hasOperatorName("-"), hasLHS(expr()), hasRHS(expr())))); + + auto isStringLike = expr(anyOf( + stringLiteral().bind("literal"), + implicitCastExpr(hasSourceExpression(stringLiteral().bind("literal"))), + declRefExpr(to(varDecl(hasType(qualType(hasDeclaration( + namedDecl(hasAnyName("::std::string", "::std::basic_string")))))))).bind("strvar"))); + + auto isSubstrCall = + cxxMemberCallExpr( + callee(memberExpr(hasDeclaration(cxxMethodDecl( + hasName("substr"), + ofClass(hasAnyName("basic_string", "string", "u16string")))))), + hasArgument(0, isZeroExpr), + hasArgument(1, expr().bind("length"))) + .bind("substr"); + + Finder->addMatcher( + binaryOperator( + anyOf(hasOperatorName("=="), hasOperatorName("!=")), + hasEitherOperand(isSubstrCall), + hasEitherOperand(isStringLike), + unless(hasType(isAnyCharacter()))) + .bind("comparison"), + this); + + Finder->addMatcher( + cxxMemberCallExpr( + callee(memberExpr(hasDeclaration(cxxMethodDecl( + hasName("substr"), + ofClass(hasAnyName("basic_string", "string", "u16string")))))), + hasArgument(0, isZeroExpr), + hasArgument(1, expr().bind("direct_length"))) + .bind("direct_substr"), + this); +} + +void SubstrToStartsWithCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Comparison = Result.Nodes.getNodeAs<BinaryOperator>("comparison"); + + if (Comparison) { + const auto *SubstrCall = Result.Nodes.getNodeAs<CXXMemberCallExpr>("substr"); + const auto *LengthArg = Result.Nodes.getNodeAs<Expr>("length"); + const auto *Literal = Result.Nodes.getNodeAs<StringLiteral>("literal"); + const auto *StrVar = Result.Nodes.getNodeAs<DeclRefExpr>("strvar"); + + if (!SubstrCall || !LengthArg || (!Literal && !StrVar)) + return; + + std::string CompareStr; + if (Literal) { + CompareStr = Literal->getString().str(); + } else if (StrVar) { + CompareStr = Lexer::getSourceText( + CharSourceRange::getTokenRange(StrVar->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()) + .str(); + } + + if (Literal) { + if (const auto *LengthLiteral = dyn_cast<IntegerLiteral>(LengthArg)) { + if (LengthLiteral->getValue() != Literal->getLength()) + return; + } + } + + std::string Replacement; + if (Comparison->getOpcode() == BO_EQ) { + Replacement = "starts_with(" + CompareStr + ")"; + } else { // BO_NE + Replacement = "!starts_with(" + CompareStr + ")"; + } + + diag(Comparison->getBeginLoc(), + "use starts_with() instead of substring comparison") + << FixItHint::CreateReplacement(Comparison->getSourceRange(), + Replacement); + } +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.h b/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.h new file mode 100644 index 00000000000000..fdb157ca0a24fb --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.h @@ -0,0 +1,30 @@ +//===--- SubstrToStartsWithCheck.h - clang-tidy ------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_SUBSTRTOSTARTSWITHCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_SUBSTRTOSTARTSWITHCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::modernize { + +/// Finds calls to substr(0, n) that can be replaced with starts_with(). +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/substr-to-starts-with.html +class SubstrToStartsWithCheck : public ClangTidyCheck { +public: + SubstrToStartsWithCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_SUBSTRTOSTARTSWITHCHECK_H diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/substr-to-starts-with.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/substr-to-starts-with.rst new file mode 100644 index 00000000000000..948c01c29b8c0a --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/substr-to-starts-with.rst @@ -0,0 +1,35 @@ +modernize-substr-to-starts-with +============================== + +Finds calls to ``substr(0, n)`` that can be replaced with ``starts_with()`` (introduced in C++20). +This makes the code's intent clearer and can be more efficient as it avoids creating temporary strings. + +For example: + +.. code-block:: c++ + + str.substr(0, 3) == "foo" // before + str.starts_with("foo") // after + + "bar" == str.substr(0, 3) // before + str.starts_with("bar") // after + + str.substr(0, n) == prefix // before + str.starts_with(prefix) // after + +The check handles various ways of expressing zero as the start index: + +.. code-block:: c++ + + const int zero = 0; + str.substr(zero, n) == prefix // converted + str.substr(x - x, n) == prefix // converted + +The check will only convert cases where: +* The substr call starts at index 0 (or equivalent) +* When comparing with string literals, the length matches exactly +* The comparison is with == or != + +.. code-block:: c++ + + auto prefix = str.substr(0, n); // warns about possible use of starts_with diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/substr-to-starts-with.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/substr-to-starts-with.cpp new file mode 100644 index 00000000000000..03703562c99115 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/substr-to-starts-with.cpp @@ -0,0 +1,42 @@ +// RUN: %check_clang_tidy %s modernize-substr-to-starts-with %t -- -std=c++20 -stdlib=libc++ + +#include <string> +#include <string_view> + +void test() { + std::string str = "hello world"; + if (str.substr(0, 5) == "hello") {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use starts_with() instead of substring comparison + // CHECK-FIXES: if (str.starts_with("hello")) {} + + if ("hello" == str.substr(0, 5)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use starts_with() instead of substring comparison + // CHECK-FIXES: if (str.starts_with("hello")) {} + + bool b = str.substr(0, 5) != "hello"; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use starts_with() instead of substring comparison + // CHECK-FIXES: bool b = !str.starts_with("hello"); + + // Variable length and string refs + std::string prefix = "hello"; + size_t len = 5; + if (str.substr(0, len) == prefix) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use starts_with() instead of substring comparison + // CHECK-FIXES: if (str.starts_with(prefix)) {} + + // Various zero expressions + const int zero = 0; + int i = 0; + if (str.substr(zero, 5) == "hello") {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use starts_with() instead of substring comparison + // CHECK-FIXES: if (str.starts_with("hello")) {} + + if (str.substr(i-i, 5) == "hello") {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use starts_with() instead of substring comparison + // CHECK-FIXES: if (str.starts_with("hello")) {} + + // Should not convert these + if (str.substr(1, 5) == "hello") {} // Non-zero start + if (str.substr(0, 4) == "hello") {} // Length mismatch + if (str.substr(0, 6) == "hello") {} // Length mismatch +} >From 06cc6fee00415d4e9c6b23a2a4f2548e70fd2a5c Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Thu, 14 Nov 2024 00:10:12 +0100 Subject: [PATCH 2/5] up --- .../modernize/SubstrToStartsWithCheck.cpp | 163 ++++++++---------- .../modernize/SubstrToStartsWithCheck.h | 18 +- .../modernize/UseStartsEndsWithCheck.cpp | 50 ++++++ 3 files changed, 131 insertions(+), 100 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp index 253e5ba3ca32ba..313c53784c8ac2 100644 --- a/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp @@ -1,11 +1,3 @@ -//===--- SubstrToStartsWithCheck.cpp - clang-tidy ------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - #include "SubstrToStartsWithCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" @@ -17,88 +9,85 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { void SubstrToStartsWithCheck::registerMatchers(MatchFinder *Finder) { - auto isZeroExpr = expr(anyOf( - integerLiteral(equals(0)), - ignoringParenImpCasts(declRefExpr( - to(varDecl(hasInitializer(integerLiteral(equals(0))))))), - binaryOperator(hasOperatorName("-"), hasLHS(expr()), hasRHS(expr())))); - - auto isStringLike = expr(anyOf( - stringLiteral().bind("literal"), - implicitCastExpr(hasSourceExpression(stringLiteral().bind("literal"))), - declRefExpr(to(varDecl(hasType(qualType(hasDeclaration( - namedDecl(hasAnyName("::std::string", "::std::basic_string")))))))).bind("strvar"))); - - auto isSubstrCall = - cxxMemberCallExpr( - callee(memberExpr(hasDeclaration(cxxMethodDecl( - hasName("substr"), - ofClass(hasAnyName("basic_string", "string", "u16string")))))), - hasArgument(0, isZeroExpr), - hasArgument(1, expr().bind("length"))) - .bind("substr"); - - Finder->addMatcher( - binaryOperator( - anyOf(hasOperatorName("=="), hasOperatorName("!=")), - hasEitherOperand(isSubstrCall), - hasEitherOperand(isStringLike), - unless(hasType(isAnyCharacter()))) - .bind("comparison"), - this); + const auto SubstrCall = cxxMemberCallExpr( + callee(cxxMethodDecl(hasName("substr"))), + hasArgument(0, integerLiteral(equals(0))), + hasArgument(1, expr().bind("length")), + on(expr().bind("str"))) + .bind("call"); + + // Helper for matching comparison operators + auto AddSimpleMatcher = [&](auto Matcher) { + Finder->addMatcher( + traverse(TK_IgnoreUnlessSpelledInSource, std::move(Matcher)), this); + }; + + // Match str.substr(0,n) == "literal" + AddSimpleMatcher( + binaryOperation( + hasOperatorName("=="), + hasEitherOperand(SubstrCall), + hasEitherOperand(expr().bind("comparison"))) + .bind("positiveComparison")); + + // Match str.substr(0,n) != "literal" + AddSimpleMatcher( + binaryOperation( + hasOperatorName("!="), + hasEitherOperand(SubstrCall), + hasEitherOperand(expr().bind("comparison"))) + .bind("negativeComparison")); +} - Finder->addMatcher( - cxxMemberCallExpr( - callee(memberExpr(hasDeclaration(cxxMethodDecl( - hasName("substr"), - ofClass(hasAnyName("basic_string", "string", "u16string")))))), - hasArgument(0, isZeroExpr), - hasArgument(1, expr().bind("direct_length"))) - .bind("direct_substr"), - this); +std::string SubstrToStartsWithCheck::getExprStr(const Expr *E, + const SourceManager &SM, + const LangOptions &LO) { + CharSourceRange Range = CharSourceRange::getTokenRange(E->getSourceRange()); + return Lexer::getSourceText(Range, SM, LO).str(); } void SubstrToStartsWithCheck::check(const MatchFinder::MatchResult &Result) { - const auto *Comparison = Result.Nodes.getNodeAs<BinaryOperator>("comparison"); - - if (Comparison) { - const auto *SubstrCall = Result.Nodes.getNodeAs<CXXMemberCallExpr>("substr"); - const auto *LengthArg = Result.Nodes.getNodeAs<Expr>("length"); - const auto *Literal = Result.Nodes.getNodeAs<StringLiteral>("literal"); - const auto *StrVar = Result.Nodes.getNodeAs<DeclRefExpr>("strvar"); - - if (!SubstrCall || !LengthArg || (!Literal && !StrVar)) - return; - - std::string CompareStr; - if (Literal) { - CompareStr = Literal->getString().str(); - } else if (StrVar) { - CompareStr = Lexer::getSourceText( - CharSourceRange::getTokenRange(StrVar->getSourceRange()), - *Result.SourceManager, Result.Context->getLangOpts()) - .str(); - } - - if (Literal) { - if (const auto *LengthLiteral = dyn_cast<IntegerLiteral>(LengthArg)) { - if (LengthLiteral->getValue() != Literal->getLength()) - return; - } - } - - std::string Replacement; - if (Comparison->getOpcode() == BO_EQ) { - Replacement = "starts_with(" + CompareStr + ")"; - } else { // BO_NE - Replacement = "!starts_with(" + CompareStr + ")"; - } - - diag(Comparison->getBeginLoc(), - "use starts_with() instead of substring comparison") - << FixItHint::CreateReplacement(Comparison->getSourceRange(), - Replacement); - } + const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call"); + if (!Call) + return; + + const auto *PositiveComparison = Result.Nodes.getNodeAs<Expr>("positiveComparison"); + const auto *NegativeComparison = Result.Nodes.getNodeAs<Expr>("negativeComparison"); + + if (!PositiveComparison && !NegativeComparison) + return; + + bool Negated = NegativeComparison != nullptr; + const auto *Comparison = Negated ? NegativeComparison : PositiveComparison; + + // Skip if in macro + if (Call->getBeginLoc().isMacroID()) + return; + + const auto *Str = Result.Nodes.getNodeAs<Expr>("str"); + const auto *CompareExpr = Result.Nodes.getNodeAs<Expr>("comparison"); + + if (!Str || !CompareExpr) + return; + + // Emit the diagnostic + auto Diag = diag(Call->getExprLoc(), + "use starts_with() instead of substr(0, n) comparison"); + + // Build the replacement text + std::string ReplacementStr = + (Negated ? "!" : "") + + Lexer::getSourceText(CharSourceRange::getTokenRange(Str->getSourceRange()), + *Result.SourceManager, getLangOpts()).str() + + ".starts_with(" + + Lexer::getSourceText(CharSourceRange::getTokenRange(CompareExpr->getSourceRange()), + *Result.SourceManager, getLangOpts()).str() + + ")"; + + // Create the fix-it + Diag << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(Comparison->getSourceRange()), + ReplacementStr); } -} // namespace clang::tidy::modernize +} // namespace clang::tidy::modernize \ No newline at end of file diff --git a/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.h b/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.h index fdb157ca0a24fb..36faaff08f8717 100644 --- a/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.h @@ -1,11 +1,3 @@ -//===--- SubstrToStartsWithCheck.h - clang-tidy ------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_SUBSTRTOSTARTSWITHCHECK_H #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_SUBSTRTOSTARTSWITHCHECK_H @@ -13,18 +5,18 @@ namespace clang::tidy::modernize { -/// Finds calls to substr(0, n) that can be replaced with starts_with(). -/// -/// For the user-facing documentation see: -/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/substr-to-starts-with.html class SubstrToStartsWithCheck : public ClangTidyCheck { public: SubstrToStartsWithCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context) {} void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + std::string getExprStr(const Expr *E, const SourceManager &SM, + const LangOptions &LO); }; } // namespace clang::tidy::modernize -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_SUBSTRTOSTARTSWITHCHECK_H +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_SUBSTRTOSTARTSWITHCHECK_H \ No newline at end of file diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index 1231f954298adc..04944426aff1c7 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -171,9 +171,59 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { hasRHS(lengthExprForStringNode("needle"))))) .bind("expr"), this); + + // Case for substr with comparison + const auto SubstrExpr = cxxMemberCallExpr( + callee(memberExpr(member(hasName("substr")))), + hasArgument(0, integerLiteral(equals(0))), + hasArgument(1, expr().bind("length")), + on(expr().bind("substr_on"))) + .bind("substr_expr"); + + // Match str.substr(0, n) == X or X == str.substr(0, n) + Finder->addMatcher( + binaryOperator( + anyOf(hasOperatorName("=="), hasOperatorName("!=")), + anyOf( + allOf(hasLHS(SubstrExpr), hasRHS(expr().bind("substr_rhs"))), + allOf(hasLHS(expr().bind("substr_rhs")), hasRHS(SubstrExpr)))) + .bind("substr_cmp"), + this); } void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *Comparison = Result.Nodes.getNodeAs<BinaryOperator>("substr_cmp")) { + if (Comparison->getBeginLoc().isMacroID()) + return; + + const auto *SubstrCall = Result.Nodes.getNodeAs<CXXMemberCallExpr>("substr_expr"); + const auto *SubstrOn = Result.Nodes.getNodeAs<Expr>("substr_on"); + const auto *RHS = Result.Nodes.getNodeAs<Expr>("substr_rhs"); + + if (!SubstrCall || !SubstrOn || !RHS) + return; + + // Build the replacement + std::string ReplacementStr = Lexer::getSourceText( + CharSourceRange::getTokenRange(SubstrOn->getSourceRange()), + *Result.SourceManager, getLangOpts()).str() + ".starts_with("; + + ReplacementStr += Lexer::getSourceText( + CharSourceRange::getTokenRange(RHS->getSourceRange()), + *Result.SourceManager, getLangOpts()).str() + ")"; + + if (Comparison->getOpcode() == BO_NE) { + ReplacementStr = "!" + ReplacementStr; + } + + // Emit diagnostic and fix + diag(SubstrCall->getBeginLoc(), "use starts_with() instead of substr(0, n) comparison") + << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(Comparison->getSourceRange()), + ReplacementStr); + return; + } + const auto *ComparisonExpr = Result.Nodes.getNodeAs<BinaryOperator>("expr"); const auto *FindExpr = Result.Nodes.getNodeAs<CXXMemberCallExpr>("find_expr"); const auto *FindFun = Result.Nodes.getNodeAs<CXXMethodDecl>("find_fun"); >From d5e2248411ef2723a9f9e2f57402c7328e2603d4 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Thu, 14 Nov 2024 01:51:26 +0100 Subject: [PATCH 3/5] now it works --- .../modernize/SubstrToStartsWithCheck.cpp | 97 +++++++++++-------- .../modernize/UseStartsEndsWithCheck.cpp | 50 ---------- 2 files changed, 59 insertions(+), 88 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp index 313c53784c8ac2..110861d446d2ca 100644 --- a/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp @@ -9,6 +9,7 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { void SubstrToStartsWithCheck::registerMatchers(MatchFinder *Finder) { + // Match the substring call const auto SubstrCall = cxxMemberCallExpr( callee(cxxMethodDecl(hasName("substr"))), hasArgument(0, integerLiteral(equals(0))), @@ -16,27 +17,46 @@ void SubstrToStartsWithCheck::registerMatchers(MatchFinder *Finder) { on(expr().bind("str"))) .bind("call"); - // Helper for matching comparison operators - auto AddSimpleMatcher = [&](auto Matcher) { - Finder->addMatcher( - traverse(TK_IgnoreUnlessSpelledInSource, std::move(Matcher)), this); - }; - - // Match str.substr(0,n) == "literal" - AddSimpleMatcher( - binaryOperation( - hasOperatorName("=="), - hasEitherOperand(SubstrCall), - hasEitherOperand(expr().bind("comparison"))) - .bind("positiveComparison")); - - // Match str.substr(0,n) != "literal" - AddSimpleMatcher( - binaryOperation( - hasOperatorName("!="), - hasEitherOperand(SubstrCall), - hasEitherOperand(expr().bind("comparison"))) - .bind("negativeComparison")); + // Match string literals on the right side + const auto StringLiteral = stringLiteral().bind("literal"); + + // Helper for matching comparison operators + auto AddSimpleMatcher = [&](auto Matcher) { + Finder->addMatcher( + traverse(TK_IgnoreUnlessSpelledInSource, std::move(Matcher)), this); + }; + + // Match str.substr(0,n) == "literal" + AddSimpleMatcher( + binaryOperation( + hasOperatorName("=="), + hasLHS(SubstrCall), + hasRHS(StringLiteral)) + .bind("positiveComparison")); + + // Also match "literal" == str.substr(0,n) + AddSimpleMatcher( + binaryOperation( + hasOperatorName("=="), + hasLHS(StringLiteral), + hasRHS(SubstrCall)) + .bind("positiveComparison")); + + // Match str.substr(0,n) != "literal" + AddSimpleMatcher( + binaryOperation( + hasOperatorName("!="), + hasLHS(SubstrCall), + hasRHS(StringLiteral)) + .bind("negativeComparison")); + + // Also match "literal" != str.substr(0,n) + AddSimpleMatcher( + binaryOperation( + hasOperatorName("!="), + hasLHS(StringLiteral), + hasRHS(SubstrCall)) + .bind("negativeComparison")); } std::string SubstrToStartsWithCheck::getExprStr(const Expr *E, @@ -60,34 +80,35 @@ void SubstrToStartsWithCheck::check(const MatchFinder::MatchResult &Result) { bool Negated = NegativeComparison != nullptr; const auto *Comparison = Negated ? NegativeComparison : PositiveComparison; - // Skip if in macro if (Call->getBeginLoc().isMacroID()) return; const auto *Str = Result.Nodes.getNodeAs<Expr>("str"); - const auto *CompareExpr = Result.Nodes.getNodeAs<Expr>("comparison"); + const auto *Literal = Result.Nodes.getNodeAs<StringLiteral>("literal"); - if (!Str || !CompareExpr) + if (!Str || !Literal) return; - // Emit the diagnostic - auto Diag = diag(Call->getExprLoc(), + // Get the string expression + std::string StrText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Str->getSourceRange()), + *Result.SourceManager, getLangOpts()).str(); + + // Get the literal text + std::string LiteralText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Literal->getSourceRange()), + *Result.SourceManager, getLangOpts()).str(); + + // Build the replacement + std::string ReplacementText = (Negated ? "!" : "") + StrText + ".starts_with(" + + LiteralText + ")"; + + auto Diag = diag(Call->getExprLoc(), "use starts_with() instead of substr(0, n) comparison"); - // Build the replacement text - std::string ReplacementStr = - (Negated ? "!" : "") + - Lexer::getSourceText(CharSourceRange::getTokenRange(Str->getSourceRange()), - *Result.SourceManager, getLangOpts()).str() + - ".starts_with(" + - Lexer::getSourceText(CharSourceRange::getTokenRange(CompareExpr->getSourceRange()), - *Result.SourceManager, getLangOpts()).str() + - ")"; - - // Create the fix-it Diag << FixItHint::CreateReplacement( CharSourceRange::getTokenRange(Comparison->getSourceRange()), - ReplacementStr); + ReplacementText); } } // namespace clang::tidy::modernize \ No newline at end of file diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index 04944426aff1c7..1231f954298adc 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -171,59 +171,9 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { hasRHS(lengthExprForStringNode("needle"))))) .bind("expr"), this); - - // Case for substr with comparison - const auto SubstrExpr = cxxMemberCallExpr( - callee(memberExpr(member(hasName("substr")))), - hasArgument(0, integerLiteral(equals(0))), - hasArgument(1, expr().bind("length")), - on(expr().bind("substr_on"))) - .bind("substr_expr"); - - // Match str.substr(0, n) == X or X == str.substr(0, n) - Finder->addMatcher( - binaryOperator( - anyOf(hasOperatorName("=="), hasOperatorName("!=")), - anyOf( - allOf(hasLHS(SubstrExpr), hasRHS(expr().bind("substr_rhs"))), - allOf(hasLHS(expr().bind("substr_rhs")), hasRHS(SubstrExpr)))) - .bind("substr_cmp"), - this); } void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) { - if (const auto *Comparison = Result.Nodes.getNodeAs<BinaryOperator>("substr_cmp")) { - if (Comparison->getBeginLoc().isMacroID()) - return; - - const auto *SubstrCall = Result.Nodes.getNodeAs<CXXMemberCallExpr>("substr_expr"); - const auto *SubstrOn = Result.Nodes.getNodeAs<Expr>("substr_on"); - const auto *RHS = Result.Nodes.getNodeAs<Expr>("substr_rhs"); - - if (!SubstrCall || !SubstrOn || !RHS) - return; - - // Build the replacement - std::string ReplacementStr = Lexer::getSourceText( - CharSourceRange::getTokenRange(SubstrOn->getSourceRange()), - *Result.SourceManager, getLangOpts()).str() + ".starts_with("; - - ReplacementStr += Lexer::getSourceText( - CharSourceRange::getTokenRange(RHS->getSourceRange()), - *Result.SourceManager, getLangOpts()).str() + ")"; - - if (Comparison->getOpcode() == BO_NE) { - ReplacementStr = "!" + ReplacementStr; - } - - // Emit diagnostic and fix - diag(SubstrCall->getBeginLoc(), "use starts_with() instead of substr(0, n) comparison") - << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange(Comparison->getSourceRange()), - ReplacementStr); - return; - } - const auto *ComparisonExpr = Result.Nodes.getNodeAs<BinaryOperator>("expr"); const auto *FindExpr = Result.Nodes.getNodeAs<CXXMemberCallExpr>("find_expr"); const auto *FindFun = Result.Nodes.getNodeAs<CXXMethodDecl>("find_fun"); >From bfc984835e0cdc227ab124dc7f5a900d5fdf21fd Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Thu, 14 Nov 2024 01:56:44 +0100 Subject: [PATCH 4/5] now it works --- .../modernize/SubstrToStartsWithCheck.cpp | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp index 110861d446d2ca..802a11359f4344 100644 --- a/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp @@ -85,10 +85,26 @@ void SubstrToStartsWithCheck::check(const MatchFinder::MatchResult &Result) { const auto *Str = Result.Nodes.getNodeAs<Expr>("str"); const auto *Literal = Result.Nodes.getNodeAs<StringLiteral>("literal"); - + const auto *Length = Result.Nodes.getNodeAs<Expr>("length"); + + if (!Str || !Literal) return; +// Check if Length is an integer literal and compare with string length + if (const auto *LengthInt = dyn_cast<IntegerLiteral>(Length)) { + unsigned LitLength = Literal->getLength(); + unsigned SubstrLength = LengthInt->getValue().getZExtValue(); + + // Only proceed if the lengths match + if (SubstrLength != LitLength) { + return; + } + } else { + // If length isn't a constant, skip the transformation + return; + } + // Get the string expression std::string StrText = Lexer::getSourceText( CharSourceRange::getTokenRange(Str->getSourceRange()), >From 1c1c344a18a0be647e035ac617bee58caf776a13 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Thu, 14 Nov 2024 16:10:49 +0100 Subject: [PATCH 5/5] up --- .../clang-tidy/modernize/CMakeLists.txt | 1 - .../modernize/ModernizeTidyModule.cpp | 3 - .../modernize/SubstrToStartsWithCheck.cpp | 130 ------------------ .../modernize/SubstrToStartsWithCheck.h | 22 --- .../modernize/UseStartsEndsWithCheck.cpp | 122 ++++++++++++++++ .../modernize/UseStartsEndsWithCheck.h | 4 + .../modernize/substr-to-starts-with.rst | 35 ----- .../checks/modernize/use-starts-ends-with.rst | 45 +++--- .../modernize/substr-to-starts-with.cpp | 42 ------ .../modernize/use-starts-ends-with.cpp | 11 +- 10 files changed, 165 insertions(+), 250 deletions(-) delete mode 100644 clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp delete mode 100644 clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.h delete mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/substr-to-starts-with.rst delete mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/substr-to-starts-with.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index ed1ba2ab62a90f..c919d49b42873a 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -49,7 +49,6 @@ add_clang_library(clangTidyModernizeModule STATIC UseTransparentFunctorsCheck.cpp UseUncaughtExceptionsCheck.cpp UseUsingCheck.cpp - SubstrToStartsWithCheck.cpp LINK_LIBS clangTidy diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 7569211d2552ea..18607593320635 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -50,7 +50,6 @@ #include "UseTransparentFunctorsCheck.h" #include "UseUncaughtExceptionsCheck.h" #include "UseUsingCheck.h" -#include "SubstrToStartsWithCheck.h" using namespace clang::ast_matchers; @@ -123,8 +122,6 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<UseUncaughtExceptionsCheck>( "modernize-use-uncaught-exceptions"); CheckFactories.registerCheck<UseUsingCheck>("modernize-use-using"); - CheckFactories.registerCheck<SubstrToStartsWithCheck>( - "modernize-substr-to-starts-with"); } }; diff --git a/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp deleted file mode 100644 index 802a11359f4344..00000000000000 --- a/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.cpp +++ /dev/null @@ -1,130 +0,0 @@ -#include "SubstrToStartsWithCheck.h" -#include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Basic/SourceManager.h" -#include "clang/Lex/Lexer.h" - -using namespace clang::ast_matchers; - -namespace clang::tidy::modernize { - -void SubstrToStartsWithCheck::registerMatchers(MatchFinder *Finder) { - // Match the substring call - const auto SubstrCall = cxxMemberCallExpr( - callee(cxxMethodDecl(hasName("substr"))), - hasArgument(0, integerLiteral(equals(0))), - hasArgument(1, expr().bind("length")), - on(expr().bind("str"))) - .bind("call"); - - // Match string literals on the right side - const auto StringLiteral = stringLiteral().bind("literal"); - - // Helper for matching comparison operators - auto AddSimpleMatcher = [&](auto Matcher) { - Finder->addMatcher( - traverse(TK_IgnoreUnlessSpelledInSource, std::move(Matcher)), this); - }; - - // Match str.substr(0,n) == "literal" - AddSimpleMatcher( - binaryOperation( - hasOperatorName("=="), - hasLHS(SubstrCall), - hasRHS(StringLiteral)) - .bind("positiveComparison")); - - // Also match "literal" == str.substr(0,n) - AddSimpleMatcher( - binaryOperation( - hasOperatorName("=="), - hasLHS(StringLiteral), - hasRHS(SubstrCall)) - .bind("positiveComparison")); - - // Match str.substr(0,n) != "literal" - AddSimpleMatcher( - binaryOperation( - hasOperatorName("!="), - hasLHS(SubstrCall), - hasRHS(StringLiteral)) - .bind("negativeComparison")); - - // Also match "literal" != str.substr(0,n) - AddSimpleMatcher( - binaryOperation( - hasOperatorName("!="), - hasLHS(StringLiteral), - hasRHS(SubstrCall)) - .bind("negativeComparison")); -} - -std::string SubstrToStartsWithCheck::getExprStr(const Expr *E, - const SourceManager &SM, - const LangOptions &LO) { - CharSourceRange Range = CharSourceRange::getTokenRange(E->getSourceRange()); - return Lexer::getSourceText(Range, SM, LO).str(); -} - -void SubstrToStartsWithCheck::check(const MatchFinder::MatchResult &Result) { - const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call"); - if (!Call) - return; - - const auto *PositiveComparison = Result.Nodes.getNodeAs<Expr>("positiveComparison"); - const auto *NegativeComparison = Result.Nodes.getNodeAs<Expr>("negativeComparison"); - - if (!PositiveComparison && !NegativeComparison) - return; - - bool Negated = NegativeComparison != nullptr; - const auto *Comparison = Negated ? NegativeComparison : PositiveComparison; - - if (Call->getBeginLoc().isMacroID()) - return; - - const auto *Str = Result.Nodes.getNodeAs<Expr>("str"); - const auto *Literal = Result.Nodes.getNodeAs<StringLiteral>("literal"); - const auto *Length = Result.Nodes.getNodeAs<Expr>("length"); - - - if (!Str || !Literal) - return; - -// Check if Length is an integer literal and compare with string length - if (const auto *LengthInt = dyn_cast<IntegerLiteral>(Length)) { - unsigned LitLength = Literal->getLength(); - unsigned SubstrLength = LengthInt->getValue().getZExtValue(); - - // Only proceed if the lengths match - if (SubstrLength != LitLength) { - return; - } - } else { - // If length isn't a constant, skip the transformation - return; - } - - // Get the string expression - std::string StrText = Lexer::getSourceText( - CharSourceRange::getTokenRange(Str->getSourceRange()), - *Result.SourceManager, getLangOpts()).str(); - - // Get the literal text - std::string LiteralText = Lexer::getSourceText( - CharSourceRange::getTokenRange(Literal->getSourceRange()), - *Result.SourceManager, getLangOpts()).str(); - - // Build the replacement - std::string ReplacementText = (Negated ? "!" : "") + StrText + ".starts_with(" + - LiteralText + ")"; - - auto Diag = diag(Call->getExprLoc(), - "use starts_with() instead of substr(0, n) comparison"); - - Diag << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange(Comparison->getSourceRange()), - ReplacementText); -} - -} // namespace clang::tidy::modernize \ No newline at end of file diff --git a/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.h b/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.h deleted file mode 100644 index 36faaff08f8717..00000000000000 --- a/clang-tools-extra/clang-tidy/modernize/SubstrToStartsWithCheck.h +++ /dev/null @@ -1,22 +0,0 @@ -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_SUBSTRTOSTARTSWITHCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_SUBSTRTOSTARTSWITHCHECK_H - -#include "../ClangTidyCheck.h" - -namespace clang::tidy::modernize { - -class SubstrToStartsWithCheck : public ClangTidyCheck { -public: - SubstrToStartsWithCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; - -private: - std::string getExprStr(const Expr *E, const SourceManager &SM, - const LangOptions &LO); -}; - -} // namespace clang::tidy::modernize - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_SUBSTRTOSTARTSWITHCHECK_H \ No newline at end of file diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index 1231f954298adc..98509d5fa05bdd 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -83,6 +83,55 @@ UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name, void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { const auto ZeroLiteral = integerLiteral(equals(0)); + // Match the substring call + const auto SubstrCall = cxxMemberCallExpr( + callee(cxxMethodDecl(hasName("substr"))), + hasArgument(0, ZeroLiteral), + hasArgument(1, expr().bind("length")), + on(expr().bind("str"))) + .bind("substr_fun"); + + // Match string literals + const auto Literal = stringLiteral().bind("literal"); + + // Helper for matching comparison operators + auto AddSubstrMatcher = [&](auto Matcher) { + Finder->addMatcher( + traverse(TK_IgnoreUnlessSpelledInSource, std::move(Matcher)), this); + }; + + // Match str.substr(0,n) == "literal" + AddSubstrMatcher( + binaryOperation( + hasOperatorName("=="), + hasLHS(SubstrCall), + hasRHS(Literal)) + .bind("positiveComparison")); + + // Also match "literal" == str.substr(0,n) + AddSubstrMatcher( + binaryOperation( + hasOperatorName("=="), + hasLHS(Literal), + hasRHS(SubstrCall)) + .bind("positiveComparison")); + + // Match str.substr(0,n) != "literal" + AddSubstrMatcher( + binaryOperation( + hasOperatorName("!="), + hasLHS(SubstrCall), + hasRHS(Literal)) + .bind("negativeComparison")); + + // Also match "literal" != str.substr(0,n) + AddSubstrMatcher( + binaryOperation( + hasOperatorName("!="), + hasLHS(Literal), + hasRHS(SubstrCall)) + .bind("negativeComparison")); + const auto ClassTypeWithMethod = [](const StringRef MethodBoundName, const auto... Methods) { return cxxRecordDecl(anyOf( @@ -173,7 +222,80 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { this); } +void UseStartsEndsWithCheck::handleSubstrMatch(const MatchFinder::MatchResult &Result) { + const auto *SubstrCall = Result.Nodes.getNodeAs<CXXMemberCallExpr>("substr_fun"); + const auto *PositiveComparison = Result.Nodes.getNodeAs<Expr>("positiveComparison"); + const auto *NegativeComparison = Result.Nodes.getNodeAs<Expr>("negativeComparison"); + + if (!SubstrCall || (!PositiveComparison && !NegativeComparison)) + return; + + bool Negated = NegativeComparison != nullptr; + const auto *Comparison = Negated ? NegativeComparison : PositiveComparison; + + if (SubstrCall->getBeginLoc().isMacroID()) + return; + + const auto *Str = Result.Nodes.getNodeAs<Expr>("str"); + const auto *Literal = Result.Nodes.getNodeAs<StringLiteral>("literal"); + const auto *Length = Result.Nodes.getNodeAs<Expr>("length"); + + if (!Str || !Literal || !Length) + return; + + // Check if Length is an integer literal and compare with string length + if (const auto *LengthInt = dyn_cast<IntegerLiteral>(Length)) { + unsigned LitLength = Literal->getLength(); + unsigned SubstrLength = LengthInt->getValue().getZExtValue(); + + // Only proceed if the lengths match + if (SubstrLength != LitLength) { + return; + } + } else { + return; // Non-constant length + } + + // Get the string expression + std::string StrText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Str->getSourceRange()), + *Result.SourceManager, getLangOpts()).str(); + + // Get the literal text + std::string LiteralText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Literal->getSourceRange()), + *Result.SourceManager, getLangOpts()).str(); + + // Build the replacement + std::string ReplacementText = (Negated ? "!" : "") + StrText + ".starts_with(" + + LiteralText + ")"; + + auto Diag = diag(SubstrCall->getExprLoc(), + "use starts_with() instead of substr(0, n) comparison"); + + Diag << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(Comparison->getSourceRange()), + ReplacementText); +} + void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) { + // Try substr pattern first + const auto *SubstrCall = Result.Nodes.getNodeAs<CXXMemberCallExpr>("substr_fun"); + if (SubstrCall) { + const auto *PositiveComparison = Result.Nodes.getNodeAs<Expr>("positiveComparison"); + const auto *NegativeComparison = Result.Nodes.getNodeAs<Expr>("negativeComparison"); + + if (PositiveComparison || NegativeComparison) { + handleSubstrMatch(Result); + return; + } + } + + // Then try find/compare patterns + handleFindCompareMatch(Result); +} + +void UseStartsEndsWithCheck::handleFindCompareMatch(const MatchFinder::MatchResult &Result) { const auto *ComparisonExpr = Result.Nodes.getNodeAs<BinaryOperator>("expr"); const auto *FindExpr = Result.Nodes.getNodeAs<CXXMemberCallExpr>("find_expr"); const auto *FindFun = Result.Nodes.getNodeAs<CXXMethodDecl>("find_fun"); diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h index 17c2999bda84cd..d06bf8c22f3ad7 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h @@ -31,6 +31,10 @@ class UseStartsEndsWithCheck : public ClangTidyCheck { std::optional<TraversalKind> getCheckTraversalKind() const override { return TK_IgnoreUnlessSpelledInSource; } +private: + void handleSubstrMatch(const ast_matchers::MatchFinder::MatchResult &Result); + void handleFindCompareMatch(const ast_matchers::MatchFinder::MatchResult &Result); + }; } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/substr-to-starts-with.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/substr-to-starts-with.rst deleted file mode 100644 index 948c01c29b8c0a..00000000000000 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/substr-to-starts-with.rst +++ /dev/null @@ -1,35 +0,0 @@ -modernize-substr-to-starts-with -============================== - -Finds calls to ``substr(0, n)`` that can be replaced with ``starts_with()`` (introduced in C++20). -This makes the code's intent clearer and can be more efficient as it avoids creating temporary strings. - -For example: - -.. code-block:: c++ - - str.substr(0, 3) == "foo" // before - str.starts_with("foo") // after - - "bar" == str.substr(0, 3) // before - str.starts_with("bar") // after - - str.substr(0, n) == prefix // before - str.starts_with(prefix) // after - -The check handles various ways of expressing zero as the start index: - -.. code-block:: c++ - - const int zero = 0; - str.substr(zero, n) == prefix // converted - str.substr(x - x, n) == prefix // converted - -The check will only convert cases where: -* The substr call starts at index 0 (or equivalent) -* When comparing with string literals, the length matches exactly -* The comparison is with == or != - -.. code-block:: c++ - - auto prefix = str.substr(0, n); // warns about possible use of starts_with diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst index 721e927e29849f..752f2cdacd065a 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst @@ -7,26 +7,39 @@ Checks for common roundabout ways to express ``starts_with`` and ``ends_with`` and suggests replacing with the simpler method when it is available. Notably, this will work with ``std::string`` and ``std::string_view``. -.. code-block:: c++ +The check handles the following expressions: - std::string s = "..."; - if (s.find("prefix") == 0) { /* do something */ } - if (s.rfind("prefix", 0) == 0) { /* do something */ } - if (s.compare(0, strlen("prefix"), "prefix") == 0) { /* do something */ } - if (s.compare(s.size() - strlen("suffix"), strlen("suffix"), "suffix") == 0) { - /* do something */ - } - if (s.rfind("suffix") == (s.length() - 6)) { - /* do something */ - } - -becomes +==================================================== =========================== +Expression Result +---------------------------------------------------- --------------------------- +``u.find(v) == 0`` ``u.starts_with(v)`` +``u.rfind(v, 0) != 0`` ``!u.starts_with(v)`` +``u.compare(0, v.size(), v) == 0`` ``u.starts_with(v)`` +``u.substr(0, v.size()) == v`` ``u.starts_with(v)`` +``v == u.substr(0, v.size())`` ``u.starts_with(v)`` +``u.substr(0, v.size()) != v`` ``!u.starts_with(v)`` +``u.compare(u.size() - v.size(), v.size(), v) == 0`` ``u.ends_with(v)`` +``u.rfind(v) == u.size() - v.size()`` ``u.ends_with(v)`` +==================================================== =========================== + +For example: .. code-block:: c++ std::string s = "..."; if (s.starts_with("prefix")) { /* do something */ } - if (s.starts_with("prefix")) { /* do something */ } - if (s.starts_with("prefix")) { /* do something */ } - if (s.ends_with("suffix")) { /* do something */ } if (s.ends_with("suffix")) { /* do something */ } + +Notes: + +* For the ``substr`` pattern, the check ensures that: + + * The substring starts at position 0 + * The length matches exactly the compared string's length + * The length is a constant value + +* Non-matching cases (will not be transformed): + + * ``s.substr(1, 5) == "hello"`` // Non-zero start position + * ``s.substr(0, 4) == "hello"`` // Length mismatch + * ``s.substr(0, len) == "hello"`` // Non-constant length \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/substr-to-starts-with.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/substr-to-starts-with.cpp deleted file mode 100644 index 03703562c99115..00000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/substr-to-starts-with.cpp +++ /dev/null @@ -1,42 +0,0 @@ -// RUN: %check_clang_tidy %s modernize-substr-to-starts-with %t -- -std=c++20 -stdlib=libc++ - -#include <string> -#include <string_view> - -void test() { - std::string str = "hello world"; - if (str.substr(0, 5) == "hello") {} - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use starts_with() instead of substring comparison - // CHECK-FIXES: if (str.starts_with("hello")) {} - - if ("hello" == str.substr(0, 5)) {} - // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use starts_with() instead of substring comparison - // CHECK-FIXES: if (str.starts_with("hello")) {} - - bool b = str.substr(0, 5) != "hello"; - // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use starts_with() instead of substring comparison - // CHECK-FIXES: bool b = !str.starts_with("hello"); - - // Variable length and string refs - std::string prefix = "hello"; - size_t len = 5; - if (str.substr(0, len) == prefix) {} - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use starts_with() instead of substring comparison - // CHECK-FIXES: if (str.starts_with(prefix)) {} - - // Various zero expressions - const int zero = 0; - int i = 0; - if (str.substr(zero, 5) == "hello") {} - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use starts_with() instead of substring comparison - // CHECK-FIXES: if (str.starts_with("hello")) {} - - if (str.substr(i-i, 5) == "hello") {} - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use starts_with() instead of substring comparison - // CHECK-FIXES: if (str.starts_with("hello")) {} - - // Should not convert these - if (str.substr(1, 5) == "hello") {} // Non-zero start - if (str.substr(0, 4) == "hello") {} // Length mismatch - if (str.substr(0, 6) == "hello") {} // Length mismatch -} 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 91477241e82e54..20548f90bec390 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 @@ -1,5 +1,5 @@ // RUN: %check_clang_tidy -std=c++20 %s modernize-use-starts-ends-with %t -- \ -// RUN: -- -isystem %clang_tidy_headers +// RUN: -- -isystem %clang_tidy_headers #include <string.h> #include <string> @@ -266,3 +266,12 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss, s.compare(0, 1, "ab") == 0; s.rfind(suffix, 1) == s.size() - suffix.size(); } + +void test_substr() { + std::string_view strs("hello world"); + + // Test basic substr pattern to ensure the modernizer catches this usage + strs.substr(0, 5) == "hello"; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of substr(0, n) comparison + // CHECK-FIXES: strs.starts_with("hello"); +} \ No newline at end of file _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits