Added `applyAtomicChanges` function.

[ioeric]

Summary

... which applies a set of AtomicChanges on code.

[klimek]

include/clang/Tooling/Refactoring/AtomicChange.h

129 ↗

(On Diff #91153)

That part of the sentence doesn't parse.

132 ↗

(On Diff #91153)

I'd add default values for everything that will otherwise be uninitialized.

147–149 ↗

(On Diff #91153)

Why?

lib/Tooling/Refactoring/AtomicChange.cpp

106 ↗

(On Diff #91153)

Won't this be incorrect if the last line (no \n) is 1 over the column limit (due to EndPos = Code.size()-1 above)?

[ioeric]

• Addressed comments; stylized code.

[ioeric]

include/clang/Tooling/Refactoring/AtomicChange.h

147–149 ↗

(On Diff #91153)

Changes might have equivalent paths in different forms (e.g. relative or absolute), and checking canonical paths are dependent on file systems and the working directory, so I think this should be checked/handled in the user level.

lib/Tooling/Refactoring/AtomicChange.cpp

106 ↗

(On Diff #91153)

Yeah, that is a bug. Should be Code.size() then.

[djasper]

include/clang/Tooling/Refactoring/AtomicChange.h

139 ↗

(On Diff #91163)

Should probably be "exceeding the column limit".

lib/Tooling/Refactoring/AtomicChange.cpp

91 ↗

(On Diff #91163)

I think LLVM style is lowerCamelCase now, right? Here and below.

113 ↗

(On Diff #91163)

nit: I'd move this down and just return {} for kNone.

127 ↗

(On Diff #91163)

Remove braces?

151 ↗

(On Diff #91163)

What is copying these vectors buying us?

164 ↗

(On Diff #91163)

I'd remove the parentheses.

196 ↗

(On Diff #91163)

Remove the braces of the for loops.

[ioeric]

• Addressed review comments.

[ioeric]

Ping ;)

[djasper]

Looks good.. Very nice :)

lib/Tooling/Refactoring/AtomicChange.cpp

102 ↗

(On Diff #91288)

nit: remove braces (at least for consistency)

[ioeric]

• addressed comment.

[chapuni]

cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h

19

It requires clangFormat in LINK_LIBS. Fixed in r298921.

[ioeric]

This (rL298913) was reverted upstream due to cyclic dependency on GreenDragon bot. Investigating...

[ioeric]

• Try to unbreak buildbots after r298913.
• Add clangFormat to dependency
• Update cmake

[ioeric]

• Merged with origin/master

cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h

 //  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/Format/Format.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,
private:
   std::string Key;
   std::string FilePath;
   std::string Error;
   std::vector<std::string> InsertedHeaders;
   std::vector<std::string> RemovedHeaders;
   tooling::Replacements Replaces;
 };
 
+// Defines specs for applying changes.
+struct ApplyChangesSpec {
+  // If true, cleans up redundant/erroneous code around changed code with
+  // clang-format's cleanup functionality, e.g. redundant commas around deleted
+  // parameter or empty namespaces introduced by deletions.
+  bool Cleanup = true;
+
+  format::FormatStyle Style = format::getNoStyle();
+
+  // Options for selectively formatting changes with clang-format:
+  // kAll: Format all changed lines.
+  // kNone: Don't format anything.
+  // kViolations: Format lines exceeding the `ColumnLimit` in `Style`.
+  enum FormatOption { kAll, kNone, kViolations };
+
+  FormatOption Format = kNone;
+};
+
+/// \brief Applies all AtomicChanges in \p Changes to the \p Code.
+///
+/// This completely ignores the file path in each change and replaces them with
+/// \p FilePath, i.e. callers are responsible for ensuring all changes are for
+/// the same file.
+///
+/// \returns The changed code if all changes are applied successfully;
+/// otherwise, an llvm::Error carrying llvm::StringError is returned (the Error
+/// message can be converted to string with `llvm::toString()` and the
+/// error_code should be ignored).
+llvm::Expected<std::string>
+applyAtomicChanges(llvm::StringRef FilePath, llvm::StringRef Code,
+                   llvm::ArrayRef<AtomicChange> Changes,
+                   const ApplyChangesSpec &Spec);
+
 } // end namespace tooling
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLING_REFACTOR_ATOMICCHANGE_H

cfe/trunk/lib/Tooling/Refactoring/AtomicChange.cpp

template <> struct MappingTraits<clang::tooling::AtomicChange> {
   }
 };
 
 } // end namespace yaml
 } // end namespace llvm
 
 namespace clang {
 namespace tooling {
+namespace {
+
+// Returns true if there is any line that violates \p ColumnLimit in range
+// [Start, End].
+bool violatesColumnLimit(llvm::StringRef Code, unsigned ColumnLimit,
+                         unsigned Start, unsigned End) {
+  auto StartPos = Code.rfind('\n', Start);
+  StartPos = (StartPos == llvm::StringRef::npos) ? 0 : StartPos + 1;
+
+  auto EndPos = Code.find("\n", End);
+  if (EndPos == llvm::StringRef::npos)
+    EndPos = Code.size();
+
+  llvm::SmallVector<llvm::StringRef, 8> Lines;
+  Code.substr(StartPos, EndPos - StartPos).split(Lines, '\n');
+  for (llvm::StringRef Line : Lines)
+    if (Line.size() > ColumnLimit)
+      return true;
+  return false;
+}
+
+std::vector<Range>
+getRangesForFormating(llvm::StringRef Code, unsigned ColumnLimit,
+                      ApplyChangesSpec::FormatOption Format,
+                      const clang::tooling::Replacements &Replaces) {
+  // kNone suppresses formatting entirely.
+  if (Format == ApplyChangesSpec::kNone)
+    return {};
+  std::vector<clang::tooling::Range> Ranges;
+  // This works assuming that replacements are ordered by offset.
+  // FIXME: use `getAffectedRanges()` to calculate when it does not include '\n'
+  // at the end of an insertion in affected ranges.
+  int Offset = 0;
+  for (const clang::tooling::Replacement &R : Replaces) {
+    int Start = R.getOffset() + Offset;
+    int End = Start + R.getReplacementText().size();
+    if (!R.getReplacementText().empty() &&
+        R.getReplacementText().back() == '\n' && R.getLength() == 0 &&
+        R.getOffset() > 0 && R.getOffset() <= Code.size() &&
+        Code[R.getOffset() - 1] == '\n')
+      // If we are inserting at the start of a line and the replacement ends in
+      // a newline, we don't need to format the subsequent line.
+      --End;
+    Offset += R.getReplacementText().size() - R.getLength();
+
+    if (Format == ApplyChangesSpec::kAll ||
+        violatesColumnLimit(Code, ColumnLimit, Start, End))
+      Ranges.emplace_back(Start, End - Start);
+  }
+  return Ranges;
+}
+
+inline llvm::Error make_string_error(const llvm::Twine &Message) {
+  return llvm::make_error<llvm::StringError>(Message,
+                                             llvm::inconvertibleErrorCode());
+}
+
+// Creates replacements for inserting/deleting #include headers.
+llvm::Expected<Replacements>
+createReplacementsForHeaders(llvm::StringRef FilePath, llvm::StringRef Code,
+                             llvm::ArrayRef<AtomicChange> Changes,
+                             const format::FormatStyle &Style) {
+  // Create header insertion/deletion replacements to be cleaned up
+  // (i.e. converted to real insertion/deletion replacements).
+  Replacements HeaderReplacements;
+  for (const auto &Change : Changes) {
+    for (llvm::StringRef Header : Change.getInsertedHeaders()) {
+      std::string EscapedHeader =
+          Header.startswith("<") || Header.startswith("\"")
+              ? Header.str()
+              : ("\"" + Header + "\"").str();
+      std::string ReplacementText = "#include " + EscapedHeader;
+      // Offset UINT_MAX and length 0 indicate that the replacement is a header
+      // insertion.
+      llvm::Error Err = HeaderReplacements.add(
+          tooling::Replacement(FilePath, UINT_MAX, 0, ReplacementText));
+      if (Err)
+        return std::move(Err);
+    }
+    for (const std::string &Header : Change.getRemovedHeaders()) {
+      // Offset UINT_MAX and length 1 indicate that the replacement is a header
+      // deletion.
+      llvm::Error Err =
+          HeaderReplacements.add(Replacement(FilePath, UINT_MAX, 1, Header));
+      if (Err)
+        return std::move(Err);
+    }
+  }
+
+  // cleanupAroundReplacements() converts header insertions/deletions into
+  // actual replacements that add/remove headers at the right location.
+  return clang::format::cleanupAroundReplacements(Code, HeaderReplacements,
+                                                  Style);
+}
+
+// Combine replacements in all Changes as a `Replacements`. This ignores the
+// file path in all replacements and replaces them with \p FilePath.
+llvm::Expected<Replacements>
+combineReplacementsInChanges(llvm::StringRef FilePath,
+                             llvm::ArrayRef<AtomicChange> Changes) {
+  Replacements Replaces;
+  for (const auto &Change : Changes)
+    for (const auto &R : Change.getReplacements())
+      if (auto Err = Replaces.add(Replacement(
+              FilePath, R.getOffset(), R.getLength(), R.getReplacementText())))
+        return std::move(Err);
+  return Replaces;
+}
+
+} // end namespace
 
 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.");

 void AtomicChange::addHeader(llvm::StringRef Header) {
   InsertedHeaders.push_back(Header);
 }
 
 void AtomicChange::removeHeader(llvm::StringRef Header) {
   RemovedHeaders.push_back(Header);
 }
 
+llvm::Expected<std::string>
+applyAtomicChanges(llvm::StringRef FilePath, llvm::StringRef Code,
+                   llvm::ArrayRef<AtomicChange> Changes,
+                   const ApplyChangesSpec &Spec) {
+  llvm::Expected<Replacements> HeaderReplacements =
+      createReplacementsForHeaders(FilePath, Code, Changes, Spec.Style);
+  if (!HeaderReplacements)
+    return make_string_error(
+        "Failed to create replacements for header changes: " +
+        llvm::toString(HeaderReplacements.takeError()));
+
+  llvm::Expected<Replacements> Replaces =
+      combineReplacementsInChanges(FilePath, Changes);
+  if (!Replaces)
+    return make_string_error("Failed to combine replacements in all changes: " +
+                             llvm::toString(Replaces.takeError()));
+
+  Replacements AllReplaces = std::move(*Replaces);
+  for (const auto &R : *HeaderReplacements) {
+    llvm::Error Err = AllReplaces.add(R);
+    if (Err)
+      return make_string_error(
+          "Failed to combine existing replacements with header replacements: " +
+          llvm::toString(std::move(Err)));
+  }
+
+  if (Spec.Cleanup) {
+    llvm::Expected<Replacements> CleanReplaces =
+        format::cleanupAroundReplacements(Code, AllReplaces, Spec.Style);
+    if (!CleanReplaces)
+      return make_string_error("Failed to cleanup around replacements: " +
+                               llvm::toString(CleanReplaces.takeError()));
+    AllReplaces = std::move(*CleanReplaces);
+  }
+
+  // Apply all replacements.
+  llvm::Expected<std::string> ChangedCode =
+      applyAllReplacements(Code, AllReplaces);
+  if (!ChangedCode)
+    return make_string_error("Failed to apply all replacements: " +
+                             llvm::toString(ChangedCode.takeError()));
+
+  // Sort inserted headers. This is done even if other formatting is turned off
+  // as incorrectly sorted headers are always just wrong, it's not a matter of
+  // taste.
+  Replacements HeaderSortingReplacements = format::sortIncludes(
+      Spec.Style, *ChangedCode, AllReplaces.getAffectedRanges(), FilePath);
+  ChangedCode = applyAllReplacements(*ChangedCode, HeaderSortingReplacements);
+  if (!ChangedCode)
+    return make_string_error(
+        "Failed to apply replacements for sorting includes: " +
+        llvm::toString(ChangedCode.takeError()));
+
+  AllReplaces = AllReplaces.merge(HeaderSortingReplacements);
+
+  std::vector<Range> FormatRanges = getRangesForFormating(
+      *ChangedCode, Spec.Style.ColumnLimit, Spec.Format, AllReplaces);
+  if (!FormatRanges.empty()) {
+    Replacements FormatReplacements =
+        format::reformat(Spec.Style, *ChangedCode, FormatRanges, FilePath);
+    ChangedCode = applyAllReplacements(*ChangedCode, FormatReplacements);
+    if (!ChangedCode)
+      return make_string_error(
+          "Failed to apply replacements for formatting changed code: " +
+          llvm::toString(ChangedCode.takeError()));
+  }
+  return ChangedCode;
+}
+
 } // end namespace tooling
 } // end namespace clang

cfe/trunk/unittests/Tooling/RefactoringTest.cpp

TEST_F(AtomicChangeTest, InsertAfterWithInvalidLocation) {
   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")));
 }
 
+class ApplyAtomicChangesTest : public ::testing::Test {
+protected:
+  ApplyAtomicChangesTest() : FilePath("file.cc") {
+    Spec.Cleanup = true;
+    Spec.Format = ApplyChangesSpec::kAll;
+    Spec.Style = format::getLLVMStyle();
+  }
+
+  ~ApplyAtomicChangesTest() override {}
+
+  void setInput(llvm::StringRef Input) {
+    Code = Input;
+    FID = Context.createInMemoryFile(FilePath, Code);
+  }
+
+  SourceLocation getLoc(unsigned Offset) const {
+    return Context.Sources.getLocForStartOfFile(FID).getLocWithOffset(Offset);
+  }
+
+  AtomicChange replacementToAtomicChange(llvm::StringRef Key, unsigned Offset,
+                                         unsigned Length,
+                                         llvm::StringRef Text) {
+    AtomicChange Change(FilePath, Key);
+    llvm::Error Err =
+        Change.replace(Context.Sources, getLoc(Offset), Length, Text);
+    EXPECT_FALSE(Err);
+    return Change;
+  }
+
+  std::string rewrite(bool FailureExpected = false) {
+    llvm::Expected<std::string> ChangedCode =
+        applyAtomicChanges(FilePath, Code, Changes, Spec);
+    EXPECT_EQ(FailureExpected, !ChangedCode);
+    if (!ChangedCode) {
+      llvm::errs() << "Failed to apply changes: "
+                   << llvm::toString(ChangedCode.takeError()) << "\n";
+      return "";
+    }
+    return *ChangedCode;
+  }
+
+  RewriterTestContext Context;
+  FileID FID;
+  ApplyChangesSpec Spec;
+  std::string Code;
+  std::string FilePath;
+  llvm::SmallVector<AtomicChange, 8> Changes;
+};
+
+TEST_F(ApplyAtomicChangesTest, BasicRefactoring) {
+  setInput("int a;");
+  AtomicChange Change(FilePath, "key1");
+  Changes.push_back(replacementToAtomicChange("key1", 4, 1, "b"));
+  EXPECT_EQ("int b;", rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, SeveralRefactorings) {
+  setInput("int a;\n"
+           "int b;");
+  Changes.push_back(replacementToAtomicChange("key1", 0, 3, "float"));
+  Changes.push_back(replacementToAtomicChange("key2", 4, 1, "f"));
+  Changes.push_back(replacementToAtomicChange("key3", 11, 1, "g"));
+  Changes.push_back(replacementToAtomicChange("key4", 7, 3, "float"));
+  EXPECT_EQ("float f;\n"
+            "float g;",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, IgnorePathsInRefactorings) {
+  setInput("int a;\n"
+           "int b;");
+  Changes.push_back(replacementToAtomicChange("key1", 4, 1, "aa"));
+
+  FileID ID = Context.createInMemoryFile("AnotherFile", "12345678912345");
+  Changes.emplace_back("AnotherFile", "key2");
+  auto Err = Changes.back().replace(
+      Context.Sources,
+      Context.Sources.getLocForStartOfFile(ID).getLocWithOffset(11), 1, "bb");
+  ASSERT_TRUE(!Err);
+  EXPECT_EQ("int aa;\n"
+            "int bb;",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, AppliesDuplicateInsertions) {
+  setInput("int a;");
+  Changes.push_back(replacementToAtomicChange("key1", 5, 0, "b"));
+  Changes.push_back(replacementToAtomicChange("key2", 5, 0, "b"));
+  EXPECT_EQ("int abb;", rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, BailsOnOverlappingRefactorings) {
+  setInput("int a;");
+  Changes.push_back(replacementToAtomicChange("key1", 0, 5, "float f"));
+  Changes.push_back(replacementToAtomicChange("key2", 4, 1, "b"));
+  EXPECT_EQ("", rewrite(/*FailureExpected=*/true));
+}
+
+TEST_F(ApplyAtomicChangesTest, BasicReformatting) {
+  setInput("int  a;");
+  Changes.push_back(replacementToAtomicChange("key1", 5, 1, "b"));
+  EXPECT_EQ("int b;", rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, OnlyFormatWhenViolateColumnLimits) {
+  Spec.Format = ApplyChangesSpec::kViolations;
+  Spec.Style.ColumnLimit = 8;
+  setInput("int  a;\n"
+           "int    a;\n"
+           "int  aaaaaaaa;\n");
+  Changes.push_back(replacementToAtomicChange("key1", 5, 1, "x"));
+  Changes.push_back(replacementToAtomicChange("key2", 15, 1, "x"));
+  Changes.push_back(replacementToAtomicChange("key3", 23, 8, "xx"));
+  EXPECT_EQ("int  x;\n"
+            "int x;\n"
+            "int  xx;\n",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, LastLineViolateColumnLimits) {
+  Spec.Format = ApplyChangesSpec::kViolations;
+  Spec.Style.ColumnLimit = 8;
+  setInput("int  a;\n"
+           "int    a;");
+  Changes.push_back(replacementToAtomicChange("key1", 0, 1, "i"));
+  Changes.push_back(replacementToAtomicChange("key2", 15, 2, "y;"));
+  EXPECT_EQ("int  a;\n"
+            "int y;",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, LastLineWithNewlineViolateColumnLimits) {
+  Spec.Format = ApplyChangesSpec::kViolations;
+  Spec.Style.ColumnLimit = 8;
+  setInput("int  a;\n"
+           "int   a;\n");
+  Changes.push_back(replacementToAtomicChange("key1", 0, 1, "i"));
+  Changes.push_back(replacementToAtomicChange("key2", 14, 3, "y;\n"));
+  EXPECT_EQ("int  a;\n"
+            "int   y;\n",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, Longer) {
+  setInput("int  a;");
+  Changes.push_back(replacementToAtomicChange("key1", 5, 1, "bbb"));
+  EXPECT_EQ("int bbb;", rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, Shorter) {
+  setInput("int  aaa;");
+  Changes.push_back(replacementToAtomicChange("key1", 5, 3, "b"));
+  EXPECT_EQ("int b;", rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, OnlyFormatChangedLines) {
+  setInput("int  aaa;\n"
+           "int a = b;\n"
+           "int  bbb;");
+  Changes.push_back(replacementToAtomicChange("key1", 14, 1, "b"));
+  EXPECT_EQ("int  aaa;\n"
+            "int b = b;\n"
+            "int  bbb;",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, DisableFormatting) {
+  Spec.Format = ApplyChangesSpec::kNone;
+  setInput("int  aaa;\n"
+           "int a   = b;\n"
+           "int  bbb;");
+  Changes.push_back(replacementToAtomicChange("key1", 14, 1, "b"));
+  EXPECT_EQ("int  aaa;\n"
+            "int b   = b;\n"
+            "int  bbb;",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, AdaptsToLocalPointerStyle) {
+  setInput("int *aaa;\n"
+           "int *bbb;");
+  Changes.push_back(replacementToAtomicChange("key1", 0, 0, "int* ccc;\n"));
+  EXPECT_EQ("int *ccc;\n"
+            "int *aaa;\n"
+            "int *bbb;",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, AcceptsSurroundingFormatting) {
+  setInput("   int  aaa;\n"
+           "   int a = b;\n"
+           "   int  bbb;");
+  Changes.push_back(replacementToAtomicChange("key1", 20, 1, "b"));
+  EXPECT_EQ("   int  aaa;\n"
+            "   int b = b;\n"
+            "   int  bbb;",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, BailsOutOnConflictingChanges) {
+  setInput("int c;\n"
+           "int f;");
+  // Insertions at the same offset are only allowed in the same AtomicChange.
+  Changes.push_back(replacementToAtomicChange("key1", 0, 0, "int a;\n"));
+  Changes.push_back(replacementToAtomicChange("key2", 0, 0, "int b;\n"));
+  EXPECT_EQ("", rewrite(/*FailureExpected=*/true));
+}
+
+TEST_F(ApplyAtomicChangesTest, InsertsNewIncludesInRightOrder) {
+  setInput("int a;");
+  Changes.emplace_back(FilePath, "key1");
+  Changes.back().addHeader("b");
+  Changes.back().addHeader("c");
+  Changes.emplace_back(FilePath, "key2");
+  Changes.back().addHeader("a");
+  EXPECT_EQ("#include \"a\"\n"
+            "#include \"b\"\n"
+            "#include \"c\"\n"
+            "int a;",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, RemoveAndSortIncludes) {
+  setInput(R"(
+#include "a"
+#include "b"
+#include "c"
+
+int a;
+      )");
+  Changes.emplace_back(FilePath, "key1");
+  Changes.back().removeHeader("b");
+  EXPECT_EQ(R"(
+#include "a"
+#include "c"
+
+int a;
+      )",
+            rewrite());
+}
+TEST_F(ApplyAtomicChangesTest, InsertsSystemIncludes) {
+  setInput("#include <asys>\n"
+           "#include <csys>\n"
+           "\n"
+           "#include \"a\"\n"
+           "#include \"c\"\n");
+  Changes.emplace_back(FilePath, "key1");
+  Changes.back().addHeader("<asys>"); // Already exists.
+  Changes.back().addHeader("<b>");
+  Changes.back().addHeader("<d>");
+  Changes.back().addHeader("\"b-already-escaped\"");
+  EXPECT_EQ("#include <asys>\n"
+            "#include <b>\n"
+            "#include <csys>\n"
+            "#include <d>\n"
+            "\n"
+            "#include \"a\"\n"
+            "#include \"b-already-escaped\"\n"
+            "#include \"c\"\n",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, RemoveSystemIncludes) {
+  setInput(R"(
+#include <a>
+#include <b>
+
+#include "c"
+
+int a;
+      )");
+  Changes.emplace_back(FilePath, "key1");
+  Changes.back().removeHeader("<a>");
+  EXPECT_EQ(R"(
+#include <b>
+
+#include "c"
+
+int a;
+      )",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest,
+       DoNotFormatFollowingLinesIfSeparatedWithNewline) {
+  setInput("#ifndef __H__\n"
+           "#define __H__\n"
+           "#include \"b\"\n"
+           "\n"
+           "int  a;\n"
+           "int  a;\n"
+           "int  a;\n"
+           "#endif // __H__\n");
+  Changes.push_back(replacementToAtomicChange("key1",
+                                              llvm::StringRef("#ifndef __H__\n"
+                                                              "#define __H__\n"
+                                                              "\n"
+                                                              "#include \"b\"\n"
+                                                              "int  a;\n"
+                                                              "int  ")
+                                                  .size(),
+                                              1, "b"));
+  Changes.back().addHeader("a");
+  EXPECT_EQ("#ifndef __H__\n"
+            "#define __H__\n"
+            "#include \"a\"\n"
+            "#include \"b\"\n"
+            "\n"
+            "int  a;\n"
+            "int b;\n"
+            "int  a;\n"
+            "#endif // __H__\n",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, FormatsCorrectLineWhenHeaderIsRemoved) {
+  setInput("#include \"a\"\n"
+           "\n"
+           "int  a;\n"
+           "int  a;\n"
+           "int  a;");
+  Changes.push_back(replacementToAtomicChange("key1", 27, 1, "b"));
+  Changes.back().removeHeader("a");
+  EXPECT_EQ("\n"
+            "int  a;\n"
+            "int b;\n"
+            "int  a;",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, CleansUpCtorInitializers) {
+  setInput("A::A() : a(), b() {}\n"
+           "A::A() : a(), b() {}\n"
+           "A::A() : a(), b() {}\n"
+           "A::A() : a()/**/, b() {}\n"
+           "A::A() : a()  ,// \n"
+           "   /**/    b()    {}");
+  Changes.emplace_back(FilePath, "key1");
+  auto Err = Changes.back().replace(Context.Sources, getLoc(9), 3, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(35), 3, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(51), 3, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(56), 3, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(72), 3, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(97), 3, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(118), 3, "");
+  ASSERT_TRUE(!Err);
+  EXPECT_EQ("A::A() : b() {}\n"
+            "A::A() : a() {}\n"
+            "A::A() {}\n"
+            "A::A() : b() {}\n"
+            "A::A() {}",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, CleansUpParameterLists) {
+  setInput("void f(int i, float f, string s);\n"
+           "f(1, 2.0f, \"a\");\n"
+           "g(1, 1);");
+  Changes.emplace_back(FilePath, "key1");
+  auto Err = Changes.back().replace(Context.Sources, getLoc(7), 5, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(23), 8, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(36), 1, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(45), 3, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(53), 1, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(56), 1, "");
+  ASSERT_TRUE(!Err);
+  EXPECT_EQ("void f(float f);\n"
+            "f(2.0f);\n"
+            "g();",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, DisableCleanup) {
+  Spec.Cleanup = false;
+  setInput("void f(int i, float f, string s);\n"
+           "f(1, 2.0f, \"a\");\n"
+           "g(1, 1);");
+  Changes.emplace_back(FilePath, "key1");
+  auto Err = Changes.back().replace(Context.Sources, getLoc(7), 5, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(23), 8, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(36), 1, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(45), 3, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(53), 1, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(56), 1, "");
+  ASSERT_TRUE(!Err);
+  EXPECT_EQ("void f(, float f, );\n"
+            "f(, 2.0f, );\n"
+            "g(, );",
+            rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, EverythingDeleted) {
+  setInput("int a;");
+  Changes.push_back(replacementToAtomicChange("key1", 0, 6, ""));
+  EXPECT_EQ("", rewrite());
+}
+
+TEST_F(ApplyAtomicChangesTest, DoesNotDeleteInserts) {
+  setInput("int a;\n"
+           "int b;");
+  Changes.emplace_back(FilePath, "key1");
+  auto Err = Changes.back().replace(Context.Sources, getLoc(4), 1, "");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(4), 0, "b");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(11), 0, "a");
+  ASSERT_TRUE(!Err);
+  Err = Changes.back().replace(Context.Sources, getLoc(11), 1, "");
+  ASSERT_TRUE(!Err);
+  EXPECT_EQ("int b;\n"
+            "int a;",
+            rewrite());
+}
+
 } // end namespace tooling
 } // end namespace clang