Tooling: When applying a set of replacements, do deletions before

insertions. It is unlikely to be the intention to delete parts of newly
inserted code. To do so, changed sorting Replacements at the same offset
to have decreasing length.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@239809 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Tooling/Core/Replacement.cpp b/lib/Tooling/Core/Replacement.cpp
index 32e8e5bd6b977907934ebe8b9d6f5a56da1a3a85..6d37a49db38159e44cde506691d8b1417d95ffae 100644 (file)
--- a/lib/Tooling/Core/Replacement.cpp
+++ b/lib/Tooling/Core/Replacement.cpp
@@ -88,8 +88,13 @@ std::string Replacement::toString() const {
 bool operator<(const Replacement &LHS, const Replacement &RHS) {
   if (LHS.getOffset() != RHS.getOffset())
     return LHS.getOffset() < RHS.getOffset();
+
+  // Apply longer replacements first, specifically so that deletions are
+  // executed before insertions. It is (hopefully) never the intention to
+  // delete parts of newly inserted code.
   if (LHS.getLength() != RHS.getLength())
-    return LHS.getLength() < RHS.getLength();
+    return LHS.getLength() > RHS.getLength();
+
   if (LHS.getFilePath() != RHS.getFilePath())
     return LHS.getFilePath() < RHS.getFilePath();
   return LHS.getReplacementText() < RHS.getReplacementText();

diff --git a/unittests/Tooling/RewriterTest.cpp b/unittests/Tooling/RewriterTest.cpp
index c53e50a87d76978638991e38c24d4bf7931fc7ac..4928b17cfa28a935107eb5314c5b2015d43ce856 100644 (file)
--- a/unittests/Tooling/RewriterTest.cpp
+++ b/unittests/Tooling/RewriterTest.cpp
@@ -8,9 +8,12 @@
 //===----------------------------------------------------------------------===//
 
 #include "RewriterTestContext.h"
+#include "clang/Tooling/Core/Replacement.h"
 #include "gtest/gtest.h"
 
 namespace clang {
+namespace tooling {
+namespace {
 
 TEST(Rewriter, OverwritesChangedFiles) {
   RewriterTestContext Context;
@@ -34,4 +37,14 @@ TEST(Rewriter, ContinuesOverwritingFilesOnError) {
             Context.getFileContentFromDisk("working.cpp")); 
 }
 
+TEST(Rewriter, AdjacentInsertAndDelete) {
+  Replacements Replaces;
+  Replaces.emplace("<file>", 6, 6, "");
+  Replaces.emplace("<file>", 6, 0, "replaced\n");
+  EXPECT_EQ("line1\nreplaced\nline3\nline4",
+            applyAllReplacements("line1\nline2\nline3\nline4", Replaces));
+}
+
+} // end namespace
+} // end namespace tooling
 } // end namespace clang