diff --git a/.gitignore b/.gitignore --- a/.gitignore +++ b/.gitignore @@ -46,8 +46,6 @@ /CMakeSettings.json # CLion project configuration /.idea -# NIX shell config file -default.nix #==============================================================================# # Directories to ignore (do not add trailing '/'s, they skip symlinks). diff --git a/clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.h b/clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.h --- a/clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.h +++ b/clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.h @@ -10,22 +10,33 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_STRINGREFERENCINGCHECK_H #include "../ClangTidyCheck.h" +#include "../utils/IncludeInserter.h" +#include "clang/Frontend/CompilerInstance.h" namespace clang { namespace tidy { namespace llvm_check { - -/// LLVM guidelines say that llvm::StringRef should be used for function parameters instead of references to std::string. +/// LThe LLVM Programmer's Manual recommends that llvm::StringRef should be +/// used for function parameters instead of references to const std::string: +/// https://llvm.org/docs/ProgrammersManual.html#the-stringref-class /// For the user-facing documentation see: /// http://clang.llvm.org/extra/clang-tidy/checks/llvm-string-referencing.html class StringReferencingCheck : public ClangTidyCheck { -const llvm::StringRef BoundNodeId = "string_reference"; + const llvm::StringRef BoundNodeId = "string_reference"; + public: StringReferencingCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + : ClangTidyCheck(Name, Context), + Inserter(Options.getLocalOrGlobal("IncludeStyle", + utils::IncludeSorter::IS_LLVM)) {} void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + utils::IncludeInserter Inserter; }; } // namespace llvm_check diff --git a/clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.cpp b/clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.cpp --- a/clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/StringReferencingCheck.cpp @@ -11,6 +11,7 @@ #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" using namespace clang::ast_matchers; @@ -19,26 +20,42 @@ namespace llvm_check { void StringReferencingCheck::registerMatchers(MatchFinder *Finder) { - // create a matcher looking for std::string& parameters - auto Matcher = parmVarDecl( - hasType( - references( - cxxRecordDecl( - matchesName("std::string"))))); + // create a matcher looking for const std::string& parameters + auto Matcher = + parmVarDecl(hasType(references(namedDecl( + hasName("string"), isInStdNamespace()))), // std::string& + hasType(references(qualType(isConstQualified()))), // const + hasType(lValueReferenceType()) // don't match std::string&& + ); // Add matcher to the finder and bound matched nodes Finder->addMatcher(Matcher.bind(BoundNodeId), this); } +void StringReferencingCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + Inserter.registerPreprocessor(PP); +} + void StringReferencingCheck::check(const MatchFinder::MatchResult &Result) { + // Retrieve parameters found by matcher const auto *ParamDecl = Result.Nodes.getNodeAs(BoundNodeId); + // Extract parameter type location in source clang::SourceRange TypeRange; - auto ParamBegin = ParamDecl->getTypeSpecStartLoc(); + auto ParamBegin = ParamDecl->getBeginLoc(); + if (!ParamBegin.isValid()) + return; TypeRange.setBegin(ParamBegin); TypeRange.setEnd(ParamDecl->getTypeSpecEndLoc()); - diag(ParamBegin, "Use of std::string& is against LLVM guidelines"); - diag(ParamBegin, "replace parameter %0 type with llvm::StringRef", DiagnosticIDs::Note) - << ParamDecl - << FixItHint::CreateReplacement(TypeRange, "llvm::StringRef"); + // Generate diagnostics and fix + diag(ParamBegin, "Use of const std::string & is discouraged in the LLVM " + "Programmer's Manual"); + diag(ParamBegin, "replace parameter %0 type with llvm::StringRef", + DiagnosticIDs::Note) + << ParamDecl + << FixItHint::CreateReplacement(TypeRange, "llvm::StringRef ") + << Inserter.createMainFileIncludeInsertion( + "llvm/include/llvm/ADT/StringRef.h", + /*IsAngled=*/false); } } // namespace llvm_check diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp @@ -1,23 +1,66 @@ // RUN: %check_clang_tidy %s llvm-string-referencing %t -namespace std{ - class string; +namespace std { +class string {}; +class u18_string_t; + } // namespace std -namespace llvm{ - class StringRef; +namespace llvm { +class StringRef; } // namespace llvm -void f(std::string& P){ -// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: Use of std::string& is against LLVM guidelines [llvm-string-referencing] -// CHECK-FIXES: void f(llvm::StringRef P){{{$}} -return; +class String; + +namespace A { +using namespace std; +void f(const string &P); +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] +// CHECK-FIXES: void f(llvm::StringRef P);{{$}} +} // namespace A + +namespace B { +using std::string; +void f1(const string &P); +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] +// CHECK-FIXES: void f1(llvm::StringRef P);{{$}} +} // namespace B + +void f2(std::string, int, const std::string &); +// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] +// CHECK-FIXES: void f2(std::string, int, llvm::StringRef );{{$}} +void f2(std::string P, int x, const std::string &P2) { + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] + // CHECK-FIXES: void f2(std::string P, int x, llvm::StringRef P2) {{{$}} + return; } -void f2(int x, std::string& P){ -// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Use of std::string& is against LLVM guidelines [llvm-string-referencing] -// CHECK-FIXES: void f2(int x, llvm::StringRef P){{{$}} +void f3(const std::string &P1, const std::string &P2); +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] +// CHECK-MESSAGES: :[[@LINE-2]]:32: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] +// CHECK-FIXES: void f3(llvm::StringRef P1, llvm::StringRef P2);{{$}} + +struct St { + void operator=(const std::string &Val) const { + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] + // CHECK-FIXES: void operator=(llvm::StringRef Val) const {{{$}} return; -} + } +}; + +void f7(const std::string &); +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Use of const std::string & is discouraged in the LLVM Programmer's Manual [llvm-string-referencing] +// CHECK-FIXES: void f7(llvm::StringRef );{{$}} + +// Functions below this line should not trigger the check +void f1(std::string &P); + +void f4(std::string *P); + +void f5(String &P); + +void f6(llvm::StringRef P); + +void f9(std::u18_string_t &P); -void f3(llvm::StringRef P); +void f10(const std::string &&P);