Improve the logic for checking setter parameter types against getter return types.

We translate the getter return types into the type context of the builder. For example, if the @autovalue class is Foo<T> then its builder must be Foo.Builder<T>. A getter like `abstract List<T> getList()` is compatible with a setter like `Builder setList(List<T> list)`, even though the <T> parameter in each case is different. So we convert `List<T-{Foo}> getList()` into `List<T{Foo.Builder}> getList()` before comparing types.

RELNOTES=Better checking of types in setters against types in getters.

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

Author: eamonnmcmanus

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

@@ -735,4 +735,39 @@ public void testTypeAnnotationOnBuilderCopiedToImplementation() {
         .asList()
         .contains(nullable());
   }
+
+  // b/127701294
+  @AutoValue
+  abstract static class OptionalOptional {
+    abstract Optional<Optional<String>> maybeJustMaybe();
+
+    static Builder builder() {
+      return new AutoValue_AutoValueJava8Test_OptionalOptional.Builder();
+    }
+
+    @AutoValue.Builder
+    abstract static class Builder {
+      abstract Builder maybeJustMaybe(Optional<String> maybe);
+      abstract OptionalOptional build();
+    }
+  }
+
+  @Test
+  public void testOptionalOptional_empty() {
+    OptionalOptional empty = OptionalOptional.builder().build();
+    assertThat(empty.maybeJustMaybe()).isEmpty();
+  }
+
+  @Test
+  public void testOptionalOptional_ofEmpty() {
+    OptionalOptional ofEmpty = OptionalOptional.builder().maybeJustMaybe(Optional.empty()).build();
+    assertThat(ofEmpty.maybeJustMaybe()).hasValue(Optional.empty());
+  }
+
+  @Test
+  public void testOptionalOptional_ofSomething() {
+    OptionalOptional ofSomething =
+        OptionalOptional.builder().maybeJustMaybe(Optional.of("foo")).build();
+    assertThat(ofSomething.maybeJustMaybe()).hasValue(Optional.of("foo"));
+  }
 }

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

@@ -3048,4 +3048,27 @@ public void testRedeclareJavaLangClasses() {
             .build();
     assertThat(x).isNotNull();
   }
+
+  // b/28382293
+  @AutoValue
+  abstract static class GenericExtends {
+    abstract ImmutableSet<Number> metrics();
+
+    static Builder builder() {
+      return new AutoValue_AutoValueTest_GenericExtends.Builder();
+    }
+
+    @AutoValue.Builder
+    abstract static class Builder {
+      abstract Builder setMetrics(ImmutableSet<? extends Number> metrics);
+      abstract GenericExtends build();
+    }
+  }
+
+  @Test
+  public void testGenericExtends() {
+    ImmutableSet<Integer> ints = ImmutableSet.of(1, 2, 3);
+    GenericExtends g = GenericExtends.builder().setMetrics(ints).build();
+    assertThat(g.metrics()).isEqualTo(ints);
+  }
 }

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

