Ensure that type annotations are placed correctly.

Treating them the same as other annotations leads to problems with nested types like `Map.Entry`. If a field or parameter is a `Map.Entry` that is `@Nullable`, and if `@Nullable` is a `TYPE_USE` annotation, then the declaration must use `Map.@Nullable Entry`. JavaPoet will do this correctly if the annotations are applied to the `TypeName` for the declaration.

Also ensure that type annotations are included in the `T` of `Provider<T>` when that appears in field or parameter declarations.

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

RELNOTES=AutoFactory type annotations are now placed correctly even for nested types.
PiperOrigin-RevId: 350930373

4 files changed, 78 insertions, 20 deletions

diff --git a/factory/src/main/java/com/google/auto/factory/processor/FactoryWriter.java b/factory/src/main/java/com/google/auto/factory/processor/FactoryWriter.java
index 3dbcf41c..32373915 100644
--- a/factory/src/main/java/com/google/auto/factory/processor/FactoryWriter.java
+++ b/factory/src/main/java/com/google/auto/factory/processor/FactoryWriter.java
@@ -26,6 +26,8 @@ import static javax.lang.model.element.Modifier.PRIVATE;
 import static javax.lang.model.element.Modifier.PUBLIC;
 import static javax.lang.model.element.Modifier.STATIC;
 
+import com.google.auto.common.AnnotationMirrors;
+import com.google.auto.common.AnnotationValues;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSetMultimap;
@@ -43,13 +45,19 @@ import com.squareup.javapoet.TypeName;
 import com.squareup.javapoet.TypeSpec;
 import com.squareup.javapoet.TypeVariableName;
 import java.io.IOException;
+import java.lang.annotation.Target;
 import java.util.Iterator;
+import java.util.List;
+import java.util.Optional;
 import java.util.stream.Stream;
 import javax.annotation.processing.Filer;
 import javax.annotation.processing.ProcessingEnvironment;
 import javax.inject.Inject;
 import javax.inject.Provider;
 import javax.lang.model.SourceVersion;
+import javax.lang.model.element.AnnotationMirror;
+import javax.lang.model.element.Element;
+import javax.lang.model.element.VariableElement;
 import javax.lang.model.type.TypeKind;
 import javax.lang.model.type.TypeMirror;
 import javax.lang.model.type.TypeVariable;
