Skip to content

Commit

Permalink
[clang-tidy] Add new modernize-use-starts-ends-with check (#72385)
Browse files Browse the repository at this point in the history
Make a modernize version of abseil-string-find-startswith using the
available C++20 `std::string::starts_with` and
`std::string_view::starts_with`. Following up from
#72283.
  • Loading branch information
nicovank committed Dec 4, 2023
1 parent 2b7191c commit bc8cff1
Show file tree
Hide file tree
Showing 14 changed files with 374 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 modernize-use-starts-ends-with when C++20 is available.
return LangOpts.CPlusPlus && !LangOpts.CPlusPlus20;
}

private:
const std::vector<StringRef> StringLikeClasses;
Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ add_clang_library(clangTidyModernizeModule
UseNoexceptCheck.cpp
UseNullptrCheck.cpp
UseOverrideCheck.cpp
UseStartsEndsWithCheck.cpp
UseStdPrintCheck.cpp
UseTrailingReturnTypeCheck.cpp
UseTransparentFunctorsCheck.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include "UseNoexceptCheck.h"
#include "UseNullptrCheck.h"
#include "UseOverrideCheck.h"
#include "UseStartsEndsWithCheck.h"
#include "UseStdPrintCheck.h"
#include "UseTrailingReturnTypeCheck.h"
#include "UseTransparentFunctorsCheck.h"
Expand Down Expand Up @@ -66,6 +67,8 @@ class ModernizeModule : public ClangTidyModule {
CheckFactories.registerCheck<MakeSharedCheck>("modernize-make-shared");
CheckFactories.registerCheck<MakeUniqueCheck>("modernize-make-unique");
CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value");
CheckFactories.registerCheck<UseStartsEndsWithCheck>(
"modernize-use-starts-ends-with");
CheckFactories.registerCheck<UseStdPrintCheck>("modernize-use-std-print");
CheckFactories.registerCheck<RawStringLiteralCheck>(
"modernize-raw-string-literal");
Expand Down
109 changes: 109 additions & 0 deletions clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
//===--- 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::modernize {

UseStartsEndsWithCheck::UseStartsEndsWithCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}

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))
.bind("starts_with_fun"));
};
const auto HasStartsWithMethod =
anyOf(HasStartsWithMethodWithName("starts_with"),
HasStartsWithMethodWithName("startsWith"),
HasStartsWithMethodWithName("startswith"));
const auto ClassWithStartsWithFunction = cxxRecordDecl(anyOf(
HasStartsWithMethod, hasAnyBase(hasType(hasCanonicalType(hasDeclaration(
cxxRecordDecl(HasStartsWithMethod)))))));

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)))));

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)))));

const auto FindOrRFindExpr =
cxxMemberCallExpr(anyOf(FindExpr, RFindExpr)).bind("find_expr");

Finder->addMatcher(
// Match [=!]= with a zero on one side and a string.(r?)find on the other.
binaryOperator(hasAnyOperatorName("==", "!="),
hasOperands(FindOrRFindExpr, ZeroLiteral))
.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 *StartsWithFunction =
Result.Nodes.getNodeAs<CXXMethodDecl>("starts_with_fun");

if (ComparisonExpr->getBeginLoc().isMacroID()) {
return;
}

const bool Neg = ComparisonExpr->getOpcode() == BO_NE;

auto Diagnostic =
diag(FindExpr->getBeginLoc(), "use %0 instead of %1() %select{==|!=}2 0")
<< StartsWithFunction->getName() << FindFun->getName() << Neg;

// Remove possible zero second argument and ' [!=]= 0' suffix.
Diagnostic << FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(
Lexer::getLocForEndOfToken(FindExpr->getArg(0)->getEndLoc(), 0,
*Result.SourceManager, getLangOpts()),
ComparisonExpr->getEndLoc()),
")");

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

// Replace '(r?)find' with 'starts_with'.
Diagnostic << FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(FindExpr->getExprLoc(),
FindExpr->getExprLoc()),
StartsWithFunction->getName());

// Add possible negation '!'.
if (Neg) {
Diagnostic << FixItHint::CreateInsertion(FindExpr->getBeginLoc(), "!");
}
}

} // namespace clang::tidy::modernize
37 changes: 37 additions & 0 deletions clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//===--- 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_MODERNIZE_USESTARTSENDSWITHCHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTARTSENDSWITHCHECK_H

#include "../ClangTidyCheck.h"

