[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
This commit is contained in:
Tom Lokovic
2020-05-28 12:35:57 -04:00
committed by Yitzhak Mandelbaum
parent 8cec5c3581
commit 7cfdff7b4a
8 changed files with 504 additions and 1 deletions
@@ -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 @@ public:
"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>(
@@ -20,6 +20,7 @@ add_clang_library(clangTidyAbseilModule
RedundantStrcatCallsCheck.cpp
StrCatAppendCheck.cpp
StringFindStartswithCheck.cpp
StringFindStrContainsCheck.cpp
TimeComparisonCheck.cpp
TimeSubtractionCheck.cpp
UpgradeDurationConversionsCheck.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
@@ -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
+7
View File
@@ -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
@@ -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``.
@@ -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"
@@ -0,0 +1,290 @@
// RUN: %check_clang_tidy %s abseil-string-find-str-contains %t -- \
// RUN: -config="{CheckOptions: []}"
using size_t = decltype(sizeof(int));
namespace std {
// Lightweight standin for std::string.
template <typename C>
class basic_string {
public:
basic_string();
basic_string(const basic_string &);
basic_string(const C *);
~basic_string();
int find(basic_string s, int pos = 0);
int find(const C *s, int pos = 0);
int find(char c, int pos = 0);
static constexpr size_t npos = -1;
};
typedef basic_string<char> string;
// Lightweight standin for std::string_view.
template <typename C>
class basic_string_view {
public:
basic_string_view();
basic_string_view(const basic_string_view &);
basic_string_view(const C *);
~basic_string_view();
int find(basic_string_view s, int pos = 0);
int find(const C *s, int pos = 0);
int find(char c, int pos = 0);
static constexpr size_t npos = -1;
};
typedef basic_string_view<char> string_view;
} // namespace std
namespace absl {
// Lightweight standin for absl::string_view.
class string_view {
public:
string_view();
string_view(const string_view &);
string_view(const char *);
~string_view();
int find(string_view s, int pos = 0);
int find(const char *s, int pos = 0);
int find(char c, int pos = 0);
static constexpr size_t npos = -1;
};
} // namespace absl
// Functions that take and return our various string-like types.
std::string foo_ss(std::string);
std::string_view foo_ssv(std::string_view);
absl::string_view foo_asv(absl::string_view);
std::string bar_ss();
std::string_view bar_ssv();
absl::string_view bar_asv();
// Confirms that find==npos and find!=npos work for each supported type, when
// npos comes from the correct type.
void basic_tests() {
std::string ss;
ss.find("a") == std::string::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of find() == npos
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ss, "a");{{$}}
ss.find("a") != std::string::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of find() != npos
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(ss, "a");{{$}}
std::string::npos != ss.find("a");
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(ss, "a");{{$}}
std::string_view ssv;
ssv.find("a") == std::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ssv, "a");{{$}}
ssv.find("a") != std::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(ssv, "a");{{$}}
std::string_view::npos != ssv.find("a");
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(ssv, "a");{{$}}
absl::string_view asv;
asv.find("a") == absl::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, "a");{{$}}
asv.find("a") != absl::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(asv, "a");{{$}}
absl::string_view::npos != asv.find("a");
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(asv, "a");{{$}}
}
// Confirms that it works even if you mix-and-match the type for find and for
// npos. (One of the reasons for this checker is to clean up cases that
// accidentally mix-and-match like this. absl::StrContains is less
// error-prone.)
void mismatched_npos() {
std::string ss;
ss.find("a") == std::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ss, "a");{{$}}
ss.find("a") != absl::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(ss, "a");{{$}}
std::string_view ssv;
ssv.find("a") == absl::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ssv, "a");{{$}}
ssv.find("a") != std::string::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(ssv, "a");{{$}}
absl::string_view asv;
asv.find("a") == std::string::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, "a");{{$}}
asv.find("a") != std::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(asv, "a");{{$}}
}
// Confirms that it works even when the needle or the haystack are more
// complicated expressions.
void subexpression_tests() {
std::string ss, ss2;
foo_ss(ss).find(ss2) == std::string::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(foo_ss(ss), ss2);{{$}}
ss.find(foo_ss(ss2)) != std::string::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(ss, foo_ss(ss2));{{$}}
foo_ss(bar_ss()).find(foo_ss(ss2)) != std::string::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(foo_ss(bar_ss()), foo_ss(ss2));{{$}}
std::string_view ssv, ssv2;
foo_ssv(ssv).find(ssv2) == std::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(foo_ssv(ssv), ssv2);{{$}}
ssv.find(foo_ssv(ssv2)) != std::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(ssv, foo_ssv(ssv2));{{$}}
foo_ssv(bar_ssv()).find(foo_ssv(ssv2)) != std::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(foo_ssv(bar_ssv()), foo_ssv(ssv2));{{$}}
absl::string_view asv, asv2;
foo_asv(asv).find(asv2) == absl::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(foo_asv(asv), asv2);{{$}}
asv.find(foo_asv(asv2)) != absl::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(asv, foo_asv(asv2));{{$}}
foo_asv(bar_asv()).find(foo_asv(asv2)) != absl::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(foo_asv(bar_asv()), foo_asv(asv2));{{$}}
}
// Confirms that it works with string literal, char* and const char* parameters.
void string_literal_and_char_ptr_tests() {
char *c = nullptr;
const char *cc = nullptr;
std::string ss;
ss.find("c") == std::string::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ss, "c");{{$}}
ss.find(c) == std::string::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ss, c);{{$}}
ss.find(cc) == std::string::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ss, cc);{{$}}
std::string_view ssv;
ssv.find("c") == std::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ssv, "c");{{$}}
ssv.find(c) == std::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ssv, c);{{$}}
ssv.find(cc) == std::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ssv, cc);{{$}}
absl::string_view asv;
asv.find("c") == absl::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, "c");{{$}}
asv.find(c) == absl::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, c);{{$}}
asv.find(cc) == absl::string_view::npos;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
// CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, cc);{{$}}
}
// Confirms that it does *not* match when the parameter to find() is a char,
// because absl::StrContains is not implemented for char.
void no_char_param_tests() {
std::string ss;
ss.find('c') == std::string::npos;
std::string_view ssv;
ssv.find('c') == std::string_view::npos;
absl::string_view asv;
asv.find('c') == absl::string_view::npos;
}
#define COMPARE_MACRO(x, y) ((x) == (y))
#define FIND_MACRO(x, y) ((x).find(y))
#define FIND_COMPARE_MACRO(x, y, z) ((x).find(y) == (z))
// Confirms that it does not match when a macro is involved.
void no_macros() {
std::string s;
COMPARE_MACRO(s.find("a"), std::string::npos);
FIND_MACRO(s, "a") == std::string::npos;
FIND_COMPARE_MACRO(s, "a", std::string::npos);
}
// Confirms that it does not match when the pos parameter is non-zero.
void no_nonzero_pos() {
std::string ss;
ss.find("a", 1) == std::string::npos;
std::string_view ssv;
ssv.find("a", 2) == std::string_view::npos;
absl::string_view asv;
asv.find("a", 3) == std::string_view::npos;
}
// Confirms that it does not match when it's compared to something other than
// npos, even if the value is the same as npos.
void no_non_npos() {
std::string ss;
ss.find("a") == 0;
ss.find("a") == 1;
ss.find("a") == -1;
std::string_view ssv;
ssv.find("a") == 0;
ssv.find("a") == 1;
ssv.find("a") == -1;
absl::string_view asv;
asv.find("a") == 0;
asv.find("a") == 1;
asv.find("a") == -1;
}
// Confirms that it does not match if the two operands are the same.
void no_symmetric_operands() {
std::string ss;
ss.find("a") == ss.find("a");
std::string::npos == std::string::npos;
}