Output a warning if a setX method in a Builder is marked `@Nullab…

…le`. If the `setX` method itself, or its return type, is `@Nullable` then a warning makes sense. The method always returns a non-null value, the `Builder` itself. RELNOTES=A warning is now produced if a `setX` method in a `Builder` or its return type is marked `@Nullable`. Those methods always return the `Builder` instance, which is never null. PiperOrigin-RevId: 547329146
google · Jul 11, 2023 · e5b4b54 · e5b4b54
1 parent 895ce2b
commit e5b4b54
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 0 deletions.
diff --git a/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java b/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java
@@ -403,6 +403,13 @@ private void classifyMethodOneArg(ExecutableElement method) {
       TypeMirror returnType = methodMirror.getReturnType();
       if (typeUtils.isSubtype(builderType.asType(), returnType)
           && !MoreTypes.isTypeOf(Object.class, returnType)) {
+        if (nullableAnnotationFor(method, returnType).isPresent()) {
+          errorReporter.
+              reportWarning(
+                  method,
+                  "[%sBuilderSetterNullable] Setter methods always return the Builder so @Nullable"
+                      + " is not appropriate", autoWhat());
+        }
         // We allow the return type to be a supertype (other than Object), to support step builders.
         TypeMirror parameterType = Iterables.getOnlyElement(methodMirror.getParameterTypes());
         propertyNameToSetters.put(

diff --git a/value/src/test/java/com/google/auto/value/processor/AutoValueCompilationTest.java b/value/src/test/java/com/google/auto/value/processor/AutoValueCompilationTest.java
@@ -2037,6 +2037,37 @@ public void autoValueBuilderWrongTypeSetter() {
         .onLineContaining("Builder blim(String x)");
   }
 
+  @Test
+  public void autoValueBuilderSetterReturnsNullable() {
+    JavaFileObject javaFileObject =
+        JavaFileObjects.forSourceLines(
+            "foo.bar.Baz",
+            "package foo.bar;",
+            "",
+            "import com.google.auto.value.AutoValue;",
+            "import javax.annotation.Nullable;",
+            "",
+            "@AutoValue",
+            "public abstract class Baz {",
+            "  abstract String blam();",
+            "",
+            "  @AutoValue.Builder",
+            "  public interface Builder {",
+            "    @Nullable Builder blam(String x);",
+            "    Baz build();",
+            "  }",
+            "}");
+    Compilation compilation =
+        javac()
+            .withProcessors(new AutoValueProcessor(), new AutoValueBuilderProcessor())
+            .compile(javaFileObject);
+    assertThat(compilation)
+        .hadWarningContaining(
+            "Setter methods always return the Builder so @Nullable is not appropriate")
+        .inFile(javaFileObject)
+        .onLineContaining("Builder blam(String x)");
+  }
+
   @Test
   public void autoValueBuilderWrongTypeSetterWithCopyOf() {
     JavaFileObject javaFileObject =