Skip to content

Commit 23b4bcd

Browse files
hjanuschkahjanuschka
committed
[clang-tidy] Enhance modernize-use-starts-ends-with with substr detection
Enhances the modernize-use-starts-ends-with check to detect substr-based patterns that can be replaced with starts_with() (C++20). This improves code readability and efficiency by avoiding temporary string creation. New patterns detected: str.substr(0, n) == "foo" -> str.starts_with("foo") "foo" == str.substr(0, n) -> str.starts_with("foo") str.substr(0, n) != "foo" -> !str.starts_with("foo") str.substr(0, strlen("foo")) == "foo" -> str.starts_with("foo") str.substr(0, prefix.size()) == "foo" -> str.starts_with("foo") The enhancement: - Integrates with existing starts_with patterns - Handles substr with zero first argument - Supports length via literals, strlen(), and size()/length() - Validates string literal length matches - Handles both == and != operators Part of modernize-use-starts-ends-with check.
1 parent 5845688 commit 23b4bcd

File tree

6 files changed

+216
-99
lines changed

6 files changed

+216
-99
lines changed

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

Lines changed: 91 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,17 @@ struct NotLengthExprForStringNode {
3030
IntegerLiteralSizeNode->getValue().getZExtValue();
3131
}
3232

33+
if (const auto *DeclRefNode = Node.get<DeclRefExpr>()) {
34+
if (const auto *VD = dyn_cast<VarDecl>(DeclRefNode->getDecl())) {
35+
if (VD->hasInit() && VD->getType().isConstQualified()) {
36+
if (const auto *Init = dyn_cast<IntegerLiteral>(VD->getInit())) {
37+
return StringLiteralNode->getLength() !=
38+
Init->getValue().getZExtValue();
39+
}
40+
}
41+
}
42+
}
43+
3344
if (const auto *StrlenNode = Node.get<CallExpr>()) {
3445
if (StrlenNode->getDirectCallee()->getName() != "strlen" ||
3546
StrlenNode->getNumArgs() != 1) {
@@ -171,10 +182,64 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
171182
hasRHS(lengthExprForStringNode("needle")))))
172183
.bind("expr"),
173184
this);
185+
186+
Finder->addMatcher(
187+
cxxOperatorCallExpr(
188+
hasAnyOperatorName("==", "!="),
189+
anyOf(
190+
hasOperands(
191+
cxxMemberCallExpr(
192+
argumentCountIs(2), hasArgument(0, ZeroLiteral),
193+
hasArgument(1, lengthExprForStringNode("needle")),
194+
callee(
195+
cxxMethodDecl(hasName("substr"),
196+
ofClass(OnClassWithStartsWithFunction))
197+
.bind("find_fun")))
198+
.bind("find_expr"),
199+
expr().bind("needle")),
200+
hasOperands(expr().bind("needle"),
201+
cxxMemberCallExpr(
202+
argumentCountIs(2), hasArgument(0, ZeroLiteral),
203+
hasArgument(1, lengthExprForStringNode("needle")),
204+
callee(cxxMethodDecl(
205+
hasName("substr"),
206+
ofClass(OnClassWithStartsWithFunction))
207+
.bind("find_fun")))
208+
.bind("find_expr"))))
209+
.bind("expr"),
210+
this);
211+
}
212+
213+
bool UseStartsEndsWithCheck::isNegativeComparison(const Expr* ComparisonExpr) {
214+
// Handle direct != operator
215+
if (const auto *BO = llvm::dyn_cast<BinaryOperator>(ComparisonExpr)) {
216+
return BO->getOpcode() == BO_NE;
217+
}
218+
219+
// Handle operator!= call
220+
if (const auto *Op = llvm::dyn_cast<CXXOperatorCallExpr>(ComparisonExpr)) {
221+
return Op->getOperator() == OO_ExclaimEqual;
222+
}
223+
224+
// Handle rewritten !(expr == expr)
225+
if (const auto *UO = llvm::dyn_cast<UnaryOperator>(ComparisonExpr)) {
226+
if (UO->getOpcode() == UO_LNot) {
227+
if (const auto *InnerBO =
228+
llvm::dyn_cast<BinaryOperator>(UO->getSubExpr()->IgnoreParens())) {
229+
return InnerBO->getOpcode() == BO_EQ;
230+
}
231+
if (const auto *InnerOp =
232+
llvm::dyn_cast<CXXOperatorCallExpr>(UO->getSubExpr()->IgnoreParens())) {
233+
return InnerOp->getOperator() == OO_EqualEqual;
234+
}
235+
}
236+
}
237+
238+
return false;
174239
}
175240

