niko created this revision.
niko added reviewers: ioeric, hokein.
Herald added subscribers: cfe-commits, mgorny.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43847
Files:
clang-tidy/CMakeLists.txt
clang-tidy/absl/AbslTidyModule.cpp
clang-tidy/absl/CMakeLists.txt
clang-tidy/absl/StringFindStartswithCheck.cpp
clang-tidy/absl/StringFindStartswithCheck.h
clang-tidy/plugin/CMakeLists.txt
clang-tidy/tool/CMakeLists.txt
clang-tidy/tool/ClangTidyMain.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/absl-string-find-startswith.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/absl-string-find-startswith.cpp
Index: test/clang-tidy/absl-string-find-startswith.cpp
===================================================================
--- test/clang-tidy/absl-string-find-startswith.cpp
+++ test/clang-tidy/absl-string-find-startswith.cpp
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s absl-string-find-startswith %t -- -- -I%S
+// -std=c++11
+
+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();
+
+void tests(std::string s) {
+ s.find("a") == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use absl::StartsWith instead of
+ // find() == 0 [absl-string-find-startswith] CHECK_FIXES:
+ // {{^}}absl::StartsWith(s, "a");{{$}}
+
+ s.find(s) == 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use absl::StartsWith instead of
+ // find() == 0 [absl-string-find-startswith] CHECK_FIXES:
+ // {{^}}absl::StartsWith(s, s);{{$}}
+
+ s.find("aaa") != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use !absl::StartsWith instead of
+ // find() != 0 [absl-string-find-startswith] CHECK_FIXES:
+ // {{^}}!absl::StartsWith(s, "aaa");{{$}}
+
+ s.find(foo(foo(bar()))) != 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use !absl::StartsWith instead of
+ // find() != 0 [absl-string-find-startswith] CHECK_FIXES:
+ // {{^}}!absl::StartsWith(s, foo(foo(foo)));{{$}}
+
+ // expressions that don't trigger the check are here.
+ s.find("a", 1) == 0;
+ s.find("a", 1) == 1;
+ s.find("a") == 1;
+}
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
=================
.. toctree::
+ absl-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/absl-string-find-startswith.rst
===================================================================
--- docs/clang-tidy/checks/absl-string-find-startswith.rst
+++ docs/clang-tidy/checks/absl-string-find-startswith.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - absl-string-find-startswith
+
+absl-string-find-startswith
+==========================================
+
+This check triggers on (in)equality comparisons between string.find()
+and zero. Comparisons like this should be replaced with
+absl::StartsWith(string, prefix). This is both a readability and a
+performance issue.
+
+::
+
+ string s = "...";
+ if (s.find("Hello World") == 0) { /* do something */ }
+
+should be replaced with
+``if (absl::StartsWith(s, "Hello World")) { /* do something */ };``
+
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,12 @@
Improvements to clang-tidy
--------------------------
+- New `absl-string-find-startswith
+ <http://clang.llvm.org/extra/clang-tidy/checks/absl-string-find-startswith.html>`_ check
+
+ Checks whether a 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
Index: clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -462,6 +462,11 @@
static int LLVM_ATTRIBUTE_UNUSED CERTModuleAnchorDestination =
CERTModuleAnchorSource;
+// This anchor is used to force the linker to link the AbslModule.
+extern volatile int AbslModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED AbslModuleAnchorDestination =
+ AbslModuleAnchorSource;
+
// This anchor is used to force the linker to link the BoostModule.
extern volatile int BoostModuleAnchorSource;
static int LLVM_ATTRIBUTE_UNUSED BoostModuleAnchorDestination =
Index: clang-tidy/tool/CMakeLists.txt
===================================================================
--- clang-tidy/tool/CMakeLists.txt
+++ clang-tidy/tool/CMakeLists.txt
@@ -18,6 +18,7 @@
clangBasic
clangTidy
clangTidyAndroidModule
+ clangTidyAbslModule
clangTidyBoostModule
clangTidyBugproneModule
clangTidyCERTModule
Index: clang-tidy/plugin/CMakeLists.txt
===================================================================
--- clang-tidy/plugin/CMakeLists.txt
+++ clang-tidy/plugin/CMakeLists.txt
@@ -9,6 +9,7 @@
clangSema
clangTidy
clangTidyAndroidModule
+ clangTidyAbslModule
clangTidyBoostModule
clangTidyCERTModule
clangTidyCppCoreGuidelinesModule
Index: clang-tidy/absl/StringFindStartswithCheck.h
===================================================================
--- clang-tidy/absl/StringFindStartswithCheck.h
+++ clang-tidy/absl/StringFindStartswithCheck.h
@@ -0,0 +1,37 @@
+//===--- 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_ABSL_STRING_FIND_STARTSWITH_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSL_STRING_FIND_STARTSWITH_H_
+
+#include "../ClangTidy.h"
+#include "../utils/IncludeInserter.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+namespace clang {
+namespace tidy {
+namespace absl {
+
+// Find string.find(...) == 0 comparisons and suggest replacing with StartsWith.
+class StringFindStartswithCheck : public ClangTidyCheck {
+public:
+ using ClangTidyCheck::ClangTidyCheck;
+ void registerPPCallbacks(CompilerInstance &compiler) override;
+ void registerMatchers(ast_matchers::MatchFinder *finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &result) override;
+
+private:
+ std::unique_ptr<clang::tidy::utils::IncludeInserter> include_inserter_;
+};
+
+} // namespace absl
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSL_STRING_FIND_STARTSWITH_H_
Index: clang-tidy/absl/StringFindStartswithCheck.cpp
===================================================================
--- clang-tidy/absl/StringFindStartswithCheck.cpp
+++ clang-tidy/absl/StringFindStartswithCheck.cpp
@@ -0,0 +1,99 @@
+#include "StringFindStartswithCheck.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+using namespace clang::ast_matchers; // NOLINT: Too many names.
+
+namespace clang {
+namespace tidy {
+namespace absl {
+
+void StringFindStartswithCheck::registerMatchers(MatchFinder *finder) {
+ auto zero = integerLiteral(equals(0));
+ // TODO(nikoweh/reviewers): There are a few other options here,
+ // like adding std::string or std::basic_string (?)
+ auto stringClassMatcher = anyOf(cxxRecordDecl(hasName("string")),
+ cxxRecordDecl(hasName("__versa_string")),
+ cxxRecordDecl(hasName("basic_string")));
+
+ 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, zero), 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(zero)),
+ hasEitherOperand(ignoringParenImpCasts(stringFind.bind("findexpr"))))
+ .bind("expr"),
+ this);
+}
+
+void StringFindStartswithCheck::check(const MatchFinder::MatchResult &result) {
+ const auto &context = *result.Context;
+ const auto &source = context.getSourceManager();
+
+ // Extract matching (sub)expressions
+ const auto *expr = result.Nodes.getNodeAs<BinaryOperator>("expr");
+ const auto *needle = result.Nodes.getNodeAs<Expr>("needle");
+ const auto *haystack = result.Nodes.getNodeAs<CXXMemberCallExpr>("findexpr")
+ ->getImplicitObjectArgument();
+
+ // Get the source code blocks (as characters) for both the string object
+ // and the search expression
+ const StringRef needle_expr_code = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(needle->getSourceRange()), source,
+ context.getLangOpts());
+ const StringRef haystack_expr_code = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(haystack->getSourceRange()), source,
+ context.getLangOpts());
+
+ // Create the StartsWith string, negating if comparison was "!=".
+ bool neg = expr->getOpcodeStr() == "!=";
+ StringRef startswith_str;
+ if (neg) {
+ startswith_str = "!absl::StartsWith";
+ } else {
+ startswith_str = "absl::StartsWith";
+ }
+
+ // Create the warning message and a FixIt hint replacing the original expr.
+ auto diag_out = diag(expr->getLocStart(),
+ (StringRef("Use ") + startswith_str +
+ " instead of find() " + expr->getOpcodeStr() + " 0")
+ .str())
+ << FixItHint::CreateReplacement(expr->getSourceRange(),
+ (startswith_str + "(" +
+ haystack_expr_code + ", " +
+ needle_expr_code + ")")
+ .str());
+
+ // Create a preprocessor #include FixIt hint (CreateIncludeInsertion checks
+ // whether this already exists).
+ auto include_hint = include_inserter_->CreateIncludeInsertion(
+ source.getFileID(expr->getLocStart()), "third_party/absl/strings/match.h",
+ false);
+ if (include_hint) {
+ diag_out << *include_hint;
+ }
+}
+
+void StringFindStartswithCheck::registerPPCallbacks(
+ CompilerInstance &compiler) {
+ include_inserter_ = llvm::make_unique<clang::tidy::utils::IncludeInserter>(
+ compiler.getSourceManager(), compiler.getLangOpts(),
+ clang::tidy::utils::IncludeSorter::IS_Google);
+ compiler.getPreprocessor().addPPCallbacks(
+ include_inserter_->CreatePPCallbacks());
+}
+
+} // namespace absl
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/absl/CMakeLists.txt
===================================================================
--- clang-tidy/absl/CMakeLists.txt
+++ clang-tidy/absl/CMakeLists.txt
@@ -0,0 +1,14 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyAbslModule
+ AbslTidyModule.cpp
+ StringFindStartswithCheck.cpp
+
+ LINK_LIBS
+ clangAST
+ clangASTMatchers
+ clangBasic
+ clangLex
+ clangTidy
+ clangTidyUtils
+ )
Index: clang-tidy/absl/AbslTidyModule.cpp
===================================================================
--- clang-tidy/absl/AbslTidyModule.cpp
+++ clang-tidy/absl/AbslTidyModule.cpp
@@ -0,0 +1,39 @@
+//===------- AbslTidyModule.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"
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace absl {
+
+class AbslModule : public ClangTidyModule {
+public:
+ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<StringFindStartswithCheck>(
+ "absl-string-find-startswith");
+ }
+};
+
+// Register the AbslModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<AbslModule> X("absl-module",
+ "Add absl checks.");
+
+} // namespace absl
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the AbslModule.
+volatile int AbslModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/CMakeLists.txt
===================================================================
--- clang-tidy/CMakeLists.txt
+++ clang-tidy/CMakeLists.txt
@@ -27,6 +27,7 @@
)
add_subdirectory(android)
+add_subdirectory(absl)
add_subdirectory(boost)
add_subdirectory(bugprone)
add_subdirectory(cert)
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
