diff --git a/clang/lib/Tooling/Transformer/Transformer.cpp b/clang/lib/Tooling/Transformer/Transformer.cpp --- a/clang/lib/Tooling/Transformer/Transformer.cpp +++ b/clang/lib/Tooling/Transformer/Transformer.cpp @@ -12,6 +12,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Tooling/Refactoring/AtomicChange.h" #include "llvm/Support/Error.h" +#include #include #include @@ -45,28 +46,39 @@ return; } - // Record the results in the AtomicChange, anchored at the location of the - // first change. - AtomicChange AC(*Result.SourceManager, - (*Transformations)[0].Range.getBegin()); + // Group the transformations, by file, into AtomicChanges, each anchored by + // the location of the first change in that file. + std::map ChangesByFileID; for (const auto &T : *Transformations) { + auto ID = Result.SourceManager->getFileID(T.Range.getBegin()); + auto Iter = ChangesByFileID + .emplace(ID, AtomicChange(*Result.SourceManager, + T.Range.getBegin())) + .first; + auto &AC = Iter->second; if (auto Err = AC.replace(*Result.SourceManager, T.Range, T.Replacement)) { Consumer(std::move(Err)); return; } } - for (const auto &I : Case.AddedIncludes) { - auto &Header = I.first; - switch (I.second) { - case transformer::IncludeFormat::Quoted: - AC.addHeader(Header); - break; - case transformer::IncludeFormat::Angled: - AC.addHeader((llvm::Twine("<") + Header + ">").str()); - break; + for (auto &IDChangePair : ChangesByFileID) { + auto &AC = IDChangePair.second; + // FIXME: this will add includes to *all* changed files, which may not be + // the intent. We should upgrade the representation to allow associating + // headers with specific edits. + for (const auto &I : Case.AddedIncludes) { + auto &Header = I.first; + switch (I.second) { + case transformer::IncludeFormat::Quoted: + AC.addHeader(Header); + break; + case transformer::IncludeFormat::Angled: + AC.addHeader((llvm::Twine("<") + Header + ">").str()); + break; + } } - } - Consumer(std::move(AC)); + Consumer(std::move(AC)); + } } diff --git a/clang/unittests/Tooling/TransformerTest.cpp b/clang/unittests/Tooling/TransformerTest.cpp --- a/clang/unittests/Tooling/TransformerTest.cpp +++ b/clang/unittests/Tooling/TransformerTest.cpp @@ -817,4 +817,46 @@ "Matcher must be.*node matcher"); } #endif + +// Edits are able to span multiple files; in this case, a header and an +// implementation file. +TEST_F(TransformerTest, MultipleFiles) { + std::string Header = R"cc(void RemoveThisFunction();)cc"; + std::string Source = R"cc(#include "input.h" + void RemoveThisFunction();)cc"; + Transformer T( + makeRule(functionDecl(hasName("RemoveThisFunction")), changeTo(cat(""))), + consumer()); + T.registerMatchers(&MatchFinder); + auto Factory = newFrontendActionFactory(&MatchFinder); + EXPECT_TRUE(runToolOnCodeWithArgs( + Factory->create(), Source, std::vector(), "input.cc", + "clang-tool", std::make_shared(), + {{"input.h", Header}})); + + std::sort(Changes.begin(), Changes.end(), + [](const AtomicChange &L, const AtomicChange &R) { + return L.getFilePath() < R.getFilePath(); + }); + + ASSERT_EQ(Changes[0].getFilePath(), "./input.h"); + EXPECT_THAT(Changes[0].getInsertedHeaders(), IsEmpty()); + EXPECT_THAT(Changes[0].getRemovedHeaders(), IsEmpty()); + llvm::Expected UpdatedCode = + clang::tooling::applyAllReplacements(Header, + Changes[0].getReplacements()); + ASSERT_TRUE(static_cast(UpdatedCode)) + << "Could not update code: " << llvm::toString(UpdatedCode.takeError()); + EXPECT_EQ(format(*UpdatedCode), format(R"cc(;)cc")); + + ASSERT_EQ(Changes[1].getFilePath(), "input.cc"); + EXPECT_THAT(Changes[1].getInsertedHeaders(), IsEmpty()); + EXPECT_THAT(Changes[1].getRemovedHeaders(), IsEmpty()); + UpdatedCode = clang::tooling::applyAllReplacements( + Source, Changes[1].getReplacements()); + ASSERT_TRUE(static_cast(UpdatedCode)) + << "Could not update code: " << llvm::toString(UpdatedCode.takeError()); + EXPECT_EQ(format(*UpdatedCode), format(R"cc(#include "input.h" + ;)cc")); +} } // namespace