Don't call Types.asMemberOf unnecessarily.

If method `m` is defined in class `C`, there's no point in calling `asMemberOf(C, m)`. `asMemberOf` is only useful when `m` is defined in a subtype of `C`.

This is a very minor optimization, but it also avoids an issue where `asMemberOf` can return a method that has lost type annotations on its return type. We still have that issue for inherited methods, but at least in the common case of locally-defined methods we can avoid it.

RELNOTES=Annotations on type parameters, like `abstract @nullable T foo()`, are now better propagated to fields and constructor parameters.
PiperOrigin-RevId: 587175179

value/src/it/functional/src/test/java/com/google/auto/value/AutoValueJava8Test.java

@@ -38,6 +38,7 @@
 import java.lang.annotation.Target;
 import java.lang.reflect.AnnotatedType;
 import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.lang.reflect.TypeVariable;
 import java.util.ArrayList;
@@ -1047,4 +1048,57 @@ public void nullableVariableBound() {
     assertThat(x.nullOne()).isNull();
     assertThat(x.nullTwo()).isNull();
   }
+
+  @AutoValue
+  public abstract static class NotNullableVariableBound<T> {
+    public abstract T t();
+
+    public abstract @Nullable T nullableT();
+
+    public abstract String string();
+
+    public static <T> Builder<T> builder() {
+      return new AutoValue_AutoValueJava8Test_NotNullableVariableBound.Builder<>();
+    }
+
+    @AutoValue.Builder
+    public abstract static class Builder<T> {
+      public abstract Builder<T> setT(T t);
+
+      public abstract Builder<T> setNullableT(@Nullable T nullableT);
+
+      public abstract Builder<T> setString(String string);
+
+      public abstract NotNullableVariableBound<T> build();
+    }
+  }
+
+  @Test
+  public void typeParameterBuilderFieldsAreNullable() throws ReflectiveOperationException {
+    assertThrows(NullPointerException.class, () -> NotNullableVariableBound.builder().setT(null));
+
+    // Even though neither t() nor string() has a @Nullable return type, the corresponding builder
+    // fields should be @Nullable. This test depends on the knowledge that for a property `t`, we
+    // will have a field also called `t`.
+    Field builderT = NotNullableVariableBound.builder().getClass().getDeclaredField("t");
+    assertThat(builderT.getAnnotatedType().getAnnotations()).asList().contains(nullable());
+    Field builderNullableT =
+        NotNullableVariableBound.builder().getClass().getDeclaredField("nullableT");
+    assertThat(builderNullableT.getAnnotatedType().getAnnotations()).asList().contains(nullable());
+
+    // Meanwhile the AutoValue class itself should have @Nullable on the private field, the getter
+    // method, and the constructor parameter for nullableT.
+    Class<?> autoValueClass = AutoValue_AutoValueJava8Test_NotNullableVariableBound.class;
+    Field nullableTField = autoValueClass.getDeclaredField("nullableT");
+    assertThat(nullableTField.getAnnotatedType().getAnnotations()).asList().contains(nullable());
+    Method nullableTMethod = autoValueClass.getMethod("nullableT");
+    assertThat(nullableTMethod.getAnnotatedReturnType().getAnnotations())
+        .asList()
+        .contains(nullable());
+    Constructor<?> autoValueConstructor =
+        autoValueClass.getDeclaredConstructor(Object.class, Object.class, String.class);
+    assertThat(autoValueConstructor.getAnnotatedParameterTypes()[1].getAnnotations())
+        .asList()
+        .contains(nullable());
+  }
 }

value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java

@@ -224,7 +224,7 @@ private static String builderInitializer(
      *
      * <ul>
      *   <li>the property is not primitive;
-     *   <li>the property is not already nullable;
+     *   <li>the property type does not already have a {@code @Nullable} annotation;
      *   <li>there is no explicit initializer (for example {@code Optional} properties start off as
      *       {@code Optional.empty()});
      *   <li>we have found a {@code @Nullable} type annotation that can be applied.

value/src/main/java/com/google/auto/value/processor/EclipseHack.java

@@ -146,21 +146,27 @@ ImmutableMap<ExecutableElement, TypeMirror> methodReturnTypes(
     ImmutableMap.Builder<ExecutableElement, TypeMirror> map = ImmutableMap.builder();
     Map<Name, ExecutableElement> noArgMethods = null;
     for (ExecutableElement method : methods) {
-      TypeMirror returnType = null;
-      try {
-        TypeMirror methodMirror = typeUtils.asMemberOf(in, method);
-        returnType = MoreTypes.asExecutable(methodMirror).getReturnType();
-      } catch (IllegalArgumentException e) {
-        if (method.getParameters().isEmpty()) {
-          if (noArgMethods == null) {
-            noArgMethods = noArgMethodsIn(in);
+      TypeMirror returnType = method.getReturnType();
+      if (!in.asElement().equals(method.getEnclosingElement())) {
+        // If this method is *not* inherited, but directly defined in `in`, then asMemberOf
+        // shouldn't have any effect. So the if-check here is an optimization. But it also avoids an
+        // issue where the compiler may return an ExecutableType that has lost any annotations that
+        // were present in the original.
+        // We can still hit that issue in the case where the method *is* inherited. Fixing it in
+        // general would probably involve keeping track of type annotations ourselves, separately
+        // from TypeMirror instances.
+        try {
+          TypeMirror methodMirror = typeUtils.asMemberOf(in, method);
+          returnType = MoreTypes.asExecutable(methodMirror).getReturnType();
+        } catch (IllegalArgumentException e) {
+          if (method.getParameters().isEmpty()) {
+            if (noArgMethods == null) {
+              noArgMethods = noArgMethodsIn(in);
+            }
+            returnType = noArgMethods.get(method.getSimpleName()).getReturnType();
           }
-          returnType = noArgMethods.get(method.getSimpleName()).getReturnType();
         }
       }
-      if (returnType == null) {
-        returnType = method.getReturnType();
-      }
       map.put(method, returnType);
     }
     return map.build();