Index: clang-tidy/CMakeLists.txt =================================================================== --- clang-tidy/CMakeLists.txt +++ clang-tidy/CMakeLists.txt @@ -27,6 +27,7 @@ ) add_subdirectory(android) +add_subdirectory(abseil) add_subdirectory(boost) add_subdirectory(bugprone) add_subdirectory(cert) Index: clang-tidy/abseil/AbseilTidyModule.cpp =================================================================== --- clang-tidy/abseil/AbseilTidyModule.cpp +++ clang-tidy/abseil/AbseilTidyModule.cpp @@ -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( + "abseil-string-find-startswith"); + } +}; + +// Register the AbseilModule using this statically initialized variable. +static ClangTidyModuleRegistry::Add 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 Index: clang-tidy/abseil/CMakeLists.txt =================================================================== --- clang-tidy/abseil/CMakeLists.txt +++ clang-tidy/abseil/CMakeLists.txt @@ -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 + ) Index: clang-tidy/abseil/StringFindStartswithCheck.h =================================================================== --- clang-tidy/abseil/StringFindStartswithCheck.h +++ clang-tidy/abseil/StringFindStartswithCheck.h @@ -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 +#include +#include + +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 IncludeInserter; + const std::vector 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_ Index: clang-tidy/abseil/StringFindStartswithCheck.cpp =================================================================== --- clang-tidy/abseil/StringFindStartswithCheck.cpp +++ clang-tidy/abseil/StringFindStartswithCheck.cpp @@ -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 + +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( + 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("expr"); + assert(ComparisonExpr != nullptr); + const auto *Needle = Result.Nodes.getNodeAs("needle"); + assert(Needle != nullptr); + const Expr *Haystack = Result.Nodes.getNodeAs("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( + 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 Index: clang-tidy/plugin/CMakeLists.txt =================================================================== --- clang-tidy/plugin/CMakeLists.txt +++ clang-tidy/plugin/CMakeLists.txt @@ -9,6 +9,7 @@ clangSema clangTidy clangTidyAndroidModule + clangTidyAbseilModule clangTidyBoostModule clangTidyCERTModule clangTidyCppCoreGuidelinesModule Index: clang-tidy/tool/CMakeLists.txt =================================================================== --- clang-tidy/tool/CMakeLists.txt +++ clang-tidy/tool/CMakeLists.txt @@ -18,6 +18,7 @@ clangBasic clangTidy clangTidyAndroidModule + clangTidyAbseilModule clangTidyBoostModule clangTidyBugproneModule clangTidyCERTModule Index: clang-tidy/tool/ClangTidyMain.cpp =================================================================== --- clang-tidy/tool/ClangTidyMain.cpp +++ clang-tidy/tool/ClangTidyMain.cpp @@ -499,6 +499,11 @@ 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 = Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -77,6 +77,15 @@ Warns if a class inherits from multiple classes that are not pure virtual. +- New `abseil` module for checks related to the `Abseil `_ + library. + +- New `abseil-string-find-startswith + `_ check + + Checks whether a ``std::string::find()`` result is compared with 0, and + suggests replacing with ``absl::StartsWith()``. + - New `fuchsia-statically-constructed-objects `_ check Index: docs/clang-tidy/checks/abseil-string-find-startswith.rst =================================================================== --- docs/clang-tidy/checks/abseil-string-find-startswith.rst +++ docs/clang-tidy/checks/abseil-string-find-startswith.rst @@ -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``. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -4,6 +4,7 @@ ================= .. toctree:: + abseil-string-find-startswith android-cloexec-accept android-cloexec-accept4 android-cloexec-creat Index: test/clang-tidy/abseil-string-find-startswith.cpp =================================================================== --- test/clang-tidy/abseil-string-find-startswith.cpp +++ test/clang-tidy/abseil-string-find-startswith.cpp @@ -0,0 +1,55 @@ +// RUN: %check_clang_tidy %s abseil-string-find-startswith %t + +namespace std { +template class allocator {}; +template class char_traits {}; +template , + typename A = std::allocator> +struct basic_string { + basic_string(); + basic_string(const basic_string &); + basic_string(const C *, const A &a = A()); + ~basic_string(); + int find(basic_string s, int pos = 0); + int find(const char *s, int pos = 0); +}; +typedef basic_string string; +typedef basic_string 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; +}