[clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

[Eugene.Zelenko]

Please add new module in docs/clang-tidy/index.rst and mention it in release notes.

clang-tidy/absl/AbslTidyModule.cpp

14 ↗

(On Diff #136204)

Please separate with empty line.

clang-tidy/absl/StringFindStartswithCheck.cpp

40 ↗

(On Diff #136204)

Please don't use auto when type could not be deduced from same statement. Same in other places.

clang-tidy/absl/StringFindStartswithCheck.h

30 ↗

(On Diff #136204)

Please include <memory>

docs/ReleaseNotes.rst

63

string::find -> std::string::find(), absl::StartsWith -> absl::StartsWith(). Please enclose them in ``.

docs/clang-tidy/checks/absl-string-find-startswith.rst

6 ↗

(On Diff #136204)

Please make first statement same as in release notes and avoid This check.

11 ↗

(On Diff #136204)

Please use .. code-block: c++

17 ↗

(On Diff #136204)

Please use .. code-block: c++

18 ↗

(On Diff #136204)

Is there any online documentation about such usage? If so please refer to in at. See other guidelines as example.

[Eugene.Zelenko]

std::basic_string::starts_with() was suggested for C++20. May be will be good idea to generalize code to create absl and modernize checks?

[aaron.ballman]

I need a bit more context because I'm unfamiliar with absl. What is this module's intended use?

clang-tidy/absl/StringFindStartswithCheck.cpp

17–19 ↗

(On Diff #136204)

These should be using elaborated type specifiers to ensure we get the correct class, not some class that happens to have the same name. Also, this list should be configurable so that users can add their own entries to it.

44–47 ↗

(On Diff #136204)

Btw, these can use auto because the type is spelled out in the initialization.

69 ↗

(On Diff #136204)

Diagnostics shouldn't start with a capital letter (or use terminating punctuation).

72 ↗

(On Diff #136204)

This fixit should be guarded in case it lands in the middle of a macro for some reason.

clang-tidy/absl/StringFindStartswithCheck.h

25 ↗

(On Diff #136204)

compiler doesn't match our naming conventions (same goes for many other identifiers in the check).

docs/ReleaseNotes.rst

60

The release notes should also document that this is adding an entirely new module to clang-tidy and what that module's purpose is.

[hokein]

In D43847#1021967, @aaron.ballman wrote:

I need a bit more context because I'm unfamiliar with absl. What is this module's intended use?

As absl has been open-sourced (https://github.com/abseil/abseil-cpp), I think there will be more absl-related checks contributed from google or external contributors in the future, so it make sense to create a new module.

@niko, could you please refine the code style of this patch to follow the LLVM code standard (especially the variable name, should be camel case "VariableName")?

[hokein]

clang-tidy/absl/StringFindStartswithCheck.cpp

1 ↗

(On Diff #136204)

nit: We need a LICENSE comment at the top of the file.

7 ↗

(On Diff #136204)

nit: no need for NOLINT.

17–19 ↗

(On Diff #136204)

+1, we could add a StringLikeClasses option.

48 ↗

(On Diff #136204)

nit: add assert to make sure these pointers are not nullptr.

81 ↗

(On Diff #136204)

The include path maybe different in different project.

I think we also need an option for the inserting header, and make it default to absl/strings/match.h.

92 ↗

(On Diff #136204)

We need to make the style customizable by adding an IncludeStyle option (see for an example http://clang.llvm.org/extra/clang-tidy/checks/performance-unnecessary-value-param.html#cmdoption-arg-includestyle), instead of hard-coding.

docs/clang-tidy/checks/absl-string-find-startswith.rst

17 ↗

(On Diff #136204)

nit: I would keep the string s = "..." here.

test/clang-tidy/absl-string-find-startswith.cpp

1 ↗

(On Diff #136204)

Since your test is isolated, // RUN: %check_clang_tidy %s absl-string-find-startswith %t should work.

27 ↗

(On Diff #136204)

nit: we usually use CHECK-FIXES in a new line: // CHECK-FIXES: absl::StartsWith(s, "a");

The same below.

30 ↗

(On Diff #136204)

Could you also add a test like if (s.find(s) == 0) { ... } to make sure the replacement range is correct.

Also maybe add 0 == s.find(s)?

[Eugene.Zelenko]

In D43847#1023452, @hokein wrote:

In D43847#1021967, @aaron.ballman wrote:

I need a bit more context because I'm unfamiliar with absl. What is this module's intended use?

As absl has been open-sourced (https://github.com/abseil/abseil-cpp), I think there will be more absl-related checks contributed from google or external contributors in the future, so it make sense to create a new module.

@niko, could you please refine the code style of this patch to follow the LLVM code standard (especially the variable name, should be camel case "VariableName")?

May be abseil is better name for module?

[Eugene.Zelenko]

By the word, may be similar check for std::string::rfind() and std::string::ends_with() (does abseil have analog) should be added too?

[alexfh]

In D43847#1023642, @Eugene.Zelenko wrote:

May be abseil is better name for module?

I'd also go for "abseil". I'll try to get abseil folks to review this.

[aaron.ballman]

In D43847#1023452, @hokein wrote:

In D43847#1021967, @aaron.ballman wrote:

I need a bit more context because I'm unfamiliar with absl. What is this module's intended use?

As absl has been open-sourced (https://github.com/abseil/abseil-cpp), I think there will be more absl-related checks contributed from google or external contributors in the future, so it make sense to create a new module.

Ah, so this is for abseil, good to know. Yeah, I think we should have a new module for it but perhaps with the full name instead of this shortened version?

[ahedberg]

In D43847#1024018, @aaron.ballman wrote:

In D43847#1023452, @hokein wrote:

In D43847#1021967, @aaron.ballman wrote:

I need a bit more context because I'm unfamiliar with absl. What is this module's intended use?

As absl has been open-sourced (https://github.com/abseil/abseil-cpp), I think there will be more absl-related checks contributed from google or external contributors in the future, so it make sense to create a new module.

Ah, so this is for abseil, good to know. Yeah, I think we should have a new module for it but perhaps with the full name instead of this shortened version?

+1 to the name abseil. Some folks from the Abseil team discussed yesterday--we want to keep other projects from using nested absl namespaces to avoid ever having to type ::absl. So it would be nice to replace that nested namespace with something else; maybe clang::tidy::abseil?

[niko]

Renamed 'absl' to 'abseil', applied reviewer suggestions

[niko]

Thanks everyone for your comments! I renamed the namespace and filenames to 'abseil'.

@Eugene.Zelenko, definitely interested in extending this to a C++20 modernize check and adding absl::EndsWith() support, would it be OK though to do this in a later patch?

clang-tidy/absl/StringFindStartswithCheck.cpp

17–19 ↗

(On Diff #136204)

I've made the list configurable. Can you clarify what I would need to add in terms of type specifiers?

44–47 ↗

(On Diff #136204)

Thanks for the clarification. Is this true even for Haystack (as ->getImplicitObjectArgument()'s type is Expr)?

72 ↗

(On Diff #136204)

Sorry, could you elaborate?

81 ↗

(On Diff #136204)

Instead of having a "AbseilStringsMatchHeader" option, does it make sense to have a "AbseilIncludeDir" option here & default that to absl? (esp. if there are going to be other abseil-checks in the future...)

docs/clang-tidy/checks/absl-string-find-startswith.rst

18 ↗

(On Diff #136204)

I don't believe there is documentation for this yet. The comment in the header explains what it does but doesn't have an explicit usage example. (If that qualifies I'll obviously include it?)

[Eugene.Zelenko]

In D43847#1025833, @niko wrote:

Thanks everyone for your comments! I renamed the namespace and filenames to 'abseil'.

@Eugene.Zelenko, definitely interested in extending this to a C++20 modernize check and adding absl::EndsWith() support, would it be OK though to do this in a later patch?

I think will be better to do this now, since you'll need to create base class and specializations for Abseil and C++17.

[aaron.ballman]

clang-tidy/absl/StringFindStartswithCheck.cpp

17–19 ↗

(On Diff #136204)

You should be looking for a record decl with the name ::std::string so that you don't run into issues with things like: namespace terrible_person { class string; }

44–47 ↗

(On Diff #136204)

It's not true for haystack, I think I highlighted one too many lines there. :-)

72 ↗

(On Diff #136204)

We generally don't issue fixit hints if the source location for the fixit is in a macro. You can test that with expr->getLocStart().isMacroID().

[hokein]

In D43847#1025846, @Eugene.Zelenko wrote:

In D43847#1025833, @niko wrote:

Thanks everyone for your comments! I renamed the namespace and filenames to 'abseil'.

@Eugene.Zelenko, definitely interested in extending this to a C++20 modernize check and adding absl::EndsWith() support, would it be OK though to do this in a later patch?

I think will be better to do this now, since you'll need to create base class and specializations for Abseil and C++17.

I'm fine with the current status. We could polish it in a follow-up patch. Let's not put too much on this patch.

clang-tidy/abseil/StringFindStartswithCheck.cpp

28

Use fully-qualified name ::std::basic_string.

docs/clang-tidy/checks/abseil-string-find-startswith.rst

5

nit: this line should align with the title.

test/clang-tidy/abseil-string-find-startswith.cpp

31

nit: you don't have to specify the full message every line (except the first line), so here and below use // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StartsWith is sufficient.

[niko]

Addressed reviewer comments.

[niko]

clang-tidy/absl/StringFindStartswithCheck.cpp

81 ↗

(On Diff #136204)

Discussed offline with @hokein. Will leave AbseilStringsMatchHeader.

[hokein]

Looks good with a few nits.

clang-tidy/abseil/AbseilTidyModule.cpp

16

What is this header used for?

clang-tidy/abseil/StringFindStartswithCheck.cpp

95

nit: we can put it at the beginning of this method to make it early return if it is in macro. I think it is fine to ignore macro cases.

docs/clang-tidy/checks/abseil-string-find-startswith.rst

31

s/consired/considered

test/clang-tidy/abseil-string-find-startswith.cpp

3

nit: -std=c++11 is not needed, it is on by default.

[niko]

clang-tidy/abseil/AbseilTidyModule.cpp

16

Sorry, ended up in the wrong file, moved to StringFindStartswithCheck.cpp.

[niko]

If this is OK as it is, can someone with commit access push this? Thanks!

[hokein]

I will commit for you.

clang-tidy/CMakeLists.txt

add_clang_library(clangTidy
   clangSema
   clangStaticAnalyzerCore
   clangStaticAnalyzerFrontend
   clangTooling
   clangToolingCore
   )
 
 add_subdirectory(android)
+add_subdirectory(abseil)
 add_subdirectory(boost)
 add_subdirectory(bugprone)
 add_subdirectory(cert)
 add_subdirectory(cppcoreguidelines)
 add_subdirectory(fuchsia)
 add_subdirectory(google)
 add_subdirectory(hicpp)
 add_subdirectory(llvm)

clang-tidy/abseil/AbseilTidyModule.cpp

+//===------- AbseilTidyModule.cpp - clang-tidy ----------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+#include "StringFindStartswithCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+class AbseilModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<StringFindStartswithCheck>(
+        "abseil-string-find-startswith");
+  }
+};
+
+// Register the AbseilModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<AbseilModule> X("abseil-module",
+                                                    "Add Abseil checks.");
+
+} // namespace abseil
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the AbseilModule.
+volatile int AbseilModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang

clang-tidy/abseil/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyAbseilModule
+  AbseilTidyModule.cpp
+  StringFindStartswithCheck.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangTidy
+  clangTidyUtils
+  )

clang-tidy/abseil/StringFindStartswithCheck.h

+//===--- StringFindStartswithCheck.h - clang-tidy----------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRING_FIND_STARTSWITH_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRING_FIND_STARTSWITH_H_
+
+#include "../ClangTidy.h"
+#include "../utils/IncludeInserter.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+#include <memory>
+#include <string>
+#include <vector>
+
+namespace clang {
+namespace tidy {
+namespace 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;
+  StringFindStartswithCheck(StringRef Name, ClangTidyContext *Context);
+  void registerPPCallbacks(CompilerInstance &Compiler) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  std::unique_ptr<clang::tidy::utils::IncludeInserter> IncludeInserter;
+  const std::vector<std::string> StringLikeClasses;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+  const std::string AbseilStringsMatchHeader;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRING_FIND_STARTSWITH_H_

clang-tidy/abseil/StringFindStartswithCheck.cpp

+//===--- StringFindStartswithCheck.cc - clang-tidy---------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "StringFindStartswithCheck.h"
+
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+#include <cassert>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+StringFindStartswithCheck::StringFindStartswithCheck(StringRef Name,
+                                                     ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      StringLikeClasses(utils::options::parseStringList(
+          Options.get("StringLikeClasses", "::std::basic_string"))),
+      IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+          Options.getLocalOrGlobal("IncludeStyle", "llvm"))),
+      AbseilStringsMatchHeader(
+          Options.get("AbseilStringsMatchHeader", "absl/strings/match.h")) {}
+
+void StringFindStartswithCheck::registerMatchers(MatchFinder *Finder) {
+  auto ZeroLiteral = integerLiteral(equals(0));
+  auto StringClassMatcher = cxxRecordDecl(hasAnyName(SmallVector<StringRef, 4>(
+      StringLikeClasses.begin(), StringLikeClasses.end())));
+
+  auto StringFind = cxxMemberCallExpr(
+      // .find()-call on a string...
+      callee(cxxMethodDecl(hasName("find"), ofClass(StringClassMatcher))),
+      // ... 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), hasArgument(1, cxxDefaultArgExpr())));
+
+  Finder->addMatcher(
+      // Match [=!]= with a zero on one side and a string.find on the other.
+      binaryOperator(
+          anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+          hasEitherOperand(ignoringParenImpCasts(ZeroLiteral)),
+          hasEitherOperand(ignoringParenImpCasts(StringFind.bind("findexpr"))))
+          .bind("expr"),
+      this);
+}
+
+void StringFindStartswithCheck::check(const MatchFinder::MatchResult &Result) {
+  const ASTContext &Context = *Result.Context;
+  const SourceManager &Source = Context.getSourceManager();
+
+  // Extract matching (sub)expressions
+  const auto *ComparisonExpr = Result.Nodes.getNodeAs<BinaryOperator>("expr");
+  assert(ComparisonExpr != nullptr);
+  const auto *Needle = Result.Nodes.getNodeAs<Expr>("needle");
+  assert(Needle != nullptr);
+  const Expr *Haystack = Result.Nodes.getNodeAs<CXXMemberCallExpr>("findexpr")
+                             ->getImplicitObjectArgument();
+  assert(Haystack != nullptr);
+
+  if (ComparisonExpr->getLocStart().isMacroID())
+    return;
+
+  // Get the source code blocks (as characters) for both the string object
+  // and the search expression
+  const StringRef NeedleExprCode = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(Needle->getSourceRange()), Source,
+      Context.getLangOpts());
+  const StringRef HaystackExprCode = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(Haystack->getSourceRange()), Source,
+      Context.getLangOpts());
+
+  // Create the StartsWith string, negating if comparison was "!=".
+  bool Neg = ComparisonExpr->getOpcodeStr() == "!=";
+  StringRef StartswithStr;
+  if (Neg) {
+    StartswithStr = "!absl::StartsWith";
+  } else {
+    StartswithStr = "absl::StartsWith";
+  }
+
+  // Create the warning message and a FixIt hint replacing the original expr.
+  auto Diagnostic =
+      diag(ComparisonExpr->getLocStart(),
+           (StringRef("use ") + StartswithStr + " instead of find() " +
+            ComparisonExpr->getOpcodeStr() + " 0")
+               .str());
+
+  Diagnostic << FixItHint::CreateReplacement(
+      ComparisonExpr->getSourceRange(),
+      (StartswithStr + "(" + HaystackExprCode + ", " + NeedleExprCode + ")")
+          .str());
+
+  // Create a preprocessor #include FixIt hint (CreateIncludeInsertion checks
+  // whether this already exists).
+  auto IncludeHint = IncludeInserter->CreateIncludeInsertion(
+      Source.getFileID(ComparisonExpr->getLocStart()), AbseilStringsMatchHeader,
+      false);
+  if (IncludeHint) {
+    Diagnostic << *IncludeHint;
+  }
+}
+
+void StringFindStartswithCheck::registerPPCallbacks(
+    CompilerInstance &Compiler) {
+  IncludeInserter = llvm::make_unique<clang::tidy::utils::IncludeInserter>(
+      Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle);
+  Compiler.getPreprocessor().addPPCallbacks(
+      IncludeInserter->CreatePPCallbacks());
+}
+
+void StringFindStartswithCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "StringLikeClasses",
+                utils::options::serializeStringList(StringLikeClasses));
+  Options.store(Opts, "IncludeStyle",
+                utils::IncludeSorter::toString(IncludeStyle));
+  Options.store(Opts, "AbseilStringsMatchHeader", AbseilStringsMatchHeader);
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang

clang-tidy/plugin/CMakeLists.txt

 add_clang_library(clangTidyPlugin
   ClangTidyPlugin.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangFrontend
   clangSema
   clangTidy
   clangTidyAndroidModule
+  clangTidyAbseilModule
   clangTidyBoostModule
   clangTidyCERTModule
   clangTidyCppCoreGuidelinesModule
   clangTidyGoogleModule
   clangTidyLLVMModule
   clangTidyMiscModule
   clangTidyModernizeModule
   clangTidyMPIModule
   clangTidyObjCModule
   clangTidyPerformanceModule
   clangTidyPortabilityModule
   clangTidyReadabilityModule
   clangTooling
   )

clang-tidy/tool/CMakeLists.txt

add_dependencies(clang-tidy
   )
 target_link_libraries(clang-tidy
   PRIVATE
   clangAST
   clangASTMatchers
   clangBasic
   clangTidy
   clangTidyAndroidModule
+  clangTidyAbseilModule
   clangTidyBoostModule
   clangTidyBugproneModule
   clangTidyCERTModule
   clangTidyCppCoreGuidelinesModule
   clangTidyFuchsiaModule
   clangTidyGoogleModule
   clangTidyHICPPModule
   clangTidyLLVMModule

clang-tidy/tool/ClangTidyMain.cpp

static int clangTidyMain(int argc, const char **argv) {
   return 0;
 }
 
 // This anchor is used to force the linker to link the CERTModule.
 extern volatile int CERTModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED CERTModuleAnchorDestination =
     CERTModuleAnchorSource;
 
+// This anchor is used to force the linker to link the AbseilModule.
+extern volatile int AbseilModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED AbseilModuleAnchorDestination =
+    AbseilModuleAnchorSource;
+
 // This anchor is used to force the linker to link the BoostModule.
 extern volatile int BoostModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED BoostModuleAnchorDestination =
     BoostModuleAnchorSource;
 
 // This anchor is used to force the linker to link the BugproneModule.
 extern volatile int BugproneModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED BugproneModuleAnchorDestination =

docs/ReleaseNotes.rst

 Improvements to clang-rename
 ----------------------------
 
 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
 - New module ``portability``.
 
 - New `bugprone-throw-keyword-missing
   <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-throw-keyword-missing.html>`_ check
 
   Diagnoses when a temporary object that appears to be an exception is
   constructed but not thrown.
 
 - New `cppcoreguidelines-avoid-goto
   <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-goto.html>`_ check
 
   The usage of ``goto`` for control flow is error prone and should be replaced
   with looping constructs. Every backward jump is rejected. Forward jumps are
   only allowed in nested loops.
 
 - New `fuchsia-multiple-inheritance
   <http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-multiple-inheritance.html>`_ check
 
   Warns if a class inherits from multiple classes that are not pure virtual.
 
+- New `abseil` module for checks related to the `Abseil <https://abseil.io>`_
+  library.
+
+- New `abseil-string-find-startswith
+  <http://clang.llvm.org/extra/clang-tidy/checks/abseil-string-find-startswith.html>`_ check
+
+  Checks whether a ``std::string::find()`` result is compared with 0, and
+  suggests replacing with ``absl::StartsWith()``.
+
 - New `fuchsia-statically-constructed-objects
   <http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-statically-constructed-objects.html>`_ check
 
   Warns if global, non-trivial objects with static storage are constructed,
   unless the object is statically initialized with a ``constexpr`` constructor
   or has no explicit constructor.
   
 - New `fuchsia-trailing-return

docs/clang-tidy/checks/abseil-string-find-startswith.rst

+.. title:: clang-tidy - abseil-string-find-startswith
+
+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.
+
+.. code-block:: c++
+
+  string s = "...";
+  if (s.find("Hello World") == 0) { /* do something */ }
+
+becomes
+
+
+.. code-block:: c++
+
+  string s = "...";
+  if (absl::StartsWith(s, "Hello World")) { /* do something */ }
+
+
+Options
+-------
+
+.. option:: StringLikeClasses
+
+   Semicolon-separated list of names of string-like classes. By default only
+   ``std::basic_string`` is considered. The list of methods to considered is
+   fixed.
+
+.. option:: IncludeStyle
+
+   A string specifying which include-style is used, `llvm` or `google`. Default
+   is `llvm`.
+
+.. option:: AbseilStringsMatchHeader
+
+   The location of Abseil's ``strings/match.h``. Defaults to
+   ``absl/strings/match.h``.

docs/clang-tidy/checks/list.rst

 .. title:: clang-tidy - Clang-Tidy Checks
 
 Clang-Tidy Checks
 =================
 
 .. toctree::
+   abseil-string-find-startswith
    android-cloexec-accept
    android-cloexec-accept4
    android-cloexec-creat
    android-cloexec-dup
    android-cloexec-epoll-create
    android-cloexec-epoll-create1
    android-cloexec-fopen
    android-cloexec-inotify-init

test/clang-tidy/abseil-string-find-startswith.cpp

+// RUN: %check_clang_tidy %s abseil-string-find-startswith %t
+
+namespace std {
+template <typename T> class allocator {};
+template <typename T> class char_traits {};
+template <typename C, typename T = std::char_traits<C>,
+          typename A = std::allocator<C>>
+struct basic_string {
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const C *, const A &a = A());
+  ~basic_string();
+  int find(basic_string<C> s, int pos = 0);
+  int find(const char *s, int pos = 0);
+};
+typedef basic_string<char> string;
+typedef basic_string<wchar_t> wstring;
+} // namespace std
+
+std::string foo(std::string);
+std::string bar();
+
+#define A_MACRO(x, y) ((x) == (y))
+
+void tests(std::string s) {
+  s.find("a") == 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StartsWith instead of find() == 0 [abseil-string-find-startswith]
+  // CHECK-FIXES: {{^[[:space:]]*}}absl::StartsWith(s, "a");{{$}}
+
+  s.find(s) == 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StartsWith
+  // CHECK-FIXES: {{^[[:space:]]*}}absl::StartsWith(s, s);{{$}}
+
+  s.find("aaa") != 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StartsWith
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StartsWith(s, "aaa");{{$}}
+
+  s.find(foo(foo(bar()))) != 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StartsWith
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StartsWith(s, foo(foo(bar())));{{$}}
+
+  if (s.find("....") == 0) { /* do something */ }
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use absl::StartsWith
+  // CHECK-FIXES: {{^[[:space:]]*}}if (absl::StartsWith(s, "....")) { /* do something */ }{{$}}
+
+  0 != s.find("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StartsWith
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StartsWith(s, "a");{{$}}
+
+  // expressions that don't trigger the check are here.
+  A_MACRO(s.find("a"), 0);
+  s.find("a", 1) == 0;
+  s.find("a", 1) == 1;
+  s.find("a") == 1;
+}