Fix replacement exceptions in auto-generated code

From google#1195
yasserg · May 30, 2020 · 9d36426 · 9d36426
1 parent ec40de7
commit 9d36426
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 12 deletions.
diff --git a/check_api/src/main/java/com/google/errorprone/JavacErrorDescriptionListener.java b/check_api/src/main/java/com/google/errorprone/JavacErrorDescriptionListener.java
@@ -22,6 +22,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.errorprone.fixes.AppliedFix;
 import com.google.errorprone.fixes.Fix;
+import com.google.errorprone.fixes.Replacement;
 import com.google.errorprone.matchers.Description;
 import com.sun.source.tree.Tree.Kind;
 import com.sun.tools.javac.tree.EndPosTable;
@@ -68,7 +69,15 @@ private JavacErrorDescriptionListener(
     checkNotNull(endPositions);
     try {
       CharSequence sourceFileContent = sourceFile.getCharContent(true);
-      fixToAppliedFix = fix -> AppliedFix.fromSource(sourceFileContent, endPositions).apply(fix);
+      fixToAppliedFix = fix -> {
+        try {
+          return AppliedFix.fromSource(sourceFileContent, endPositions).apply(fix);
+        } catch (Replacement.InvalidReplacementPositionException e) {
+          // Not possible to auto-suggest a fix, for example inside
+          // Lombok auto-generated code
+          return null;
+        }
+      };
     } catch (IOException e) {
       throw new UncheckedIOException(e);
     }

diff --git a/check_api/src/main/java/com/google/errorprone/fixes/Replacement.java b/check_api/src/main/java/com/google/errorprone/fixes/Replacement.java
@@ -16,8 +16,6 @@
 
 package com.google.errorprone.fixes;
 
-import static com.google.common.base.Preconditions.checkArgument;
-
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.Range;
 
@@ -34,12 +32,13 @@ public abstract class Replacement {
    * @param replaceWith the replacement text
    */
   public static Replacement create(int startPosition, int endPosition, String replaceWith) {
-    checkArgument(
-        startPosition >= 0 && startPosition <= endPosition,
-        "invalid replacement: [%s, %s) (%s)",
-        startPosition,
-        endPosition,
-        replaceWith);
+    if (startPosition < 0 || startPosition > endPosition) {
+      throw new InvalidReplacementPositionException(String.format(
+              "invalid replacement: [%s, %s) (%s)",
+              startPosition,
+              endPosition,
+              replaceWith));
+    }
     return new AutoValue_Replacement(Range.closedOpen(startPosition, endPosition), replaceWith);
   }
 
@@ -63,4 +62,11 @@ public int endPosition() {
 
   /** The source text to appear in the output. */
   public abstract String replaceWith();
+
+  public static class InvalidReplacementPositionException extends IllegalArgumentException {
+    InvalidReplacementPositionException(String message) {
+      super(message);
+    }
+  }
+
 }
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractReferenceEquality.java b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractReferenceEquality.java
@@ -21,10 +21,14 @@
 import static com.google.errorprone.matchers.Matchers.instanceMethod;
 import static com.google.errorprone.matchers.Matchers.staticEqualsInvocation;
 
+import java.util.List;
+import java.util.Optional;
+
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker.BinaryTreeMatcher;
 import com.google.errorprone.dataflow.nullnesspropagation.Nullness;
 import com.google.errorprone.fixes.Fix;
+import com.google.errorprone.fixes.Replacement;
 import com.google.errorprone.fixes.SuggestedFix;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
@@ -35,8 +39,6 @@
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.Tree.Kind;
 import com.sun.source.util.TreePath;
-import java.util.List;
-import java.util.Optional;
 
 /**
  * Abstract implementation of a BugPattern that detects the use of reference equality to compare
@@ -79,7 +81,12 @@ public final Description matchBinary(BinaryTree tree, VisitorState state) {
     }
 
     Description.Builder builder = buildDescription(tree);
-    addFixes(builder, tree, state);
+    try {
+      addFixes(builder, tree, state);
+    } catch (Replacement.InvalidReplacementPositionException e) {
+      // Not possible to auto-suggest a fix, for example inside
+      // Lombok auto-generated setters which use "=="
+    }
     return builder.build();
   }