From ba8f9a285f542c8b628cf50e365bd0270b0aac45 Mon Sep 17 00:00:00 2001 From: Error Prone Team Date: Tue, 6 Aug 2024 12:10:10 -0700 Subject: [PATCH] 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 --- .../bugpatterns/AutoValueBoxedValues.java | 7 +++ .../bugpatterns/AutoValueBoxedValuesTest.java | 58 +++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AutoValueBoxedValues.java b/core/src/main/java/com/google/errorprone/bugpatterns/AutoValueBoxedValues.java index 06025ebd325..c47831af89d 100644 --- 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 index a7c6cc92aa9..adc536f4801 100644 --- 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) {