diff --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp --- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp @@ -71,14 +71,6 @@ if (Result.Context->getDiagnostics().hasErrorOccurred()) return; - // Verify the existence and validity of the AST node that roots this rule. - const ast_matchers::BoundNodes::IDToNodeMap &NodesMap = Result.Nodes.getMap(); - auto Root = NodesMap.find(RewriteRule::RootID); - assert(Root != NodesMap.end() && "Transformation failed: missing root node."); - SourceLocation RootLoc = Result.SourceManager->getExpansionLoc( - Root->second.getSourceRange().getBegin()); - assert(RootLoc.isValid() && "Invalid location for Root node of match."); - assert(Rule && "check() should not fire if Rule is None"); RewriteRule::Case Case = tooling::detail::findSelectedCase(Result, *Rule); Expected> Transformations = @@ -99,10 +91,12 @@ << llvm::toString(Explanation.takeError()) << "\n"; return; } - DiagnosticBuilder Diag = diag(RootLoc, *Explanation); - for (const auto &T : *Transformations) { + + // Associate the diagnostic with the location of the first change. + DiagnosticBuilder Diag = + diag((*Transformations)[0].Range.getBegin(), *Explanation); + for (const auto &T : *Transformations) Diag << FixItHint::CreateReplacement(T.Range, T.Replacement); - } for (const auto &I : Case.AddedIncludes) { auto &Header = I.first; diff --git a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp --- a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp @@ -18,18 +18,18 @@ namespace tidy { namespace utils { namespace { +using namespace ::clang::ast_matchers; + using tooling::change; using tooling::IncludeFormat; +using tooling::node; using tooling::RewriteRule; +using tooling::statement; using tooling::text; using tooling::stencil::cat; // Invert the code of an if-statement, while maintaining its semantics. RewriteRule invertIf() { - using namespace ::clang::ast_matchers; - using tooling::node; - using tooling::statement; - StringRef C = "C", T = "T", E = "E"; RewriteRule Rule = tooling::makeRule( ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)), @@ -67,6 +67,57 @@ EXPECT_EQ(Expected, test::runCheckOnCode(Input)); } +class IntLitCheck : public TransformerClangTidyCheck { +public: + IntLitCheck(StringRef Name, ClangTidyContext *Context) + : TransformerClangTidyCheck(tooling::makeRule(integerLiteral(), + change(text("LIT")), + text("no message")), + Name, Context) {} +}; + +// Tests that two changes in a single macro expansion do not lead to conflicts +// in applying the changes. +TEST(TransformerClangTidyCheckTest, TwoChangesInOneMacroExpansion) { + const std::string Input = R"cc( +#define PLUS(a,b) (a) + (b) + int f() { return PLUS(3, 4); } + )cc"; + const std::string Expected = R"cc( +#define PLUS(a,b) (a) + (b) + int f() { return PLUS(LIT, LIT); } + )cc"; + + EXPECT_EQ(Expected, test::runCheckOnCode(Input)); +} + +class BinOpCheck : public TransformerClangTidyCheck { +public: + BinOpCheck(StringRef Name, ClangTidyContext *Context) + : TransformerClangTidyCheck( + tooling::makeRule( + binaryOperator(hasOperatorName("+"), hasRHS(expr().bind("r"))), + change(node("r"), text("RIGHT")), text("no message")), + Name, Context) {} +}; + +// Tests case where the rule's match spans both source from the macro and its +// argument, while the change spans only the argument AND there are two such +// matches. Here, we expect a conflict between the two matches and the second +// to be ignored. +TEST(TransformerClangTidyCheckTest, TwoMatchesInMacroExpansion) { + const std::string Input = R"cc( +#define M(a,b) (1 + a) * (1 + b) + int f() { return M(3, 4); } + )cc"; + const std::string Expected = R"cc( +#define M(a,b) (1 + a) * (1 + b) + int f() { return M(RIGHT, RIGHT); } + )cc"; + + EXPECT_EQ(Expected, test::runCheckOnCode(Input)); +} + // A trivial rewrite-rule generator that requires Objective-C code. Optional needsObjC(const LangOptions &LangOpts, const ClangTidyCheck::OptionsView &Options) {