Make insertion coalescing in Replacements#add explicit

Instead of silently concatenating overlapping insertions together,
require callers of Replacements#add to explicitly requires a policy for
concatenating them (either existing-first, or new-first). The default
behaviour now throws an exception for overlapping insertions.

MOE_MIGRATED_REVID=212536627

check_api/src/main/java/com/google/errorprone/apply/DescriptionBasedDiff.java

@@ -119,7 +119,8 @@ public void applyDifferences(SourceFile sourceFile) throws DiffNotApplicableExce
             Replacement.create(
                 importStatements.getStartPos(),
                 importStatements.getEndPos(),
-                importStatements.toString()));
+                importStatements.toString()),
+            Replacements.CoalescePolicy.REPLACEMENT_FIRST);
       }
     }
     for (Replacement replacement : replacements.descending()) {

check_api/src/main/java/com/google/errorprone/fixes/Replacements.java

@@ -53,7 +53,46 @@ public int compare(Range<Integer> o1, Range<Integer> o2) {
   private final RangeMap<Integer, Replacement> overlaps = TreeRangeMap.create();
   private final TreeSet<Integer> zeroLengthRanges = new TreeSet<>();
 
+  /** A policy for handling overlapping insertions. */
+  public enum CoalescePolicy {
+    /** Reject overlapping insertions and throw an {@link IllegalArgumentException}. */
+    REJECT {
+      @Override
+      public String coalesce(String replacement, String existing) {
+        throw new IllegalArgumentException(
+            String.format("%s conflicts with existing replacement %s", replacement, existing));
+      }
+    },
+    /** Accept overlapping insertions, with the new insertion before the existing one. */
+    REPLACEMENT_FIRST {
+      @Override
+      public String coalesce(String replacement, String existing) {
+        return replacement + existing;
+      }
+    },
+    /** Accept overlapping insertions, with the existing insertion before the new one. */
+    EXISTING_FIRST {
+      @Override
+      public String coalesce(String replacement, String existing) {
+        return existing + replacement;
+      }
+    };
+
+    /**
+     * Handle an overlapping insert.
+     *
+     * @param replacement the replacement being added.
+     * @param existing the existing insert at this range.
+     * @return the coalesced replacement.
+     */
+    public abstract String coalesce(String replacement, String existing);
+  }
+
   public Replacements add(Replacement replacement) {
+    return add(replacement, CoalescePolicy.REJECT);
+  }
+
+  public Replacements add(Replacement replacement, CoalescePolicy coalescePolicy) {
     if (replacements.containsKey(replacement.range())) {
       Replacement existing = replacements.get(replacement.range());
       if (!existing.equals(replacement)) {
@@ -64,7 +103,7 @@ public Replacements add(Replacement replacement) {
               Replacement.create(
                   existing.startPosition(),
                   existing.endPosition(),
-                  existing.replaceWith() + replacement.replaceWith());
+                  coalescePolicy.coalesce(replacement.replaceWith(), existing.replaceWith()));
         } else {
           throw new IllegalArgumentException(
               String.format("%s conflicts with existing replacement %s", replacement, existing));

check_api/src/main/java/com/google/errorprone/fixes/SuggestedFix.java

@@ -87,7 +87,8 @@ public Set<Replacement> getReplacements(EndPosTable endPositions) {
     }
     Replacements replacements = new Replacements();
     for (FixOperation fix : fixes) {
-      replacements.add(fix.getReplacement(endPositions));
+      replacements.add(
+          fix.getReplacement(endPositions), Replacements.CoalescePolicy.EXISTING_FIRST);
     }
     return replacements.descending();
   }

check_api/src/test/java/com/google/errorprone/fixes/ReplacementsTest.java

@@ -23,6 +23,7 @@
 import com.google.common.base.Function;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Range;
+import com.google.errorprone.fixes.Replacements.CoalescePolicy;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -96,24 +97,54 @@ public void identicalDuplicatesOK() {
   }
 
   @Test
-  public void multipleInsertionsAtSamePointAreCoalescedInOrder() {
+  public void coalesceExistingFirst() {
     // A replacement of an empty region represents an insertion.
     // Multiple, differing insertions at the same insertion point are allowed, and will be
     // coalesced into a single Replacement at that insertion point.
     assertThat(
             new Replacements()
                 .add(Replacement.create(42, 42, "hello;"))
-                .add(Replacement.create(42, 42, "goodbye;"))
+                .add(Replacement.create(42, 42, "goodbye;"), CoalescePolicy.EXISTING_FIRST)
                 .descending())
         .containsExactly(Replacement.create(42, 42, "hello;goodbye;"));
     assertThat(
             new Replacements()
                 .add(Replacement.create(42, 42, "goodbye;"))
-                .add(Replacement.create(42, 42, "hello;"))
+                .add(Replacement.create(42, 42, "hello;"), CoalescePolicy.EXISTING_FIRST)
                 .descending())
         .containsExactly(Replacement.create(42, 42, "goodbye;hello;"));
   }
 
+  @Test
+  public void coalesceReplacementFirst() {
+    // A replacement of an empty region represents an insertion.
+    // Multiple, differing insertions at the same insertion point are allowed, and will be
+    // coalesced into a single Replacement at that insertion point.
+    assertThat(
+            new Replacements()
+                .add(Replacement.create(42, 42, "hello;"))
+                .add(Replacement.create(42, 42, "goodbye;"), CoalescePolicy.REPLACEMENT_FIRST)
+                .descending())
+        .contains(Replacement.create(42, 42, "goodbye;hello;"));
+    assertThat(
+            new Replacements()
+                .add(Replacement.create(42, 42, "goodbye;"))
+                .add(Replacement.create(42, 42, "hello;"), CoalescePolicy.REPLACEMENT_FIRST)
+                .descending())
+        .containsExactly(Replacement.create(42, 42, "hello;goodbye;"));
+  }
+
+  @Test
+  public void coalesceReject() {
+    assertThrows(
+        IllegalArgumentException.class,
+        () ->
+            new Replacements()
+                .add(Replacement.create(42, 42, "hello;"))
+                .add(Replacement.create(42, 42, "goodbye;"), CoalescePolicy.REJECT)
+                .descending());
+  }
+
   @Test
   public void multipleInsertionsAreDeduplicated() {
     assertThat(

core/src/test/java/com/google/errorprone/fixes/SuggestedFixesTest.java

@@ -57,6 +57,7 @@
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.tree.DCTree;
 import com.sun.tools.javac.tree.JCTree;
+import java.io.IOException;
 import java.lang.annotation.Retention;
 import java.util.stream.Stream;
 import javax.lang.model.element.Modifier;
@@ -1229,4 +1230,38 @@ public void removeAddModifier_rangesCompatible() {
         .addOutputLines("out/Test.java", "abstract class Test {}")
         .doTest();
   }
+
+  /** A bugchecker for testing suggested fixes. */
+  @BugPattern(
+      name = "PrefixAddImportCheck",
+      summary = "A bugchecker for testing suggested fixes.",
+      severity = ERROR,
+      providesFix = ProvidesFix.REQUIRES_HUMAN_ATTENTION)
+  public static class PrefixAddImportCheck extends BugChecker implements ClassTreeMatcher {
+    @Override
+    public Description matchClass(ClassTree tree, VisitorState state) {
+      return describeMatch(
+          tree,
+          SuggestedFix.builder()
+              .prefixWith(tree, "@Deprecated\n")
+              .addImport("java.util.List")
+              .build());
+    }
+  }
+
+  @Test
+  public void prefixAddImport() throws IOException {
+    BugCheckerRefactoringTestHelper.newInstance(new PrefixAddImportCheck(), getClass())
+        .addInputLines(
+            "in/Test.java", //
+            "package p;",
+            "class Test {}")
+        .addOutputLines(
+            "out/Test.java", //
+            "package p;",
+            "import java.util.List;",
+            "@Deprecated",
+            "class Test {}")
+        .doTest();
+  }
 }