Skip to content

Commit 425cc9b

Browse files
nicovankDanielCChen
nicovank
authored and
DanielCChen
committed
[clang-tidy][modernize-use-starts-ends-with] Add support for two ends_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()); ```
1 parent 6f9c7e8 commit 425cc9b

File tree

6 files changed

+215
-94
lines changed

6 files changed

+215
-94
lines changed

‎clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp‎

Lines changed: 135 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "UseStartsEndsWithCheck.h"
1010

11+
#include "../utils/ASTUtils.h"
1112
#include "../utils/OptionsUtils.h"
1213
#include "clang/Lex/Lexer.h"
1314

@@ -16,13 +17,71 @@
1617
using namespace clang::ast_matchers;
1718

1819
namespace clang::tidy::modernize {
20+
struct NotLengthExprForStringNode {
21+
NotLengthExprForStringNode(std::string ID, DynTypedNode Node,
22+
ASTContext *Context)
23+
: ID(std::move(ID)), Node(std::move(Node)), Context(Context) {}
24+
bool operator()(const internal::BoundNodesMap &Nodes) const {
25+
// Match a string literal and an integer size or strlen() call.
26+
if (const auto *StringLiteralNode = Nodes.getNodeAs<StringLiteral>(ID)) {
27+
if (const auto *IntegerLiteralSizeNode = Node.get<IntegerLiteral>()) {
28+
return StringLiteralNode->getLength() !=
29+
IntegerLiteralSizeNode->getValue().getZExtValue();
30+
}
31+
32+
if (const auto *StrlenNode = Node.get<CallExpr>()) {
33+
if (StrlenNode->getDirectCallee()->getName() != "strlen" ||
34+
StrlenNode->getNumArgs() != 1) {
35+
return true;
36+
}
37+
38+
if (const auto *StrlenArgNode = dyn_cast<StringLiteral>(
39+
StrlenNode->getArg(0)->IgnoreParenImpCasts())) {
40+
return StrlenArgNode->getLength() != StringLiteralNode->getLength();
41+
}
42+
}
43+
}
44+
45+
// Match a string variable and a call to length() or size().
46+
if (const auto *ExprNode = Nodes.getNodeAs<Expr>(ID)) {
47+
if (const auto *MemberCallNode = Node.get<CXXMemberCallExpr>()) {
48+
const CXXMethodDecl *MethodDeclNode = MemberCallNode->getMethodDecl();
49+
const StringRef Name = MethodDeclNode->getName();
50+
if (!MethodDeclNode->isConst() || MethodDeclNode->getNumParams() != 0 ||
51+
(Name != "size" && Name != "length")) {
52+
return true;
53+
}
54+
55+
if (const auto *OnNode =
56+
dyn_cast<Expr>(MemberCallNode->getImplicitObjectArgument())) {
57+
return !utils::areStatementsIdentical(OnNode->IgnoreParenImpCasts(),
58+
ExprNode->IgnoreParenImpCasts(),
59+
*Context);
60+
}
61+
}
62+
}
63+
64+
return true;
65+
}
66+
67+
private:
68+
std::string ID;
69+
DynTypedNode Node;
70+
ASTContext *Context;
71+
};
72+
73+
AST_MATCHER_P(Expr, lengthExprForStringNode, std::string, ID) {
74+
return Builder->removeBindings(NotLengthExprForStringNode(
75+
ID, DynTypedNode::create(Node), &(Finder->getASTContext())));
76+
}
1977

2078
UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name,
2179
ClangTidyContext *Context)
2280
: ClangTidyCheck(Name, Context) {}
2381

2482
void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
2583
const auto ZeroLiteral = integerLiteral(equals(0));
84+
2685
const auto HasStartsWithMethodWithName = [](const std::string &Name) {
2786
return hasMethod(
2887
cxxMethodDecl(hasName(Name), isConst(), parameterCountIs(1))
@@ -32,119 +91,104 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
3291
anyOf(HasStartsWithMethodWithName("starts_with"),
3392
HasStartsWithMethodWithName("startsWith"),
3493
HasStartsWithMethodWithName("startswith"));
35-
const auto ClassWithStartsWithFunction = cxxRecordDecl(anyOf(
36-
HasStartsWithMethod, hasAnyBase(hasType(hasCanonicalType(hasDeclaration(
37-
cxxRecordDecl(HasStartsWithMethod)))))));
94+
const auto OnClassWithStartsWithFunction =
95+
on(hasType(hasCanonicalType(hasDeclaration(cxxRecordDecl(
96+
anyOf(HasStartsWithMethod,
97+
hasAnyBase(hasType(hasCanonicalType(
98+
hasDeclaration(cxxRecordDecl(HasStartsWithMethod)))))))))));
3899

100+
const auto HasEndsWithMethodWithName = [](const std::string &Name) {
101+
return hasMethod(
102+
cxxMethodDecl(hasName(Name), isConst(), parameterCountIs(1))
103+
.bind("ends_with_fun"));
104+
};
105+
const auto HasEndsWithMethod = anyOf(HasEndsWithMethodWithName("ends_with"),
106+
HasEndsWithMethodWithName("endsWith"),
107+
HasEndsWithMethodWithName("endswith"));
108+
const auto OnClassWithEndsWithFunction =
109+
on(expr(hasType(hasCanonicalType(hasDeclaration(cxxRecordDecl(
110+
anyOf(HasEndsWithMethod,
111+
hasAnyBase(hasType(hasCanonicalType(hasDeclaration(
112+
cxxRecordDecl(HasEndsWithMethod)))))))))))
113+
.bind("haystack"));
114+
115+
// Case 1: X.find(Y) [!=]= 0 -> starts_with.
39116
const auto FindExpr = cxxMemberCallExpr(
40-
// A method call with no second argument or the second argument is zero...
41117
anyOf(argumentCountIs(1), hasArgument(1, ZeroLiteral)),
42-
// ... named find...
43118
callee(cxxMethodDecl(hasName("find")).bind("find_fun")),
44-
// ... on a class with a starts_with function.
45-
on(hasType(
46-
hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction)))),
47-
// Bind search expression.
48-
hasArgument(0, expr().bind("search_expr")));
119+
OnClassWithStartsWithFunction, hasArgument(0, expr().bind("needle")));
49120

121+
// Case 2: X.rfind(Y, 0) [!=]= 0 -> starts_with.
50122
const auto RFindExpr = cxxMemberCallExpr(
51-
// A method call with a second argument of zero...
52123
hasArgument(1, ZeroLiteral),
53-
// ... named rfind...
54124
callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")),
55-
// ... on a class with a starts_with function.
56-
on(hasType(
57-
hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction)))),
58-
// Bind search expression.
59-
hasArgument(0, expr().bind("search_expr")));
60-
61-
// Match a string literal and an integer or strlen() call matching the length.
62-
const auto HasStringLiteralAndLengthArgs = [](const auto StringArgIndex,
63-
const auto LengthArgIndex) {
64-
return allOf(
65-
hasArgument(StringArgIndex, stringLiteral().bind("string_literal_arg")),
66-
hasArgument(LengthArgIndex,
67-
anyOf(integerLiteral().bind("integer_literal_size_arg"),
68-
callExpr(callee(functionDecl(parameterCountIs(1),
69-
hasName("strlen"))),
70-
hasArgument(0, stringLiteral().bind(
71-
"strlen_arg"))))));
72-
};
73-
74-
// Match a string variable and a call to length() or size().
75-
const auto HasStringVariableAndSizeCallArgs = [](const auto StringArgIndex,
76-
const auto LengthArgIndex) {
77-
return allOf(
78-
hasArgument(StringArgIndex, declRefExpr(hasDeclaration(
79-
decl().bind("string_var_decl")))),
80-
hasArgument(LengthArgIndex,
81-
cxxMemberCallExpr(
82-
callee(cxxMethodDecl(isConst(), parameterCountIs(0),
83-
hasAnyName("size", "length"))),
84-
on(declRefExpr(
85-
to(decl(equalsBoundNode("string_var_decl"))))))));
86-
};
87-
88-
// Match either one of the two cases above.
89-
const auto HasStringAndLengthArgs =
90-
[HasStringLiteralAndLengthArgs, HasStringVariableAndSizeCallArgs](
91-
const auto StringArgIndex, const auto LengthArgIndex) {
92-
return anyOf(
93-
HasStringLiteralAndLengthArgs(StringArgIndex, LengthArgIndex),
94-
HasStringVariableAndSizeCallArgs(StringArgIndex, LengthArgIndex));
95-
};
125+
OnClassWithStartsWithFunction, hasArgument(0, expr().bind("needle")));
96126

127+
// Case 3: X.compare(0, LEN(Y), Y) [!=]= 0 -> starts_with.
97128
const auto CompareExpr = cxxMemberCallExpr(
98-
// A method call with three arguments...
99-
argumentCountIs(3),
100-
// ... where the first argument is zero...
101-
hasArgument(0, ZeroLiteral),
102-
// ... named compare...
129+
argumentCountIs(3), hasArgument(0, ZeroLiteral),
103130
callee(cxxMethodDecl(hasName("compare")).bind("find_fun")),
104-
// ... on a class with a starts_with function...
105-
on(hasType(
106-
hasCanonicalType(hasDeclaration(ClassWithStartsWithFunction)))),
107-
// ... where the third argument is some string and the second a length.
108-
HasStringAndLengthArgs(2, 1),
109-
// Bind search expression.
110-
hasArgument(2, expr().bind("search_expr")));
131+
OnClassWithStartsWithFunction, hasArgument(2, expr().bind("needle")),
132+
hasArgument(1, lengthExprForStringNode("needle")));
111133

134+
// Case 4: X.compare(LEN(X) - LEN(Y), LEN(Y), Y) [!=]= 0 -> ends_with.
135+
const auto CompareEndsWithExpr = cxxMemberCallExpr(
136+
argumentCountIs(3),
137+
callee(cxxMethodDecl(hasName("compare")).bind("find_fun")),
138+
OnClassWithEndsWithFunction, hasArgument(2, expr().bind("needle")),
139+
hasArgument(1, lengthExprForStringNode("needle")),
140+
hasArgument(0,
141+
binaryOperator(hasOperatorName("-"),
142+
hasLHS(lengthExprForStringNode("haystack")),
143+
hasRHS(lengthExprForStringNode("needle")))));
144+
145+
// All cases comparing to 0.
112146
Finder->addMatcher(
113-
// Match [=!]= with a zero on one side and (r?)find|compare on the other.
114147
binaryOperator(
115148
hasAnyOperatorName("==", "!="),
116-
hasOperands(cxxMemberCallExpr(anyOf(FindExpr, RFindExpr, CompareExpr))
149+
hasOperands(cxxMemberCallExpr(anyOf(FindExpr, RFindExpr, CompareExpr,
150+
CompareEndsWithExpr))
117151
.bind("find_expr"),
118152
ZeroLiteral))
119153
.bind("expr"),
120154
this);
155+
156+
// Case 5: X.rfind(Y) [!=]= LEN(X) - LEN(Y) -> ends_with.
157+
Finder->addMatcher(
158+
binaryOperator(
159+
hasAnyOperatorName("==", "!="),
160+
hasOperands(
161+
cxxMemberCallExpr(
162+
anyOf(
163+
argumentCountIs(1),
164+
allOf(argumentCountIs(2),
165+
hasArgument(
166+
1,
167+
anyOf(declRefExpr(to(varDecl(hasName("npos")))),
168+
memberExpr(member(hasName("npos"))))))),
169+
callee(cxxMethodDecl(hasName("rfind")).bind("find_fun")),
170+
OnClassWithEndsWithFunction,
171+
hasArgument(0, expr().bind("needle")))
172+
.bind("find_expr"),
173+
binaryOperator(hasOperatorName("-"),
174+
hasLHS(lengthExprForStringNode("haystack")),
175+
hasRHS(lengthExprForStringNode("needle")))))
176+
.bind("expr"),
177+
this);
121178
}
122179

123180
void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
124181
const auto *ComparisonExpr = Result.Nodes.getNodeAs<BinaryOperator>("expr");
125182
const auto *FindExpr = Result.Nodes.getNodeAs<CXXMemberCallExpr>("find_expr");
126183
const auto *FindFun = Result.Nodes.getNodeAs<CXXMethodDecl>("find_fun");
127-
const auto *SearchExpr = Result.Nodes.getNodeAs<Expr>("search_expr");
184+
const auto *SearchExpr = Result.Nodes.getNodeAs<Expr>("needle");
128185
const auto *StartsWithFunction =
129186
Result.Nodes.getNodeAs<CXXMethodDecl>("starts_with_fun");
130-
131-
const auto *StringLiteralArg =
132-
Result.Nodes.getNodeAs<StringLiteral>("string_literal_arg");
133-
const auto *IntegerLiteralSizeArg =
134-
Result.Nodes.getNodeAs<IntegerLiteral>("integer_literal_size_arg");
135-
const auto *StrlenArg = Result.Nodes.getNodeAs<StringLiteral>("strlen_arg");
136-
137-
// Filter out compare cases where the length does not match string literal.
138-
if (StringLiteralArg && IntegerLiteralSizeArg &&
139-
StringLiteralArg->getLength() !=
140-
IntegerLiteralSizeArg->getValue().getZExtValue()) {
141-
return;
142-
}
143-
144-
if (StringLiteralArg && StrlenArg &&
145-
StringLiteralArg->getLength() != StrlenArg->getLength()) {
146-
return;
147-
}
187+
const auto *EndsWithFunction =
188+
Result.Nodes.getNodeAs<CXXMethodDecl>("ends_with_fun");
189+
assert(bool(StartsWithFunction) != bool(EndsWithFunction));
190+
const CXXMethodDecl *ReplacementFunction =
191+
StartsWithFunction ? StartsWithFunction : EndsWithFunction;
148192

149193
if (ComparisonExpr->getBeginLoc().isMacroID()) {
150194
return;
@@ -154,26 +198,26 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
154198

155199
auto Diagnostic =
156200
diag(FindExpr->getExprLoc(), "use %0 instead of %1() %select{==|!=}2 0")
157-
<< StartsWithFunction->getName() << FindFun->getName() << Neg;
201+
<< ReplacementFunction->getName() << FindFun->getName() << Neg;
158202

159-
// Remove possible arguments after search expression and ' [!=]= 0' suffix.
203+
// Remove possible arguments after search expression and ' [!=]= .+' suffix.
160204
Diagnostic << FixItHint::CreateReplacement(
161205
CharSourceRange::getTokenRange(
162206
Lexer::getLocForEndOfToken(SearchExpr->getEndLoc(), 0,
163207
*Result.SourceManager, getLangOpts()),
164208
ComparisonExpr->getEndLoc()),
165209
")");
166210

167-
// Remove possible '0 [!=]= ' prefix.
211+
// Remove possible '.+ [!=]= ' prefix.
168212
Diagnostic << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
169213
ComparisonExpr->getBeginLoc(), FindExpr->getBeginLoc()));
170214

171-
// Replace method name by 'starts_with'.
215+
// Replace method name by '(starts|ends)_with'.
172216
// Remove possible arguments before search expression.
173217
Diagnostic << FixItHint::CreateReplacement(
174218
CharSourceRange::getCharRange(FindExpr->getExprLoc(),
175219
SearchExpr->getBeginLoc()),
176-
(StartsWithFunction->getName() + "(").str());
220+
(ReplacementFunction->getName() + "(").str());
177221

178222
// Add possible negation '!'.
179223
if (Neg) {

‎clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
namespace clang::tidy::modernize {
1515

1616
/// Checks for common roundabout ways to express ``starts_with`` and
17-
/// ``ends_with`` and suggests replacing with ``starts_with`` when the method is
17+
/// ``ends_with`` and suggests replacing with the simpler method when it is
1818
/// available. Notably, this will work with ``std::string`` and
1919
/// ``std::string_view``.
2020
///

‎clang-tools-extra/docs/ReleaseNotes.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,10 @@ Changes in existing checks
202202
<clang-tidy/checks/modernize/use-nullptr>` check to also recognize
203203
``NULL``/``__null`` (but not ``0``) when used with a templated type.
204204

