ioeric updated this revision to Diff 90018. ioeric marked 3 inline comments as done. ioeric added a comment.
- Return const refs in getXXX. https://reviews.llvm.org/D27054 Files: include/clang/Tooling/Refactoring/AtomicChange.h lib/Tooling/CMakeLists.txt lib/Tooling/Refactoring/AtomicChange.cpp lib/Tooling/Refactoring/CMakeLists.txt unittests/Tooling/CMakeLists.txt unittests/Tooling/RefactoringTest.cpp
Index: unittests/Tooling/RefactoringTest.cpp =================================================================== --- unittests/Tooling/RefactoringTest.cpp +++ unittests/Tooling/RefactoringTest.cpp @@ -26,6 +26,7 @@ #include "clang/Frontend/TextDiagnosticPrinter.h" #include "clang/Rewrite/Core/Rewriter.h" #include "clang/Tooling/Refactoring.h" +#include "clang/Tooling/Refactoring/AtomicChange.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/SmallString.h" #include "gtest/gtest.h" @@ -102,10 +103,10 @@ // Checks that an llvm::Error instance contains a ReplacementError with expected // error code, expected new replacement, and expected existing replacement. -static bool checkReplacementError( - llvm::Error&& Error, replacement_error ExpectedErr, - llvm::Optional<Replacement> ExpectedExisting, - llvm::Optional<Replacement> ExpectedNew) { +static bool checkReplacementError(llvm::Error &&Error, + replacement_error ExpectedErr, + llvm::Optional<Replacement> ExpectedExisting, + llvm::Optional<Replacement> ExpectedNew) { if (!Error) { llvm::errs() << "Error is a success."; return false; @@ -1089,5 +1090,187 @@ EXPECT_TRUE(FileToReplaces.empty()); } +class AtomicChangeTest : public ::testing::Test { + protected: + void setUp() { + DefaultFileID = Context.createInMemoryFile("input.cpp", DefaultCode); + DefaultLoc = Context.Sources.getLocForStartOfFile(DefaultFileID) + .getLocWithOffset(20); + assert(DefaultLoc.isValid() && "Default location must be valid."); + } + + RewriterTestContext Context; + std::string DefaultCode = std::string(100, 'a'); + unsigned DefaultOffset = 20; + SourceLocation DefaultLoc; + FileID DefaultFileID; +}; + +TEST_F(AtomicChangeTest, AtomicChangeToYAML) { + setUp(); + AtomicChange Change(Context.Sources, DefaultLoc); + llvm::Error Err = + Change.insert(Context.Sources, DefaultLoc, "aa", /*InsertAfter=*/false); + ASSERT_TRUE(!Err); + Err = Change.insert(Context.Sources, DefaultLoc.getLocWithOffset(10), "bb", + /*InsertAfter=*/false); + ASSERT_TRUE(!Err); + Change.addHeader("a.h"); + Change.removeHeader("b.h"); + std::string YAMLString = Change.toYAMLString(); + + // NOTE: If this test starts to fail for no obvious reason, check whitespace. + ASSERT_STREQ("---\n" + "Key: 'input.cpp:20'\n" + "FilePath: input.cpp\n" + "Error: ''\n" + "InsertedHeaders: [ a.h ]\n" + "RemovedHeaders: [ b.h ]\n" + "Replacements: \n" // Extra whitespace here! + " - FilePath: input.cpp\n" + " Offset: 20\n" + " Length: 0\n" + " ReplacementText: aa\n" + " - FilePath: input.cpp\n" + " Offset: 30\n" + " Length: 0\n" + " ReplacementText: bb\n" + "...\n", + YAMLString.c_str()); +} + +TEST_F(AtomicChangeTest, YAMLToAtomicChange) { + setUp(); + std::string YamlContent = "---\n" + "Key: 'input.cpp:20'\n" + "FilePath: input.cpp\n" + "Error: 'ok'\n" + "InsertedHeaders: [ a.h ]\n" + "RemovedHeaders: [ b.h ]\n" + "Replacements: \n" // Extra whitespace here! + " - FilePath: input.cpp\n" + " Offset: 20\n" + " Length: 0\n" + " ReplacementText: aa\n" + " - FilePath: input.cpp\n" + " Offset: 30\n" + " Length: 0\n" + " ReplacementText: bb\n" + "...\n"; + AtomicChange ExpectedChange(Context.Sources, DefaultLoc); + llvm::Error Err = ExpectedChange.insert(Context.Sources, DefaultLoc, "aa", + /*InsertAfter=*/false); + ASSERT_TRUE(!Err); + Err = ExpectedChange.insert(Context.Sources, DefaultLoc.getLocWithOffset(10), + "bb", /*InsertAfter=*/false); + ASSERT_TRUE(!Err); + + ExpectedChange.addHeader("a.h"); + ExpectedChange.removeHeader("b.h"); + ExpectedChange.setError("ok"); + + AtomicChange ActualChange = AtomicChange::convertFromYAML(YamlContent); + EXPECT_EQ(ExpectedChange.getKey(), ActualChange.getKey()); + EXPECT_EQ(ExpectedChange.getFilePath(), ActualChange.getFilePath()); + EXPECT_EQ(ExpectedChange.getError(), ActualChange.getError()); + EXPECT_EQ(ExpectedChange.getInsertedHeaders(), ActualChange.getInsertedHeaders()); + EXPECT_EQ(ExpectedChange.getRemovedHeaders(), ActualChange.getRemovedHeaders()); + EXPECT_EQ(ExpectedChange.getReplacements().size(), + ActualChange.getReplacements().size()); + EXPECT_EQ(2u, ActualChange.getReplacements().size()); + EXPECT_EQ(*ExpectedChange.getReplacements().begin(), + *ActualChange.getReplacements().begin()); + EXPECT_EQ(*(++ExpectedChange.getReplacements().begin()), + *(++ActualChange.getReplacements().begin())); +} + +TEST_F(AtomicChangeTest, CheckKeyAndKeyFile) { + setUp(); + AtomicChange Change(Context.Sources, DefaultLoc); + EXPECT_EQ("input.cpp:20", Change.getKey()); + EXPECT_EQ("input.cpp", Change.getFilePath()); +} + +TEST_F(AtomicChangeTest, InsertBefore) { + setUp(); + AtomicChange Change(Context.Sources, DefaultLoc); + llvm::Error Err = Change.insert(Context.Sources, DefaultLoc, "aa"); + ASSERT_TRUE(!Err); + EXPECT_EQ(Change.getReplacements().size(), 1u); + EXPECT_EQ(*Change.getReplacements().begin(), + Replacement(Context.Sources, DefaultLoc, 0, "aa")); + Err = Change.insert(Context.Sources, DefaultLoc, "b", /*InsertAfter=*/false); + ASSERT_TRUE(!Err); + EXPECT_EQ(Change.getReplacements().size(), 1u); + EXPECT_EQ(*Change.getReplacements().begin(), + Replacement(Context.Sources, DefaultLoc, 0, "baa")); +} + +TEST_F(AtomicChangeTest, InsertAfter) { + setUp(); + AtomicChange Change(Context.Sources, DefaultLoc); + llvm::Error Err = Change.insert(Context.Sources, DefaultLoc, "aa"); + ASSERT_TRUE(!Err); + EXPECT_EQ(Change.getReplacements().size(), 1u); + EXPECT_EQ(*Change.getReplacements().begin(), + Replacement(Context.Sources, DefaultLoc, 0, "aa")); + Err = Change.insert(Context.Sources, DefaultLoc, "b"); + ASSERT_TRUE(!Err); + EXPECT_EQ(Change.getReplacements().size(), 1u); + EXPECT_EQ(*Change.getReplacements().begin(), + Replacement(Context.Sources, DefaultLoc, 0, "aab")); +} + +TEST_F(AtomicChangeTest, InsertBeforeWithInvalidLocation) { + setUp(); + AtomicChange Change(Context.Sources, DefaultLoc); + llvm::Error Err = + Change.insert(Context.Sources, DefaultLoc, "a", /*InsertAfter=*/false); + ASSERT_TRUE(!Err); + + // Invalid location. + Err = Change.insert(Context.Sources, SourceLocation(), "a", + /*InsertAfter=*/false); + ASSERT_TRUE((bool)Err); + EXPECT_TRUE(checkReplacementError( + std::move(Err), replacement_error::wrong_file_path, + Replacement(Context.Sources, DefaultLoc, 0, "a"), + Replacement(Context.Sources, SourceLocation(), 0, "a"))); + +} + +TEST_F(AtomicChangeTest, InsertBeforeToWrongFile) { + setUp(); + AtomicChange Change(Context.Sources, DefaultLoc); + llvm::Error Err = + Change.insert(Context.Sources, DefaultLoc, "a", /*InsertAfter=*/false); + ASSERT_TRUE(!Err); + + // Inserting at a different file. + FileID NewID = Context.createInMemoryFile("extra.cpp", DefaultCode); + SourceLocation NewLoc = Context.Sources.getLocForStartOfFile(NewID); + Err = Change.insert(Context.Sources, NewLoc, "b", /*InsertAfter=*/false); + ASSERT_TRUE((bool)Err); + EXPECT_TRUE( + checkReplacementError(std::move(Err), replacement_error::wrong_file_path, + Replacement(Context.Sources, DefaultLoc, 0, "a"), + Replacement(Context.Sources, NewLoc, 0, "b"))); +} + +TEST_F(AtomicChangeTest, InsertAfterWithInvalidLocation) { + setUp(); + AtomicChange Change(Context.Sources, DefaultLoc); + llvm::Error Err = Change.insert(Context.Sources, DefaultLoc, "a"); + ASSERT_TRUE(!Err); + + // Invalid location. + Err = Change.insert(Context.Sources, SourceLocation(), "b"); + ASSERT_TRUE((bool)Err); + EXPECT_TRUE(checkReplacementError( + std::move(Err), replacement_error::wrong_file_path, + Replacement(Context.Sources, DefaultLoc, 0, "a"), + Replacement(Context.Sources, SourceLocation(), 0, "b"))); +} + } // end namespace tooling } // end namespace clang Index: unittests/Tooling/CMakeLists.txt =================================================================== --- unittests/Tooling/CMakeLists.txt +++ unittests/Tooling/CMakeLists.txt @@ -13,7 +13,7 @@ add_clang_unittest(ToolingTests CommentHandlerTest.cpp CompilationDatabaseTest.cpp - FixItTest.cpp + FixItTest.cpp LookupTest.cpp QualTypeNamesTest.cpp RecursiveASTVisitorTest.cpp @@ -38,4 +38,5 @@ clangRewrite clangTooling clangToolingCore + clangToolingRefactor ) Index: lib/Tooling/Refactoring/CMakeLists.txt =================================================================== --- /dev/null +++ lib/Tooling/Refactoring/CMakeLists.txt @@ -0,0 +1,12 @@ +set(LLVM_LINK_COMPONENTS + Option + Support + ) + +add_clang_library(clangToolingRefactor + AtomicChange.cpp + + LINK_LIBS + clangBasic + clangToolingCore + ) Index: lib/Tooling/Refactoring/AtomicChange.cpp =================================================================== --- /dev/null +++ lib/Tooling/Refactoring/AtomicChange.cpp @@ -0,0 +1,167 @@ +//===--- AtomicChange.cpp - AtomicChange implementation -----------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "clang/Tooling/Refactoring/AtomicChange.h" +#include "clang/Tooling/ReplacementsYaml.h" +#include "llvm/Support/YAMLTraits.h" +#include <string> + +LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(std::string) +LLVM_YAML_IS_SEQUENCE_VECTOR(clang::tooling::AtomicChange) + +namespace { +/// \brief Helper to (de)serialize an AtomicChange since we don't have direct +/// access to its data members. +/// Data members of a normalized AtomicChange can be directly mapped from/to +/// YAML string. +struct NormalizedAtomicChange { + NormalizedAtomicChange() = default; + + NormalizedAtomicChange(const llvm::yaml::IO &) {} + + // This converts AtomicChange's internal implementation of the replacements + // set to a vector of replacements. + NormalizedAtomicChange(const llvm::yaml::IO &, + const clang::tooling::AtomicChange &E) + : Key(E.getKey()), FilePath(E.getFilePath()), Error(E.getError()), + InsertedHeaders(E.getInsertedHeaders()), + RemovedHeaders(E.getRemovedHeaders()), + Replaces(E.getReplacements().begin(), E.getReplacements().end()) {} + + // This is not expected to be called but needed for template instantiation. + clang::tooling::AtomicChange denormalize(const llvm::yaml::IO &) { + llvm_unreachable("Do not convert YAML to AtomicChange directly with '>>'. " + "Use AtomicChange::convertFromYAML instead."); + } + std::string Key; + std::string FilePath; + std::string Error; + std::vector<std::string> InsertedHeaders; + std::vector<std::string> RemovedHeaders; + std::vector<clang::tooling::Replacement> Replaces; +}; +} // anonymous namespace + +namespace llvm { +namespace yaml { + +/// \brief Specialized MappingTraits to describe how an AtomicChange is +/// (de)serialized. +template <> struct MappingTraits<NormalizedAtomicChange> { + static void mapping(IO &Io, NormalizedAtomicChange &Doc) { + Io.mapRequired("Key", Doc.Key); + Io.mapRequired("FilePath", Doc.FilePath); + Io.mapRequired("Error", Doc.Error); + Io.mapRequired("InsertedHeaders", Doc.InsertedHeaders); + Io.mapRequired("RemovedHeaders", Doc.RemovedHeaders); + Io.mapRequired("Replacements", Doc.Replaces); + } +}; + +/// \brief Specialized MappingTraits to describe how an AtomicChange is +/// (de)serialized. +template <> struct MappingTraits<clang::tooling::AtomicChange> { + static void mapping(IO &Io, clang::tooling::AtomicChange &Doc) { + MappingNormalization<NormalizedAtomicChange, clang::tooling::AtomicChange> + Keys(Io, Doc); + Io.mapRequired("Key", Keys->Key); + Io.mapRequired("FilePath", Keys->FilePath); + Io.mapRequired("Error", Keys->Error); + Io.mapRequired("InsertedHeaders", Keys->InsertedHeaders); + Io.mapRequired("RemovedHeaders", Keys->RemovedHeaders); + Io.mapRequired("Replacements", Keys->Replaces); + } +}; + +} // end namespace yaml +} // end namespace llvm + +namespace clang { +namespace tooling { + +AtomicChange::AtomicChange(const SourceManager &SM, + SourceLocation KeyPosition) { + const FullSourceLoc FullKeyPosition(KeyPosition, SM); + std::pair<FileID, unsigned> FileIDAndOffset = + FullKeyPosition.getSpellingLoc().getDecomposedLoc(); + const FileEntry *FE = SM.getFileEntryForID(FileIDAndOffset.first); + assert(FE && "Cannot create AtomicChange with invalid location."); + FilePath = FE->getName(); + Key = FilePath + ":" + std::to_string(FileIDAndOffset.second); +} + +AtomicChange::AtomicChange(std::string Key, std::string FilePath, + std::string Error, + std::vector<std::string> InsertedHeaders, + std::vector<std::string> RemovedHeaders, + clang::tooling::Replacements Replaces) + : Key(std::move(Key)), FilePath(std::move(FilePath)), + Error(std::move(Error)), InsertedHeaders(std::move(InsertedHeaders)), + RemovedHeaders(std::move(RemovedHeaders)), Replaces(std::move(Replaces)) { +} + +std::string AtomicChange::toYAMLString() { + std::string YamlContent; + llvm::raw_string_ostream YamlContentStream(YamlContent); + + llvm::yaml::Output YAML(YamlContentStream); + YAML << *this; + YamlContentStream.flush(); + return YamlContent; +} + +AtomicChange AtomicChange::convertFromYAML(llvm::StringRef YAMLContent) { + NormalizedAtomicChange NE; + llvm::yaml::Input YAML(YAMLContent); + YAML >> NE; + AtomicChange E(NE.Key, NE.FilePath, NE.Error, NE.InsertedHeaders, + NE.RemovedHeaders, tooling::Replacements()); + for (const auto &R : NE.Replaces) { + llvm::Error Err = E.Replaces.add(R); + if (Err) + llvm_unreachable( + "Failed to add replacement when Converting YAML to AtomicChange."); + llvm::consumeError(std::move(Err)); + } + return E; +} + +llvm::Error AtomicChange::insert(const SourceManager &SM, SourceLocation Loc, + llvm::StringRef Text, bool InsertAfter) { + if (Text.empty()) + return llvm::Error::success(); + Replacement R(SM, Loc, 0, Text); + llvm::Error Err = Replaces.add(R); + if (Err) { + return llvm::handleErrors( + std::move(Err), [&](const ReplacementError &RE) -> llvm::Error { + if (RE.get() != replacement_error::insert_conflict) + return llvm::make_error<ReplacementError>(RE); + unsigned NewOffset = Replaces.getShiftedCodePosition(R.getOffset()); + if (!InsertAfter) + NewOffset -= + RE.getExistingReplacement()->getReplacementText().size(); + Replacement NewR(R.getFilePath(), NewOffset, 0, Text); + Replaces = Replaces.merge(Replacements(NewR)); + return llvm::Error::success(); + }); + } + return llvm::Error::success(); +} + +void AtomicChange::addHeader(llvm::StringRef Header) { + InsertedHeaders.push_back(Header); +} + +void AtomicChange::removeHeader(llvm::StringRef Header) { + RemovedHeaders.push_back(Header); +} + +} // end namespace tooling +} // end namespace clang Index: lib/Tooling/CMakeLists.txt =================================================================== --- lib/Tooling/CMakeLists.txt +++ lib/Tooling/CMakeLists.txt @@ -4,6 +4,7 @@ ) add_subdirectory(Core) +add_subdirectory(Refactoring) add_clang_library(clangTooling ArgumentsAdjusters.cpp Index: include/clang/Tooling/Refactoring/AtomicChange.h =================================================================== --- /dev/null +++ include/clang/Tooling/Refactoring/AtomicChange.h @@ -0,0 +1,129 @@ +//===--- AtomicChange.h - AtomicChange class --------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file defines AtomicChange which is used to create a set of source +// changes, e.g. replacements and header insertions. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLING_REFACTOR_ATOMICCHANGE_H +#define LLVM_CLANG_TOOLING_REFACTOR_ATOMICCHANGE_H + +#include "clang/Basic/SourceManager.h" +#include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Error.h" + +namespace clang { +namespace tooling { + +/// \brief An atomic change is used to create and group a set of source edits, +/// e.g. replacements or header insertions. Edits in an AtomicChange should be +/// related, e.g. replacements for the same type reference and the corresponding +/// header insertion/deletion. +/// +/// An AtomicChange is uniquely identified by a key and will either be fully +/// applied or not applied at all. +/// +/// Calling setError on an AtomicChange stores the error message and marks it as +/// bad, i.e. none of its source edits will be applied. +class AtomicChange { +public: + /// \brief Creates an atomic change around \p KeyPosition with the key being a + /// concatenation of the file name and the offset of \p KeyPosition. + /// \p KeyPosition should be the location of the key syntactical element that + /// is being changed, e.g. the call to a refactored method. + AtomicChange(const SourceManager &SM, SourceLocation KeyPosition); + + /// \brief Creates an atomic change for \p FilePath with a customized key. + AtomicChange(llvm::StringRef FilePath, llvm::StringRef Key) + : Key(Key), FilePath(FilePath) {} + + /// \brief Returns the atomic change as a YAML string. + std::string toYAMLString(); + + /// \brief Converts a YAML-encoded automic change to AtomicChange. + static AtomicChange convertFromYAML(llvm::StringRef YAMLContent); + + /// \brief Returns the key of this change, which is a concatenation of the + /// file name and offset of the key position. + const std::string &getKey() const { return Key; } + + /// \brief Returns the path of the file containing this atomic change. + const std::string &getFilePath() const { return FilePath; } + + /// \brief If this change could not be created successfully, e.g. because of + /// conflicts among replacements, use this to set an error description. + /// Thereby, places that cannot be fixed automatically can be gathered when + /// applying changes. + void setError(llvm::StringRef Error) { this->Error = Error; } + + /// \brief Returns whether an error has been set on this list. + bool hasError() const { return !Error.empty(); } + + /// \brief Returns the error message or an empty string if it does not exist. + const std::string &getError() const { return Error; } + + /// \brief Adds a replacement that replaces range [Loc, Loc+Length) with + /// \p Text. + /// \returns An llvm::Error carrying ReplacementError on error. + llvm::Error replace(const SourceManager &SM, SourceLocation Loc, + unsigned Length, llvm::StringRef Text); + + /// \brief Adds a replacement that inserts \p Text at \p Loc. If this + /// insertion conflicts with an existing insertion (at the same position), + /// this will be inserted before/after the existing insertion depending on + /// \p InsertAfter. Users should use `replace` with `Length=0` instead if they + /// do not want conflict resolving by default. If the conflicting replacement + /// is not an insertion, an error is returned. + /// + /// \returns An llvm::Error carrying ReplacementError on error. + llvm::Error insert(const SourceManager &SM, SourceLocation Loc, + llvm::StringRef Text, bool InsertAfter = true); + + /// \brief Adds a header into the file that contains the key position. + /// Header can be in angle brackets or double quotation marks. By default + /// (header is not quoted), header will be surrounded with double quotes. + void addHeader(llvm::StringRef Header); + + /// \brief Removes a header from the file that contains the key position. + void removeHeader(llvm::StringRef Header); + + /// \brief Returns a const reference to existing replacements. + const Replacements &getReplacements() const { return Replaces; } + + llvm::ArrayRef<std::string> getInsertedHeaders() const { + return InsertedHeaders; + } + + llvm::ArrayRef<std::string> getRemovedHeaders() const { + return RemovedHeaders; + } + +private: + AtomicChange() {} + + AtomicChange(std::string Key, std::string FilePath, std::string Error, + std::vector<std::string> InsertedHeaders, + std::vector<std::string> RemovedHeaders, + clang::tooling::Replacements Replaces); + + // This uniquely identifies an AtomicChange. + std::string Key; + std::string FilePath; + std::string Error; + std::vector<std::string> InsertedHeaders; + std::vector<std::string> RemovedHeaders; + tooling::Replacements Replaces; +}; + +} // end namespace tooling +} // end namespace clang + +#endif // LLVM_CLANG_TOOLING_REFACTOR_ATOMICCHANGE_H
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits