[clang-tools-extra] [clang-tidy] Add modernize-substr-to-starts-with check (PR #116033)
Helmut Januschka via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 14 07:11:22 PST 2024
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 <helmut at 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 <helmut at 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 <helmut at 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 <helmut at 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 <helmut at 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
More information about the cfe-commits
mailing list