Fix a bug with @Provides/@produces methods in generic modules.

RELNOTES=Fixed a bug with `@Provides`/`@Produces` methods in generic modules.

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

java/dagger/internal/codegen/FactoryGenerator.java

@@ -35,7 +35,6 @@
 import static dagger.internal.codegen.SourceFiles.generatedClassNameForBinding;
 import static dagger.internal.codegen.SourceFiles.parameterizedGeneratedTypeNameForBinding;
 import static dagger.internal.codegen.TypeNames.factoryOf;
-import static dagger.model.BindingKind.INJECTION;
 import static dagger.model.BindingKind.PROVISION;
 import static javax.lang.model.element.Modifier.FINAL;
 import static javax.lang.model.element.Modifier.PRIVATE;
@@ -52,7 +51,6 @@
 import com.squareup.javapoet.ParameterSpec;
 import com.squareup.javapoet.TypeName;
 import com.squareup.javapoet.TypeSpec;
-import com.squareup.javapoet.TypeVariableName;
 import dagger.internal.Factory;
 import dagger.internal.Preconditions;
 import dagger.internal.codegen.InjectionMethods.InjectionSiteMethod;
@@ -114,7 +112,7 @@ private TypeSpec.Builder factoryBuilder(ProvisionBinding binding) {
         classBuilder(nameGeneratedType(binding))
             .addModifiers(PUBLIC, FINAL)
             .addSuperinterface(factoryTypeName(binding))
-            .addTypeVariables(typeParameters(binding));
+            .addTypeVariables(bindingTypeElementTypeVariableNames(binding));
 
     addConstructorAndFields(binding, factoryBuilder);
     factoryBuilder.addMethod(getMethod(binding));
@@ -183,15 +181,15 @@ private void addCreateMethod(ProvisionBinding binding, TypeSpec.Builder factoryB
         methodBuilder("create")
             .addModifiers(PUBLIC, STATIC)
             .returns(parameterizedGeneratedTypeNameForBinding(binding))
-            .addTypeVariables(typeParameters(binding));
+            .addTypeVariables(bindingTypeElementTypeVariableNames(binding));
 
     switch (binding.factoryCreationStrategy()) {
       case SINGLETON_INSTANCE:
         FieldSpec.Builder instanceFieldBuilder =
             FieldSpec.builder(nameGeneratedType(binding), "INSTANCE", PRIVATE, STATIC, FINAL)
                 .initializer("new $T()", nameGeneratedType(binding));
 
-        if (!typeParameters(binding).isEmpty()) {
+        if (!bindingTypeElementTypeVariableNames(binding).isEmpty()) {
           // If the factory has type parameters, ignore them in the field declaration & initializer
           instanceFieldBuilder.addAnnotation(suppressWarnings(RAWTYPES));
 
@@ -277,14 +275,6 @@ private static ParameterSpec toParameter(FieldSpec field) {
     return ParameterSpec.builder(field.type, field.name).build();
   }
 
-  private static ImmutableList<TypeVariableName> typeParameters(ProvisionBinding binding) {
-    // Use type parameters from the injected type or the module instance *only* if we require it.
-    if (binding.kind().equals(INJECTION) || binding.requiresModuleInstance()) {
-      return bindingTypeElementTypeVariableNames(binding);
-    }
-    return ImmutableList.of();
-  }
-
   /**
    * Returns {@code Preconditions.checkNotNull(providesMethodInvocation)} with a message suitable
    * for {@code @Provides} methods.

java/dagger/internal/codegen/MembersInjectionBinding.java

@@ -156,6 +156,8 @@ int indexAmongAtInjectMembersWithSameSimpleName() {
     }
   }
 
+  /* TODO(dpb): Combine ProvisionBinding.Factory, ProductionBinding.Factory, and
+   * MembersInjectionBinding.Factory into one BindingFactory class.*/
   static final class Factory {
     private final Elements elements;
     private final DaggerTypes types;

java/dagger/internal/codegen/ProducerFactoryGenerator.java

@@ -25,6 +25,7 @@
 import static dagger.internal.codegen.CodeBlocks.makeParametersCodeBlock;
 import static dagger.internal.codegen.GwtCompatibility.gwtIncompatibleAnnotation;
 import static dagger.internal.codegen.MapKeys.mapKeyFactoryMethod;
+import static dagger.internal.codegen.SourceFiles.bindingTypeElementTypeVariableNames;
 import static dagger.internal.codegen.SourceFiles.frameworkTypeUsageStatement;
 import static dagger.internal.codegen.SourceFiles.generateBindingFieldsForDependencies;
 import static dagger.internal.codegen.SourceFiles.generatedClassNameForBinding;
@@ -100,6 +101,8 @@ Optional<? extends Element> getElementForErrorReporting(ProductionBinding bindin
 
   @Override
   Optional<TypeSpec.Builder> write(ClassName generatedTypeName, ProductionBinding binding) {
+    // We don't want to write out resolved bindings -- we want to write out the generic version.
+    checkArgument(!binding.unresolved().isPresent());
     checkArgument(binding.bindingElement().isPresent());
 
     TypeName providedTypeName = TypeName.get(binding.contributedType());
@@ -108,7 +111,8 @@ Optional<TypeSpec.Builder> write(ClassName generatedTypeName, ProductionBinding
     TypeSpec.Builder factoryBuilder =
         classBuilder(generatedTypeName)
             .addModifiers(PUBLIC, FINAL)
-            .superclass(abstractProducerOf(providedTypeName));
+            .superclass(abstractProducerOf(providedTypeName))
+            .addTypeVariables(bindingTypeElementTypeVariableNames(binding));
 
     UniqueNameSet uniqueFieldNames = new UniqueNameSet();
     ImmutableMap.Builder<Key, FieldSpec> fieldsBuilder = ImmutableMap.builder();

java/dagger/internal/codegen/ProductionBinding.java

@@ -16,6 +16,7 @@
 
 package dagger.internal.codegen;
 
+import static com.google.auto.common.MoreElements.asType;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static dagger.internal.codegen.DaggerStreams.toImmutableSet;
@@ -63,9 +64,7 @@ public BindingType bindingType() {
   }
 
   @Override
-  Optional<ProductionBinding> unresolved() {
-    return Optional.empty();
-  }
+  abstract Optional<ProductionBinding> unresolved();
 
   @Override
   ImmutableSet<DependencyRequest> implicitDependencies() {
@@ -128,6 +127,8 @@ abstract static class Builder extends ContributionBinding.Builder<Builder> {
 
     abstract Builder productionKind(ProductionKind productionKind);
 
+    abstract Builder unresolved(ProductionBinding unresolved);
+
     abstract Builder thrownTypes(Iterable<? extends TypeMirror> thrownTypes);
 
     abstract Builder executorRequest(DependencyRequest executorRequest);
@@ -138,6 +139,8 @@ abstract static class Builder extends ContributionBinding.Builder<Builder> {
     abstract ProductionBinding build();
   }
 
+  /* TODO(dpb): Combine ProvisionBinding.Factory, ProductionBinding.Factory, and
+   * MembersInjectionBinding.Factory into one BindingFactory class.*/
   static final class Factory {
     private final Types types;
     private final KeyFactory keyFactory;
@@ -155,13 +158,12 @@ ProductionBinding forProducesMethod(
       checkArgument(producesMethod.getKind().equals(METHOD));
       ContributionType contributionType = ContributionType.fromBindingMethod(producesMethod);
       Key key = keyFactory.forProducesMethod(producesMethod, contributedBy);
-      ExecutableType resolvedMethod =
+      ExecutableType methodType =
           MoreTypes.asExecutable(
               types.asMemberOf(MoreTypes.asDeclared(contributedBy.asType()), producesMethod));
       ImmutableSet<DependencyRequest> dependencies =
           dependencyRequestFactory.forRequiredResolvedVariables(
-              producesMethod.getParameters(),
-              resolvedMethod.getParameterTypes());
+              producesMethod.getParameters(), methodType.getParameterTypes());
       DependencyRequest executorRequest =
           dependencyRequestFactory.forProductionImplementationExecutor();
       DependencyRequest monitorRequest = dependencyRequestFactory.forProductionComponentMonitor();
@@ -175,19 +177,24 @@ && isFutureType(SetType.from(producesMethod.getReturnType()).elementType())) {
         productionKind = ProductionKind.IMMEDIATE;
       }
       // TODO(beder): Add nullability checking with Java 8.
-      return ProductionBinding.builder()
-          .contributionType(contributionType)
-          .bindingElement(producesMethod)
-          .contributingModule(contributedBy)
-          .key(key)
-          .explicitDependencies(dependencies)
-          .wrappedMapKey(wrapOptionalInEquivalence(getMapKey(producesMethod)))
-          .kind(PRODUCTION)
-          .productionKind(productionKind)
-          .thrownTypes(producesMethod.getThrownTypes())
-          .executorRequest(executorRequest)
-          .monitorRequest(monitorRequest)
-          .build();
+      ProductionBinding.Builder builder =
+          ProductionBinding.builder()
+              .contributionType(contributionType)
+              .bindingElement(producesMethod)
+              .contributingModule(contributedBy)
+              .key(key)
+              .explicitDependencies(dependencies)
+              .wrappedMapKey(wrapOptionalInEquivalence(getMapKey(producesMethod)))
+              .kind(PRODUCTION)
+              .productionKind(productionKind)
+              .thrownTypes(producesMethod.getThrownTypes())
+              .executorRequest(executorRequest)
+              .monitorRequest(monitorRequest);
+      if (!types.isSameType(methodType, producesMethod.asType())) {
+        builder.unresolved(
+            forProducesMethod(producesMethod, asType(producesMethod.getEnclosingElement())));
+      }
+      return builder.build();
     }
 
     /**

java/dagger/internal/codegen/ProvisionBinding.java

@@ -159,6 +159,8 @@ abstract static class Builder extends ContributionBinding.Builder<Builder> {
     abstract ProvisionBinding build();
   }
 
+  /* TODO(dpb): Combine ProvisionBinding.Factory, ProductionBinding.Factory, and
+   * MembersInjectionBinding.Factory into one BindingFactory class.*/
   static final class Factory {
     private final DaggerTypes types;
     private final KeyFactory keyFactory;
@@ -236,25 +238,30 @@ ProvisionBinding forInjectConstructor(
     ProvisionBinding forProvidesMethod(
         ExecutableElement providesMethod, TypeElement contributedBy) {
       checkArgument(providesMethod.getKind().equals(METHOD));
-      ExecutableType resolvedMethod =
+      ExecutableType methodType =
           MoreTypes.asExecutable(
               types.asMemberOf(MoreTypes.asDeclared(contributedBy.asType()), providesMethod));
       Key key = keyFactory.forProvidesMethod(providesMethod, contributedBy);
       ImmutableSet<DependencyRequest> dependencies =
           dependencyRequestFactory.forRequiredResolvedVariables(
-              providesMethod.getParameters(),
-              resolvedMethod.getParameterTypes());
-      return ProvisionBinding.builder()
-          .contributionType(ContributionType.fromBindingMethod(providesMethod))
-          .bindingElement(providesMethod)
-          .contributingModule(contributedBy)
-          .key(key)
-          .provisionDependencies(dependencies)
-          .nullableType(ConfigurationAnnotations.getNullableType(providesMethod))
-          .wrappedMapKey(wrapOptionalInEquivalence(getMapKey(providesMethod)))
-          .kind(PROVISION)
-          .scope(uniqueScopeOf(providesMethod))
-          .build();
+              providesMethod.getParameters(), methodType.getParameterTypes());
+      ProvisionBinding.Builder builder =
+          ProvisionBinding.builder()
+              .contributionType(ContributionType.fromBindingMethod(providesMethod))
+              .bindingElement(providesMethod)
+              .contributingModule(contributedBy)
+              .key(key)
+              .provisionDependencies(dependencies)
+              .nullableType(ConfigurationAnnotations.getNullableType(providesMethod))
+              .wrappedMapKey(wrapOptionalInEquivalence(getMapKey(providesMethod)))
+              .kind(PROVISION)
+              .scope(uniqueScopeOf(providesMethod));
+      if (!types.isSameType(methodType, providesMethod.asType())) {
+        builder.unresolved(
+            forProvidesMethod(
+                providesMethod, MoreElements.asType(providesMethod.getEnclosingElement())));
+      }
+      return builder.build();
     }
 
     /**

javatests/dagger/functional/GenericComponent.java

@@ -17,10 +17,16 @@
 package dagger.functional;
 
 import dagger.Component;
+import dagger.Module;
+import dagger.Provides;
+import dagger.functional.GenericComponent.NongenericModule;
 import dagger.functional.sub.Exposed;
 import dagger.functional.sub.PublicSubclass;
+import java.util.Arrays;
+import java.util.List;
+import javax.inject.Provider;
 
-@Component(modules = {ChildDoubleModule.class, ChildIntegerModule.class})
+@Component(modules = {ChildDoubleModule.class, ChildIntegerModule.class, NongenericModule.class})
 interface GenericComponent {
   ReferencesGeneric referencesGeneric();
   GenericDoubleReferences<A> doubleGenericA();
@@ -37,4 +43,26 @@ interface GenericComponent {
 
   Iterable<Integer> iterableInt();
   Iterable<Double> iterableDouble();
+
+  Provider<List<String>> stringsProvider(); // b/71595104
+
+  // b/71595104
+  @Module
+  abstract class GenericModule<T> {
+    // Note that for subclasses that use String for T, this factory will still need two
+    // Provider<String> framework dependencies.
+    @Provides
+    List<T> list(T t, String string) {
+      return Arrays.asList(t);
+    }
+  }
+
+  // b/71595104
+  @Module
+  class NongenericModule extends GenericModule<String> {
+    @Provides
+    static String string() {
+      return "string";
+    }
+  }
 }

javatests/dagger/functional/producers/GenericComponent.java

@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2017 The Dagger Authors.
+ *
+ * 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 dagger.functional.producers;
+
+import com.google.common.util.concurrent.ListenableFuture;
+import dagger.functional.producers.GenericComponent.NongenericModule;
+import dagger.producers.ProducerModule;
+import dagger.producers.Produces;
+import dagger.producers.ProductionComponent;
+import java.util.Arrays;
+import java.util.List;
+
+@ProductionComponent(modules = {ExecutorModule.class, NongenericModule.class})
+interface GenericComponent {
+
+  ListenableFuture<List<String>> list(); // b/71595104
+
+  // b/71595104
+  @ProducerModule
+  abstract class GenericModule<T> {
+
+    @Produces
+    List<T> list(T t, String string) {
+      return Arrays.asList(t);
+    }
+  }
+
+  // b/71595104
+  @ProducerModule
+  class NongenericModule extends GenericModule<String> {
+    @Produces
+    static String string() {
+      return "string";
+    }
+  }
+}