diff --git a/compiler/src/it/functional-tests/src/main/java/test/subcomponent/MultibindingSubcomponents.java b/compiler/src/it/functional-tests/src/main/java/test/subcomponent/MultibindingSubcomponents.java index 7857e3df0e0..fa53807cb8f 100644 --- a/compiler/src/it/functional-tests/src/main/java/test/subcomponent/MultibindingSubcomponents.java +++ b/compiler/src/it/functional-tests/src/main/java/test/subcomponent/MultibindingSubcomponents.java @@ -152,6 +152,37 @@ static BoundInChild onlyInChildEntry() { } } + @Module + abstract static class ChildMultibindingModuleWithOnlyBindsMultibindings { + @Provides + static BoundInParentAndChild provideBoundInParentAndChildForBinds() { + return BoundInParentAndChild.IN_CHILD; + } + + @Binds + @IntoSet + abstract BoundInParentAndChild bindsLocalContribution(BoundInParentAndChild instance); + + @Binds + @IntoMap + @StringKey("child key") + abstract BoundInParentAndChild inParentAndChildEntry(BoundInParentAndChild instance); + + @Provides + static BoundInChild provideBoundInChildForBinds() { + return BoundInChild.INSTANCE; + } + + @Binds + @IntoSet + abstract BoundInChild inChild(BoundInChild instance); + + @Binds + @IntoMap + @StringKey("child key") + abstract BoundInChild inChildEntry(BoundInChild instance); + } + interface ProvidesBoundInParent { RequiresMultibindings requiresMultibindingsBoundInParent(); } @@ -211,4 +242,13 @@ interface ChildWithProvision interface Grandchild extends ProvidesBoundInParent, ProvidesBoundInParentAndChild, ProvidesBoundInChild, ProvidesSetOfRequiresMultibindings {} + + @Component(modules = ParentMultibindingModule.class) + interface ParentWithProvisionHasChildWithBinds extends ParentWithProvision { + ChildWithBinds childWithBinds(); + } + + @Subcomponent(modules = ChildMultibindingModuleWithOnlyBindsMultibindings.class) + interface ChildWithBinds extends ChildWithProvision {} + } diff --git a/compiler/src/it/functional-tests/src/test/java/test/subcomponent/SubcomponentMultibindingsTest.java b/compiler/src/it/functional-tests/src/test/java/test/subcomponent/SubcomponentMultibindingsTest.java index 3d2c8e5d858..3e0e52702ae 100644 --- a/compiler/src/it/functional-tests/src/test/java/test/subcomponent/SubcomponentMultibindingsTest.java +++ b/compiler/src/it/functional-tests/src/test/java/test/subcomponent/SubcomponentMultibindingsTest.java @@ -225,6 +225,13 @@ public void testParentWithProvisionHasChildWithProvision() { .requiresMultibindingsBoundInParentAndChild()) .isEqualTo(BOUND_IN_PARENT_AND_CHILD); + // https://github.com/google/dagger/issues/401 + assertThat( + DaggerMultibindingSubcomponents_ParentWithProvisionHasChildWithBinds.create() + .childWithBinds() + .requiresMultibindingsBoundInParentAndChild()) + .isEqualTo(BOUND_IN_PARENT_AND_CHILD); + /* * Even though the multibinding for Set does not itself have a * contribution from the child, it must be pushed down to (not duplicated in) the child because diff --git a/compiler/src/main/java/dagger/internal/codegen/BindingGraph.java b/compiler/src/main/java/dagger/internal/codegen/BindingGraph.java index a71d2e36926..e880aa0094a 100644 --- a/compiler/src/main/java/dagger/internal/codegen/BindingGraph.java +++ b/compiler/src/main/java/dagger/internal/codegen/BindingGraph.java @@ -19,6 +19,7 @@ import static com.google.auto.common.MoreElements.getAnnotationMirror; import static com.google.auto.common.MoreElements.hasModifiers; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Predicates.in; import static com.google.common.base.Predicates.not; import static com.google.common.base.Verify.verify; @@ -302,17 +303,12 @@ private final class Resolver { ImmutableSetMultimap explicitBindings, ImmutableSetMultimap multibindingDeclarations, ImmutableSetMultimap delegateDeclarations) { - assert parentResolver != null; - this.parentResolver = parentResolver; - assert componentDescriptor != null; - this.componentDescriptor = componentDescriptor; - assert explicitBindings != null; - this.explicitBindings = explicitBindings; + this.parentResolver = checkNotNull(parentResolver); + this.componentDescriptor = checkNotNull(componentDescriptor); + this.explicitBindings = checkNotNull(explicitBindings); this.explicitBindingsSet = ImmutableSet.copyOf(explicitBindings.values()); - assert multibindingDeclarations != null; - this.multibindingDeclarations = multibindingDeclarations; - assert delegateDeclarations != null; - this.delegateDeclarations = delegateDeclarations; + this.multibindingDeclarations = checkNotNull(multibindingDeclarations); + this.delegateDeclarations = checkNotNull(delegateDeclarations); this.resolvedBindings = Maps.newLinkedHashMap(); this.explicitMultibindings = multibindingContributionsByMultibindingKey(explicitBindingsSet); @@ -361,11 +357,7 @@ ResolvedBindings lookUpBindings(DependencyRequest request) { for (Key key : keysMatchingRequest(requestKey)) { contributionBindings.addAll(getExplicitBindings(key)); - contributionBindings.addAll(getDelegateBindings(key)); - - multibindingContributionsBuilder.addAll(getExplicitMultibindingContributions(key)); - multibindingContributionsBuilder.addAll(getDelegateMultibindingContributions(key)); - + multibindingContributionsBuilder.addAll(getExplicitMultibindings(key)); multibindingDeclarationsBuilder.addAll(getMultibindingDeclarations(key)); } @@ -463,9 +455,7 @@ private FluentIterable multibindingContributionsForValueMap new Function>() { @Override public Iterable apply(Key key) { - return Iterables.concat( - getExplicitMultibindingContributions(key), - getDelegateMultibindingContributions(key)); + return getExplicitMultibindings(key); } }); } @@ -676,25 +666,36 @@ private ImmutableList getResolverLineage() { * this and all ancestor resolvers. */ private ImmutableSet getExplicitBindings(Key requestKey) { - ImmutableSet.Builder explicitBindingsForKey = ImmutableSet.builder(); + ImmutableSet.Builder bindings = ImmutableSet.builder(); + Key delegateDeclarationKey = keyFactory.convertToDelegateKey(requestKey); for (Resolver resolver : getResolverLineage()) { - explicitBindingsForKey.addAll(resolver.explicitBindings.get(requestKey)); + bindings + .addAll(resolver.explicitBindings.get(requestKey)) + .addAll( + createDelegateBindings( + resolver.delegateDeclarations.get(delegateDeclarationKey))); } - return explicitBindingsForKey.build(); + return bindings.build(); } /** * Returns the explicit multibinding contributions that contribute to the map or set requested * by {@code requestKey} from this and all ancestor resolvers. */ - private ImmutableSet getExplicitMultibindingContributions( - Key requestKey) { - ImmutableSet.Builder explicitMultibindingsForKey = - ImmutableSet.builder(); + private ImmutableSet getExplicitMultibindings(Key requestKey) { + ImmutableSet.Builder multibindings = ImmutableSet.builder(); + Key delegateDeclarationKey = keyFactory.convertToDelegateKey(requestKey); for (Resolver resolver : getResolverLineage()) { - explicitMultibindingsForKey.addAll(resolver.explicitMultibindings.get(requestKey)); + multibindings.addAll(resolver.explicitMultibindings.get(requestKey)); + if (!MapType.isMap(requestKey) || MapType.from(requestKey).valuesAreFrameworkType()) { + // There are no @Binds @IntoMap delegate declarations for Map requests. All + // @IntoMap requests must be for Map>. + multibindings.addAll( + createDelegateBindings( + resolver.delegateMultibindingDeclarations.get(delegateDeclarationKey))); + } } - return explicitMultibindingsForKey.build(); + return multibindings.build(); } /** @@ -710,37 +711,6 @@ private ImmutableSet getMultibindingDeclarations(Key ke return multibindingDeclarations.build(); } - private ImmutableSet getDelegateBindings(Key requestKey) { - Key delegateDeclarationKey = keyFactory.convertToDelegateKey(requestKey); - ImmutableSet.Builder delegateBindings = ImmutableSet.builder(); - for (Resolver resolver : getResolverLineage()) { - delegateBindings.addAll( - createDelegateBindings(resolver.delegateDeclarations.get(delegateDeclarationKey))); - } - return delegateBindings.build(); - } - - /** - * Returns the delegate multibinding contribution declarations that contribute to the map or - * set requested by {@code requestKey} from this and all ancestor resolvers. - */ - private ImmutableSet getDelegateMultibindingContributions( - Key requestKey) { - if (MapType.isMap(requestKey) && !MapType.from(requestKey).valuesAreFrameworkType()) { - // There are no @Binds @IntoMap delegate declarations for Map requests. All @IntoMap - // requests must be for Map>. - return ImmutableSet.of(); - } - Key delegateDeclarationKey = keyFactory.convertToDelegateKey(requestKey); - ImmutableSet.Builder delegateMultibindings = ImmutableSet.builder(); - for (Resolver resolver : getResolverLineage()) { - delegateMultibindings.addAll( - createDelegateBindings( - resolver.delegateMultibindingDeclarations.get(delegateDeclarationKey))); - } - return delegateMultibindings.build(); - } - private Optional getPreviouslyResolvedBindings( final BindingKey bindingKey) { Optional result = Optional.fromNullable(resolvedBindings.get(bindingKey)); @@ -925,10 +895,12 @@ public Boolean call() { } private boolean isMultibindingsWithLocalContributions(ResolvedBindings resolvedBindings) { + Key key = resolvedBindings.key(); return FluentIterable.from(resolvedBindings.contributionBindings()) .transform(ContributionBinding.KIND) .anyMatch(IS_SYNTHETIC_MULTIBINDING_KIND) - && explicitMultibindings.containsKey(resolvedBindings.key()); + && !getExplicitMultibindings(key) + .equals(parentResolver.get().getExplicitMultibindings(key)); } } }