[clang-tools-extra] fd8fc5e - [clang-tidy] abseil-string-find-startswith: detect `s.rfind(z, 0) == 0`
Dmitri Gribenko via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 22 07:49:55 PST 2021
Author: Ivan Gerasimov
Date: 2021-12-22T16:45:51+01:00
New Revision: fd8fc5e8d93849f4a2c8dea087690b1a0e6ea7df
URL: https://github.com/llvm/llvm-project/commit/fd8fc5e8d93849f4a2c8dea087690b1a0e6ea7df
DIFF: https://github.com/llvm/llvm-project/commit/fd8fc5e8d93849f4a2c8dea087690b1a0e6ea7df.diff
LOG: [clang-tidy] abseil-string-find-startswith: detect `s.rfind(z, 0) == 0`
Suggest converting `std::string::rfind()` calls to `absl::StartsWith()`
where possible.
Added:
Modified:
clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
clang-tools-extra/docs/clang-tidy/checks/abseil-string-find-startswith.rst
clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-startswith.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp b/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
index 5741c0d505d51..e834c8a124590 100644
--- a/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
+++ b/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
@@ -40,7 +40,7 @@ void StringFindStartswithCheck::registerMatchers(MatchFinder *Finder) {
auto StringFind = cxxMemberCallExpr(
// .find()-call on a string...
- callee(cxxMethodDecl(hasName("find"))),
+ callee(cxxMethodDecl(hasName("find")).bind("findfun")),
on(hasType(StringType)),
// ... with some search expression ...
hasArgument(0, expr().bind("needle")),
@@ -55,6 +55,25 @@ void StringFindStartswithCheck::registerMatchers(MatchFinder *Finder) {
ignoringParenImpCasts(StringFind.bind("findexpr"))))
.bind("expr"),
this);
+
+ auto StringRFind = cxxMemberCallExpr(
+ // .rfind()-call on a string...
+ callee(cxxMethodDecl(hasName("rfind")).bind("findfun")),
+ on(hasType(StringType)),
+ // ... with some search expression ...
+ hasArgument(0, expr().bind("needle")),
+ // ... and "0" as second argument.
+ hasArgument(1, ZeroLiteral));
+
+ Finder->addMatcher(
+ // Match [=!]= with either a zero or npos on one side and a string.rfind
+ // on the other.
+ binaryOperator(
+ hasAnyOperatorName("==", "!="),
+ hasOperands(ignoringParenImpCasts(ZeroLiteral),
+ ignoringParenImpCasts(StringRFind.bind("findexpr"))))
+ .bind("expr"),
+ this);
}
void StringFindStartswithCheck::check(const MatchFinder::MatchResult &Result) {
@@ -69,6 +88,11 @@ void StringFindStartswithCheck::check(const MatchFinder::MatchResult &Result) {
const Expr *Haystack = Result.Nodes.getNodeAs<CXXMemberCallExpr>("findexpr")
->getImplicitObjectArgument();
assert(Haystack != nullptr);
+ const CXXMethodDecl *FindFun =
+ Result.Nodes.getNodeAs<CXXMethodDecl>("findfun");
+ assert(FindFun != nullptr);
+
+ bool Rev = FindFun->getName().contains("rfind");
if (ComparisonExpr->getBeginLoc().isMacroID())
return;
@@ -86,10 +110,11 @@ void StringFindStartswithCheck::check(const MatchFinder::MatchResult &Result) {
bool Neg = ComparisonExpr->getOpcode() == BO_NE;
// Create the warning message and a FixIt hint replacing the original expr.
- auto Diagnostic = diag(ComparisonExpr->getBeginLoc(),
- "use %select{absl::StartsWith|!absl::StartsWith}0 "
- "instead of find() %select{==|!=}0 0")
- << Neg;
+ auto Diagnostic =
+ diag(ComparisonExpr->getBeginLoc(),
+ "use %select{absl::StartsWith|!absl::StartsWith}0 "
+ "instead of %select{find()|rfind()}1 %select{==|!=}0 0")
+ << Neg << Rev;
Diagnostic << FixItHint::CreateReplacement(
ComparisonExpr->getSourceRange(),
diff --git a/clang-tools-extra/docs/clang-tidy/checks/abseil-string-find-startswith.rst b/clang-tools-extra/docs/clang-tidy/checks/abseil-string-find-startswith.rst
index 43f35ac66c8ac..8224c37a087b8 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/abseil-string-find-startswith.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/abseil-string-find-startswith.rst
@@ -3,14 +3,15 @@
abseil-string-find-startswith
=============================
-Checks whether a ``std::string::find()`` result is compared with 0, and
-suggests replacing with ``absl::StartsWith()``. This is both a readability and
-performance issue.
+Checks whether a ``std::string::find()`` or ``std::string::rfind()`` result is
+compared with 0, and suggests replacing with ``absl::StartsWith()``. This is
+both a readability and performance issue.
.. code-block:: c++
string s = "...";
if (s.find("Hello World") == 0) { /* do something */ }
+ if (s.rfind("Hello World", 0) == 0) { /* do something */ }
becomes
@@ -19,6 +20,7 @@ becomes
string s = "...";
if (absl::StartsWith(s, "Hello World")) { /* do something */ }
+ if (absl::StartsWith(s, "Hello World")) { /* do something */ }
Options
diff --git a/clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-startswith.cpp b/clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-startswith.cpp
index 89365bfa1f815..7a1fd8233d837 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-startswith.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-startswith.cpp
@@ -1,6 +1,8 @@
// RUN: %check_clang_tidy %s abseil-string-find-startswith %t -- \
// RUN: -config="{CheckOptions: [{key: 'abseil-string-find-startswith.StringLikeClasses', value: '::std::basic_string;::basic_string'}]}"
+using size_t = decltype(sizeof(int));
+
namespace std {
template <typename T> class allocator {};
template <typename T> class char_traits {};
@@ -13,12 +15,17 @@ struct basic_string {
~basic_string();
int find(basic_string<C> s, int pos = 0);
int find(const char *s, int pos = 0);
+ int rfind(basic_string<C> s, int pos = npos);
+ int rfind(const char *s, int pos = npos);
+ static constexpr size_t npos = -1;
};
typedef basic_string<char> string;
typedef basic_string<wchar_t> wstring;
struct cxx_string {
int find(const char *s, int pos = 0);
+ int rfind(const char *s, int pos = npos);
+ static constexpr size_t npos = -1;
};
} // namespace std
@@ -61,9 +68,42 @@ void tests(std::string s, global_string s2) {
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StartsWith
// CHECK-FIXES: {{^[[:space:]]*}}absl::StartsWith(s2, "a");{{$}}
+ s.rfind("a", 0) == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StartsWith instead of rfind() == 0 [abseil-string-find-startswith]
+ // CHECK-FIXES: {{^[[:space:]]*}}absl::StartsWith(s, "a");{{$}}
+
+ s.rfind(s, 0) == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StartsWith
+ // CHECK-FIXES: {{^[[:space:]]*}}absl::StartsWith(s, s);{{$}}
+
+ s.rfind("aaa", 0) != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StartsWith
+ // CHECK-FIXES: {{^[[:space:]]*}}!absl::StartsWith(s, "aaa");{{$}}
+
+ s.rfind(foo(foo(bar())), 0) != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StartsWith
+ // CHECK-FIXES: {{^[[:space:]]*}}!absl::StartsWith(s, foo(foo(bar())));{{$}}
+
+ if (s.rfind("....", 0) == 0) { /* do something */ }
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use absl::StartsWith
+ // CHECK-FIXES: {{^[[:space:]]*}}if (absl::StartsWith(s, "....")) { /* do something */ }{{$}}
+
+ 0 != s.rfind("a", 0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StartsWith
+ // CHECK-FIXES: {{^[[:space:]]*}}!absl::StartsWith(s, "a");{{$}}
+
+ s2.rfind("a", 0) == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StartsWith
+ // CHECK-FIXES: {{^[[:space:]]*}}absl::StartsWith(s2, "a");{{$}}
+
// expressions that don't trigger the check are here.
A_MACRO(s.find("a"), 0);
+ A_MACRO(s.rfind("a", 0), 0);
s.find("a", 1) == 0;
s.find("a", 1) == 1;
s.find("a") == 1;
+ s.rfind("a", 1) == 0;
+ s.rfind("a", 1) == 1;
+ s.rfind("a") == 0;
+ s.rfind("a") == 1;
}
More information about the cfe-commits
mailing list