Introducing clang::tooling::AtomicChange for refactoring tools.

[ioeric]

Summary

An AtomicChange 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 position and will either be
fully applied or not applied at all. The key position should be the location
of the key syntactical element that is being changed, e.g. the call to a
refactored method.

Next step: add a tool that applies AtomicChange.

[ioeric]

Ping.

Daniel, could you take a look? Manuel and I would like your opinion on this.

[djasper]

include/clang/Tooling/Refactoring/EditList.h

42

I wonder whether we should always use a SourceLocation as key or whether we want to leave this up to the users. E.g. we could make this take a string parameter and provide a

string getKeyForLocation(const SourceManager &SM, SourceLocation KeyPosition);

44

Does this need to be public? Specifically, couldn't the YamlTraits create a NormalizedEditList and then convertFromYAML can create the EditList object, which should have access to a private constructor?

83–95

Do we currently actually need these functions? They seem a bit dangerous.

118

Maybe add a short description of what "Key" contains. The others are pretty intuitive.

lib/Tooling/Refactoring/EditList.cpp

26

I don't understand (without more comments) why this is normalized/denormalized.

unittests/Tooling/RefactoringTest.cpp

1113

I think these need to be ASSERT_TRUE.

[ioeric]

• Addressed review comments.

include/clang/Tooling/Refactoring/EditList.h

42

I think the key idea of EditList is that it groups a set of edits around a key position. For refactoring, using position as key makes sense to me (since all edits will be around some position), and I couldn't think of other things that are good to be a key here.

44

You are right! Make this private now.

83–95

I think these functions are helpful. Insertion conflict is by far the most common conflict I've seen, and several users have asked for ways to resolve this. And generally, resolving such conflict is not straight-forward. Also notice that these interfaces only resolve insertion conflict, other conflicts (e.g. overlaps) will still raise errors.

Although inserting all texts at one location as a single replacement is preferred, I've seen several tools that use these in some way, i.e. generating all insertions at the same location is hard or impossible.

[klimek]

include/clang/Tooling/Refactoring/EditList.h

42

An idea would be an edit that just inserts multiple headers, but does not directly touch a source location (thus, we might want to key things off of files?).

[ioeric]

include/clang/Tooling/Refactoring/EditList.h

42

We could make the key position the start of the file in this case. But looks like a customize-able key could potentially enable more opportunities, I'll go with Daniel's suggestion. Thanks!

[ioeric]

• Make key customize-able.

include/clang/Tooling/Refactoring/EditList.h

42

So I tried the getKeyForLocation way, but we still need FilePath in the constructor to fully initialize an EditList.

I think it might be better if we keep the current constructor (since this will be the most-commonly used one IMO) and adds another constructor that takes FilePath and Key.

[djasper]

include/clang/Tooling/Refactoring/EditList.h

42

Ah, but you might want to include more details in the Key, e.g. the branch or source revision. My point is that the user might be better equipped to know what a valid key might be. But I am happy to keep it as is and add another constructor where we pass in a key when needed.

83–95

I still have several concerns:

1. This interface actually exposes two different layers: It has special function to insert text and headers and also the functions that add or merge replacements. IMO, that's not ideal.
2. There is now no function insert() if users actually want to get the error for a conflict. Yes, they can then instead use addReplacement, but again, that's a completely separate layer of the interface.
3. The logic of before after AND that this only applies to two conflicting insertions is a bit hard to explain.

Not sure what the right trade-off for all of these is. Maybe we should just remove addReplacement and mergeReplacements and instead add a replace() function? Maybe we should just have a single insert() function with an optional fourth parameter that can be used to control conflict resolution.

lib/Tooling/Refactoring/EditList.cpp

165

Don't duplicate this code. Add an internal function that takes an extra parameter that controls conflict resolution.

[ioeric]

include/clang/Tooling/Refactoring/EditList.h

83–95

replace + insert sounds like a good way to go. Do we want to support resolving general conflicts besides insertions though?

[djasper]

include/clang/Tooling/Refactoring/EditList.h

83–95

I think that might not be necessary. If so, we might need to see a few use cases to be sure to get it right.

[ioeric]

• Added replace() and insert() to replace current replacement interfaces.

[ioeric]

• s/EditList/AtomicChange/

[alexander-shaposhnikov]

include/clang/Tooling/Refactoring/AtomicChange.h

59 ↗

