Internal Changes

dagger-compiler/main/java/dagger/internal/codegen/writing/ProvisionBindingRepresentation.java

@@ -26,7 +26,6 @@
 import dagger.internal.codegen.binding.BindingRequest;
 import dagger.internal.codegen.binding.ContributionBinding;
 import dagger.internal.codegen.binding.DelegateBinding;
-import dagger.internal.codegen.model.DependencyRequest;
 import dagger.internal.codegen.model.RequestKind;
 import dagger.internal.codegen.writing.ComponentImplementation.CompilerMode;
 
@@ -81,8 +80,6 @@ private boolean usesDirectInstanceExpression(RequestKind requestKind) {
     }
 
     switch (binding.kind()) {
-      case SUBCOMPONENT_CREATOR:
-        return true;
       case MEMBERS_INJECTOR:
         // Currently, we always use a framework instance for MembersInjectors, e.g.
         // InstanceFactory.create(Foo_MembersInjector.create(...)).
@@ -97,19 +94,9 @@ private boolean usesDirectInstanceExpression(RequestKind requestKind) {
             "Assisted injection binding shouldn't be requested with an instance request.");
       default:
         // We don't need to use Provider#get() if there's no caching, so use a direct instance.
-        // However, if there's no caching needed but we already have a framework instance requested
-        // for this binding, we can reuse that framework instance by calling Provider#get() instead
-        // of generating a direct instance.
-        // TODO(emjich): To be even more accurate, we should consider delegate bindings here
-        // too. For example, if we have:
-        //   @Binds Foo -> @Binds FooIntermediate -> @Provides FooImpl
-        // Then we technically should be checking all of bindings for hasFrameworkRequest,
-        // e.g. if someone requests a Provider<Foo> we should be able to reuse that same
-        // provider for FooIntermediate and FooImpl since they are all just delegates of
-        // each other.
-        return !needsCaching(binding, graph)
-            && !(graph.topLevelBindingGraph().hasFrameworkRequest(binding)
-                && bindingHasDependencies(binding, graph));
+        // TODO(bcorso): This can be optimized in cases where we know a Provider field already
+        // exists, in which case even if it's not scoped we might as well call Provider#get().
+        return !needsCaching(binding, graph);
     }
   }
 
@@ -129,26 +116,6 @@ static boolean needsCaching(ContributionBinding binding, BindingGraph graph) {
     return true;
   }
 
-  /**
-   * Returns {@code true} if {@code binding} has dependencies.
-   *
-   * <p>If {@code binding} is a {@code DELEGATE}, it is only considered to have dependencies if the
-   * binding it delegates to has dependencies. Otherwise, a binding has dependencies if {@code
-   * binding.dependencies()} is not empty.
-   */
-  private static boolean bindingHasDependencies(ContributionBinding binding, BindingGraph graph) {
-    if (binding.dependencies().isEmpty()) {
-      return false;
-    }
-    if (!binding.kind().equals(DELEGATE)) {
-      return true;
-    }
-    return binding.dependencies().stream()
-        .map(DependencyRequest::key)
-        .map(graph::contributionBinding)
-        .anyMatch(b -> bindingHasDependencies(b, graph));
-  }
-
   @AssistedFactory
   static interface Factory {
     ProvisionBindingRepresentation create(ContributionBinding binding);

javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_DEFAULT_MODE

@@ -50,14 +50,18 @@ public final class DaggerTestComponent {
 
     }
 
+    Foo foo() {
+      return new Foo(new Bar());
+    }
+
     @SuppressWarnings("unchecked")
     private void initialize() {
       this.fooProvider = Foo_Factory.create(Bar_Factory.create());
     }
 
     @Override
     public SomeEntryPoint someEntryPoint() {
-      return new SomeEntryPoint(fooProvider.get(), fooProvider);
+      return new SomeEntryPoint(foo(), fooProvider);
     }
   }
 }

javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_KT_DEFAULT_MODE

@@ -43,8 +43,8 @@ public final class DaggerTestComponent {
     private final TestComponentImpl testComponentImpl = this;
 
     /**
-      * {@code Provider<Foo>}
-      */
+     * {@code Provider<Foo>}
+     */
     Provider<?> fooProvider;
 
     TestComponentImpl() {
@@ -53,14 +53,18 @@ public final class DaggerTestComponent {
 
     }
 
+    Object foo() {
+      return Foo_Factory.newInstance(Bar_Factory.newInstance());
+    }
+
     @SuppressWarnings("unchecked")
     private void initialize() {
       this.fooProvider = Foo_Factory.create(Bar_Factory.create());
     }
 
     @Override
     public SomeEntryPoint someEntryPoint() {
-      return (SomeEntryPoint) ((Object) (SomeEntryPoint_Factory.newInstance(fooProvider.get(), fooProvider)));
+      return (SomeEntryPoint) ((Object) (SomeEntryPoint_Factory.newInstance(foo(), fooProvider)));
     }
   }
 }

