From e5b4b548486549ba096983d6d79ed39f9a0e16b7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89amonn=20McManus?= <emcmanus@google.com>
Date: Tue, 11 Jul 2023 16:25:07 -0700
Subject: [PATCH] Output a warning if a `setX` method in a `Builder` is marked
 `@Nullable`.

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
---
 .../processor/BuilderMethodClassifier.java    |  7 +++++
 .../processor/AutoValueCompilationTest.java   | 31 +++++++++++++++++++
 2 files changed, 38 insertions(+)

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
index b0872009d6..591b0cc8cc 100644
--- 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
index 01ec81fefb..e9fdcbfd6e 100644
--- 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 =