(On Diff #90011)

i assume i might be missing smth - why here and above (in getKey, getFilePath, getError) std::string is returned by value ?

[alexander-shaposhnikov]

lib/Tooling/Refactoring/AtomicChange.cpp

35 ↗

(On Diff #90011)

if i am not mistaken this can be done in the intialization list:

Replaces(E.getReplacements().begin(), E.getReplacements().end())

[ioeric]

include/clang/Tooling/Refactoring/AtomicChange.h

59 ↗

(On Diff #90011)

Otherwise, users would need to worry about the lifetimes of the object. And these methods are not expected to be called intensively, so performance is not an issue.

lib/Tooling/Refactoring/AtomicChange.cpp

35 ↗

(On Diff #90011)

You are right.

[ioeric]

• Addressed comments.

[alexander-shaposhnikov]

include/clang/Tooling/Refactoring/AtomicChange.h

59 ↗

(On Diff #90011)

i see, thanks for the explanation, probably not particularly important
i thought that if a caller wanted to get a copy / take ownership
it would just accept it by value, i.e.

auto path = change.getFilePath() /* assuming getFilePath returns const ref */

At the same time if for example we need to check the filepath (or just print it),
not copies would be required:

llvm::errs() << change.getFilePath();

[ioeric]

• Return const refs in getXXX.

include/clang/Tooling/Refactoring/AtomicChange.h

59 ↗

(On Diff #90011)

I don't really have strong opinion about this. Another thing I was considering is whether the returned value always has an associated object in the class. For example, getError might return Error plus some extra message so that the return string can't be a reference. But these interfaces look pretty stable for now. And your examples make sense. Changed them to const ref instead.

[klimek]

lg

include/clang/Tooling/Refactoring/EditList.h

+//===--- EditList.h - Refactoring edits -------------------------*- 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 EditList which is used to create a set of source eidts,
+//  e.g. replacements and header insertions.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLING_REFACTOR_EDITLIST_H
+#define LLVM_CLANG_TOOLING_REFACTOR_EDITLIST_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 edit list is used to create and group a set of source edits, e.g.
+/// replacements or header insertions. Edits in an EditList should be related,
+/// e.g. replacements for the same type reference and the corresponding header
+/// insertion/deletion.
+///
+/// An EditList is uniquely identified by a key and will either be fully applied
+/// or not applied at all.
+///
+/// Calling setError on an EditList stores the error message and marks it as
+/// bad, i.e. none of its source edits will be applied.
+class EditList {
+public:
+  /// \brief Creates an edit list 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.
+  EditList(const SourceManager &SM, SourceLocation KeyPosition);
+
+  /// \brief Creates an edit list for \p FilePath with a customized key.
+  EditList(llvm::StringRef FilePath, llvm::StringRef Key)
+      : Key(Key), FilePath(FilePath) {}
+
+  /// \brief Returns the edit as a YAML string.
+  std::string toYAMLString();
+
+  /// \brief Converts a YAML-encoded edit to EditList.
+  static EditList convertFromYAML(llvm::StringRef YAMLContent);
+
+  /// \brief Returns the key of this edit, which is a concatenation of the file
+  /// name and offset of the key position.
+  std::string getKey() const { return Key; }
+
+  /// \brief Returns the path of the file containing this edit.
+  std::string getFilePath() const { return FilePath; }
+
+  /// \brief If this edit 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 edits.
+  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.
+  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:
+  EditList() {}
+
+  EditList(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 EditList.
+  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_EDITLIST_H

lib/Tooling/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Option
   Support
   )
 
 add_subdirectory(Core)
+add_subdirectory(Refactoring)
 
 add_clang_library(clangTooling
   ArgumentsAdjusters.cpp
   CommonOptionsParser.cpp
   CompilationDatabase.cpp
   FileMatchTrie.cpp
   FixIt.cpp
   JSONCompilationDatabase.cpp

lib/Tooling/Refactoring/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS
+  Option
+  Support
+  )
+
+add_clang_library(clangToolingRefactor
+  EditList.cpp
+
+  LINK_LIBS
+  clangBasic
+  clangToolingCore
+  )

lib/Tooling/Refactoring/EditList.cpp

+//===--- EditList.cpp - EditList 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/EditList.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::EditList)
+
+namespace {
+/// \brief Helper to (de)serialize an EditList since we don't have direct
+/// access to its data members.
+/// Data members of a normalized editlist can be directly mapped from/to YAML
+/// string.
+struct NormalizedEditList {
+  NormalizedEditList() = default;
+
+  NormalizedEditList(const llvm::yaml::IO &) {}
+
+  // This converts EditList's internal implementation of the replacements set
+  // to a vector of replacements.
+  NormalizedEditList(const llvm::yaml::IO &, const clang::tooling::EditList &E)
+      : Key(E.getKey()), FilePath(E.getFilePath()), Error(E.getError()),
+        InsertedHeaders(E.getInsertedHeaders()),
+        RemovedHeaders(E.getRemovedHeaders()) {
+    std::copy(E.getReplacements().begin(), E.getReplacements().end(),
+              std::back_inserter(Replaces));
+  }
+
+  // This is not expected to be called but needed for template instantiation.
+  clang::tooling::EditList denormalize(const llvm::yaml::IO &) {
+    llvm_unreachable("Do not convert YAML to EditList directly with '>>'. "
+                     "Use EditList::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 EditList is
+/// (de)serialized.
+template <> struct MappingTraits<NormalizedEditList> {
+  static void mapping(IO &Io, NormalizedEditList &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 EditList is
+/// (de)serialized.
+template <> struct MappingTraits<clang::tooling::EditList> {
+  static void mapping(IO &Io,
+                      clang::tooling::EditList &Doc) {
+    MappingNormalization<NormalizedEditList, clang::tooling::EditList>
+    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 {
+
+EditList::EditList(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 EditList with invalid location.");
+  FilePath = FE->getName();
+  Key = FilePath + ":" + std::to_string(FileIDAndOffset.second);
+}
+
+EditList::EditList(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 EditList::toYAMLString() {
+  std::string YamlContent;
+  llvm::raw_string_ostream YamlContentStream(YamlContent);
+
+  llvm::yaml::Output YAML(YamlContentStream);
+  YAML << *this;
+  YamlContentStream.flush();
+  return YamlContent;
+}
+
+EditList EditList::convertFromYAML(llvm::StringRef YAMLContent) {
+  NormalizedEditList NE;
+  llvm::yaml::Input YAML(YAMLContent);
+  YAML >> NE;
+  EditList 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 EditList.");
+    llvm::consumeError(std::move(Err));
+  }
+  return E;
+}
+
+llvm::Error EditList::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 EditList::addHeader(llvm::StringRef Header) {
+  InsertedHeaders.push_back(Header);
+}
+
+void EditList::removeHeader(llvm::StringRef Header) {
+  RemovedHeaders.push_back(Header);
+}
+
+} // end namespace tooling
+} // end namespace clang

unittests/Tooling/CMakeLists.txt

target_link_libraries(ToolingTests
   clangASTMatchers
   clangBasic
   clangFormat
   clangFrontend
   clangLex
   clangRewrite
   clangTooling
   clangToolingCore
+  clangToolingRefactor
   )

unittests/Tooling/RefactoringTest.cpp

 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Refactoring/EditList.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/SmallString.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace tooling {
 
 TEST_F(ReplacementTest, CanDeleteAllText) {
TEST_F(ReplacementTest, ReturnsInvalidPath) {
   EXPECT_TRUE(Replace1.getFilePath().empty());
 
   Replacement Replace2;
   EXPECT_TRUE(Replace2.getFilePath().empty());
 }
 
 // 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,
+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;
   }
   std::string ErrorMessage;
   llvm::raw_string_ostream OS(ErrorMessage);
   llvm::handleAllErrors(std::move(Error), [&](const ReplacementError &RE) {
     llvm::errs() << "Handling error...\n";
#else
   StringRef Path2 = "a\\b\\c.h";
 #endif
   FileToReplaces[Path1] = Replacements();
   FileToReplaces[Path2] = Replacements();
   FileToReplaces = groupReplacementsByFile(FileMgr, FileToReplaces);
   EXPECT_TRUE(FileToReplaces.empty());
 }
 
+class EditListTest : 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(EditListTest, EditListToYAML) {
+  setUp();
+  EditList Edit(Context.Sources, DefaultLoc);
+  llvm::Error Err =
+      Edit.insert(Context.Sources, DefaultLoc, "aa", /*InsertAfter=*/false);
+  ASSERT_TRUE(!Err);
+  Err = Edit.insert(Context.Sources, DefaultLoc.getLocWithOffset(10), "bb",
+                    /*InsertAfter=*/false);
+  ASSERT_TRUE(!Err);
+  Edit.addHeader("a.h");
+  Edit.removeHeader("b.h");
+  std::string YAMLString = Edit.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(EditListTest, YAMLToEditList) {
+  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";
+  EditList ExpectedEdit(Context.Sources, DefaultLoc);
+  llvm::Error Err = ExpectedEdit.insert(Context.Sources, DefaultLoc, "aa",
+                                        /*InsertAfter=*/false);
+  ASSERT_TRUE(!Err);
+  Err = ExpectedEdit.insert(Context.Sources, DefaultLoc.getLocWithOffset(10),
+                            "bb", /*InsertAfter=*/false);
+  ASSERT_TRUE(!Err);
+
+  ExpectedEdit.addHeader("a.h");
+  ExpectedEdit.removeHeader("b.h");
+  ExpectedEdit.setError("ok");
+
+  EditList ActualEdit = EditList::convertFromYAML(YamlContent);
+  EXPECT_EQ(ExpectedEdit.getKey(), ActualEdit.getKey());
+  EXPECT_EQ(ExpectedEdit.getFilePath(), ActualEdit.getFilePath());
+  EXPECT_EQ(ExpectedEdit.getError(), ActualEdit.getError());
+  EXPECT_EQ(ExpectedEdit.getInsertedHeaders(), ActualEdit.getInsertedHeaders());
+  EXPECT_EQ(ExpectedEdit.getRemovedHeaders(), ActualEdit.getRemovedHeaders());
+  EXPECT_EQ(ExpectedEdit.getReplacements().size(),
+            ActualEdit.getReplacements().size());
+  EXPECT_EQ(2u, ActualEdit.getReplacements().size());
+  EXPECT_EQ(*ExpectedEdit.getReplacements().begin(),
+            *ActualEdit.getReplacements().begin());
+  EXPECT_EQ(*(++ExpectedEdit.getReplacements().begin()),
+            *(++ActualEdit.getReplacements().begin()));
+}
+
+TEST_F(EditListTest, CheckKeyAndKeyFile) {
+  setUp();
+  EditList Edit(Context.Sources, DefaultLoc);
+  EXPECT_EQ("input.cpp:20", Edit.getKey());
+  EXPECT_EQ("input.cpp", Edit.getFilePath());
+}
+
+TEST_F(EditListTest, InsertBefore) {
+  setUp();
+  EditList Edit(Context.Sources, DefaultLoc);
+  llvm::Error Err = Edit.insert(Context.Sources, DefaultLoc, "aa");
+  ASSERT_TRUE(!Err);
+  EXPECT_EQ(Edit.getReplacements().size(), 1u);
+  EXPECT_EQ(*Edit.getReplacements().begin(),
+            Replacement(Context.Sources, DefaultLoc, 0, "aa"));
+  Err = Edit.insert(Context.Sources, DefaultLoc, "b", /*InsertAfter=*/false);
+  ASSERT_TRUE(!Err);
+  EXPECT_EQ(Edit.getReplacements().size(), 1u);
+  EXPECT_EQ(*Edit.getReplacements().begin(),
+            Replacement(Context.Sources, DefaultLoc, 0, "baa"));
+}
+
+TEST_F(EditListTest, InsertAfter) {
+  setUp();
+  EditList Edit(Context.Sources, DefaultLoc);
+  llvm::Error Err = Edit.insert(Context.Sources, DefaultLoc, "aa");
+  ASSERT_TRUE(!Err);
+  EXPECT_EQ(Edit.getReplacements().size(), 1u);
+  EXPECT_EQ(*Edit.getReplacements().begin(),
+            Replacement(Context.Sources, DefaultLoc, 0, "aa"));
+  Err = Edit.insert(Context.Sources, DefaultLoc, "b");
+  ASSERT_TRUE(!Err);
+  EXPECT_EQ(Edit.getReplacements().size(), 1u);
+  EXPECT_EQ(*Edit.getReplacements().begin(),
+            Replacement(Context.Sources, DefaultLoc, 0, "aab"));
+}
+
+TEST_F(EditListTest, InsertBeforeWithInvalidLocation) {
+  setUp();
+  EditList Edit(Context.Sources, DefaultLoc);
+  llvm::Error Err =
+      Edit.insert(Context.Sources, DefaultLoc, "a", /*InsertAfter=*/false);
+  ASSERT_TRUE(!Err);
+
+  // Invalid location.
+  Err = Edit.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(EditListTest, InsertBeforeToWrongFile) {
+  setUp();
+  EditList Edit(Context.Sources, DefaultLoc);
+  llvm::Error Err =
+      Edit.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 = Edit.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(EditListTest, InsertAfterWithInvalidLocation) {
+  setUp();
+  EditList Edit(Context.Sources, DefaultLoc);
+  llvm::Error Err = Edit.insert(Context.Sources, DefaultLoc, "a");
+  ASSERT_TRUE(!Err);
+
+  // Invalid location.
+  Err = Edit.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