Do not update getters that override methods from a superclass.

The change would be incorrect as the modified method is no longer matching the signature of the method in the superclass. PiperOrigin-RevId: 660049434
google · Aug 6, 2024 · ba8f9a2 · ba8f9a2
1 parent a706e8d
commit ba8f9a2
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 0 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AutoValueBoxedValues.java b/core/src/main/java/com/google/errorprone/bugpatterns/AutoValueBoxedValues.java
@@ -20,6 +20,7 @@
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
 import static com.google.errorprone.matchers.Description.NO_MATCH;
 import static com.google.errorprone.matchers.Matchers.hasModifier;
+import static com.google.errorprone.util.ASTHelpers.findSuperMethods;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
 import static com.google.errorprone.util.ASTHelpers.getType;
 import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
@@ -121,6 +122,7 @@ private Getter maybeFixGetter(MethodTree method, VisitorState state) {
     Type type = getType(method.getReturnType());
     if (!isSuppressed(method, state)
         && !hasNullableAnnotation(method)
+        && !isOverride(method, state)
         && isBoxedPrimitive(state, type)) {
       suggestRemoveUnnecessaryBoxing(method.getReturnType(), state, type, getter.fix());
     }
@@ -211,6 +213,11 @@ private static boolean hasNullableAnnotation(Tree tree) {
     return NullnessAnnotations.fromAnnotationsOn(getSymbol(tree)).orElse(null) == Nullness.NULLABLE;
   }
 
+  /** Returns true if the method overrides another method. */
+  private static boolean isOverride(MethodTree methodTree, VisitorState state) {
+    return !findSuperMethods(getSymbol(methodTree), state.getTypes()).isEmpty();
+  }
+
   /** Returns the primitive type corresponding to a boxed type. */
   private static Type unbox(VisitorState state, Type type) {
     return state.getTypes().unboxedType(type);

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/AutoValueBoxedValuesTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/AutoValueBoxedValuesTest.java
@@ -425,6 +425,64 @@ public void unnecessaryBoxedTypes_suppressWarnings() {
         .doTest();
   }
 
+  @Test
+  public void unnecessaryBoxedTypes_overrides() {
+    refactoringHelper
+        .addInputLines(
+            "in/Test.java",
+            mergeLines(
+                lines(
+                    "import com.google.auto.value.AutoValue;",
+                    "class Test {",
+                    "  interface SuperClass {",
+                    "    Long superClassLongId();",
+                    "  }",
+                    "  @AutoValue",
+                    "  static abstract class BaseClass implements SuperClass {",
+                    "    public abstract Long longId();",
+                    "    @Override",
+                    "    public abstract Long superClassLongId();"),
+                linesWithoutBuilder(
+                    "    static BaseClass create(Long longId, Long superClassLongId) {",
+                    "      return new AutoValue_Test_BaseClass(longId, superClassLongId);",
+                    "    }"),
+                linesWithBuilder(
+                    "    @AutoValue.Builder",
+                    "    abstract static class Builder {",
+                    "      abstract Builder setLongId(Long value);",
+                    "      abstract Builder setSuperClassLongId(Long value);",
+                    "      abstract BaseClass build();",
+                    "    }"),
+                lines("  }", "}")))
+        .addOutputLines(
+            "out/Test.java",
+            mergeLines(
+                lines(
+                    "import com.google.auto.value.AutoValue;",
+                    "class Test {",
+                    "  interface SuperClass {",
+                    "    Long superClassLongId();",
+                    "  }",
+                    "  @AutoValue",
+                    "  static abstract class BaseClass implements SuperClass {",
+                    "    public abstract long longId();",
+                    "    @Override",
+                    "    public abstract Long superClassLongId();"),
+                linesWithoutBuilder(
+                    "    static BaseClass create(long longId, Long superClassLongId) {",
+                    "      return new AutoValue_Test_BaseClass(longId, superClassLongId);",
+                    "    }"),
+                linesWithBuilder(
+                    "    @AutoValue.Builder",
+                    "    abstract static class Builder {",
+                    "      abstract Builder setLongId(long value);",
+                    "      abstract Builder setSuperClassLongId(Long value);",
+                    "      abstract BaseClass build();",
+                    "    }"),
+                lines("  }", "}")))
+        .doTest();
+  }
+
   @Test
   public void nullableGettersWithNonNullableSetters_noChange() {
     if (!withBuilder) {