Fix getter in Builder class if the corresponding getter was changed.

PiperOrigin-RevId: 663771111
google · Aug 16, 2024 · cad8b20 · cad8b20
1 parent 841fa1d
commit cad8b20
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 1 deletion.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AutoValueBoxedValues.java b/core/src/main/java/com/google/errorprone/bugpatterns/AutoValueBoxedValues.java
@@ -130,13 +130,14 @@ && isBoxedPrimitive(state, type)) {
   }
 
   /**
-   * Identifies and fixes the setters in the {@link AutoValue.Builder} class.
+   * Identifies and fixes both the setters and the getters in the {@link AutoValue.Builder} class.
    *
    * @param classTree The {@link AutoValue.Builder} class tree.
    * @param state The visitor state.
    * @param getters The {@link List} of {@link Getter} in the {@link AutoValue} class.
    */
   private void handleSetterMethods(ClassTree classTree, VisitorState state, List<Getter> getters) {
+    // Identify and try to fix the setters.
     classTree.getMembers().stream()
         .filter(MethodTree.class::isInstance)
         .map(memberTree -> (MethodTree) memberTree)
@@ -146,6 +147,14 @@ private void handleSetterMethods(ClassTree classTree, VisitorState state, List<G
                     && methodTree.getParameters().size() == 1
                     && isSameType(getType(methodTree.getReturnType()), getType(classTree), state))
         .forEach(methodTree -> maybeFixSetter(methodTree, state, getters));
+    // Identify and try to fix the getters.
+    classTree.getMembers().stream()
+        .filter(MethodTree.class::isInstance)
+        .map(memberTree -> (MethodTree) memberTree)
+        .filter(
+            methodTree ->
+                ABSTRACT_MATCHER.matches(methodTree, state) && methodTree.getParameters().isEmpty())
+        .forEach(methodTree -> maybeFixGetterInBuilder(methodTree, state, getters));
   }
 
   /** Given a setter, it tries to apply a fix if the corresponding getter was also fixed. */
@@ -170,6 +179,30 @@ && matchGetterAndSetter(getter.method(), methodTree, allGettersPrefixed))
     }
   }
 
+  /**
+   * Given a getter in the Builder class, it tries to apply a fix if the corresponding getter in the
+   * parent {@link AutoValue} class was also fixed.
+   */
+  private static void maybeFixGetterInBuilder(
+      MethodTree methodTree, VisitorState state, List<Getter> getters) {
+    Optional<Getter> fixedGetter =
+        getters.stream()
+            .filter(
+                getter ->
+                    !getter.fix().isEmpty()
+                        && getter.method().getName().contentEquals(methodTree.getName().toString())
+                        && isSameType(
+                            getType(getter.method().getReturnType()),
+                            getType(methodTree.getReturnType()),
+                            state))
+            .findAny();
+    if (fixedGetter.isPresent()) {
+      Type type = getType(methodTree.getReturnType());
+      suggestRemoveUnnecessaryBoxing(
+          methodTree.getReturnType(), state, type, fixedGetter.get().fix());
+    }
+  }
+
   /**
    * Identifies and fixes the factory method in the {@link AutoValue} class.
    *

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/AutoValueBoxedValuesTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/AutoValueBoxedValuesTest.java
@@ -660,6 +660,47 @@ public void allGettersWithPrefix_ignoreToBuilder() {
         .doTest();
   }
 
+  @Test
+  public void getterInBuilderClass() {
+    if (!withBuilder) {
+      return;
+    }
+    refactoringHelper
+        .addInputLines(
+            "in/Test.java",
+            "import com.google.auto.value.AutoValue;",
+            "@AutoValue",
+            "abstract class Test {",
+            "  public abstract Long longId();",
+            "  public abstract Boolean booleanId();",
+            "  public abstract Builder toBuilder();",
+            "  @AutoValue.Builder",
+            "  abstract static class Builder {",
+            "    abstract Builder longId(Long value);",
+            "    abstract Builder booleanId(Boolean value);",
+            "    abstract Boolean booleanId();",
+            "    abstract Test build();",
+            "  }",
+            "}")
+        .addOutputLines(
+            "out/Test.java",
+            "import com.google.auto.value.AutoValue;",
+            "@AutoValue",
+            "abstract class Test {",
+            "  public abstract long longId();",
+            "  public abstract boolean booleanId();",
+            "  public abstract Builder toBuilder();",
+            "  @AutoValue.Builder",
+            "  abstract static class Builder {",
+            "    abstract Builder longId(long value);",
+            "    abstract Builder booleanId(boolean value);",
+            "    abstract boolean booleanId();",
+            "    abstract Test build();",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   private static List<String> lines(String... lines) {
     return Arrays.asList(lines);
   }