[clang-tools-extra] r327111 - [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 9 02:47:15 PST 2018
Author: hokein
Date: Fri Mar 9 02:47:14 2018
New Revision: 327111
URL: http://llvm.org/viewvc/llvm-project?rev=327111&view=rev
Log:
[clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith
Patch by Niko Weh!
Reviewers: hokein
Subscribers: klimek, cfe-commits, ioeric, ilya-biryukov, ahedberg
Differential Revision: https://reviews.llvm.org/D43847
Added:
clang-tools-extra/trunk/clang-tidy/abseil/
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-string-find-startswith.rst
clang-tools-extra/trunk/test/clang-tidy/abseil-string-find-startswith.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/CMakeLists.txt?rev=327111&r1=327110&r2=327111&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt Fri Mar 9 02:47:14 2018
@@ -27,6 +27,7 @@ add_clang_library(clangTidy
)
add_subdirectory(android)
+add_subdirectory(abseil)
add_subdirectory(boost)
add_subdirectory(bugprone)
add_subdirectory(cert)
Added: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=327111&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Fri Mar 9 02:47:14 2018
@@ -0,0 +1,38 @@
+//===------- 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
Added: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=327111&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Fri Mar 9 02:47:14 2018
@@ -0,0 +1,14 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyAbseilModule
+ AbseilTidyModule.cpp
+ StringFindStartswithCheck.cpp
+
+ LINK_LIBS
+ clangAST
+ clangASTMatchers
+ clangBasic
+ clangLex
+ clangTidy
+ clangTidyUtils
+ )
Added: clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.cpp?rev=327111&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.cpp Fri Mar 9 02:47:14 2018
@@ -0,0 +1,133 @@
+//===--- 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
Added: clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.h?rev=327111&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.h Fri Mar 9 02:47:14 2018
@@ -0,0 +1,48 @@
+//===--- 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_
Modified: clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt?rev=327111&r1=327110&r2=327111&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt Fri Mar 9 02:47:14 2018
@@ -9,6 +9,7 @@ add_clang_library(clangTidyPlugin
clangSema
clangTidy
clangTidyAndroidModule
+ clangTidyAbseilModule
clangTidyBoostModule
clangTidyCERTModule
clangTidyCppCoreGuidelinesModule
Modified: clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt?rev=327111&r1=327110&r2=327111&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt Fri Mar 9 02:47:14 2018
@@ -18,6 +18,7 @@ target_link_libraries(clang-tidy
clangBasic
clangTidy
clangTidyAndroidModule
+ clangTidyAbseilModule
clangTidyBoostModule
clangTidyBugproneModule
clangTidyCERTModule
Modified: clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp?rev=327111&r1=327110&r2=327111&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp Fri Mar 9 02:47:14 2018
@@ -499,6 +499,11 @@ extern volatile int CERTModuleAnchorSour
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 =
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=327111&r1=327110&r2=327111&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Fri Mar 9 02:47:14 2018
@@ -77,6 +77,15 @@ Improvements to clang-tidy
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
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-string-find-startswith.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-string-find-startswith.rst?rev=327111&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-string-find-startswith.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-string-find-startswith.rst Fri Mar 9 02:47:14 2018
@@ -0,0 +1,41 @@
+.. 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``.
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=327111&r1=327110&r2=327111&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Fri Mar 9 02:47:14 2018
@@ -4,6 +4,7 @@ Clang-Tidy Checks
=================
.. toctree::
+ abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Added: clang-tools-extra/trunk/test/clang-tidy/abseil-string-find-startswith.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/abseil-string-find-startswith.cpp?rev=327111&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/abseil-string-find-startswith.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/abseil-string-find-startswith.cpp Fri Mar 9 02:47:14 2018
@@ -0,0 +1,55 @@
+// 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;
+}
More information about the cfe-commits
mailing list