When checking builder setter parameters, use the final type. The final type is the type after type-variable substitution. Report this type in error messages, since it may not be obvious.

(See AutoValueNotEclipseTest for an example of the problem this is fixing.)

Fix CompileWithEclipseTest so that it actually does exclude AutoValueNotEclipseTest.java from the compilation, as intended. Move the existing test within that file into AutoValueTest.java, since apparently the Eclipse bug it was provoking has been fixed. Add the test for the bug being fixed here into AutoValueNotEclipseTest.java, because it hits another Eclipse bug.

Fixes https://github.com/google/auto/issues/802.

RELNOTES=Fixed an issue with type checking of setter parameters in the presence of inheritance and generics.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=287580372

4 files changed, 56 insertions, 30 deletions

diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueNotEclipseTest.java b/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueNotEclipseTest.java
index 17cd6352..4d008c71 100644
--- a/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueNotEclipseTest.java
+++ b/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueNotEclipseTest.java
@@ -15,9 +15,10 @@
  */
 package com.google.auto.value;
 
-import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
 
-import com.google.common.collect.ImmutableList;
+import java.util.Optional;
+import javax.annotation.Nullable;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -29,37 +30,36 @@ import org.junit.runners.JUnit4;
  */
 @RunWith(JUnit4.class)
 public class AutoValueNotEclipseTest {
-  interface ImmutableListOf<T> {
-    ImmutableList<T> list();
+  // This produced the following error with JDT 4.6:
+  // Internal compiler error: java.lang.Exception: java.lang.IllegalArgumentException: element
+  // public abstract B setOptional(T)  is not a member of the containing type
+  // com.google.auto.value.AutoValueTest.ConcreteOptional.Builder nor any of its superclasses at
+  // org.eclipse.jdt.internal.compiler.apt.dispatch.RoundDispatcher.handleProcessor(RoundDispatcher.java:169)
+  interface AbstractOptional<T> {
+    Optional<T> optional();
+
+    interface Builder<T, B extends Builder<T, B>> {
+      B setOptional(@Nullable T t);
+    }
   }
 
-  // This provoked the following with the Eclipse compiler:
-  // java.lang.NullPointerException
-  //   at org.eclipse.jdt.internal.compiler.lookup.ParameterizedTypeBinding.readableName(ParameterizedTypeBinding.java:1021)
-  //   at org.eclipse.jdt.internal.compiler.apt.model.DeclaredTypeImpl.toString(DeclaredTypeImpl.java:118)
-  //   at java.lang.String.valueOf(String.java:2996)
-  //   at java.lang.StringBuilder.append(StringBuilder.java:131)
-  //   at org.eclipse.jdt.internal.compiler.apt.model.TypesImpl.asMemberOf(TypesImpl.java:130)
-  //   at com.google.auto.value.processor.EclipseHack.methodReturnType(EclipseHack.java:124)
-  //   at com.google.auto.value.processor.TypeVariables.lambda$rewriteReturnTypes$1(TypeVariables.java:106)
   @AutoValue
-  abstract static class PropertyBuilderInheritsType implements ImmutableListOf<String> {
+  abstract static class ConcreteOptional implements AbstractOptional<String> {
     static Builder builder() {
-      return new AutoValue_AutoValueNotEclipseTest_PropertyBuilderInheritsType.Builder();
+      return new AutoValue_AutoValueNotEclipseTest_ConcreteOptional.Builder();
     }
 
     @AutoValue.Builder
-    abstract static class Builder {
-      abstract ImmutableList.Builder<String> listBuilder();
-      abstract PropertyBuilderInheritsType build();
+    interface Builder extends AbstractOptional.Builder<String, Builder> {
+      ConcreteOptional build();
     }
   }
 
   @Test
-  public void propertyBuilderInheritsType() {
-    PropertyBuilderInheritsType.Builder builder = PropertyBuilderInheritsType.builder();
-    builder.listBuilder().add("foo", "bar");
-    PropertyBuilderInheritsType x = builder.build();
-    assertThat(x.list()).containsExactly("foo", "bar").inOrder();
+  public void genericOptionalOfNullable() {
+    ConcreteOptional empty = ConcreteOptional.builder().build();
+    assertThat(empty.optional()).isEmpty();
+    ConcreteOptional notEmpty = ConcreteOptional.builder().setOptional("foo").build();
+    assertThat(notEmpty.optional()).hasValue("foo");
   }
 }
diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java b/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java
index 04d9218e..ab6468bf 100644
--- a/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java
+++ b/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java
@@ -2160,6 +2160,31 @@ public class AutoValueTest {
     }
   }
 
+  interface ImmutableListOf<T> {
+    ImmutableList<T> list();
+  }
+
+  @AutoValue
+  abstract static class PropertyBuilderInheritsType implements ImmutableListOf<String> {
+    static Builder builder() {
+      return new AutoValue_AutoValueTest_PropertyBuilderInheritsType.Builder();
+    }
+
+    @AutoValue.Builder
+    abstract static class Builder {
+      abstract ImmutableList.Builder<String> listBuilder();
+      abstract PropertyBuilderInheritsType build();
+    }
+  }
+
+  @Test
+  public void propertyBuilderInheritsType() {
+    PropertyBuilderInheritsType.Builder builder = PropertyBuilderInheritsType.builder();
+    builder.listBuilder().add("foo", "bar");
+    PropertyBuilderInheritsType x = builder.build();
+    assertThat(x.list()).containsExactly("foo", "bar").inOrder();
+  }
+
   @AutoValue
   public abstract static class BuilderWithExoticPropertyBuilders<
       K extends Number, V extends Comparable<K>> {
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 f611d6b9..6663b1bd 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
@@ -434,7 +434,7 @@ class BuilderMethodClassifier {
     // Parameter type is not equal to property type, but might be convertible with copyOf.
     ImmutableList<ExecutableElement> copyOfMethods = copyOfMethods(targetType, setter);
     if (!copyOfMethods.isEmpty()) {
-      return getConvertingSetterFunction(copyOfMethods, valueGetter, setter);
+      return getConvertingSetterFunction(copyOfMethods, valueGetter, setter, parameterType);
     }
     String error =
         String.format(
@@ -452,9 +452,9 @@ class BuilderMethodClassifier {
   private Optional<Function<String, String>> getConvertingSetterFunction(
       ImmutableList<ExecutableElement> copyOfMethods,
       ExecutableElement valueGetter,
-      ExecutableElement setter) {
+      ExecutableElement setter,
+      TypeMirror parameterType) {
     DeclaredType targetType = MoreTypes.asDeclared(getterToPropertyType.get(valueGetter));
-    TypeMirror parameterType = setter.getParameters().get(0).asType();
     for (ExecutableElement copyOfMethod : copyOfMethods) {
       Optional<Function<String, String>> function =
           getConvertingSetterFunction(copyOfMethod, targetType, parameterType);
@@ -465,8 +465,9 @@ class BuilderMethodClassifier {
     String targetTypeSimpleName = targetType.asElement().getSimpleName().toString();
     String error =
         String.format(
-            "Parameter type of setter method should be %s to match getter %s.%s, or it should be a "
-                + "type that can be passed to %s.%s to produce %s",
+            "Parameter type %s of setter method should be %s to match getter %s.%s,"
+                + " or it should be a type that can be passed to %s.%s to produce %s",
+            parameterType,
             targetType,
             autoValueClass,
             valueGetter.getSimpleName(),
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 724ea6b9..77390765 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
@@ -1599,7 +1599,7 @@ public class AutoValueCompilationTest {
             .compile(javaFileObject);
     assertThat(compilation)
         .hadErrorContaining(
-            "Parameter type of setter method should be "
+            "Parameter type java.lang.String of setter method should be "
                 + "com.google.common.collect.ImmutableList<java.lang.String> to match getter "
                 + "foo.bar.Baz.blam, or it should be a type that can be passed to "
                 + "ImmutableList.copyOf")
@@ -1636,7 +1636,7 @@ public class AutoValueCompilationTest {
             .compile(javaFileObject);
     assertThat(compilation)
         .hadErrorContaining(
-            "Parameter type of setter method should be "
+            "Parameter type java.util.Collection<java.lang.Integer> of setter method should be "
                 + "com.google.common.collect.ImmutableList<java.lang.String> to match getter "
                 + "foo.bar.Baz.blam, or it should be a type that can be passed to "
                 + "ImmutableList.copyOf to produce "