Restructure PropertyBuilderClassifier to use more general types.

RELNOTES=n/a
PiperOrigin-RevId: 363994601

4 files changed, 59 insertions, 55 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
index 08eba48d..0f550728 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
@@ -38,6 +38,7 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
+import java.util.stream.Stream;
 import javax.annotation.processing.ProcessingEnvironment;
 import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.Modifier;
@@ -65,7 +66,7 @@ class BuilderMethodClassifier {
   private final TypeElement autoValueClass;
   private final TypeElement builderType;
   private final ImmutableBiMap<ExecutableElement, String> getterToPropertyName;
-  private final ImmutableMap<ExecutableElement, TypeMirror> getterToPropertyType;
+  private final ImmutableMap<String, TypeMirror> propertyTypes;
   private final ImmutableMap<String, ExecutableElement> getterNameToGetter;
 
   private final Set<ExecutableElement> buildMethods = new LinkedHashSet<>();
@@ -85,14 +86,14 @@ class BuilderMethodClassifier {
       TypeElement autoValueClass,
       TypeElement builderType,
       ImmutableBiMap<ExecutableElement, String> getterToPropertyName,
-      ImmutableMap<ExecutableElement, TypeMirror> getterToPropertyType) {
+      ImmutableMap<String, TypeMirror> propertyTypes) {
     this.errorReporter = errorReporter;
     this.typeUtils = processingEnv.getTypeUtils();
     this.elementUtils = processingEnv.getElementUtils();
     this.autoValueClass = autoValueClass;
     this.builderType = builderType;
     this.getterToPropertyName = getterToPropertyName;
-    this.getterToPropertyType = getterToPropertyType;
+    this.propertyTypes = propertyTypes;
     ImmutableMap.Builder<String, ExecutableElement> getterToPropertyNameBuilder =
         ImmutableMap.builder();
     for (ExecutableElement getter : getterToPropertyName.keySet()) {
@@ -111,9 +112,9 @@ class BuilderMethodClassifier {
    * @param autoValueClass the {@code AutoValue} class containing the builder.
    * @param builderType the builder class or interface within {@code autoValueClass}.
    * @param getterToPropertyName a map from getter methods to the properties they get.
-   * @param getterToPropertyType a map from getter methods to their return types. The return types
-   *     here use type parameters from the builder class (if any) rather than from the {@code
-   *     AutoValue} class, even though the getter methods are in the latter.
+   * @param propertyTypes a map from property names to types. The types here use type parameters
+   *     from the builder class (if any) rather than from the {@code AutoValue} class, even though
+   *     the getter methods are in the latter.
    * @param autoValueHasToBuilder true if the containing {@code @AutoValue} class has a {@code
    *     toBuilder()} method.
    * @return an {@code Optional} that contains the results of the classification if it was
@@ -126,7 +127,7 @@ class BuilderMethodClassifier {
       TypeElement autoValueClass,
       TypeElement builderType,
       ImmutableBiMap<ExecutableElement, String> getterToPropertyName,
-      ImmutableMap<ExecutableElement, TypeMirror> getterToPropertyType,
+      ImmutableMap<String, TypeMirror> propertyTypes,
       boolean autoValueHasToBuilder) {
     BuilderMethodClassifier classifier =
         new BuilderMethodClassifier(
@@ -135,7 +136,7 @@ class BuilderMethodClassifier {
             autoValueClass,
             builderType,
             getterToPropertyName,
-            getterToPropertyType);
+            propertyTypes);
     if (classifier.classifyMethods(methods, autoValueHasToBuilder)) {
       return Optional.of(classifier);
     } else {
@@ -147,8 +148,8 @@ class BuilderMethodClassifier {
    * Returns a multimap from the name of a property to the methods that set it. If the property is
    * defined by an abstract method in the {@code @AutoValue} class called {@code foo()} or {@code
    * getFoo()} then the name of the property is {@code foo} and there will be an entry in the map
-   * where the key is {@code "foo"} and the value describes a method in the builder called
-   * {@code foo} or {@code setFoo}.
+   * where the key is {@code "foo"} and the value describes a method in the builder called {@code
+   * foo} or {@code setFoo}.
    */
   ImmutableMultimap<String, PropertySetter> propertyNameToSetters() {
     return ImmutableMultimap.copyOf(
@@ -203,7 +204,7 @@ class BuilderMethodClassifier {
     }
     getterToPropertyName.forEach(
         (getter, property) -> {
-          TypeMirror propertyType = getterToPropertyType.get(getter);
+          TypeMirror propertyType = propertyTypes.get(property);
           boolean hasSetter = propertyNameToSetter.containsKey(property);
           PropertyBuilder propertyBuilder = propertyNameToPropertyBuilder.get(property);
           boolean hasBuilder = propertyBuilder != null;
@@ -286,8 +287,8 @@ class BuilderMethodClassifier {
                 typeUtils,
                 elementUtils,
                 this,
-                getterToPropertyName,
-                getterToPropertyType,
+                this::propertyIsNullable,
+                propertyTypes,
                 eclipseHack);
         Optional<PropertyBuilder> propertyBuilder =
             propertyBuilderClassifier.makePropertyBuilder(method, property);
@@ -313,7 +314,7 @@ class BuilderMethodClassifier {
 
   private void classifyGetter(ExecutableElement builderGetter, ExecutableElement originalGetter) {
     String propertyName = getterToPropertyName.get(originalGetter);
-    TypeMirror originalGetterType = getterToPropertyType.get(originalGetter);
+    TypeMirror originalGetterType = propertyTypes.get(propertyName);
     TypeMirror builderGetterType = builderMethodReturnType(builderGetter);
     String builderGetterTypeString = TypeEncoder.encodeWithAnnotations(builderGetterType);
     if (TYPE_EQUIVALENCE.equivalent(builderGetterType, originalGetterType)) {
@@ -449,8 +450,8 @@ class BuilderMethodClassifier {
             typeUtils,
             elementUtils,
             this,
-            getterToPropertyName,
-            getterToPropertyType,
+            this::propertyIsNullable,
+            propertyTypes,
             eclipseHack);
     Optional<PropertyBuilder> maybePropertyBuilder =
         propertyBuilderClassifier.makePropertyBuilder(method, property);
@@ -491,7 +492,7 @@ class BuilderMethodClassifier {
     VariableElement parameterElement = Iterables.getOnlyElement(setter.getParameters());
     boolean nullableParameter =
         nullableAnnotationFor(parameterElement, parameterElement.asType()).isPresent();
-    TypeMirror targetType = getterToPropertyType.get(valueGetter);
+    TypeMirror targetType = propertyTypes.get(getterToPropertyName.get(valueGetter));
     ExecutableType finalSetter =
         MoreTypes.asExecutable(
             typeUtils.asMemberOf(MoreTypes.asDeclared(builderType.asType()), setter));
@@ -525,7 +526,10 @@ class BuilderMethodClassifier {
     errorReporter.reportError(
         setter,
         "[AutoValueGetVsSet] Parameter type %s of setter method should be %s to match getter %s.%s",
-        parameterType, targetType, autoValueClass, valueGetter.getSimpleName());
+        parameterType,
+        targetType,
+        autoValueClass,
+        valueGetter.getSimpleName());
     return Optional.empty();
   }
 
@@ -539,7 +543,8 @@ class BuilderMethodClassifier {
       ExecutableElement valueGetter,
       ExecutableElement setter,
       TypeMirror parameterType) {
-    DeclaredType targetType = MoreTypes.asDeclared(getterToPropertyType.get(valueGetter));
+    DeclaredType targetType =
+        MoreTypes.asDeclared(propertyTypes.get(getterToPropertyName.get(valueGetter)));
     for (ExecutableElement copyOfMethod : copyOfMethods) {
       Optional<Copier> function =
           getConvertingSetterFunction(copyOfMethod, targetType, parameterType);
@@ -693,4 +698,16 @@ class BuilderMethodClassifier {
   private String typeParamsString() {
     return TypeSimplifier.actualTypeParametersString(autoValueClass);
   }
+
+  /**
+   * True if the given property is nullable, either because its type has a {@code @Nullable} type
+   * annotation, or because its getter method has a {@code @Nullable} method annotation.
+   */
+  private boolean propertyIsNullable(String property) {
+    ExecutableElement getter = getterToPropertyName.inverse().get(property);
+    return Stream.of(getter, getter.getReturnType())
+        .flatMap(ac -> ac.getAnnotationMirrors().stream())
+        .map(a -> a.getAnnotationType().asElement().getSimpleName())
+        .anyMatch(n -> n.contentEquals("Nullable"));
+  }
 }
diff --git a/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java b/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java
index 5d4e3721..2021bd84 100644
--- a/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java
+++ b/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java
@@ -260,7 +260,8 @@ class BuilderSpec {
     void defineVars(
         AutoValueTemplateVars vars,
         ImmutableBiMap<ExecutableElement, String> getterToPropertyName) {
-      Iterable<ExecutableElement> builderMethods = abstractMethods(builderTypeElement);
+      Iterable<ExecutableElement> builderMethods =
+          abstractMethods(builderTypeElement, processingEnv);
       boolean autoValueHasToBuilder = !toBuilderMethods.isEmpty();
       ImmutableMap<ExecutableElement, TypeMirror> getterToPropertyType =
           TypeVariables.rewriteReturnTypes(
@@ -269,6 +270,9 @@ class BuilderSpec {
               getterToPropertyName.keySet(),
               autoValueClass,
               builderTypeElement);
+      ImmutableMap.Builder<String, TypeMirror> propertyTypes = ImmutableMap.builder();
+      getterToPropertyType.forEach(
+          (getter, type) -> propertyTypes.put(getterToPropertyName.get(getter), type));
       Optional<BuilderMethodClassifier> optionalClassifier =
           BuilderMethodClassifier.classify(
               builderMethods,
@@ -277,7 +281,7 @@ class BuilderSpec {
               autoValueClass,
               builderTypeElement,
               getterToPropertyName,
-              getterToPropertyType,
+              propertyTypes.build(),
               autoValueHasToBuilder);
       if (!optionalClassifier.isPresent()) {
         return;
@@ -532,7 +536,8 @@ class BuilderSpec {
    * then this method will throw an exception that will cause us to defer processing of the current
    * class until a later annotation-processing round.
    */
-  private ImmutableSet<ExecutableElement> abstractMethods(TypeElement typeElement) {
+  static ImmutableSet<ExecutableElement> abstractMethods(
+      TypeElement typeElement, ProcessingEnvironment processingEnv) {
     Set<ExecutableElement> methods =
         getLocalAndInheritedMethods(
             typeElement, processingEnv.getTypeUtils(), processingEnv.getElementUtils());
diff --git a/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java b/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java
index 384d730a..61af680b 100644
--- a/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java
+++ b/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java
@@ -21,16 +21,13 @@ import static java.util.stream.Collectors.toMap;
 
 import com.google.auto.common.MoreElements;
 import com.google.auto.common.MoreTypes;
-import com.google.common.collect.ImmutableBiMap;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Function;
-import javax.lang.model.element.AnnotationMirror;
+import java.util.function.Predicate;
 import javax.lang.model.element.ElementKind;
 import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.Modifier;
@@ -57,8 +54,8 @@ class PropertyBuilderClassifier {
   private final Types typeUtils;
   private final Elements elementUtils;
   private final BuilderMethodClassifier builderMethodClassifier;
-  private final ImmutableBiMap<ExecutableElement, String> getterToPropertyName;
-  private final ImmutableMap<ExecutableElement, TypeMirror> getterToPropertyType;
+  private final Predicate<String> propertyIsNullable;
+  private final ImmutableMap<String, TypeMirror> propertyTypes;
   private final EclipseHack eclipseHack;
 
   PropertyBuilderClassifier(
@@ -66,15 +63,15 @@ class PropertyBuilderClassifier {
       Types typeUtils,
       Elements elementUtils,
       BuilderMethodClassifier builderMethodClassifier,
-      ImmutableBiMap<ExecutableElement, String> getterToPropertyName,
-      ImmutableMap<ExecutableElement, TypeMirror> getterToPropertyType,
+      Predicate<String> propertyIsNullable,
+      ImmutableMap<String, TypeMirror> propertyTypes,
       EclipseHack eclipseHack) {
     this.errorReporter = errorReporter;
     this.typeUtils = typeUtils;
     this.elementUtils = elementUtils;
     this.builderMethodClassifier = builderMethodClassifier;
-    this.getterToPropertyName = getterToPropertyName;
-    this.getterToPropertyType = getterToPropertyType;
+    this.propertyIsNullable = propertyIsNullable;
+    this.propertyTypes = propertyTypes;
     this.eclipseHack = eclipseHack;
   }
 
@@ -219,8 +216,7 @@ class PropertyBuilderClassifier {
     TypeElement barBuilderTypeElement = MoreTypes.asTypeElement(barBuilderTypeMirror);
     Map<String, ExecutableElement> barBuilderNoArgMethods = noArgMethodsOf(barBuilderTypeElement);
 
-    ExecutableElement barGetter = getterToPropertyName.inverse().get(property);
-    TypeMirror barTypeMirror = getterToPropertyType.get(barGetter);
+    TypeMirror barTypeMirror = propertyTypes.get(property);
     if (barTypeMirror.getKind() != TypeKind.DECLARED) {
       errorReporter.reportError(
           method,
@@ -229,10 +225,10 @@ class PropertyBuilderClassifier {
           property);
       return Optional.empty();
     }
-    if (isNullable(barGetter)) {
+    if (propertyIsNullable.test(property)) {
       errorReporter.reportError(
-          barGetter,
-          "[AutoValueNullBuilder] Property %s has a property builder so it cannot be @Nullable",
+          method,
+          "[AutoValueNullBuilder] Property %s is @Nullable so it cannot have a property builder",
           property);
     }
     TypeElement barTypeElement = MoreTypes.asTypeElement(barTypeMirror);
@@ -455,18 +451,4 @@ class PropertyBuilderClassifier {
             })
         .findFirst();
   }
-
-  private static boolean isNullable(ExecutableElement getter) {
-    List<List<? extends AnnotationMirror>> annotationLists =
-        ImmutableList.of(
-            getter.getAnnotationMirrors(), getter.getReturnType().getAnnotationMirrors());
-    for (List<? extends AnnotationMirror> annotations : annotationLists) {
-      for (AnnotationMirror annotation : annotations) {
-        if (annotation.getAnnotationType().asElement().getSimpleName().contentEquals("Nullable")) {
-          return true;
-        }
-      }
-    }
-    return false;
-  }
 }
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 e1ff5372..8d770614 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
@@ -1929,9 +1929,9 @@ public class AutoValueCompilationTest {
             .withProcessors(new AutoValueProcessor(), new AutoValueBuilderProcessor())
             .compile(javaFileObject);
     assertThat(compilation)
-        .hadErrorContaining("Property strings has a property builder so it cannot be @Nullable")
+        .hadErrorContaining("Property strings is @Nullable so it cannot have a property builder")
         .inFile(javaFileObject)
-        .onLineContaining("@Nullable ImmutableList<String> strings()");
+        .onLineContaining("stringsBuilder()");
   }
 
   @Test
@@ -1963,9 +1963,9 @@ public class AutoValueCompilationTest {
             .withProcessors(new AutoValueProcessor(), new AutoValueBuilderProcessor())
             .compile(javaFileObject);
     assertThat(compilation)
-        .hadErrorContaining("Property strings has a property builder so it cannot be @Nullable")
+        .hadErrorContaining("Property strings is @Nullable so it cannot have a property builder")
         .inFile(javaFileObject)
-        .onLineContaining("@Nullable ImmutableList<String> strings()");
+        .onLineContaining("stringsBuilder()");
   }
 
   @Test