diff --git a/dagger-compiler/main/java/dagger/internal/codegen/writing/ProvisionBindingRepresentation.java b/dagger-compiler/main/java/dagger/internal/codegen/writing/ProvisionBindingRepresentation.java index 518d110b45b..35cdb0d2dc7 100644 --- a/dagger-compiler/main/java/dagger/internal/codegen/writing/ProvisionBindingRepresentation.java +++ b/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 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. - * - *

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); diff --git a/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_DEFAULT_MODE b/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_DEFAULT_MODE index 8b87e6daab3..96dcdc3ca74 100644 --- a/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_DEFAULT_MODE +++ b/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_DEFAULT_MODE @@ -50,6 +50,10 @@ public final class DaggerTestComponent { } + Foo foo() { + return new Foo(new Bar()); + } + @SuppressWarnings("unchecked") private void initialize() { this.fooProvider = Foo_Factory.create(Bar_Factory.create()); @@ -57,7 +61,7 @@ public final class DaggerTestComponent { @Override public SomeEntryPoint someEntryPoint() { - return new SomeEntryPoint(fooProvider.get(), fooProvider); + return new SomeEntryPoint(foo(), fooProvider); } } } diff --git a/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_KT_DEFAULT_MODE b/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_KT_DEFAULT_MODE index 1968e001b1e..0ba5565fe71 100644 --- a/javatests/dagger/internal/codegen/goldens/ComponentProcessorTest/providerComponentType/test.DaggerTestComponent_KT_DEFAULT_MODE +++ b/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} - */ + * {@code Provider} + */ Provider fooProvider; TestComponentImpl() { @@ -53,6 +53,10 @@ 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()); @@ -60,7 +64,7 @@ public final class DaggerTestComponent { @Override public SomeEntryPoint someEntryPoint() { - return (SomeEntryPoint) ((Object) (SomeEntryPoint_Factory.newInstance(fooProvider.get(), fooProvider))); + return (SomeEntryPoint) ((Object) (SomeEntryPoint_Factory.newInstance(foo(), fooProvider))); } } } diff --git a/javatests/dagger/internal/codegen/goldens/LazyClassKeyMapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE b/javatests/dagger/internal/codegen/goldens/LazyClassKeyMapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE index bfcb93eb002..7d9b1b3512c 100644 --- a/javatests/dagger/internal/codegen/goldens/LazyClassKeyMapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE +++ b/javatests/dagger/internal/codegen/goldens/LazyClassKeyMapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE @@ -1,5 +1,6 @@ package test; +import com.google.common.collect.ImmutableMap; import dagger.internal.DaggerGenerated; import dagger.internal.LazyClassKeyMap; import dagger.internal.MapFactory; @@ -7,6 +8,7 @@ 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, Integer> classKey() { - return mapOfClassOfAndIntegerProvider.get(); + return LazyClassKeyMap.of(ImmutableMap.of(MapModule_ClassKey_LazyMapKey.lazyClassKeyName, MapModule.classKey())); } @Override @@ -96,7 +98,7 @@ final class DaggerTestComponent { @Override public Map complexKey() { - return mapOfComplexKeyAndIntegerProvider.get(); + return ImmutableMap.of(MapModule_ComplexKeyWithInaccessibleValueMapKey.create(), MapModule.complexKeyWithInaccessibleValue(), MapModule_ComplexKeyWithInaccessibleArrayValueMapKey.create(), MapModule.complexKeyWithInaccessibleArrayValue(), MapModule_ComplexKeyWithInaccessibleAnnotationValueMapKey.create(), MapModule.complexKeyWithInaccessibleAnnotationValue()); } @Override diff --git a/javatests/dagger/internal/codegen/goldens/MapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE b/javatests/dagger/internal/codegen/goldens/MapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE index fa8f494b584..94c6cba54fd 100644 --- a/javatests/dagger/internal/codegen/goldens/MapBindingComponentProcessorTest/mapBindingsWithInaccessibleKeys/test.DaggerTestComponent_DEFAULT_MODE +++ b/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, Integer> classKey() { - return mapOfClassOfAndIntegerProvider.get(); + return ImmutableMap., 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 complexKey() { - return mapOfComplexKeyAndIntegerProvider.get(); + return ImmutableMap.of(MapModule_ComplexKeyWithInaccessibleValueMapKey.create(), MapModule.complexKeyWithInaccessibleValue(), MapModule_ComplexKeyWithInaccessibleArrayValueMapKey.create(), MapModule.complexKeyWithInaccessibleArrayValue(), MapModule_ComplexKeyWithInaccessibleAnnotationValueMapKey.create(), MapModule.complexKeyWithInaccessibleAnnotationValue()); } @Override diff --git a/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE b/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE index df4c586b9fa..e2f9a67ed45 100644 --- a/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE +++ b/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,13 +59,29 @@ final class DaggerTestComponent { } + Map mapOfStringAndLazyOfIntegerBuilder() { + MapBuilder mapBuilder = MapBuilder.>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.>>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 mapOfStringAndLazyOfIntegerBuilder() { + MapLazyFactory mapOfStringAndLazyOfIntegerBuilder2() { MapLazyFactory.Builder builder = MapLazyFactory.builder(3); builder.put("key0", LazyMaps_TestModule_Int0Factory.create()); builder.put("key1", LazyMaps_TestModule_Int1Factory.create()); @@ -70,7 +89,7 @@ final class DaggerTestComponent { return builder.build(); } - MapProviderLazyFactory mapOfStringAndProviderOfLazyOfIntegerBuilder() { + MapProviderLazyFactory mapOfStringAndProviderOfLazyOfIntegerBuilder2() { MapProviderLazyFactory.Builder 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> mapOfLazy() { - return mapOfStringAndLazyOfIntegerProvider.get(); + return mapOfStringAndLazyOfIntegerBuilder(); } @Override public Map>> mapOfProviderOfLazy() { - return (Map>>) (mapOfStringAndProviderOfLazyOfIntegerProvider.get()); + return mapOfStringAndProviderOfLazyOfIntegerBuilder(); } @Override diff --git a/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE b/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE index cebfcbec991..a30bcc588b9 100644 --- a/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_DEFAULT_MODE +++ b/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 mapOfString() { - return ImmutableMap.of("key0", string0Provider.get(), "key1", string1Provider.get(), "key2", string2Provider.get()); + return ImmutableMap.of("key0", string0(), "key1", string1(), "key2", string2()); } @Override public Map> mapOfLazy() { - return mapOfStringAndLazyOfStringProvider.get(); + return ImmutableMap.>of("key0", DoubleCheck.lazy(string0Provider), "key1", DoubleCheck.lazy(string1Provider), "key2", DoubleCheck.lazy(string2Provider)); } @Override public Map>> mapOfProviderOfLazy() { - return (Map>>) (mapOfStringAndProviderOfLazyOfStringProvider.get()); + return ImmutableMap.>>of("key0", ProviderOfLazy.create(string0Provider), "key1", ProviderOfLazy.create(string1Provider), "key2", ProviderOfLazy.create(string2Provider)); } @Override diff --git a/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_KT_DEFAULT_MODE b/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_KT_DEFAULT_MODE index 8c87d73be33..79b3650bdd7 100644 --- a/javatests/dagger/internal/codegen/goldens/MapRequestRepresentationWithGuavaTest/lazyMaps/test.DaggerTestComponent_KT_DEFAULT_MODE +++ b/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 mapOfString() { - return (Map) ((Object) (ImmutableMap.of("key0", string0Provider.get(), "key1", string1Provider.get(), "key2", string2Provider.get()))); + return (Map) ((Object) (ImmutableMap.of("key0", string0(), "key1", string1(), "key2", string2()))); } @Override public Map> mapOfLazy() { - return (Map>) ((Object) (mapOfStringAndLazyOfStringProvider.get())); + return (Map>) ((Object) (ImmutableMap.of("key0", DoubleCheck.lazy(string0Provider), "key1", DoubleCheck.lazy(string1Provider), "key2", DoubleCheck.lazy(string2Provider)))); } @Override public Map>> mapOfProviderOfLazy() { - return (Map>>) ((Object) ((Map>>) (mapOfStringAndProviderOfLazyOfStringProvider.get()))); + return (Map>>) ((Object) (ImmutableMap.of("key0", ProviderOfLazy.create(string0Provider), "key1", ProviderOfLazy.create(string1Provider), "key2", ProviderOfLazy.create(string2Provider)))); } @Override