[clang-tidy][modernize-use-starts-ends-with] Add support for two ends_with patterns #110448
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang-tools-extra Author: Nicolas van Kempen (nicovank) ChangesAdd support for the following two patterns: Patch is 21.20 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/110448.diff 6 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
index 89ee45faecd7f3..5eb3267adb0799 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
@@ -8,6 +8,7 @@
#include "UseStartsEndsWithCheck.h"
+#include "../utils/ASTUtils.h"
#include "../utils/OptionsUtils.h"
#include "clang/Lex/Lexer.h"
@@ -16,6 +17,63 @@
using namespace clang::ast_matchers;
namespace clang::tidy::modernize {
+struct NotLengthExprForStringNode {
+ NotLengthExprForStringNode(std::string ID, DynTypedNode Node,
+ ASTContext *Context)
+ : ID(std::move(ID)), Node(std::move(Node)), Context(Context) {}
+ bool operator()(const internal::BoundNodesMap &Nodes) const {
+ // Match a string literal and an integer size or strlen() call.
+ if (const auto *StringLiteralNode = Nodes.getNodeAs<StringLiteral>(ID)) {
+ if (const auto *IntegerLiteralSizeNode = Node.get<IntegerLiteral>()) {
+ return StringLiteralNode->getLength() !=
+ IntegerLiteralSizeNode->getValue().getZExtValue();
+ }
+
+ if (const auto *StrlenNode = Node.get<CallExpr>()) {
+ if (StrlenNode->getDirectCallee()->getName() != "strlen" ||
+ StrlenNode->getNumArgs() != 1) {
+ return true;
+ }
+
+ if (const auto *StrlenArgNode = dyn_cast<StringLiteral>(
+ StrlenNode->getArg(0)->IgnoreParenImpCasts())) {
+ return StrlenArgNode->getLength() != StringLiteralNode->getLength();
+ }
+ }
+ }
+
+ // Match a string variable and a call to length() or size().
+ if (const auto *ExprNode = Nodes.getNodeAs<Expr>(ID)) {
+ if (const auto *MemberCallNode = Node.get<CXXMemberCallExpr>()) {
+ const CXXMethodDecl *MethodDeclNode = MemberCallNode->getMethodDecl();
+ const StringRef Name = MethodDeclNode->getName();
+ if (!MethodDeclNode->isConst() || MethodDeclNode->getNumParams() != 0 ||
+ (Name != "size" && Name != "length")) {
+ return true;
+ }
+
+ if (const auto *OnNode =
+ dyn_cast<Expr>(MemberCallNode->getImplicitObjectArgument())) {
+ return !utils::areStatementsIdentical(OnNode->IgnoreParenImpCasts(),
+ ExprNode->IgnoreParenImpCasts(),
+ *Context);
+ }
+ }
+ }
+
+ return true;
+ }
+
+private:
+ std::string ID;
+ DynTypedNode Node;
+ ASTContext *Context;
+};
+
+AST_MATCHER_P(Expr, lengthExprForStringNode, std::string, ID) {
+ return Builder->removeBindings(NotLengthExprForStringNode(
+ ID, DynTypedNode::create(Node), &(Finder->getASTContext())));
+}
UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name,
ClangTidyContext *Context)
@@ -23,6 +81,7 @@ UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name,
void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
const auto ZeroLiteral = integerLiteral(equals(0));
+
const auto HasStartsWithMethodWithName = [](const std::string &Name) {
return hasMethod(
cxxMethodDecl(hasName(Name), isConst(), parameterCountIs(1))
@@ -32,119 +91,104 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
anyOf(HasStartsWithMethodWithName("starts_with"),
HasStartsWithMethodWithName("startsWith"),
HasStartsWithMethodWithName("startswith"));
- const auto ClassWithStartsWithFunction = cxxRecordDecl(anyOf(
- HasStartsWithMethod, hasAnyBase(hasType(hasCanonicalType(hasDeclaration(
- cxxRecordDecl(HasStartsWithMethod)))))));
+ const auto OnClassWithStartsWithFunction =
+ on(hasType(hasCanonicalType(hasDeclaration(cxxRecordDecl(
+ anyOf(HasStartsWithMethod,
+ hasAnyBase(hasType(hasCanonicalType(
+ hasDeclaration(cxxRecordDecl(HasStartsWithMethod)))))))))));
+ const auto HasEndsWithMethodWithName = [](const std::string &Name) {
+ return hasMethod(
+ cxxMethodDecl(hasName(Name), isConst(), parameterCountIs(1))
+ .bind("ends_with_fun"));
+ };
+ const auto HasEndsWithMethod = anyOf(HasEndsWithMethodWithName("ends_with"),
+ HasEndsWithMethodWithName("endsWith"),
+ HasEndsWithMethodWithName("endswith"));
+ const auto OnClassWithEndsWithFunction =
+ on(expr(hasType(hasCanonicalType(hasDeclaration(cxxRecordDecl(
+ anyOf(HasEndsWithMethod,
+ hasAnyBase(hasType(hasCanonicalType(hasDeclaration(
+ cxxRecordDecl(HasEndsWithMethod)))))))))))
+ .bind("haystack"));
+
+ // Case 1: X.find(Y) [!=]= 0 -> starts_with.
const auto FindExpr = cxxMemberCallExpr(
- // A method call with no second argument or the second argument is zero...
anyOf(argumentCountIs(1), hasArgument(1, ZeroLiteral)),
- // ... named find...
callee(cxxMethodDecl(hasName("find")).bind("find_fun")),
- // ... on a class with a starts_with function.
- on(hasType(
- hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction)))),
- // Bind search expression.
- hasArgument(0, expr().bind("search_expr")));
+ OnClassWithStartsWithFunction, hasArgument(0, expr().bind("needle")));
+ // Case 2: X.rfind(Y, 0) [!=]= 0 -> starts_with.
const auto RFindExpr = cxxMemberCallExpr(
- // A method call with a second argument of zero...
hasArgument(1, ZeroLiteral),
- // ... named rfind...
callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")),
- // ... on a class with a starts_with function.
- on(hasType(
- hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction)))),
- // Bind search expression.
- hasArgument(0, expr().bind("search_expr")));
-
- // Match a string literal and an integer or strlen() call matching the length.
- const auto HasStringLiteralAndLengthArgs = [](const auto StringArgIndex,
- const auto LengthArgIndex) {
- return allOf(
- hasArgument(StringArgIndex, stringLiteral().bind("string_literal_arg")),
- hasArgument(LengthArgIndex,
- anyOf(integerLiteral().bind("integer_literal_size_arg"),
- callExpr(callee(functionDecl(parameterCountIs(1),
- hasName("strlen"))),
- hasArgument(0, stringLiteral().bind(
- "strlen_arg"))))));
- };
-
- // Match a string variable and a call to length() or size().
- const auto HasStringVariableAndSizeCallArgs = [](const auto StringArgIndex,
- const auto LengthArgIndex) {
- return allOf(
- hasArgument(StringArgIndex, declRefExpr(hasDeclaration(
- decl().bind("string_var_decl")))),
- hasArgument(LengthArgIndex,
- cxxMemberCallExpr(
- callee(cxxMethodDecl(isConst(), parameterCountIs(0),
- hasAnyName("size", "length"))),
- on(declRefExpr(
- to(decl(equalsBoundNode("string_var_decl"))))))));
- };
-
- // Match either one of the two cases above.
- const auto HasStringAndLengthArgs =
- [HasStringLiteralAndLengthArgs, HasStringVariableAndSizeCallArgs](
- const auto StringArgIndex, const auto LengthArgIndex) {
- return anyOf(
- HasStringLiteralAndLengthArgs(StringArgIndex, LengthArgIndex),
- HasStringVariableAndSizeCallArgs(StringArgIndex, LengthArgIndex));
- };
+ OnClassWithStartsWithFunction, hasArgument(0, expr().bind("needle")));
+ // Case 3: X.compare(0, LEN(Y), Y) [!=]= 0 -> starts_with.
const auto CompareExpr = cxxMemberCallExpr(
- // A method call with three arguments...
- argumentCountIs(3),
- // ... where the first argument is zero...
- hasArgument(0, ZeroLiteral),
- // ... named compare...
+ argumentCountIs(3), hasArgument(0, ZeroLiteral),
callee(cxxMethodDecl(hasName("compare")).bind("find_fun")),
- // ... on a class with a starts_with function...
- on(hasType(
- hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction)))),
- // ... where the third argument is some string and the second a length.
- HasStringAndLengthArgs(2, 1),
- // Bind search expression.
- hasArgument(2, expr().bind("search_expr")));
+ OnClassWithStartsWithFunction, hasArgument(2, expr().bind("needle")),
+ hasArgument(1, lengthExprForStringNode("needle")));
+ // Case 4: X.compare(LEN(X) - LEN(Y), LEN(Y), Y) [!=]= 0 -> ends_with.
+ const auto CompareEndsWithExpr = cxxMemberCallExpr(
+ argumentCountIs(3),
+ callee(cxxMethodDecl(hasName("compare")).bind("find_fun")),
+ OnClassWithEndsWithFunction, hasArgument(2, expr().bind("needle")),
+ hasArgument(1, lengthExprForStringNode("needle")),
+ hasArgument(0,
+ binaryOperator(hasOperatorName("-"),
+ hasLHS(lengthExprForStringNode("haystack")),
+ hasRHS(lengthExprForStringNode("needle")))));
+
+ // All cases comparing to 0.
Finder->addMatcher(
- // Match [=!]= with a zero on one side and (r?)find|compare on the other.
binaryOperator(
hasAnyOperatorName("==", "!="),
- hasOperands(cxxMemberCallExpr(anyOf(FindExpr, RFindExpr, CompareExpr))
+ hasOperands(cxxMemberCallExpr(anyOf(FindExpr, RFindExpr, CompareExpr,
+ CompareEndsWithExpr))
.bind("find_expr"),
ZeroLiteral))
.bind("expr"),
this);
+
+ // Case 5: X.rfind(Y) [!=]= LEN(X) - LEN(Y) -> ends_with.
+ Finder->addMatcher(
+ binaryOperator(
+ hasAnyOperatorName("==", "!="),
+ hasOperands(
+ cxxMemberCallExpr(
+ anyOf(
+ argumentCountIs(1),
+ allOf(argumentCountIs(2),
+ hasArgument(
+ 1,
+ anyOf(declRefExpr(to(varDecl(hasName("npos")))),
+ memberExpr(member(hasName("npos"))))))),
+ callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")),
+ OnClassWithEndsWithFunction,
+ hasArgument(0, expr().bind("needle")))
+ .bind("find_expr"),
+ binaryOperator(hasOperatorName("-"),
+ hasLHS(lengthExprForStringNode("haystack")),
+ hasRHS(lengthExprForStringNode("needle")))))
+ .bind("expr"),
+ this);
}
void UseStartsEndsWithCheck::check(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");
- const auto *SearchExpr = Result.Nodes.getNodeAs<Expr>("search_expr");
+ const auto *SearchExpr = Result.Nodes.getNodeAs<Expr>("needle");
const auto *StartsWithFunction =
Result.Nodes.getNodeAs<CXXMethodDecl>("starts_with_fun");
-
- const auto *StringLiteralArg =
- Result.Nodes.getNodeAs<StringLiteral>("string_literal_arg");
- const auto *IntegerLiteralSizeArg =
- Result.Nodes.getNodeAs<IntegerLiteral>("integer_literal_size_arg");
- const auto *StrlenArg = Result.Nodes.getNodeAs<StringLiteral>("strlen_arg");
-
- // Filter out compare cases where the length does not match string literal.
- if (StringLiteralArg && IntegerLiteralSizeArg &&
- StringLiteralArg->getLength() !=
- IntegerLiteralSizeArg->getValue().getZExtValue()) {
- return;
- }
-
- if (StringLiteralArg && StrlenArg &&
- StringLiteralArg->getLength() != StrlenArg->getLength()) {
- return;
- }
+ const auto *EndsWithFunction =
+ Result.Nodes.getNodeAs<CXXMethodDecl>("ends_with_fun");
+ assert(bool(StartsWithFunction) != bool(EndsWithFunction));
+ const CXXMethodDecl *ReplacementFunction =
+ StartsWithFunction ? StartsWithFunction : EndsWithFunction;
if (ComparisonExpr->getBeginLoc().isMacroID()) {
return;
@@ -154,9 +198,9 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
auto Diagnostic =
diag(FindExpr->getExprLoc(), "use %0 instead of %1() %select{==|!=}2 0")
- << StartsWithFunction->getName() << FindFun->getName() << Neg;
+ << ReplacementFunction->getName() << FindFun->getName() << Neg;
- // Remove possible arguments after search expression and ' [!=]= 0' suffix.
+ // Remove possible arguments after search expression and ' [!=]= .+' suffix.
Diagnostic << FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(
Lexer::getLocForEndOfToken(SearchExpr->getEndLoc(), 0,
@@ -164,16 +208,16 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
ComparisonExpr->getEndLoc()),
")");
- // Remove possible '0 [!=]= ' prefix.
+ // Remove possible '.+ [!=]= ' prefix.
Diagnostic << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
ComparisonExpr->getBeginLoc(), FindExpr->getBeginLoc()));
- // Replace method name by 'starts_with'.
+ // Replace method name by '(starts|ends)_with'.
// Remove possible arguments before search expression.
Diagnostic << FixItHint::CreateReplacement(
CharSourceRange::getCharRange(FindExpr->getExprLoc(),
SearchExpr->getBeginLoc()),
- (StartsWithFunction->getName() + "(").str());
+ (ReplacementFunction->getName() + "(").str());
// Add possible negation '!'.
if (Neg) {
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h
index 840191f321493f..17c2999bda84cd 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h
@@ -14,7 +14,7 @@
namespace clang::tidy::modernize {
/// Checks for common roundabout ways to express ``starts_with`` and
-/// ``ends_with`` and suggests replacing with ``starts_with`` when the method is
+/// ``ends_with`` and suggests replacing with the simpler method when it is
/// available. Notably, this will work with ``std::string`` and
/// ``std::string_view``.
///
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 7d37a4b03222cf..5bcca66df13b0e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -161,6 +161,10 @@ Changes in existing checks
as a replacement for parameters of incomplete C array type in C++20 and
``std::array`` or ``std::vector`` before C++20.
+- Improved :doc:`modernize-use-starts-ends-with
+ <clang-tidy/checks/modernize/use-starts-ends-with>` check to handle two cases
+ that can be replaced with ``ends_with``.
+
- Improved :doc:`modernize-use-std-format
<clang-tidy/checks/modernize/use-std-format>` check to support replacing
member function calls too.
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 34237ede30a30b..721e927e29849f 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
@@ -4,8 +4,8 @@ modernize-use-starts-ends-with
==============================
Checks for common roundabout ways to express ``starts_with`` and ``ends_with``
-and suggests replacing with ``starts_with`` when the method is available.
-Notably, this will work with ``std::string`` and ``std::string_view``.
+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++
@@ -13,6 +13,12 @@ Notably, this will work with ``std::string`` and ``std::string_view``.
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
@@ -22,3 +28,5 @@ becomes
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 */ }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
index 0c160bc182b6eb..a0e8ebbb267cd0 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
@@ -64,6 +64,10 @@ struct basic_string {
constexpr bool starts_with(C ch) const noexcept;
constexpr bool starts_with(const C* s) const;
+ constexpr bool ends_with(std::basic_string_view<C, T> sv) const noexcept;
+ constexpr bool ends_with(C ch) const noexcept;
+ constexpr bool ends_with(const C* s) const;
+
_Type& operator[](size_type);
const _Type& operator[](size_type) const;
@@ -108,6 +112,10 @@ struct basic_string_view {
constexpr bool starts_with(C ch) const noexcept;
constexpr bool starts_with(const C* s) const;
+ constexpr bool ends_with(basic_string_view sv) const noexcept;
+ constexpr bool ends_with(C ch) const noexcept;
+ constexpr bool ends_with(const C* s) const;
+
constexpr int compare(basic_string_view sv) const noexcept;
static constexpr size_t npos = -1;
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 c5b2c86befd1fe..798af260a3b66c 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
@@ -208,6 +208,62 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss,
// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
// CHECK-FIXES: !s.starts_with(sv);
+ s.compare(s.size() - 6, 6, "suffix") == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with("suffix");
+
+ s.compare(s.size() - 6, strlen("abcdef"), "suffix") == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with("suffix");
+
+ std::string suffix = "suffix";
+ s.compare(s.size() - suffix.size(), suffix.size(), suffix) == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with(suffix);
+
+ s.rfind("suffix") == s.size() - 6;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with("suffix");
+
+ s.rfind("suffix") == s.size() - strlen("suffix");
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with("suffix");
+
+ s.rfind(suffix) == s.size() - suffix.size();
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with(suffix);
+
+ s.rfind(suffix, std::string::npos) == s.size() - suffix.size();
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with(suffix);
+
+ s.rfind(suffix) == (s.size() - suffix.size());
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with(s...
[truncated]
|
|
@llvm/pr-subscribers-clang-tidy Author: Nicolas van Kempen (nicovank) ChangesAdd support for the following two patterns: Patch is 21.20 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/110448.diff 6 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
index 89ee45faecd7f3..5eb3267adb0799 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
@@ -8,6 +8,7 @@
#include "UseStartsEndsWithCheck.h"
+#include "../utils/ASTUtils.h"
#include "../utils/OptionsUtils.h"
#include "clang/Lex/Lexer.h"
@@ -16,6 +17,63 @@
using namespace clang::ast_matchers;
namespace clang::tidy::modernize {
+struct NotLengthExprForStringNode {
+ NotLengthExprForStringNode(std::string ID, DynTypedNode Node,
+ ASTContext *Context)
+ : ID(std::move(ID)), Node(std::move(Node)), Context(Context) {}
+ bool operator()(const internal::BoundNodesMap &Nodes) const {
+ // Match a string literal and an integer size or strlen() call.
+ if (const auto *StringLiteralNode = Nodes.getNodeAs<StringLiteral>(ID)) {
+ if (const auto *IntegerLiteralSizeNode = Node.get<IntegerLiteral>()) {
+ return StringLiteralNode->getLength() !=
+ IntegerLiteralSizeNode->getValue().getZExtValue();
+ }
+
+ if (const auto *StrlenNode = Node.get<CallExpr>()) {
+ if (StrlenNode->getDirectCallee()->getName() != "strlen" ||
+ StrlenNode->getNumArgs() != 1) {
+ return true;
+ }
+
+ if (const auto *StrlenArgNode = dyn_cast<StringLiteral>(
+ StrlenNode->getArg(0)->IgnoreParenImpCasts())) {
+ return StrlenArgNode->getLength() != StringLiteralNode->getLength();
+ }
+ }
+ }
+
+ // Match a string variable and a call to length() or size().
+ if (const auto *ExprNode = Nodes.getNodeAs<Expr>(ID)) {
+ if (const auto *MemberCallNode = Node.get<CXXMemberCallExpr>()) {
+ const CXXMethodDecl *MethodDeclNode = MemberCallNode->getMethodDecl();
+ const StringRef Name = MethodDeclNode->getName();
+ if (!MethodDeclNode->isConst() || MethodDeclNode->getNumParams() != 0 ||
+ (Name != "size" && Name != "length")) {
+ return true;
+ }
+
+ if (const auto *OnNode =
+ dyn_cast<Expr>(MemberCallNode->getImplicitObjectArgument())) {
+ return !utils::areStatementsIdentical(OnNode->IgnoreParenImpCasts(),
+ ExprNode->IgnoreParenImpCasts(),
+ *Context);
+ }
+ }
+ }
+
+ return true;
+ }
+
+private:
+ std::string ID;
+ DynTypedNode Node;
+ ASTContext *Context;
+};
+
+AST_MATCHER_P(Expr, lengthExprForStringNode, std::string, ID) {
+ return Builder->removeBindings(NotLengthExprForStringNode(
+ ID, DynTypedNode::create(Node), &(Finder->getASTContext())));
+}
UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name,
ClangTidyContext *Context)
@@ -23,6 +81,7 @@ UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name,
void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
const auto ZeroLiteral = integerLiteral(equals(0));
+
const auto HasStartsWithMethodWithName = [](const std::string &Name) {
return hasMethod(
cxxMethodDecl(hasName(Name), isConst(), parameterCountIs(1))
@@ -32,119 +91,104 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
anyOf(HasStartsWithMethodWithName("starts_with"),
HasStartsWithMethodWithName("startsWith"),
HasStartsWithMethodWithName("startswith"));
- const auto ClassWithStartsWithFunction = cxxRecordDecl(anyOf(
- HasStartsWithMethod, hasAnyBase(hasType(hasCanonicalType(hasDeclaration(
- cxxRecordDecl(HasStartsWithMethod)))))));
+ const auto OnClassWithStartsWithFunction =
+ on(hasType(hasCanonicalType(hasDeclaration(cxxRecordDecl(
+ anyOf(HasStartsWithMethod,
+ hasAnyBase(hasType(hasCanonicalType(
+ hasDeclaration(cxxRecordDecl(HasStartsWithMethod)))))))))));
+ const auto HasEndsWithMethodWithName = [](const std::string &Name) {
+ return hasMethod(
+ cxxMethodDecl(hasName(Name), isConst(), parameterCountIs(1))
+ .bind("ends_with_fun"));
+ };
+ const auto HasEndsWithMethod = anyOf(HasEndsWithMethodWithName("ends_with"),
+ HasEndsWithMethodWithName("endsWith"),
+ HasEndsWithMethodWithName("endswith"));
+ const auto OnClassWithEndsWithFunction =
+ on(expr(hasType(hasCanonicalType(hasDeclaration(cxxRecordDecl(
+ anyOf(HasEndsWithMethod,
+ hasAnyBase(hasType(hasCanonicalType(hasDeclaration(
+ cxxRecordDecl(HasEndsWithMethod)))))))))))
+ .bind("haystack"));
+
+ // Case 1: X.find(Y) [!=]= 0 -> starts_with.
const auto FindExpr = cxxMemberCallExpr(
- // A method call with no second argument or the second argument is zero...
anyOf(argumentCountIs(1), hasArgument(1, ZeroLiteral)),
- // ... named find...
callee(cxxMethodDecl(hasName("find")).bind("find_fun")),
- // ... on a class with a starts_with function.
- on(hasType(
- hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction)))),
- // Bind search expression.
- hasArgument(0, expr().bind("search_expr")));
+ OnClassWithStartsWithFunction, hasArgument(0, expr().bind("needle")));
+ // Case 2: X.rfind(Y, 0) [!=]= 0 -> starts_with.
const auto RFindExpr = cxxMemberCallExpr(
- // A method call with a second argument of zero...
hasArgument(1, ZeroLiteral),
- // ... named rfind...
callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")),
- // ... on a class with a starts_with function.
- on(hasType(
- hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction)))),
- // Bind search expression.
- hasArgument(0, expr().bind("search_expr")));
-
- // Match a string literal and an integer or strlen() call matching the length.
- const auto HasStringLiteralAndLengthArgs = [](const auto StringArgIndex,
- const auto LengthArgIndex) {
- return allOf(
- hasArgument(StringArgIndex, stringLiteral().bind("string_literal_arg")),
- hasArgument(LengthArgIndex,
- anyOf(integerLiteral().bind("integer_literal_size_arg"),
- callExpr(callee(functionDecl(parameterCountIs(1),
- hasName("strlen"))),
- hasArgument(0, stringLiteral().bind(
- "strlen_arg"))))));
- };
-
- // Match a string variable and a call to length() or size().
- const auto HasStringVariableAndSizeCallArgs = [](const auto StringArgIndex,
- const auto LengthArgIndex) {
- return allOf(
- hasArgument(StringArgIndex, declRefExpr(hasDeclaration(
- decl().bind("string_var_decl")))),
- hasArgument(LengthArgIndex,
- cxxMemberCallExpr(
- callee(cxxMethodDecl(isConst(), parameterCountIs(0),
- hasAnyName("size", "length"))),
- on(declRefExpr(
- to(decl(equalsBoundNode("string_var_decl"))))))));
- };
-
- // Match either one of the two cases above.
- const auto HasStringAndLengthArgs =
- [HasStringLiteralAndLengthArgs, HasStringVariableAndSizeCallArgs](
- const auto StringArgIndex, const auto LengthArgIndex) {
- return anyOf(
- HasStringLiteralAndLengthArgs(StringArgIndex, LengthArgIndex),
- HasStringVariableAndSizeCallArgs(StringArgIndex, LengthArgIndex));
- };
+ OnClassWithStartsWithFunction, hasArgument(0, expr().bind("needle")));
+ // Case 3: X.compare(0, LEN(Y), Y) [!=]= 0 -> starts_with.
const auto CompareExpr = cxxMemberCallExpr(
- // A method call with three arguments...
- argumentCountIs(3),
- // ... where the first argument is zero...
- hasArgument(0, ZeroLiteral),
- // ... named compare...
+ argumentCountIs(3), hasArgument(0, ZeroLiteral),
callee(cxxMethodDecl(hasName("compare")).bind("find_fun")),
- // ... on a class with a starts_with function...
- on(hasType(
- hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction)))),
- // ... where the third argument is some string and the second a length.
- HasStringAndLengthArgs(2, 1),
- // Bind search expression.
- hasArgument(2, expr().bind("search_expr")));
+ OnClassWithStartsWithFunction, hasArgument(2, expr().bind("needle")),
+ hasArgument(1, lengthExprForStringNode("needle")));
+ // Case 4: X.compare(LEN(X) - LEN(Y), LEN(Y), Y) [!=]= 0 -> ends_with.
+ const auto CompareEndsWithExpr = cxxMemberCallExpr(
+ argumentCountIs(3),
+ callee(cxxMethodDecl(hasName("compare")).bind("find_fun")),
+ OnClassWithEndsWithFunction, hasArgument(2, expr().bind("needle")),
+ hasArgument(1, lengthExprForStringNode("needle")),
+ hasArgument(0,
+ binaryOperator(hasOperatorName("-"),
+ hasLHS(lengthExprForStringNode("haystack")),
+ hasRHS(lengthExprForStringNode("needle")))));
+
+ // All cases comparing to 0.
Finder->addMatcher(
- // Match [=!]= with a zero on one side and (r?)find|compare on the other.
binaryOperator(
hasAnyOperatorName("==", "!="),
- hasOperands(cxxMemberCallExpr(anyOf(FindExpr, RFindExpr, CompareExpr))
+ hasOperands(cxxMemberCallExpr(anyOf(FindExpr, RFindExpr, CompareExpr,
+ CompareEndsWithExpr))
.bind("find_expr"),
ZeroLiteral))
.bind("expr"),
this);
+
+ // Case 5: X.rfind(Y) [!=]= LEN(X) - LEN(Y) -> ends_with.
+ Finder->addMatcher(
+ binaryOperator(
+ hasAnyOperatorName("==", "!="),
+ hasOperands(
+ cxxMemberCallExpr(
+ anyOf(
+ argumentCountIs(1),
+ allOf(argumentCountIs(2),
+ hasArgument(
+ 1,
+ anyOf(declRefExpr(to(varDecl(hasName("npos")))),
+ memberExpr(member(hasName("npos"))))))),
+ callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")),
+ OnClassWithEndsWithFunction,
+ hasArgument(0, expr().bind("needle")))
+ .bind("find_expr"),
+ binaryOperator(hasOperatorName("-"),
+ hasLHS(lengthExprForStringNode("haystack")),
+ hasRHS(lengthExprForStringNode("needle")))))
+ .bind("expr"),
+ this);
}
void UseStartsEndsWithCheck::check(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");
- const auto *SearchExpr = Result.Nodes.getNodeAs<Expr>("search_expr");
+ const auto *SearchExpr = Result.Nodes.getNodeAs<Expr>("needle");
const auto *StartsWithFunction =
Result.Nodes.getNodeAs<CXXMethodDecl>("starts_with_fun");
-
- const auto *StringLiteralArg =
- Result.Nodes.getNodeAs<StringLiteral>("string_literal_arg");
- const auto *IntegerLiteralSizeArg =
- Result.Nodes.getNodeAs<IntegerLiteral>("integer_literal_size_arg");
- const auto *StrlenArg = Result.Nodes.getNodeAs<StringLiteral>("strlen_arg");
-
- // Filter out compare cases where the length does not match string literal.
- if (StringLiteralArg && IntegerLiteralSizeArg &&
- StringLiteralArg->getLength() !=
- IntegerLiteralSizeArg->getValue().getZExtValue()) {
- return;
- }
-
- if (StringLiteralArg && StrlenArg &&
- StringLiteralArg->getLength() != StrlenArg->getLength()) {
- return;
- }
+ const auto *EndsWithFunction =
+ Result.Nodes.getNodeAs<CXXMethodDecl>("ends_with_fun");
+ assert(bool(StartsWithFunction) != bool(EndsWithFunction));
+ const CXXMethodDecl *ReplacementFunction =
+ StartsWithFunction ? StartsWithFunction : EndsWithFunction;
if (ComparisonExpr->getBeginLoc().isMacroID()) {
return;
@@ -154,9 +198,9 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
auto Diagnostic =
diag(FindExpr->getExprLoc(), "use %0 instead of %1() %select{==|!=}2 0")
- << StartsWithFunction->getName() << FindFun->getName() << Neg;
+ << ReplacementFunction->getName() << FindFun->getName() << Neg;
- // Remove possible arguments after search expression and ' [!=]= 0' suffix.
+ // Remove possible arguments after search expression and ' [!=]= .+' suffix.
Diagnostic << FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(
Lexer::getLocForEndOfToken(SearchExpr->getEndLoc(), 0,
@@ -164,16 +208,16 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
ComparisonExpr->getEndLoc()),
")");
- // Remove possible '0 [!=]= ' prefix.
+ // Remove possible '.+ [!=]= ' prefix.
Diagnostic << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
ComparisonExpr->getBeginLoc(), FindExpr->getBeginLoc()));
- // Replace method name by 'starts_with'.
+ // Replace method name by '(starts|ends)_with'.
// Remove possible arguments before search expression.
Diagnostic << FixItHint::CreateReplacement(
CharSourceRange::getCharRange(FindExpr->getExprLoc(),
SearchExpr->getBeginLoc()),
- (StartsWithFunction->getName() + "(").str());
+ (ReplacementFunction->getName() + "(").str());
// Add possible negation '!'.
if (Neg) {
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h
index 840191f321493f..17c2999bda84cd 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h
@@ -14,7 +14,7 @@
namespace clang::tidy::modernize {
/// Checks for common roundabout ways to express ``starts_with`` and
-/// ``ends_with`` and suggests replacing with ``starts_with`` when the method is
+/// ``ends_with`` and suggests replacing with the simpler method when it is
/// available. Notably, this will work with ``std::string`` and
/// ``std::string_view``.
///
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 7d37a4b03222cf..5bcca66df13b0e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -161,6 +161,10 @@ Changes in existing checks
as a replacement for parameters of incomplete C array type in C++20 and
``std::array`` or ``std::vector`` before C++20.
+- Improved :doc:`modernize-use-starts-ends-with
+ <clang-tidy/checks/modernize/use-starts-ends-with>` check to handle two cases
+ that can be replaced with ``ends_with``.
+
- Improved :doc:`modernize-use-std-format
<clang-tidy/checks/modernize/use-std-format>` check to support replacing
member function calls too.
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 34237ede30a30b..721e927e29849f 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
@@ -4,8 +4,8 @@ modernize-use-starts-ends-with
==============================
Checks for common roundabout ways to express ``starts_with`` and ``ends_with``
-and suggests replacing with ``starts_with`` when the method is available.
-Notably, this will work with ``std::string`` and ``std::string_view``.
+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++
@@ -13,6 +13,12 @@ Notably, this will work with ``std::string`` and ``std::string_view``.
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
@@ -22,3 +28,5 @@ becomes
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 */ }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
index 0c160bc182b6eb..a0e8ebbb267cd0 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
@@ -64,6 +64,10 @@ struct basic_string {
constexpr bool starts_with(C ch) const noexcept;
constexpr bool starts_with(const C* s) const;
+ constexpr bool ends_with(std::basic_string_view<C, T> sv) const noexcept;
+ constexpr bool ends_with(C ch) const noexcept;
+ constexpr bool ends_with(const C* s) const;
+
_Type& operator[](size_type);
const _Type& operator[](size_type) const;
@@ -108,6 +112,10 @@ struct basic_string_view {
constexpr bool starts_with(C ch) const noexcept;
constexpr bool starts_with(const C* s) const;
+ constexpr bool ends_with(basic_string_view sv) const noexcept;
+ constexpr bool ends_with(C ch) const noexcept;
+ constexpr bool ends_with(const C* s) const;
+
constexpr int compare(basic_string_view sv) const noexcept;
static constexpr size_t npos = -1;
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 c5b2c86befd1fe..798af260a3b66c 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
@@ -208,6 +208,62 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss,
// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
// CHECK-FIXES: !s.starts_with(sv);
+ s.compare(s.size() - 6, 6, "suffix") == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with("suffix");
+
+ s.compare(s.size() - 6, strlen("abcdef"), "suffix") == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with("suffix");
+
+ std::string suffix = "suffix";
+ s.compare(s.size() - suffix.size(), suffix.size(), suffix) == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with(suffix);
+
+ s.rfind("suffix") == s.size() - 6;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with("suffix");
+
+ s.rfind("suffix") == s.size() - strlen("suffix");
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with("suffix");
+
+ s.rfind(suffix) == s.size() - suffix.size();
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with(suffix);
+
+ s.rfind(suffix, std::string::npos) == s.size() - suffix.size();
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with(suffix);
+
+ s.rfind(suffix) == (s.size() - suffix.size());
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with
+ // CHECK-FIXES: s.ends_with(s...
[truncated]
|
|
Oops, for some reason my software duplicated #105366 when I meant to just rebase and ping. |
nicovank
requested review from
PiotrZSL,
SimplyDanny,
5chmidti and
HerrCai0907
5chmidti
approved these changes
Oct 11, 2024
|
Sounds good, thanks! I will go ahead and rebase and merge this. |
…_with patterns Add support for the following two patterns: ``` haystack.compare(haystack.length() - needle.length(), needle.length(), needle) == 0; haystack.rfind(needle) == (haystack.size() - needle.size()); ```
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
…_with patterns (llvm#110448) Add support for the following two patterns: ``` haystack.compare(haystack.length() - needle.length(), needle.length(), needle) == 0; haystack.rfind(needle) == (haystack.size() - needle.size()); ```
bricknerb
pushed a commit
to bricknerb/llvm-project
that referenced
this pull request
Oct 17, 2024
…_with patterns (llvm#110448) Add support for the following two patterns: ``` haystack.compare(haystack.length() - needle.length(), needle.length(), needle) == 0; haystack.rfind(needle) == (haystack.size() - needle.size()); ```
EricWF
pushed a commit
to efcs/llvm-project
that referenced
this pull request
Oct 22, 2024
…_with patterns (llvm#110448) Add support for the following two patterns: ``` haystack.compare(haystack.length() - needle.length(), needle.length(), needle) == 0; haystack.rfind(needle) == (haystack.size() - needle.size()); ```
nicovank
added a commit
to nicovank/llvm-project
that referenced
this pull request
Nov 14, 2024
…or message In one of the cases recently added to this check in llvm#110448, the error message is no longer accurate as the comparison is not with zero. llvm#116033 brought my attention to this as it may add another comparison without zero. Remove the `[!=] 0` part of the diagnostic. This is in line with some other checks e.g. modernize-use-emplace. ``` > cat tmp.cpp #include <string> bool f(std::string u, std::string v) { return u.rfind(v) == u.size() - v.size(); } # Before. > ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp -- -std=c++20 tmp.cpp:3:12: warning: use ends_with instead of rfind() == 0 [modernize-use-starts-ends-with] 3 | return u.rfind(v) == u.size() - v.size(); | ^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~ | ends_with( ) # After. > ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp -- -std=c++20 tmp.cpp:3:12: warning: use ends_with instead of rfind [modernize-use-starts-ends-with] 3 | return u.rfind(v) == u.size() - v.size(); | ^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~ | ends_with( ) ```
nicovank
added a commit
to nicovank/llvm-project
that referenced
this pull request
Nov 16, 2024
…or message In one of the cases recently added to this check in llvm#110448, the error message is no longer accurate as the comparison is not with zero. llvm#116033 brought my attention to this as it may add another comparison without zero. Remove the `[!=] 0` part of the diagnostic. This is in line with some other checks e.g. modernize-use-emplace. ``` > cat tmp.cpp #include <string> bool f(std::string u, std::string v) { return u.rfind(v) == u.size() - v.size(); } # Before. > ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp -- -std=c++20 tmp.cpp:3:12: warning: use ends_with instead of rfind() == 0 [modernize-use-starts-ends-with] 3 | return u.rfind(v) == u.size() - v.size(); | ^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~ | ends_with( ) # After. > ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp -- -std=c++20 tmp.cpp:3:12: warning: use ends_with instead of rfind [modernize-use-starts-ends-with] 3 | return u.rfind(v) == u.size() - v.size(); | ^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~ | ends_with( ) ```
nicovank
added a commit
to nicovank/llvm-project
that referenced
this pull request
Nov 16, 2024
…or message In one of the cases recently added to this check in llvm#110448, the error message is no longer accurate as the comparison is not with zero. llvm#116033 brought my attention to this as it may add another comparison without zero. Remove the `[!=] 0` part of the diagnostic. This is in line with some other checks e.g. modernize-use-emplace. ``` > cat tmp.cpp #include <string> bool f(std::string u, std::string v) { return u.rfind(v) == u.size() - v.size(); } # Before. > ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp -- -std=c++20 tmp.cpp:3:12: warning: use ends_with instead of rfind() == 0 [modernize-use-starts-ends-with] 3 | return u.rfind(v) == u.size() - v.size(); | ^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~ | ends_with( ) # After. > ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp -- -std=c++20 tmp.cpp:3:12: warning: use ends_with instead of rfind [modernize-use-starts-ends-with] 3 | return u.rfind(v) == u.size() - v.size(); | ^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~ | ends_with( ) ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add support for the following two patterns: