[clang-tidy] Add abseil-string-find-str-contains checker.

Summary: This adds a checker which suggests replacing string.find(...) == npos with absl::StrContains.

Reviewers: alexfh, hokein, aaron.ballman, njames93, ymandel

Reviewed By: ymandel

Subscribers: ymandel, Eugene.Zelenko, xazax.hun, mgorny, Charusso, phosek, cfe-commits

Tags: #clang, #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D80023

clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp

@@ -21,8 +21,9 @@
 #include "NoInternalDependenciesCheck.h"
 #include "NoNamespaceCheck.h"
 #include "RedundantStrcatCallsCheck.h"
-#include "StringFindStartswithCheck.h"
 #include "StrCatAppendCheck.h"
+#include "StringFindStartswithCheck.h"
+#include "StringFindStrContainsCheck.h"
 #include "TimeComparisonCheck.h"
 #include "TimeSubtractionCheck.h"
 #include "UpgradeDurationConversionsCheck.h"
@@ -61,6 +62,8 @@ class AbseilModule : public ClangTidyModule {
         "abseil-str-cat-append");
     CheckFactories.registerCheck<StringFindStartswithCheck>(
         "abseil-string-find-startswith");
+    CheckFactories.registerCheck<StringFindStrContainsCheck>(
+        "abseil-string-find-str-contains");
     CheckFactories.registerCheck<TimeComparisonCheck>(
         "abseil-time-comparison");
     CheckFactories.registerCheck<TimeSubtractionCheck>(

clang-tools-extra/clang-tidy/abseil/CMakeLists.txt

@@ -20,6 +20,7 @@ add_clang_library(clangTidyAbseilModule
   RedundantStrcatCallsCheck.cpp
   StrCatAppendCheck.cpp
   StringFindStartswithCheck.cpp
+  StringFindStrContainsCheck.cpp
   TimeComparisonCheck.cpp
   TimeSubtractionCheck.cpp
   UpgradeDurationConversionsCheck.cpp

clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp

@@ -0,0 +1,110 @@
+//===--- StringFindStrContainsCheck.cc - 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 "StringFindStrContainsCheck.h"
+
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Tooling/Transformer/RewriteRule.h"
+#include "clang/Tooling/Transformer/Stencil.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+using ::clang::transformer::applyFirst;
+using ::clang::transformer::cat;
+using ::clang::transformer::change;
+using ::clang::transformer::makeRule;
+using ::clang::transformer::node;
+
+static const char DefaultStringLikeClasses[] = "::std::basic_string;"
+                                               "::std::basic_string_view;"
+                                               "::absl::string_view";
+static const char DefaultAbseilStringsMatchHeader[] = "absl/strings/match.h";
+
+static llvm::Optional<transformer::RewriteRule>
+MakeRule(const LangOptions &LangOpts,
+         const ClangTidyCheck::OptionsView &Options) {
+  // Parse options.
+  //
+  // FIXME(tdl-g): These options are being parsed redundantly with the
+  // constructor because TransformerClangTidyCheck forces us to provide MakeRule
+  // before "this" is fully constructed, but StoreOptions requires us to store
+  // the parsed options in "this".  We need to fix TransformerClangTidyCheck and
+  // then we can clean this up.
+  const std::vector<std::string> StringLikeClassNames =
+      utils::options::parseStringList(
+          Options.get("StringLikeClasses", DefaultStringLikeClasses));
+  const std::string AbseilStringsMatchHeader =
+      Options.get("AbseilStringsMatchHeader", DefaultAbseilStringsMatchHeader);
+
+  auto StringLikeClass = cxxRecordDecl(hasAnyName(SmallVector<StringRef, 4>(
+      StringLikeClassNames.begin(), StringLikeClassNames.end())));
+  auto StringType =
+      hasUnqualifiedDesugaredType(recordType(hasDeclaration(StringLikeClass)));
+  auto CharStarType =
+      hasUnqualifiedDesugaredType(pointerType(pointee(isAnyCharacter())));
+  auto StringNpos = declRefExpr(
+      to(varDecl(hasName("npos"), hasDeclContext(StringLikeClass))));
+  auto StringFind = cxxMemberCallExpr(
+      callee(cxxMethodDecl(
+          hasName("find"),
+          hasParameter(0, parmVarDecl(anyOf(hasType(StringType),
+                                            hasType(CharStarType)))))),
+      on(hasType(StringType)), hasArgument(0, expr().bind("parameter_to_find")),
+      anyOf(hasArgument(1, integerLiteral(equals(0))),
+            hasArgument(1, cxxDefaultArgExpr())),
+      onImplicitObjectArgument(expr().bind("string_being_searched")));
+
+  tooling::RewriteRule rule = applyFirst(
+      {makeRule(binaryOperator(hasOperatorName("=="),
+                               hasOperands(ignoringParenImpCasts(StringNpos),
+                                           ignoringParenImpCasts(StringFind))),
+                change(cat("!absl::StrContains(", node("string_being_searched"),
+                           ", ", node("parameter_to_find"), ")")),
+                cat("use !absl::StrContains instead of find() == npos")),
+       makeRule(binaryOperator(hasOperatorName("!="),
+                               hasOperands(ignoringParenImpCasts(StringNpos),
+                                           ignoringParenImpCasts(StringFind))),
+                change(cat("absl::StrContains(", node("string_being_searched"),
+                           ", ", node("parameter_to_find"), ")")),
+                cat("use absl::StrContains instead of find() != npos"))});
+  addInclude(rule, AbseilStringsMatchHeader);
+  return rule;
+}
+
+StringFindStrContainsCheck::StringFindStrContainsCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : TransformerClangTidyCheck(&MakeRule, Name, Context),
+      StringLikeClassesOption(utils::options::parseStringList(
+          Options.get("StringLikeClasses", DefaultStringLikeClasses))),
+      AbseilStringsMatchHeaderOption(Options.get(
+          "AbseilStringsMatchHeader", DefaultAbseilStringsMatchHeader)) {}
+
+bool StringFindStrContainsCheck::isLanguageVersionSupported(
+    const LangOptions &LangOpts) const {
+  return LangOpts.CPlusPlus11;
+}
+
+void StringFindStrContainsCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  TransformerClangTidyCheck::storeOptions(Opts);
+  Options.store(Opts, "StringLikeClasses",
+                utils::options::serializeStringList(StringLikeClassesOption));
+  Options.store(Opts, "AbseilStringsMatchHeader",
+                AbseilStringsMatchHeaderOption);
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.h

@@ -0,0 +1,39 @@
+//===--- StringFindStrContainsCheck.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_ABSEIL_STRINGFINDSTRCONTAINSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRINGFINDSTRCONTAINSCHECK_H
+
+#include "../ClangTidy.h"
+#include "../utils/TransformerClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Finds s.find(...) == string::npos comparisons (for various string-like
+/// types) and suggests replacing with absl::StrContains.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-string-find-str-contains.html
+class StringFindStrContainsCheck : public utils::TransformerClangTidyCheck {
+public:
+  StringFindStrContainsCheck(StringRef Name, ClangTidyContext *Context);
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  const std::vector<std::string> StringLikeClassesOption;
+  const std::string AbseilStringsMatchHeaderOption;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRINGFINDSTRCONTAINSCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -75,6 +75,13 @@ New module
 
 New checks
 ^^^^^^^^^^
+
+- New :doc:`abseil-string-find-str-contains
+  <clang-tidy/checks/abseil-string-find-str-contains>` check.
+
+  Finds ``s.find(...) == string::npos`` comparisons (for various string-like types)
+  and suggests replacing with ``absl::StrContains()``.
+
 - New :doc:`cppcoreguidelines-avoid-non-const-global-variables
   <clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables>` check.
   Finds non-const global variables as described in check I.2 of C++ Core

clang-tools-extra/docs/clang-tidy/checks/abseil-string-find-str-contains.rst

@@ -0,0 +1,52 @@
+.. title:: clang-tidy - abseil-string-find-str-contains
+
+abseil-string-find-str-contains
+===============================
+
+Finds ``s.find(...) == string::npos`` comparisons (for various string-like types)
+and suggests replacing with ``absl::StrContains()``.
+
+This improves readability and reduces the likelihood of accidentally mixing
+``find()`` and ``npos`` from different string-like types.
+
+By default, "string-like types" includes ``::std::basic_string``,
+``::std::basic_string_view``, and ``::absl::string_view``.  See the
+StringLikeClasses option to change this.
+
+.. code-block:: c++
+
+  std::string s = "...";
+  if (s.find("Hello World") == std::string::npos) { /* do something */ }
+
+  absl::string_view a = "...";
+  if (absl::string_view::npos != a.find("Hello World")) { /* do something */ }
+
+becomes
+
+.. code-block:: c++
+
+  std::string s = "...";
+  if (!absl::StrContains(s, "Hello World")) { /* do something */ }
+
+  absl::string_view a = "...";
+  if (absl::StrContains(a, "Hello World")) { /* do something */ }
+
+
+Options
+-------
+
+.. option:: StringLikeClasses
+
+   Semicolon-separated list of names of string-like classes. By default includes
+   ``::std::basic_string``, ``::std::basic_string_view``, and
+   ``::absl::string_view``.
+
+.. 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``.

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -26,6 +26,7 @@ Clang-Tidy Checks
    `abseil-redundant-strcat-calls <abseil-redundant-strcat-calls.html>`_, "Yes"
    `abseil-str-cat-append <abseil-str-cat-append.html>`_, "Yes"
    `abseil-string-find-startswith <abseil-string-find-startswith.html>`_, "Yes"
+   `abseil-string-find-str-contains <abseil-string-find-str-contains.html>`_, "Yes"
    `abseil-time-comparison <abseil-time-comparison.html>`_, "Yes"
    `abseil-time-subtraction <abseil-time-subtraction.html>`_, "Yes"
    `abseil-upgrade-duration-conversions <abseil-upgrade-duration-conversions.html>`_, "Yes"