@@ -143,7 +151,7 @@ final class FactoryWriter {
       ImmutableSet<TypeVariableName> factoryTypeVariables) {
     for (FactoryMethodDescriptor methodDescriptor : descriptor.methodDescriptors()) {
       MethodSpec.Builder method =
-          MethodSpec.methodBuilder(methodDescriptor.name())
+          methodBuilder(methodDescriptor.name())
               .addTypeVariables(getMethodTypeVariables(methodDescriptor, factoryTypeVariables))
               .returns(TypeName.get(methodDescriptor.returnType()))
               .varargs(methodDescriptor.isVarArgs());
@@ -219,17 +227,45 @@ final class FactoryWriter {
   private ImmutableList<ParameterSpec> parameters(Iterable<Parameter> parameters) {
     ImmutableList.Builder<ParameterSpec> builder = ImmutableList.builder();
     for (Parameter parameter : parameters) {
-      ParameterSpec.Builder parameterBuilder =
-          ParameterSpec.builder(resolveTypeName(parameter.type().get()), parameter.name());
-      Stream.of(parameter.nullable(), parameter.key().qualifier())
-          .flatMap(Streams::stream)
-          .map(AnnotationSpec::get)
-          .forEach(parameterBuilder::addAnnotation);
-      builder.add(parameterBuilder.build());
+      TypeName type = resolveTypeName(parameter.type().get());
+      // Remove TYPE_USE annotations, since resolveTypeName will already have included those in
+      // the TypeName it returns.
+      List<AnnotationSpec> annotations =
+          Stream.of(parameter.nullable(), parameter.key().qualifier())
+              .flatMap(Streams::stream)
+              .filter(a -> !isTypeUseAnnotation(a))
+              .map(AnnotationSpec::get)
+              .collect(toList());
+      ParameterSpec parameterSpec =
+          ParameterSpec.builder(type, parameter.name())
+              .addAnnotations(annotations)
+              .build();
+      builder.add(parameterSpec);
     }
     return builder.build();
   }
 
+  private static boolean isTypeUseAnnotation(AnnotationMirror mirror) {
+    Element annotationElement = mirror.getAnnotationType().asElement();
+    // This is basically equivalent to:
+    //    Target target = annotationElement.getAnnotation(Target.class);
+    //    return target != null
+    //        && Arrays.asList(annotationElement.getAnnotation(Target.class)).contains(TYPE_USE);
+    // but that might blow up if the annotation is being compiled at the same time and has an
+    // undefined identifier in its @Target values. The rigmarole below avoids that problem.
+    Optional<AnnotationMirror> maybeTargetMirror =
+        Mirrors.getAnnotationMirror(annotationElement, Target.class);
+    return maybeTargetMirror
+        .map(
+            targetMirror ->
+                AnnotationValues.getEnums(
+                        AnnotationMirrors.getAnnotationValue(targetMirror, "value"))
+                    .stream()
+                    .map(VariableElement::getSimpleName)
+                    .anyMatch(name -> name.contentEquals("TYPE_USE")))
+        .orElse(false);
+  }
+
   private static void addCheckNotNullMethod(
       TypeSpec.Builder factory, FactoryDescriptor descriptor) {
     if (shouldGenerateCheckNotNull(descriptor)) {
@@ -284,17 +320,20 @@ final class FactoryWriter {
    * {@code @AutoFactory class Foo} has a constructor parameter of type {@code BarFactory} and
    * {@code @AutoFactory class Bar} has a constructor parameter of type {@code FooFactory}. We did
    * in fact find instances of this in Google's source base.
+   *
+   * <p>If the type has type annotations then include those in the returned {@link TypeName}.
    */
   private TypeName resolveTypeName(TypeMirror type) {
-    if (type.getKind() != TypeKind.ERROR) {
-      return TypeName.get(type);
-    }
-    ImmutableSet<PackageAndClass> factoryNames = factoriesBeingCreated.get(type.toString());
-    if (factoryNames.size() == 1) {
-      PackageAndClass packageAndClass = Iterables.getOnlyElement(factoryNames);
-      return ClassName.get(packageAndClass.packageName(), packageAndClass.className());
+    TypeName typeName = TypeName.get(type);
+    if (type.getKind() == TypeKind.ERROR) {
+      ImmutableSet<PackageAndClass> factoryNames = factoriesBeingCreated.get(type.toString());
+      if (factoryNames.size() == 1) {
+        PackageAndClass packageAndClass = Iterables.getOnlyElement(factoryNames);
+        typeName = ClassName.get(packageAndClass.packageName(), packageAndClass.className());
+      }
     }
-    return TypeName.get(type);
+    return typeName.annotated(
+        type.getAnnotationMirrors().stream().map(AnnotationSpec::get).collect(toList()));
   }
 
   private static ImmutableSet<TypeVariableName> getFactoryTypeVariables(
diff --git a/factory/src/test/java/com/google/auto/factory/processor/AutoFactoryProcessorTest.java b/factory/src/test/java/com/google/auto/factory/processor/AutoFactoryProcessorTest.java
index 9593139a..49a5357e 100644
--- a/factory/src/test/java/com/google/auto/factory/processor/AutoFactoryProcessorTest.java
+++ b/factory/src/test/java/com/google/auto/factory/processor/AutoFactoryProcessorTest.java
@@ -15,6 +15,8 @@
  */
 package com.google.auto.factory.processor;
 
+import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION;
+import static com.google.common.truth.TruthJUnit.assume;
 import static com.google.testing.compile.CompilationSubject.assertThat;
 import static java.lang.Math.max;
 import static java.lang.Math.min;
@@ -427,6 +429,11 @@ public class AutoFactoryProcessorTest {
 
   @Test
   public void checkerFrameworkNullableType() {
+    // TYPE_USE annotations are pretty much unusable with annotation processors on Java 8 because
+    // of bugs that mean they only appear in the javax.lang.model API when the compiler feels like
+    // it. Checking for a java.specification.version that does not start with "1." eliminates 8 and
+    // any earlier version.
+    assume().that(JAVA_SPECIFICATION_VERSION.value()).doesNotMatch("1\\..*");
     Compilation compilation =
         javac.compile(JavaFileObjects.forResource("good/CheckerFrameworkNullable.java"));
     assertThat(compilation).succeededWithoutWarnings();
diff --git a/factory/src/test/resources/expected/CheckerFrameworkNullableFactory.java b/factory/src/test/resources/expected/CheckerFrameworkNullableFactory.java
index 79175c7e..8e4b1ea4 100644
--- a/factory/src/test/resources/expected/CheckerFrameworkNullableFactory.java
+++ b/factory/src/test/resources/expected/CheckerFrameworkNullableFactory.java
@@ -15,6 +15,7 @@
  */
 package tests;
 
+import java.util.Map;
 import javax.annotation.processing.Generated;
 import javax.inject.Inject;
 import javax.inject.Provider;
@@ -29,19 +30,27 @@ final class CheckerFrameworkNullableFactory {
 
   private final Provider<String> java_lang_StringProvider;
 
+  private final Provider<Map.@NullableType Entry<?, ?>> providedNestedNullableTypeProvider;
+
   @Inject
   CheckerFrameworkNullableFactory(
-      Provider<String> java_lang_StringProvider) {
+      Provider<String> java_lang_StringProvider,
+      Provider<Map.@NullableType Entry<?, ?>> providedNestedNullableTypeProvider) {
     this.java_lang_StringProvider = checkNotNull(java_lang_StringProvider, 1);
+    this.providedNestedNullableTypeProvider = checkNotNull(providedNestedNullableTypeProvider, 2);
   }
 
   CheckerFrameworkNullable create(
-      @NullableDecl String nullableDecl, @NullableType String nullableType) {
+      @NullableDecl String nullableDecl,
+      @NullableType String nullableType,
+      Map.@NullableType Entry<?, ?> nestedNullableType) {
     return new CheckerFrameworkNullable(
         nullableDecl,
         java_lang_StringProvider.get(),
         nullableType,
-        java_lang_StringProvider.get());
+        java_lang_StringProvider.get(),
+        nestedNullableType,
+        providedNestedNullableTypeProvider.get());
   }
 
   private static <T> T checkNotNull(T reference, int argumentIndex) {
diff --git a/factory/src/test/resources/good/CheckerFrameworkNullable.java b/factory/src/test/resources/good/CheckerFrameworkNullable.java
index 7f2a0fee..8b9cbc26 100644
--- a/factory/src/test/resources/good/CheckerFrameworkNullable.java
+++ b/factory/src/test/resources/good/CheckerFrameworkNullable.java
@@ -17,6 +17,7 @@ package tests;
 
 import com.google.auto.factory.AutoFactory;
 import com.google.auto.factory.Provided;
+import java.util.Map;
 import org.checkerframework.checker.nullness.compatqual.NullableDecl;
 import org.checkerframework.checker.nullness.compatqual.NullableType;
 
@@ -27,5 +28,7 @@ final class CheckerFrameworkNullable {
       @NullableDecl String nullableDecl,
       @Provided @NullableDecl String providedNullableDecl,
       @NullableType String nullableType,
-      @Provided @NullableType String providedNullableType) {}
+      @Provided @NullableType String providedNullableType,
+      Map.@NullableType Entry<?, ?> nestedNullableType,
+      @Provided Map.@NullableType Entry<?, ?> providedNestedNullableType) {}
 }