[clang] ff2743b - [libTooling] In Transformer, allow atomic changes to span multiple files.
Yitzhak Mandelbaum via cfe-commits
cfe-commits at lists.llvm.org
Tue May 26 06:23:10 PDT 2020
Author: Yitzhak Mandelbaum
Date: 2020-05-26T09:17:35-04:00
New Revision: ff2743bf047deac7ef6cc6c3efd30ff05e55b2ad
URL: https://github.com/llvm/llvm-project/commit/ff2743bf047deac7ef6cc6c3efd30ff05e55b2ad
DIFF: https://github.com/llvm/llvm-project/commit/ff2743bf047deac7ef6cc6c3efd30ff05e55b2ad.diff
LOG: [libTooling] In Transformer, allow atomic changes to span multiple files.
Summary:
Currently, all changes returned by a single application of a rule must fit in
one atomic change and therefore must apply to one file. However, there are
patterns in which a single rule will want to modify multiple files; for example,
a header and implementation to change a declaration and its definition. This
patch relaxes Transformer, libTooling's interpreter of RewriteRules, to support
multiple changes.
Reviewers: gribozavr
Subscribers: mgrang, jfb, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D80239
Added:
Modified:
clang/lib/Tooling/Transformer/Transformer.cpp
clang/unittests/Tooling/TransformerTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Tooling/Transformer/Transformer.cpp b/clang/lib/Tooling/Transformer/Transformer.cpp
index 93c2c0912d21..71340bf2f676 100644
--- 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 <map>
#include <utility>
#include <vector>
@@ -45,28 +46,39 @@ void Transformer::run(const MatchFinder::MatchResult &Result) {
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<FileID, AtomicChange> 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
index 1d955cf5e9b8..c8c6db059fed 100644
--- a/clang/unittests/Tooling/TransformerTest.cpp
+++ b/clang/unittests/Tooling/TransformerTest.cpp
@@ -817,4 +817,46 @@ TEST(TransformerDeathTest, OrderedRuleTypes) {
"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<std::string>(), "input.cc",
+ "clang-tool", std::make_shared<PCHContainerOperations>(),
+ {{"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<std::string> UpdatedCode =
+ clang::tooling::applyAllReplacements(Header,
+ Changes[0].getReplacements());
+ ASSERT_TRUE(static_cast<bool>(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<bool>(UpdatedCode))
+ << "Could not update code: " << llvm::toString(UpdatedCode.takeError());
+ EXPECT_EQ(format(*UpdatedCode), format(R"cc(#include "input.h"
+ ;)cc"));
+}
} // namespace
More information about the cfe-commits
mailing list