namespace clang::tidy::modernize {

/// Checks whether a ``find`` or ``rfind`` result is compared with 0 and
/// suggests replacing with ``starts_with`` when the method exists in the class.
/// Notably, this will work with ``std::string`` and ``std::string_view``.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/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;
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus;
}
std::optional<TraversalKind> getCheckTraversalKind() const override {
return TK_IgnoreUnlessSpelledInSource;
}
};

} // namespace clang::tidy::modernize

#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTARTSENDSWITHCHECK_H
7 changes: 7 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,13 @@ New checks

Replace ``enable_if`` with C++20 requires clauses.

- New :doc:`modernize-use-starts-ends-with
<clang-tidy/checks/modernize/use-starts-ends-with>` check.

Checks whether a ``find`` or ``rfind`` result is compared with 0 and suggests
replacing with ``starts_with`` when the method exists in the class. Notably,
this will work with ``std::string`` and ``std::string_view``.

- New :doc:`performance-enum-size
<clang-tidy/checks/performance/enum-size>` check.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 :doc:`modernize-use-starts-ends-with
<../modernize/use-starts-ends-with>` instead of this check.

.. code-block:: c++

string s = "...";
Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/docs/clang-tidy/checks/list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ Clang-Tidy Checks
:doc:`modernize-use-noexcept <modernize/use-noexcept>`, "Yes"
:doc:`modernize-use-nullptr <modernize/use-nullptr>`, "Yes"
:doc:`modernize-use-override <modernize/use-override>`, "Yes"
:doc:`modernize-use-starts-ends-with <modernize/use-starts-ends-with>`, "Yes"
:doc:`modernize-use-std-print <modernize/use-std-print>`, "Yes"
:doc:`modernize-use-trailing-return-type <modernize/use-trailing-return-type>`, "Yes"
:doc:`modernize-use-transparent-functors <modernize/use-transparent-functors>`, "Yes"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
.. title:: clang-tidy - modernize-use-starts-ends-with

modernize-use-starts-ends-with
==============================

Checks whether a ``find`` or ``rfind`` result is compared with 0 and suggests
replacing with ``starts_with`` when the method exists in the class. Notably,
this will work with ``std::string`` and ``std::string_view``.

.. code-block:: c++

std::string s = "...";
if (s.find("prefix") == 0) { /* do something */ }
if (s.rfind("prefix", 0) == 0) { /* do something */ }
becomes

.. code-block:: c++

std::string s = "...";
if (s.starts_with("prefix")) { /* do something */ }
if (s.starts_with("prefix")) { /* do something */ }
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@
typedef __PTRDIFF_TYPE__ ptrdiff_t;
typedef __SIZE_TYPE__ size_t;

#endif _STDDEF_H_
#endif // _STDDEF_H_
16 changes: 15 additions & 1 deletion clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,13 @@ typedef unsigned __INT32_TYPE__ char32;
namespace std {
template <typename T>
class allocator {};

template <typename T>
class char_traits {};

template <typename C, typename T = char_traits<C>>
struct basic_string_view;

template <typename C, typename T = char_traits<C>, typename A = allocator<C>>
struct basic_string {
typedef size_t size_type;
Expand Down Expand Up @@ -52,6 +57,10 @@ struct basic_string {
_Type& insert(size_type pos, const C* s);
_Type& insert(size_type pos, const C* s, size_type n);

constexpr bool starts_with(std::basic_string_view<C, T> sv) const noexcept;
constexpr bool starts_with(C ch) const noexcept;
constexpr bool starts_with(const C* s) const;

_Type& operator[](size_type);
const _Type& operator[](size_type) const;

Expand All @@ -68,7 +77,7 @@ typedef basic_string<wchar_t> wstring;
typedef basic_string<char16> u16string;
typedef basic_string<char32> u32string;

template <typename C, typename T = char_traits<C>>
template <typename C, typename T>
struct basic_string_view {
typedef size_t size_type;
typedef basic_string_view<C, T> _Type;
Expand All @@ -86,8 +95,13 @@ struct basic_string_view {
size_type rfind(const C* s, size_type pos, size_type count) const;
size_type rfind(const C* s, size_type pos = npos) const;

constexpr bool starts_with(basic_string_view sv) const noexcept;
constexpr bool starts_with(C ch) const noexcept;
constexpr bool starts_with(const C* s) const;

static constexpr size_t npos = -1;
};

typedef basic_string_view<char> string_view;
typedef basic_string_view<wchar_t> wstring_view;
typedef basic_string_view<char16> u16string_view;
Expand Down
Original file line number Diff line number Diff line change
@@ -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'}}" \
Expand Down

0 comments on commit bc8cff1

Please sign in to comment.