[clang-tools-extra] [clang-tidy] Add new modernize-string-find-startswith check (PR #72385)
Nicolas van Kempen via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 15 09:51:51 PST 2023
https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/72385
>From fd3e09fddae97549c327cf224d66de144221fe83 Mon Sep 17 00:00:00 2001
From: Nicolas van Kempen <nvankempen at fb.com>
Date: Wed, 15 Nov 2023 01:13:10 -0800
Subject: [PATCH] [clang-tidy] Add new modernize-string-find-startswith check
Matchers are copied over from abseil-string-find-startswith, only the error
message is different and suggests `std::{string|string_view}::starts_with`
instead of the Abseil equivalent.
---
.../abseil/StringFindStartswithCheck.h | 5 +-
.../clang-tidy/performance/CMakeLists.txt | 1 +
.../performance/PerformanceTidyModule.cpp | 3 +
.../performance/UseStartsEndsWithCheck.cpp | 106 ++++++++++++++++++
.../performance/UseStartsEndsWithCheck.h | 44 ++++++++
clang-tools-extra/docs/ReleaseNotes.rst | 7 ++
.../checks/abseil/string-find-startswith.rst | 4 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../performance/use-starts-ends-with.rst | 31 +++++
.../abseil/string-find-startswith.cpp | 2 +-
.../performance/use-starts-ends-with.cpp | 77 +++++++++++++
11 files changed, 279 insertions(+), 2 deletions(-)
create mode 100644 clang-tools-extra/clang-tidy/performance/UseStartsEndsWithCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/performance/UseStartsEndsWithCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/performance/use-starts-ends-with.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/use-starts-ends-with.cpp
diff --git a/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.h b/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.h
index 923b5caece5439b..09773139daa1d66 100644
--- a/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.h
+++ b/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.h
@@ -21,7 +21,6 @@ namespace clang::tidy::abseil {
// Find string.find(...) == 0 comparisons and suggest replacing with StartsWith.
// FIXME(niko): Add similar check for EndsWith
-// FIXME(niko): Add equivalent modernize checks for C++20's std::starts_With
class StringFindStartswithCheck : public ClangTidyCheck {
public:
using ClangTidyCheck::ClangTidyCheck;
@@ -31,6 +30,10 @@ class StringFindStartswithCheck : public ClangTidyCheck {
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ // Prefer performance-use-starts-ends-with when C++20 is available.
+ return LangOpts.CPlusPlus && !LangOpts.CPlusPlus20;
+ }
private:
const std::vector<StringRef> StringLikeClasses;
diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
index 81128ff086021ed..fc88156d8c5f395 100644
--- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
@@ -25,6 +25,7 @@ add_clang_library(clangTidyPerformanceModule
TypePromotionInMathFnCheck.cpp
UnnecessaryCopyInitialization.cpp
UnnecessaryValueParamCheck.cpp
+ UseStartsEndsWithCheck.cpp
LINK_LIBS
clangTidy
diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
index 9e0fa6f88b36a00..3405b5514cbce44 100644
--- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
@@ -28,6 +28,7 @@
#include "TypePromotionInMathFnCheck.h"
#include "UnnecessaryCopyInitialization.h"
#include "UnnecessaryValueParamCheck.h"
+#include "UseStartsEndsWithCheck.h"
namespace clang::tidy {
namespace performance {
@@ -70,6 +71,8 @@ class PerformanceModule : public ClangTidyModule {
"performance-unnecessary-copy-initialization");
CheckFactories.registerCheck<UnnecessaryValueParamCheck>(
"performance-unnecessary-value-param");
+ CheckFactories.registerCheck<UseStartsEndsWithCheck>(
+ "performance-use-starts-ends-with");
}
};
diff --git a/clang-tools-extra/clang-tidy/performance/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/performance/UseStartsEndsWithCheck.cpp
new file mode 100644
index 000000000000000..988eb7da008fb71
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/UseStartsEndsWithCheck.cpp
@@ -0,0 +1,106 @@
+//===--- UseStartsEndsWithCheck.cpp - clang-tidy --------------------------===//
+//
+// 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 "UseStartsEndsWithCheck.h"
+
+#include "../utils/OptionsUtils.h"
+#include "clang/Lex/Lexer.h"
+
+#include <string>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::performance {
+
+const auto DefaultStringLikeClasses =
+ "::std::basic_string;::std::basic_string_view";
+
+UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ StringLikeClasses(utils::options::parseStringList(
+ Options.get("StringLikeClasses", DefaultStringLikeClasses))) {}
+
+void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
+ const auto ZeroLiteral = integerLiteral(equals(0));
+ const auto StringClassMatcher = cxxRecordDecl(hasAnyName(StringLikeClasses));
+ const auto StringType = hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(StringClassMatcher)));
+
+ const auto StringFind = cxxMemberCallExpr(
+ // .find()-call on a string...
+ callee(cxxMethodDecl(hasName("find")).bind("findfun")),
+ on(hasType(StringType)),
+ // ... with some search expression ...
+ hasArgument(0, expr().bind("needle")),
+ // ... and either "0" as second argument or the default argument (also 0).
+ anyOf(hasArgument(1, ZeroLiteral), argumentCountIs(1)));
+
+ const auto StringRFind = cxxMemberCallExpr(
+ // .rfind()-call on a string...
+ callee(cxxMethodDecl(hasName("rfind")).bind("rfindfun")),
+ on(hasType(StringType)),
+ // ... with some search expression ...
+ hasArgument(0, expr().bind("needle")),
+ // ... and "0" as second argument.
+ hasArgument(1, ZeroLiteral));
+
+ Finder->addMatcher(
+ // Match [=!]= with a zero on one side and a string.(r?)find on the other.
+ binaryOperator(
+ hasAnyOperatorName("==", "!="),
+ hasOperands(ZeroLiteral,
+ cxxMemberCallExpr(anyOf(StringFind, StringRFind))
+ .bind("findexpr")))
+ .bind("expr"),
+ this);
+}
+
+void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Context = *Result.Context;
+ const SourceManager &Source = Context.getSourceManager();
+
+ const auto *ComparisonExpr = Result.Nodes.getNodeAs<BinaryOperator>("expr");
+ const auto *Needle = Result.Nodes.getNodeAs<Expr>("needle");
+ const Expr *Haystack = Result.Nodes.getNodeAs<CXXMemberCallExpr>("findexpr")
+ ->getImplicitObjectArgument();
+ const auto *FindFun = Result.Nodes.getNodeAs<CXXMethodDecl>("findfun");
+ const auto *RFindFun = Result.Nodes.getNodeAs<CXXMethodDecl>("rfindfun");
+ assert(!FindFun != !RFindFun); // XOR.
+
+ if (ComparisonExpr->getBeginLoc().isMacroID()) {
+ return;
+ }
+
+ const StringRef NeedleExprCode = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Needle->getSourceRange()), Source,
+ Context.getLangOpts());
+ const StringRef HaystackExprCode = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Haystack->getSourceRange()), Source,
+ Context.getLangOpts());
+
+ const bool Rev = RFindFun != nullptr;
+ const bool Neg = ComparisonExpr->getOpcode() == BO_NE;
+ const std::string ReplacementCode = ((Neg ? "!" : "") + HaystackExprCode +
+ ".starts_with(" + NeedleExprCode + ")")
+ .str();
+
+ diag(ComparisonExpr->getBeginLoc(),
+ "use starts_with "
+ "instead of %select{find()|rfind()}0 %select{==|!=}1 0")
+ << Rev << Neg
+ << FixItHint::CreateReplacement(ComparisonExpr->getSourceRange(),
+ ReplacementCode);
+}
+
+void UseStartsEndsWithCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "StringLikeClasses",
+ utils::options::serializeStringList(StringLikeClasses));
+}
+
+} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/clang-tidy/performance/UseStartsEndsWithCheck.h b/clang-tools-extra/clang-tidy/performance/UseStartsEndsWithCheck.h
new file mode 100644
index 000000000000000..80ce29b99d33d8b
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/UseStartsEndsWithCheck.h
@@ -0,0 +1,44 @@
+//===--- UseStartsEndsWithCheck.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_PERFORMANCE_USESTARTSENDSWITHCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_USESTARTSENDSWITHCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+#include <vector>
+
+namespace clang::tidy::performance {
+
+/// Checks whether a ``std::string::find()`` or ``std::string::rfind()`` (and
+/// corresponding ``std::string_view`` methods) result is compared with 0, and
+/// suggests replacing with ``starts_with()``. This is both a readability and a
+/// performance issue.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance/use-starts-ends-with.html
+class UseStartsEndsWithCheck : public ClangTidyCheck {
+public:
+ UseStartsEndsWithCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus20;
+ }
+ std::optional<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
+
+private:
+ const std::vector<StringRef> StringLikeClasses;
+};
+
+} // namespace clang::tidy::performance
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_USESTARTSENDSWITHCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 353c6fe20269274..2cdd58b1a9dd068 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,6 +192,13 @@ New checks
Recommends the smallest possible underlying type for an ``enum`` or ``enum``
class based on the range of its enumerators.
+- New :doc:`performance-use-starts-ends-with
+ <clang-tidy/checks/performance/use-starts-ends-with>` check.
+
+ Checks whether a ``std::string::find()`` or ``std::string::rfind()`` (and
+ corresponding ``std::string_view`` methods) result is compared with 0, and
+ suggests replacing with ``starts_with()``.
+
- New :doc:`readability-reference-to-constructed-temporary
<clang-tidy/checks/readability/reference-to-constructed-temporary>` check.
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 c82c38772a5c9a8..28988ce627ec884 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
@@ -8,6 +8,10 @@ corresponding ``std::string_view`` methods) result is compared with 0, and
suggests replacing with ``absl::StartsWith()``. This is both a readability and
performance issue.
+``starts_with`` was added as a built-in function on those types in C++20. If
+available, prefer enabling performance-use-starts-ends-with instead of this
+check.
+
.. code-block:: c++
string s = "...";
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 6f987ba1672e3f2..d9d339aaa8fb80b 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -329,6 +329,7 @@ Clang-Tidy Checks
:doc:`performance-type-promotion-in-math-fn <performance/type-promotion-in-math-fn>`, "Yes"
:doc:`performance-unnecessary-copy-initialization <performance/unnecessary-copy-initialization>`, "Yes"
:doc:`performance-unnecessary-value-param <performance/unnecessary-value-param>`, "Yes"
+ :doc:`performance-use-starts-ends-with <performance/use-starts-ends-with>`, "Yes"
:doc:`portability-restrict-system-includes <portability/restrict-system-includes>`, "Yes"
:doc:`portability-simd-intrinsics <portability/simd-intrinsics>`,
:doc:`portability-std-allocator-const <portability/std-allocator-const>`,
diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/use-starts-ends-with.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/use-starts-ends-with.rst
new file mode 100644
index 000000000000000..419e7fbc9a814e1
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/use-starts-ends-with.rst
@@ -0,0 +1,31 @@
+.. title:: clang-tidy - performance-use-starts-ends-with
+
+performance-use-starts-ends-with
+================================
+
+Checks whether a ``std::string::find()`` or ``std::string::rfind()`` (and
+corresponding ``std::string_view`` methods) result is compared with 0, and
+suggests replacing with ``starts_with()``. This is both a readability and a
+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
+
+.. code-block:: c++
+
+ string s = "...";
+ if (s.starts_with("Hello World")) { /* do something */ }
+ if (s.starts_with("Hello World")) { /* do something */ }
+
+Options
+-------
+
+.. option:: StringLikeClasses
+
+ Semicolon-separated list of names of string-like classes. By default both
+ ``std::basic_string`` and ``std::basic_string_view`` are considered.
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 417598790bc007f..aabb30fe34f782c 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,4 +1,4 @@
-// RUN: %check_clang_tidy %s abseil-string-find-startswith %t -- \
+// RUN: %check_clang_tidy -std=c++17 %s abseil-string-find-startswith %t -- \
// RUN: -config="{CheckOptions: \
// RUN: {abseil-string-find-startswith.StringLikeClasses: \
// RUN: '::std::basic_string;::std::basic_string_view;::basic_string'}}" \
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/use-starts-ends-with.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/use-starts-ends-with.cpp
new file mode 100644
index 000000000000000..dde083e911185d2
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/use-starts-ends-with.cpp
@@ -0,0 +1,77 @@
+// RUN: %check_clang_tidy -std=c++20 %s performance-use-starts-ends-with %t -- -- -isystem %clang_tidy_headers
+
+#include <string>
+
+std::string foo(std::string);
+std::string bar();
+
+#define A_MACRO(x, y) ((x) == (y))
+
+void test(std::string s, std::string_view sv) {
+ s.find("a") == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of find() == 0
+ // CHECK-FIXES: s.starts_with("a");
+
+ s.find(s) == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: s.starts_with(s);
+
+ s.find("aaa") != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: !s.starts_with("aaa");
+
+ s.find(foo(foo(bar()))) != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: !s.starts_with(foo(foo(bar())));
+
+ if (s.find("....") == 0) { /* do something */ }
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: if (s.starts_with("...."))
+
+ 0 != s.find("a");
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: !s.starts_with("a");
+
+ s.rfind("a", 0) == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of rfind() == 0
+ // CHECK-FIXES: s.starts_with("a");
+
+ s.rfind(s, 0) == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: s.starts_with(s);
+
+ s.rfind("aaa", 0) != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: !s.starts_with("aaa");
+
+ s.rfind(foo(foo(bar())), 0) != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: !s.starts_with(foo(foo(bar())));
+
+ if (s.rfind("....", 0) == 0) { /* do something */ }
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use starts_with
+ // CHECK-FIXES: if (s.starts_with("...."))
+
+ 0 != s.rfind("a", 0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: !s.starts_with("a");
+
+ sv.find("a") == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: sv.starts_with("a");
+
+ sv.rfind("a", 0) != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with
+ // CHECK-FIXES: !sv.starts_with("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