javatests/dagger/internal/codegen/goldens/LazyClassKeyMapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE

@@ -1,12 +1,14 @@
 package test;
 
+import com.google.common.collect.ImmutableMap;
 import dagger.internal.DaggerGenerated;
 import dagger.internal.LazyClassKeyMap;
 import dagger.internal.MapFactory;
 import dagger.internal.Provider;
 import java.util.Map;
 import javax.annotation.processing.Generated;
 import mapkeys.MapKeys;
+import mapkeys.MapModule;
 import mapkeys.MapModule_ClassKeyFactory;
 import mapkeys.MapModule_ClassKey_LazyMapKey;
 import mapkeys.MapModule_ComplexKeyWithInaccessibleAnnotationValueFactory;
@@ -86,7 +88,7 @@ final class DaggerTestComponent {
 
     @Override
     public Map<Class<?>, Integer> classKey() {
-      return mapOfClassOfAndIntegerProvider.get();
+      return LazyClassKeyMap.<Integer>of(ImmutableMap.<String, Integer>of(MapModule_ClassKey_LazyMapKey.lazyClassKeyName, MapModule.classKey()));
     }
 
     @Override
@@ -96,7 +98,7 @@ final class DaggerTestComponent {
 
     @Override
     public Map<MapKeys.ComplexKey, Integer> complexKey() {
-      return mapOfComplexKeyAndIntegerProvider.get();
+      return ImmutableMap.<MapKeys.ComplexKey, Integer>of(MapModule_ComplexKeyWithInaccessibleValueMapKey.create(), MapModule.complexKeyWithInaccessibleValue(), MapModule_ComplexKeyWithInaccessibleArrayValueMapKey.create(), MapModule.complexKeyWithInaccessibleArrayValue(), MapModule_ComplexKeyWithInaccessibleAnnotationValueMapKey.create(), MapModule.complexKeyWithInaccessibleAnnotationValue());
     }
 
     @Override

javatests/dagger/internal/codegen/goldens/MapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE

@@ -1,11 +1,13 @@
 package test;
 
+import com.google.common.collect.ImmutableMap;
 import dagger.internal.DaggerGenerated;
 import dagger.internal.MapFactory;
 import dagger.internal.Provider;
 import java.util.Map;
 import javax.annotation.processing.Generated;
 import mapkeys.MapKeys;
+import mapkeys.MapModule;
 import mapkeys.MapModule_ClassKeyFactory;
 import mapkeys.MapModule_ClassKeyMapKey;
 import mapkeys.MapModule_ComplexKeyWithInaccessibleAnnotationValueFactory;
@@ -68,6 +70,10 @@ final class DaggerTestComponent {
 
     }
 
+    Map mapOfPackagePrivateEnumAndInteger() {
+      return ImmutableMap.of(MapModule_EnumKeyMapKey.create(), MapModule.enumKey());
+    }
+
     @SuppressWarnings("unchecked")
     private void initialize() {
       this.mapOfClassOfAndIntegerProvider = mapOfClassOfAndIntegerBuilder();
@@ -97,7 +103,7 @@ final class DaggerTestComponent {
 
     @Override
     public Map<Class<?>, Integer> classKey() {
-      return mapOfClassOfAndIntegerProvider.get();
+      return ImmutableMap.<Class<?>, Integer>of(MapModule_ClassKeyMapKey.create(), MapModule.classKey());
     }
 
     @Override
@@ -107,7 +113,7 @@ final class DaggerTestComponent {
 
     @Override
     public Object inaccessibleEnum() {
-      return mapOfPackagePrivateEnumAndIntegerProvider.get();
+      return mapOfPackagePrivateEnumAndInteger();
     }
 
     @Override
@@ -117,7 +123,7 @@ final class DaggerTestComponent {
 
     @Override
     public Map<MapKeys.ComplexKey, Integer> complexKey() {
-      return mapOfComplexKeyAndIntegerProvider.get();
+      return ImmutableMap.<MapKeys.ComplexKey, Integer>of(MapModule_ComplexKeyWithInaccessibleValueMapKey.create(), MapModule.complexKeyWithInaccessibleValue(), MapModule_ComplexKeyWithInaccessibleArrayValueMapKey.create(), MapModule.complexKeyWithInaccessibleArrayValue(), MapModule_ComplexKeyWithInaccessibleAnnotationValueMapKey.create(), MapModule.complexKeyWithInaccessibleAnnotationValue());
     }
 
     @Override

javatests/dagger/internal/codegen/goldens/MapRequestRepresentationTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE

@@ -2,9 +2,12 @@ package test;
 
 import dagger.Lazy;
 import dagger.internal.DaggerGenerated;
+import dagger.internal.DoubleCheck;
+import dagger.internal.MapBuilder;
 import dagger.internal.MapLazyFactory;
 import dagger.internal.MapProviderLazyFactory;
 import dagger.internal.Provider;
+import dagger.internal.ProviderOfLazy;
 import java.util.Map;
 import javax.annotation.processing.Generated;
 
@@ -56,21 +59,37 @@ final class DaggerTestComponent {
 
     }
 
+    Map mapOfStringAndLazyOfIntegerBuilder() {
+      MapBuilder mapBuilder = MapBuilder.<String, Lazy<Integer>>newMapBuilder(3);
+      mapBuilder.put("key0", DoubleCheck.lazy(LazyMaps_TestModule_Int0Factory.create()));
+      mapBuilder.put("key1", DoubleCheck.lazy(LazyMaps_TestModule_Int1Factory.create()));
+      mapBuilder.put("key2", DoubleCheck.lazy(LazyMaps_TestModule_Int2Factory.create()));
+      return mapBuilder.build();
+    }
+
+    Map mapOfStringAndProviderOfLazyOfIntegerBuilder() {
+      MapBuilder mapBuilder = MapBuilder.<String, javax.inject.Provider<Lazy<Integer>>>newMapBuilder(3);
+      mapBuilder.put("key0", ProviderOfLazy.create(LazyMaps_TestModule_Int0Factory.create()));
+      mapBuilder.put("key1", ProviderOfLazy.create(LazyMaps_TestModule_Int1Factory.create()));
+      mapBuilder.put("key2", ProviderOfLazy.create(LazyMaps_TestModule_Int2Factory.create()));
+      return mapBuilder.build();
+    }
+
     @SuppressWarnings("unchecked")
     private void initialize() {
-      this.mapOfStringAndLazyOfIntegerProvider = mapOfStringAndLazyOfIntegerBuilder();
-      this.mapOfStringAndProviderOfLazyOfIntegerProvider = mapOfStringAndProviderOfLazyOfIntegerBuilder();
+      this.mapOfStringAndLazyOfIntegerProvider = mapOfStringAndLazyOfIntegerBuilder2();
+      this.mapOfStringAndProviderOfLazyOfIntegerProvider = mapOfStringAndProviderOfLazyOfIntegerBuilder2();
     }
 
-    MapLazyFactory<String, Integer> mapOfStringAndLazyOfIntegerBuilder() {
+    MapLazyFactory<String, Integer> mapOfStringAndLazyOfIntegerBuilder2() {
       MapLazyFactory.Builder<String, Integer> builder = MapLazyFactory.builder(3);
       builder.put("key0", LazyMaps_TestModule_Int0Factory.create());
       builder.put("key1", LazyMaps_TestModule_Int1Factory.create());
       builder.put("key2", LazyMaps_TestModule_Int2Factory.create());
       return builder.build();
     }
 
-    MapProviderLazyFactory<String, Integer> mapOfStringAndProviderOfLazyOfIntegerBuilder() {
+    MapProviderLazyFactory<String, Integer> mapOfStringAndProviderOfLazyOfIntegerBuilder2() {
       MapProviderLazyFactory.Builder<String, Integer> builder = MapProviderLazyFactory.builder(3);
       builder.put("key0", LazyMaps_TestModule_Int0Factory.create());
       builder.put("key1", LazyMaps_TestModule_Int1Factory.create());
@@ -80,12 +99,12 @@ final class DaggerTestComponent {
 
     @Override
     public Map<String, Lazy<Integer>> mapOfLazy() {
-      return mapOfStringAndLazyOfIntegerProvider.get();
+      return mapOfStringAndLazyOfIntegerBuilder();
     }
 
     @Override
     public Map<String, javax.inject.Provider<Lazy<Integer>>> mapOfProviderOfLazy() {
-      return (Map<String, javax.inject.Provider<Lazy<Integer>>>) (mapOfStringAndProviderOfLazyOfIntegerProvider.get());
+      return mapOfStringAndProviderOfLazyOfIntegerBuilder();
     }
 
     @Override

javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE

@@ -7,6 +7,7 @@ import dagger.internal.DoubleCheck;
 import dagger.internal.MapLazyFactory;
 import dagger.internal.MapProviderLazyFactory;
 import dagger.internal.Provider;
+import dagger.internal.ProviderOfLazy;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 import javax.annotation.processing.Generated;
@@ -69,6 +70,22 @@ final class DaggerTestComponent {
 
     }
 
+    String string() {
+      return LazyMaps_TestModule_ProvideStringFactory.provideString(provideAtomicIntegerProvider.get());
+    }
+
+    String string0() {
+      return LazyMaps_TestModule_String0Factory.string0(string());
+    }
+
+    String string1() {
+      return LazyMaps_TestModule_String1Factory.string1(string());
+    }
+
+    String string2() {
+      return LazyMaps_TestModule_String2Factory.string2(string());
+    }
+
     @SuppressWarnings("unchecked")
     private void initialize() {
       this.provideAtomicIntegerProvider = DoubleCheck.provider(LazyMaps_TestModule_ProvideAtomicIntegerFactory.create());
@@ -98,17 +115,17 @@ final class DaggerTestComponent {
 
     @Override
     public Map<String, String> mapOfString() {
-      return ImmutableMap.<String, String>of("key0", string0Provider.get(), "key1", string1Provider.get(), "key2", string2Provider.get());
+      return ImmutableMap.<String, String>of("key0", string0(), "key1", string1(), "key2", string2());
     }
 
     @Override
     public Map<String, Lazy<String>> mapOfLazy() {
-      return mapOfStringAndLazyOfStringProvider.get();
+      return ImmutableMap.<String, Lazy<String>>of("key0", DoubleCheck.lazy(string0Provider), "key1", DoubleCheck.lazy(string1Provider), "key2", DoubleCheck.lazy(string2Provider));
     }
 
     @Override
     public Map<String, javax.inject.Provider<Lazy<String>>> mapOfProviderOfLazy() {
-      return (Map<String, javax.inject.Provider<Lazy<String>>>) (mapOfStringAndProviderOfLazyOfStringProvider.get());
+      return ImmutableMap.<String, javax.inject.Provider<Lazy<String>>>of("key0", ProviderOfLazy.create(string0Provider), "key1", ProviderOfLazy.create(string1Provider), "key2", ProviderOfLazy.create(string2Provider));
     }
 
     @Override

javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_KT_DEFAULT_MODE

@@ -7,6 +7,7 @@ import dagger.internal.DoubleCheck;
 import dagger.internal.MapLazyFactory;
 import dagger.internal.MapProviderLazyFactory;
 import dagger.internal.Provider;
+import dagger.internal.ProviderOfLazy;
 import java.util.Map;
 import javax.annotation.processing.Generated;
 
@@ -89,6 +90,22 @@ final class DaggerTestComponent {
 
     }
 
+    Object string() {
+      return LazyMaps_TestModule_ProvideStringFactory.provideString(provideAtomicIntegerProvider.get());
+    }
+
+    Object string0() {
+      return LazyMaps_TestModule_String0Factory.string0(string());
+    }
+
+    Object string1() {
+      return LazyMaps_TestModule_String1Factory.string1(string());
+    }
+
+    Object string2() {
+      return LazyMaps_TestModule_String2Factory.string2(string());
+    }
+
     @SuppressWarnings("unchecked")
     private void initialize() {
       this.provideAtomicIntegerProvider = DoubleCheck.provider(LazyMaps_TestModule_ProvideAtomicIntegerFactory.create());
@@ -118,17 +135,17 @@ final class DaggerTestComponent {
 
     @Override
     public Map<String, String> mapOfString() {
-      return (Map<String, String>) ((Object) (ImmutableMap.of("key0", string0Provider.get(), "key1", string1Provider.get(), "key2", string2Provider.get())));
+      return (Map<String, String>) ((Object) (ImmutableMap.of("key0", string0(), "key1", string1(), "key2", string2())));
     }
 
     @Override
     public Map<String, Lazy<String>> mapOfLazy() {
-      return (Map<String, Lazy<String>>) ((Object) (mapOfStringAndLazyOfStringProvider.get()));
+      return (Map<String, Lazy<String>>) ((Object) (ImmutableMap.of("key0", DoubleCheck.lazy(string0Provider), "key1", DoubleCheck.lazy(string1Provider), "key2", DoubleCheck.lazy(string2Provider))));
     }
 
     @Override
     public Map<String, javax.inject.Provider<Lazy<String>>> mapOfProviderOfLazy() {
-      return (Map<String, javax.inject.Provider<Lazy<String>>>) ((Object) ((Map<String, javax.inject.Provider<Lazy<String>>>) (mapOfStringAndProviderOfLazyOfStringProvider.get())));
+      return (Map<String, javax.inject.Provider<Lazy<String>>>) ((Object) (ImmutableMap.of("key0", ProviderOfLazy.create(string0Provider), "key1", ProviderOfLazy.create(string1Provider), "key2", ProviderOfLazy.create(string2Provider))));
     }
 
     @Override