@@ -415,8 +415,6 @@ private void defineVarsForType(
     // We can't use ImmutableList.toImmutableList() for obscure Google-internal reasons.
     vars.toBuilderMethods =
         ImmutableList.copyOf(toBuilderMethods.stream().map(SimpleMethod::new).collect(toList()));
-    ImmutableBiMap<ExecutableElement, String> methodToPropertyName =
-        propertyNameToMethodMap(propertyMethods).inverse();
     ImmutableListMultimap<ExecutableElement, AnnotationMirror> annotatedPropertyFields =
         propertyFieldAnnotationMap(type, propertyMethods);
     ImmutableListMultimap<ExecutableElement, AnnotationMirror> annotatedPropertyMethods =
@@ -426,6 +424,8 @@ private void defineVarsForType(
     vars.serialVersionUID = getSerialVersionUID(type);
     // Check for @AutoValue.Builder and add appropriate variables if it is present.
     if (builder.isPresent()) {
+      ImmutableBiMap<ExecutableElement, String> methodToPropertyName =
+          propertyNameToMethodMap(propertyMethods).inverse();
       builder.get().defineVars(vars, methodToPropertyName);
     }
   }

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

@@ -30,7 +30,6 @@
 import com.google.common.collect.Iterables;
 import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import javax.annotation.processing.ProcessingEnvironment;
@@ -42,6 +41,7 @@
 import javax.lang.model.element.TypeParameterElement;
 import javax.lang.model.element.VariableElement;
 import javax.lang.model.type.DeclaredType;
+import javax.lang.model.type.ExecutableType;
 import javax.lang.model.type.TypeKind;
 import javax.lang.model.type.TypeMirror;
 import javax.lang.model.util.ElementFilter;
@@ -173,6 +173,13 @@ void defineVars(
         ImmutableBiMap<ExecutableElement, String> getterToPropertyName) {
       Iterable<ExecutableElement> builderMethods = abstractMethods(builderTypeElement);
       boolean autoValueHasToBuilder = !toBuilderMethods.isEmpty();
+      ImmutableMap<ExecutableElement, TypeMirror> getterToPropertyType =
+          TypeVariables.rewriteReturnTypes(
+              processingEnv.getElementUtils(),
+              processingEnv.getTypeUtils(),
+              getterToPropertyName.keySet(),
+              autoValueClass,
+              builderTypeElement);
       Optional<BuilderMethodClassifier> optionalClassifier =
           BuilderMethodClassifier.classify(
               builderMethods,
@@ -181,6 +188,7 @@ void defineVars(
               autoValueClass,
               builderTypeElement,
               getterToPropertyName,
+              getterToPropertyType,
               autoValueHasToBuilder);
       if (!optionalClassifier.isPresent()) {
         return;
@@ -207,15 +215,21 @@ void defineVars(
       vars.buildMethod = Optional.of(new SimpleMethod(buildMethod));
       vars.builderGetters = classifier.builderGetters();
 
+      DeclaredType builderTypeMirror = MoreTypes.asDeclared(builderTypeElement.asType());
       ImmutableMultimap.Builder<String, PropertySetter> setterBuilder = ImmutableMultimap.builder();
-      for (Map.Entry<String, ExecutableElement> entry :
-          classifier.propertyNameToSetters().entries()) {
-        String property = entry.getKey();
-        ExecutableElement setter = entry.getValue();
-        TypeMirror propertyType = getterToPropertyName.inverse().get(property).getReturnType();
-        setterBuilder.put(
-            property, new PropertySetter(setter, propertyType, processingEnv.getTypeUtils()));
-      }
+      classifier.propertyNameToSetters().forEach(
+          (property, setter) -> {
+        ExecutableElement getter = getterToPropertyName.inverse().get(property);
+        // Get the effective parameter type, which might need asMemberOf in cases where the method
+        // is inherited from a generic ancestor type and references type parameters.
+        ExecutableType setterMirror = MoreTypes.asExecutable(
+            processingEnv.getTypeUtils().asMemberOf(builderTypeMirror, setter));
+        TypeMirror parameterType = Iterables.getOnlyElement(setterMirror.getParameterTypes());
+        TypeMirror propertyType = getterToPropertyType.get(getter);
+        PropertySetter propertySetter = new PropertySetter(
+            setter, parameterType, propertyType, processingEnv.getTypeUtils());
+        setterBuilder.put(property, propertySetter);
+      });
       vars.builderSetters = setterBuilder.build();
 
       vars.builderPropertyBuilders =
@@ -297,13 +311,16 @@ public static class PropertySetter {
     private final String nullableAnnotation;
     private final String copyOf;
 
-    PropertySetter(ExecutableElement setter, TypeMirror propertyType, Types typeUtils) {
+    PropertySetter(
+        ExecutableElement setter,
+        TypeMirror parameterType,
+        TypeMirror propertyType,
+        Types typeUtils) {
       this.access = SimpleMethod.access(setter);
       this.name = setter.getSimpleName().toString();
-      VariableElement parameterElement = Iterables.getOnlyElement(setter.getParameters());
-      TypeMirror parameterType = parameterElement.asType();
       primitiveParameter = parameterType.getKind().isPrimitive();
       this.parameterTypeString = parameterTypeString(setter, parameterType);
+      VariableElement parameterElement = Iterables.getOnlyElement(setter.getParameters());
       Optional<String> maybeNullable =
           AutoValueOrOneOfProcessor.nullableAnnotationFor(parameterElement, parameterType);
       this.nullableAnnotation = maybeNullable.orElse("");
@@ -325,13 +342,13 @@ private static String parameterTypeString(ExecutableElement setter, TypeMirror p
       }
     }
 
-    private static String copyOfString(
+    private String copyOfString(
         TypeMirror propertyType, TypeMirror parameterType, Types typeUtils, boolean nullable) {
-      TypeMirror erasedPropertyType = typeUtils.erasure(propertyType);
-      boolean sameType = typeUtils.isSameType(typeUtils.erasure(parameterType), erasedPropertyType);
+      boolean sameType = typeUtils.isAssignable(parameterType, propertyType);
       if (sameType) {
         return null;
       }
+      TypeMirror erasedPropertyType = typeUtils.erasure(propertyType);
       String rawTarget = TypeEncoder.encodeRaw(erasedPropertyType);
       String of;
       Optionalish optionalish = Optionalish.createIfOptional(propertyType);

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

@@ -40,14 +40,19 @@
  * @author Éamonn McManus
  */
 class EclipseHack {
-  private final ProcessingEnvironment processingEnv;
+  private final Elements elementUtils;
+  private final Types typeUtils;
 
   EclipseHack(ProcessingEnvironment processingEnv) {
-    this.processingEnv = processingEnv;
+    this(processingEnv.getElementUtils(), processingEnv.getTypeUtils());
+  }
+
+  EclipseHack(Elements elementUtils, Types typeUtils) {
+    this.elementUtils = elementUtils;
+    this.typeUtils = typeUtils;
   }
 
   TypeMirror methodReturnType(ExecutableElement method, DeclaredType in) {
-    Types typeUtils = processingEnv.getTypeUtils();
     try {
       TypeMirror methodMirror = typeUtils.asMemberOf(in, method);
       return MoreTypes.asExecutable(methodMirror).getReturnType();
@@ -71,7 +76,6 @@ TypeMirror methodReturnType(ExecutableElement method, DeclaredType in) {
    */
   ImmutableMap<ExecutableElement, TypeMirror> methodReturnTypes(
       Set<ExecutableElement> methods, DeclaredType in) {
-    Types typeUtils = processingEnv.getTypeUtils();
     ImmutableMap.Builder<ExecutableElement, TypeMirror> map = ImmutableMap.builder();
     Map<Name, ExecutableElement> noArgMethods = null;
     for (ExecutableElement method : methods) {
@@ -102,8 +106,6 @@ ImmutableMap<ExecutableElement, TypeMirror> methodReturnTypes(
    * does in Eclipse.
    */
   private Map<Name, ExecutableElement> noArgMethodsIn(DeclaredType in) {
-    Types typeUtils = processingEnv.getTypeUtils();
-    Elements elementUtils = processingEnv.getElementUtils();
     TypeElement autoValueType = MoreElements.asType(typeUtils.asElement(in));
     List<ExecutableElement> allMethods =
         ElementFilter.methodsIn(elementUtils.getAllMembers(autoValueType));

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

@@ -23,12 +23,11 @@
 /**
  * Handle error reporting for an annotation processor.
  *
- * @see AutoValue
  * @author Éamonn McManus
  */
 class ErrorReporter {
   private final Messager messager;
-  private boolean anyErrors;
+  private int errorCount;
 
   ErrorReporter(ProcessingEnvironment processingEnv) {
     this.messager = processingEnv.getMessager();
@@ -65,7 +64,7 @@ void reportWarning(String msg, Element e) {
    */
   void reportError(String msg, Element e) {
     messager.printMessage(Diagnostic.Kind.ERROR, msg, e);
-    anyErrors = true;
+    errorCount++;
   }
 
   /**
@@ -80,9 +79,14 @@ void abortWithError(String msg, Element e) {
     throw new AbortProcessingException();
   }
 
+  /** The number of errors that have been output by calls to {@link #reportError}. */
+  int errorCount() {
+    return errorCount;
+  }
+
   /** Abandon the processing of this class if any errors have been output. */
   void abortIfAnyError() {
-    if (anyErrors) {
+    if (errorCount > 0) {
       throw new AbortProcessingException();
     }
   }

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

@@ -0,0 +1,102 @@
+/*
+ * Copyright (C) 2019 Google, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.auto.value.processor;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import java.util.Collection;
+import java.util.List;
+import javax.lang.model.element.ExecutableElement;
+import javax.lang.model.element.TypeElement;
+import javax.lang.model.element.TypeParameterElement;
+import javax.lang.model.type.DeclaredType;
+import javax.lang.model.type.TypeMirror;
+import javax.lang.model.util.Elements;
+import javax.lang.model.util.Types;
+
+/**
+ * Methods for handling type variables.
+ */
+final class TypeVariables {
+  private TypeVariables() {}
+
+  /**
+   * Returns a map from methods to return types, where the return types are not necessarily the
+   * original return types of the methods. Consider this example:
+   *
+   * <pre>
+   * &#64;AutoValue class {@code Foo<T>} {
+   *   abstract T getFoo();
+   *
+   *   &#64;AutoValue.Builder
+   *   abstract class {@code Builder<T>} {
+   *     abstract Builder setFoo(T t);
+   *     abstract {@code Foo<T>} build();
+   *   }
+   * }
+   * </pre>
+   *
+   * We want to be able to check that the parameter type of {@code setFoo} is the same as the
+   * return type of {@code getFoo}. But in fact it isn't, because the {@code T} of {@code Foo<T>}
+   * is not the same as the {@code T} of {@code Foo.Builder<T>}. So we create a parallel
+   * {@code Foo<T>} where the {@code T} <i>is</i> the one from {@code Foo.Builder<T>}. That way the
+   * types do correspond. This method then returns the return types of the given methods as they
+   * appear in that parallel class, meaning the type given for {@code getFoo()} is the {@code T} of
+   * {@code Foo.Builder<T>}.
+   *
+   * <p>We do the rewrite this way around (applying the type parameter from {@code Foo.Builder} to
+   * {@code Foo}) because if we hit one of the historical Eclipse bugs with {@link Types#asMemberOf}
+   * then {@link EclipseHack#methodReturnType} can use fallback logic, which only works for methods
+   * with no arguments.
+   *
+   * @param methods the methods whose return types are to be rewritten.
+   * @param sourceType the class containing those methods ({@code Foo} in the example).
+   * @param targetType the class to translate the methods into ({@code Foo.Builder<T>}) in the
+   *     example.
+   */
+  static ImmutableMap<ExecutableElement, TypeMirror> rewriteReturnTypes(
+      Elements elementUtils,
+      Types typeUtils,
+      Collection<ExecutableElement> methods,
+      TypeElement sourceType,
+      TypeElement targetType) {
+    List<? extends TypeParameterElement> sourceTypeParameters = sourceType.getTypeParameters();
+    List<? extends TypeParameterElement> targetTypeParameters = targetType.getTypeParameters();
+    Preconditions.checkArgument(
+        sourceTypeParameters.toString().equals(targetTypeParameters.toString()),
+        "%s != %s",
+        sourceTypeParameters,
+        targetTypeParameters);
+    // What we're doing is only valid if the type parameters are "the same". The check here even
+    // requires the names to be the same. The logic would still work without that, but we impose
+    // that requirement elsewhere and it means we can check in this simple way.
+
+    if (sourceTypeParameters.isEmpty()) {
+      return methods.stream()
+          .collect(ImmutableMap.toImmutableMap(m -> m, ExecutableElement::getReturnType));
+    }
+    EclipseHack eclipseHack = new EclipseHack(elementUtils, typeUtils);
+    TypeMirror[] targetTypeParameterMirrors = new TypeMirror[targetTypeParameters.size()];
+    for (int i = 0; i < targetTypeParameters.size(); i++) {
+      targetTypeParameterMirrors[i] = targetTypeParameters.get(i).asType();
+    }
+    DeclaredType parallelSource = typeUtils.getDeclaredType(sourceType, targetTypeParameterMirrors);
+    return methods.stream()
+        .collect(
+            ImmutableMap.toImmutableMap(
+                m -> m, m -> eclipseHack.methodReturnType(m, parallelSource)));
+  }
+}