Index: clang-tidy/ClangTidyDiagnosticConsumer.h =================================================================== --- clang-tidy/ClangTidyDiagnosticConsumer.h +++ clang-tidy/ClangTidyDiagnosticConsumer.h @@ -173,6 +173,10 @@ /// \brief Store an \p Error. void storeError(const ClangTidyError &Error); + /// \brief Applies \c ClangTidyOptions::Substitutions to each + /// \c ClangTidyError::Fix's replacement text. + void applySubstitutions(ClangTidyError &Error) const; + std::vector Errors; DiagnosticsEngine *DiagEngine; std::unique_ptr OptionsProvider; Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp =================================================================== --- clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -229,6 +229,36 @@ /// \brief Store a \c ClangTidyError. void ClangTidyContext::storeError(const ClangTidyError &Error) { Errors.push_back(Error); + applySubstitutions(Errors.back()); +} + +void ClangTidyContext::applySubstitutions(ClangTidyError &Error) const { + std::vector> + ChangedReplacements; + for (const auto &Replacement : Error.Fix) { + std::string ReplacementText = Replacement.getReplacementText().str(); + for (const auto &Substitution : getOptions().Substitutions) { + const std::string &From = Substitution.first; + const std::string &To = Substitution.second; + std::string::size_type Pos = 0; + while ((Pos = ReplacementText.find(From, Pos)) != std::string::npos) { + ReplacementText = ReplacementText.substr(0, Pos) + To + + ReplacementText.substr(Pos + From.size()); + Pos += To.size(); + } + } + if (Replacement.getReplacementText() != ReplacementText) { + ChangedReplacements.emplace_back( + Replacement, tooling::Replacement( + Replacement.getFilePath(), Replacement.getOffset(), + Replacement.getLength(), ReplacementText)); + } + } + + for (const auto &ChangedReplacement : ChangedReplacements) { + Error.Fix.erase(ChangedReplacement.first); + Error.Fix.insert(ChangedReplacement.second); + } } StringRef ClangTidyContext::getCheckName(unsigned DiagnosticID) const { Index: clang-tidy/ClangTidyOptions.h =================================================================== --- clang-tidy/ClangTidyOptions.h +++ clang-tidy/ClangTidyOptions.h @@ -77,6 +77,20 @@ /// \brief Key-value mapping used to store check-specific options. OptionMap CheckOptions; + + /// \brief Maps placeholders to the content that they should be replaced with. + /// + /// ClangTidy supports substitiution of placeholders in replacement strings of + /// the \c FixItHints. \c Substitutions maps placeholders to the text inserted + /// instead. + OptionMap Substitutions; + + /// \brief Placeholders available globally. + struct Placeholders { + /// \brief This placeholder can be used whenever the user's name/email is + /// needed (e.g. in TODO() comments in certain styles). + static const char User[]; + }; }; /// \brief Abstract interface for retrieving various ClangTidy options. Index: clang-tidy/ClangTidyOptions.cpp =================================================================== --- clang-tidy/ClangTidyOptions.cpp +++ clang-tidy/ClangTidyOptions.cpp @@ -82,10 +82,21 @@ static void mapping(IO &IO, ClangTidyOptions &Options) { MappingNormalization NOpts( IO, Options.CheckOptions); + MappingNormalization NSubsts( + IO, Options.Substitutions); IO.mapOptional("Checks", Options.Checks); IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex); IO.mapOptional("AnalyzeTemporaryDtors", Options.AnalyzeTemporaryDtors); IO.mapOptional("CheckOptions", NOpts->Options); + IO.mapOptional("Substitutions", NSubsts->Options); + } + static StringRef validate(IO &io, ClangTidyOptions &Options) { + for (const auto &S : Options.Substitutions) { + StringRef Key = S.first; + if (!Key.startswith("${") || !Key.endswith("}")) + return "Substitution name has to be enclosed in ${}"; + } + return StringRef(); } }; @@ -95,6 +106,8 @@ namespace clang { namespace tidy { +const char ClangTidyOptions::Placeholders::User[] = "${ClangTidy.User}"; + ClangTidyOptions ClangTidyOptions::mergeWith(const ClangTidyOptions &Other) const { ClangTidyOptions Result = *this; @@ -113,6 +126,9 @@ for (const auto &KeyValue : Other.CheckOptions) Result.CheckOptions[KeyValue.first] = KeyValue.second; + for (const auto &KeyValue : Other.Substitutions) + Result.Substitutions[KeyValue.first] = KeyValue.second; + return Result; } Index: clang-tidy/google/TodoCommentCheck.h =================================================================== --- clang-tidy/google/TodoCommentCheck.h +++ clang-tidy/google/TodoCommentCheck.h @@ -22,7 +22,6 @@ class TodoCommentCheck : public ClangTidyCheck { public: TodoCommentCheck(StringRef Name, ClangTidyContext *Context); - ~TodoCommentCheck(); void registerPPCallbacks(CompilerInstance &Compiler) override; private: Index: clang-tidy/google/TodoCommentCheck.cpp =================================================================== --- clang-tidy/google/TodoCommentCheck.cpp +++ clang-tidy/google/TodoCommentCheck.cpp @@ -35,13 +35,9 @@ if (!Username.empty()) return false; - // If the username is missing put in the current user's name. Not ideal but - // works for running tidy locally. - // FIXME: Can we get this from a more reliable source? - const char *User = std::getenv("USER"); - if (!User) - User = "unknown"; - std::string NewText = ("// TODO(" + Twine(User) + "): " + Comment).str(); + std::string NewText = + ("// TODO(" + Twine(ClangTidyOptions::Placeholders::User) + "): " + + Comment).str(); Check.diag(Range.getBegin(), "missing username/bug in TODO") << FixItHint::CreateReplacement(CharSourceRange::getCharRange(Range), @@ -58,8 +54,6 @@ : ClangTidyCheck(Name, Context), Handler(llvm::make_unique(*this)) {} -TodoCommentCheck::~TodoCommentCheck() {} - void TodoCommentCheck::registerPPCallbacks(CompilerInstance &Compiler) { Compiler.getPreprocessor().addCommentHandler(Handler.get()); } Index: clang-tidy/tool/ClangTidyMain.cpp =================================================================== --- clang-tidy/tool/ClangTidyMain.cpp +++ clang-tidy/tool/ClangTidyMain.cpp @@ -17,6 +17,7 @@ #include "../ClangTidy.h" #include "clang/Tooling/CommonOptionsParser.h" +#include "llvm/Support/Process.h" using namespace clang::ast_matchers; using namespace clang::driver; @@ -156,6 +157,10 @@ if (AnalyzeTemporaryDtors.getNumOccurrences() > 0) OverrideOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors; + // FIXME: Allow this to be overridden from a config file. + if (llvm::Optional User = llvm::sys::Process::GetEnv("USER")) + OverrideOptions.Substitutions[ClangTidyOptions::Placeholders::User] = *User; + auto OptionsProvider = llvm::make_unique( GlobalOptions, FallbackOptions, OverrideOptions); Index: unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp =================================================================== --- unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp +++ unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp @@ -1,5 +1,6 @@ #include "ClangTidy.h" #include "ClangTidyTest.h" +#include "clang/Lex/Lexer.h" #include "gtest/gtest.h" namespace clang { @@ -29,6 +30,49 @@ EXPECT_EQ("variable", Errors[1].Message.Message); } +TEST(ClangTidyDiagnosticConsumer, HandlesSubstitutions) { + class SubstitutionsTestCheck : public TestCheck { + public: + SubstitutionsTestCheck(StringRef Name, ClangTidyContext *Context) + : TestCheck(Name, Context) {} + void check(const ast_matchers::MatchFinder::MatchResult &Result) override { + const VarDecl *Var = Result.Nodes.getNodeAs("var"); + diag(Var->getTypeSpecStartLoc(), "around type") + << FixItHint::CreateInsertion(Var->getTypeSpecStartLoc(), + ClangTidyOptions::Placeholders::User) + << FixItHint::CreateInsertion( + Lexer::getLocForEndOfToken(Var->getTypeSpecStartLoc(), 0, + *Result.SourceManager, + Result.Context->getLangOpts()), + ClangTidyOptions::Placeholders::User); + diag(Var->getLocation(), "instead of name") + << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(Var->getLocation(), + Var->getLocation()), + "\\${aaa}/${aaa}${aaa}${}}}{{"); + } + }; + + std::vector Errors; + ClangTidyOptions Options; + std::string ActualUser = "<<>>"; + Options.Substitutions[ClangTidyOptions::Placeholders::User] = ActualUser; + Options.Substitutions["${aaa}"] = "123"; + std::string Replacement2 = "\\123/123123${}}}{{"; + std::string Result = runCheckOnCode( + "int abc;", &Errors, "input.cc", None, Options); + EXPECT_EQ(ActualUser + "int" + ActualUser + " " + Replacement2 + ";", Result); + EXPECT_EQ(2ul, Errors.size()); + EXPECT_EQ("around type", Errors[0].Message.Message); + EXPECT_EQ(2ul, Errors[0].Fix.size()); + for (const auto &Fix : Errors[0].Fix) + EXPECT_EQ(ActualUser, Fix.getReplacementText()); + + EXPECT_EQ("instead of name", Errors[1].Message.Message); + EXPECT_EQ(1ul, Errors[1].Fix.size()); + EXPECT_EQ(Replacement2, Errors[1].Fix.begin()->getReplacementText()); +} + TEST(GlobList, Empty) { GlobList Filter(""); Index: unittests/clang-tidy/ClangTidyTest.h =================================================================== --- unittests/clang-tidy/ClangTidyTest.h +++ unittests/clang-tidy/ClangTidyTest.h @@ -40,12 +40,15 @@ }; template -std::string runCheckOnCode(StringRef Code, - std::vector *Errors = nullptr, - const Twine &Filename = "input.cc", - ArrayRef ExtraArgs = None) { +std::string +runCheckOnCode(StringRef Code, std::vector *Errors = nullptr, + const Twine &Filename = "input.cc", + ArrayRef ExtraArgs = None, + llvm::Optional OverrideOptions = None) { ClangTidyOptions Options; Options.Checks = "*"; + if (OverrideOptions) + Options = Options.mergeWith(*OverrideOptions); ClangTidyContext Context(llvm::make_unique( ClangTidyGlobalOptions(), Options)); ClangTidyDiagnosticConsumer DiagConsumer(Context);