205+
- Improved :doc:`modernize-use-starts-ends-with
206+
<clang-tidy/checks/modernize/use-starts-ends-with>` check to handle two cases
207+
that can be replaced with ``ends_with``
208+
205209
- Improved :doc:`modernize-use-std-format
206210
<clang-tidy/checks/modernize/use-std-format>` check to support replacing
207211
member function calls too and to only expand macros starting with ``PRI``

‎clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,21 @@ modernize-use-starts-ends-with
44
==============================
55

66
Checks for common roundabout ways to express ``starts_with`` and ``ends_with``
7-
and suggests replacing with ``starts_with`` when the method is available.
8-
Notably, this will work with ``std::string`` and ``std::string_view``.
7+
and suggests replacing with the simpler method when it is available. Notably,
8+
this will work with ``std::string`` and ``std::string_view``.
99

1010
.. code-block:: c++
1111

1212
std::string s = "...";
1313
if (s.find("prefix") == 0) { /* do something */ }
1414
if (s.rfind("prefix", 0) == 0) { /* do something */ }
1515
if (s.compare(0, strlen("prefix"), "prefix") == 0) { /* do something */ }
16+
if (s.compare(s.size() - strlen("suffix"), strlen("suffix"), "suffix") == 0) {
17+
/* do something */
18+
}
19+
if (s.rfind("suffix") == (s.length() - 6)) {
20+
/* do something */
21+
}
1622
1723
becomes
1824