176241
void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
177-
const auto *ComparisonExpr = Result.Nodes.getNodeAs<BinaryOperator>("expr");
242+
const auto *ComparisonExpr = Result.Nodes.getNodeAs<Expr>("expr");
178243
const auto *FindExpr = Result.Nodes.getNodeAs<CXXMemberCallExpr>("find_expr");
179244
const auto *FindFun = Result.Nodes.getNodeAs<CXXMethodDecl>("find_fun");
180245
const auto *SearchExpr = Result.Nodes.getNodeAs<Expr>("needle");
@@ -183,40 +248,43 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
183248
const auto *EndsWithFunction =
184249
Result.Nodes.getNodeAs<CXXMethodDecl>("ends_with_fun");
185250
assert(bool(StartsWithFunction) != bool(EndsWithFunction));
251+
186252
const CXXMethodDecl *ReplacementFunction =
187253
StartsWithFunction ? StartsWithFunction : EndsWithFunction;
188254

189255
if (ComparisonExpr->getBeginLoc().isMacroID())
190256
return;
191257

192-
const bool Neg = ComparisonExpr->getOpcode() == BO_NE;
258+
bool Neg = isNegativeComparison(ComparisonExpr);
193259

194-
auto Diagnostic =
195-
diag(FindExpr->getExprLoc(), "use %0 instead of %1() %select{==|!=}2 0")
196-
<< ReplacementFunction->getName() << FindFun->getName() << Neg;
260+
// Retrieve the source text of the search expression.
261+
const auto SearchExprText = Lexer::getSourceText(
262+
CharSourceRange::getTokenRange(SearchExpr->getSourceRange()),
263+
*Result.SourceManager, Result.Context->getLangOpts());
197264

198-
// Remove possible arguments after search expression and ' [!=]= .+' suffix.
199-
Diagnostic << FixItHint::CreateReplacement(
200-
CharSourceRange::getTokenRange(
201-
Lexer::getLocForEndOfToken(SearchExpr->getEndLoc(), 0,
202-
*Result.SourceManager, getLangOpts()),
203-
ComparisonExpr->getEndLoc()),
204-
")");
265+
auto Diag = diag(ComparisonExpr->getBeginLoc(),
266+
"use %0 instead of %1 %select{==|!=}2 ")
267+
<< ReplacementFunction->getName() << FindFun->getNameAsString()
268+
<< Neg;
205269

206-
// Remove possible '.+ [!=]= ' prefix.
207-
Diagnostic << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
270+
// Remove everything before the function call.
271+
Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
208272
ComparisonExpr->getBeginLoc(), FindExpr->getBeginLoc()));
209273

210-
// Replace method name by '(starts|ends)_with'.
211-
// Remove possible arguments before search expression.
212-
Diagnostic << FixItHint::CreateReplacement(
213-
CharSourceRange::getCharRange(FindExpr->getExprLoc(),
214-
SearchExpr->getBeginLoc()),
215-
(ReplacementFunction->getName() + "(").str());
274+
// Rename the function to `starts_with` or `ends_with`.
275+
Diag << FixItHint::CreateReplacement(FindExpr->getExprLoc(),
276+
ReplacementFunction->getName());
216277