@@ -22,3 +28,5 @@ becomes
2228
if (s.starts_with("prefix")) { /* do something */ }
2329
if (s.starts_with("prefix")) { /* do something */ }
2430
if (s.starts_with("prefix")) { /* do something */ }
31+
if (s.ends_with("suffix")) { /* do something */ }
32+
if (s.ends_with("suffix")) { /* do something */ }

‎clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ struct basic_string {
6464
constexpr bool starts_with(C ch) const noexcept;
6565
constexpr bool starts_with(const C* s) const;
6666

67+
constexpr bool ends_with(std::basic_string_view<C, T> sv) const noexcept;
68+
constexpr bool ends_with(C ch) const noexcept;
69+
constexpr bool ends_with(const C* s) const;
70+
6771
_Type& operator[](size_type);
6872
const _Type& operator[](size_type) const;
6973

@@ -108,6 +112,10 @@ struct basic_string_view {
108112
constexpr bool starts_with(C ch) const noexcept;
109113
constexpr bool starts_with(const C* s) const;
110114

115+
constexpr bool ends_with(basic_string_view sv) const noexcept;
116+
constexpr bool ends_with(C ch) const noexcept;
117+
constexpr bool ends_with(const C* s) const;
118+
111119
constexpr int compare(basic_string_view sv) const noexcept;
112120

113121
static constexpr size_t npos = -1;

0 commit comments

Comments
 (0)