217-
// Add possible negation '!'.
218-
if (Neg)
219-
Diagnostic << FixItHint::CreateInsertion(FindExpr->getBeginLoc(), "!");
278+
// Replace arguments and everything after the function call.
279+
Diag << FixItHint::CreateReplacement(
280+
CharSourceRange::getTokenRange(FindExpr->getArg(0)->getBeginLoc(),
281+
ComparisonExpr->getEndLoc()),
282+
(SearchExprText + ")").str());
283+
284+
// Add negation if necessary.
285+
if (Neg) {
286+
Diag << FixItHint::CreateInsertion(FindExpr->getBeginLoc(), "!");
287+
}
220288
}
221289

222290
} // namespace clang::tidy::modernize

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class UseStartsEndsWithCheck : public ClangTidyCheck {
2525
UseStartsEndsWithCheck(StringRef Name, ClangTidyContext *Context);
2626
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
2727
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
28+
bool isNegativeComparison(const Expr* ComparisonExpr);
2829
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
2930
return LangOpts.CPlusPlus;
3031
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,10 @@ Changes in existing checks
244244

245245
- Improved :doc:`modernize-use-starts-ends-with
246246
<clang-tidy/checks/modernize/use-starts-ends-with>` check to handle two cases
247-
that can be replaced with ``ends_with``
247+
that can be replaced with ``ends_with`` and detect patterns using ``substr``
248+
that can be replaced with ``starts_with``. Now handles cases like
249+
``str.substr(0, n) == "literal"``, with support for length determination through
250+
integer literals, ``strlen()``, and ``size()``/``length()`` member functions.
248251

249252
- Improved :doc:`modernize-use-std-format
250253
<clang-tidy/checks/modernize/use-std-format>` check to support replacing

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

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,26 +7,16 @@ Checks for common roundabout ways to express ``starts_with`` and ``ends_with``
77
and suggests replacing with the simpler method when it is available. Notably,
88
this will work with ``std::string`` and ``std::string_view``.
99

10-
.. code-block:: c++
10+
The check handles the following expressions:
1111

12-
std::string s = "...";
13-
if (s.find("prefix") == 0) { /* do something */ }
14-
if (s.rfind("prefix", 0) == 0) { /* do something */ }
15-
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-
}
22-
23-
becomes
24-
25-
.. code-block:: c++
26-
27-
std::string s = "...";
28-
if (s.starts_with("prefix")) { /* do something */ }
29-
if (s.starts_with("prefix")) { /* do something */ }
30-
if (s.starts_with("prefix")) { /* do something */ }
31-
if (s.ends_with("suffix")) { /* do something */ }
32-
if (s.ends_with("suffix")) { /* do something */ }
12+
==================================================== =====================
13+
Expression Replacement
14+
---------------------------------------------------- ---------------------
15+
``u.find(v) == 0`` ``u.starts_with(v)``
16+
``u.rfind(v, 0) != 0`` ``!u.starts_with(v)``
17+
``u.compare(0, v.size(), v) == 0`` ``u.starts_with(v)``
18+
``u.substr(0, v.size()) == v`` ``u.starts_with(v)``
19+
``v != u.substr(0, v.size())`` ``!u.starts_with(v)``
20+
``u.compare(u.size() - v.size(), v.size(), v) == 0`` ``u.ends_with(v)``
21+
``u.rfind(v) == u.size() - v.size()`` ``u.ends_with(v)``
22+
==================================================== =====================

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ struct basic_string {
6060
_Type& insert(size_type pos, const C* s);
6161
_Type& insert(size_type pos, const C* s, size_type n);
6262

63+
_Type substr(size_type pos = 0, size_type count = npos) const;
64+
6365
constexpr bool starts_with(std::basic_string_view<C, T> sv) const noexcept;
6466
constexpr bool starts_with(C ch) const noexcept;
6567
constexpr bool starts_with(const C* s) const;

0 commit comments

Comments
 (0)