Skip to content

Commit

Permalink
Detect local multibinding contributions from @BINDS methods too.
Browse files Browse the repository at this point in the history 
Also stop distinguishing between explicit bindings and delegate declarations during resolution time; it's confusing and unnecessary.

Github: Fixes square#401

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=127997438
  • Loading branch information
@ronshapiro @cgruber
ronshapiro authored and cgruber committed Jul 25, 2016
1 parent 4938083 commit d697327
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 59 deletions.
40 changes: 40 additions & 0 deletions ...er/src/it/functional-tests/src/main/java/test/subcomponent/MultibindingSubcomponents.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<BoundInParent> requiresMultibindingsBoundInParent();
}
Expand Down Expand Up @@ -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 {}

}
7 changes: 7 additions & 0 deletions ...rc/it/functional-tests/src/test/java/test/subcomponent/SubcomponentMultibindingsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<RequiresMultiboundObjects> does not itself have a
* contribution from the child, it must be pushed down to (not duplicated in) the child because
Expand Down
90 changes: 31 additions & 59 deletions compiler/src/main/java/dagger/internal/codegen/BindingGraph.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -302,17 +303,12 @@ private final class Resolver {
ImmutableSetMultimap<Key, ContributionBinding> explicitBindings,
ImmutableSetMultimap<Key, MultibindingDeclaration> multibindingDeclarations,
ImmutableSetMultimap<Key, DelegateDeclaration> 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);
Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -463,9 +455,7 @@ private FluentIterable<ContributionBinding> multibindingContributionsForValueMap
new Function<Key, Iterable<ContributionBinding>>() {
@Override
public Iterable<ContributionBinding> apply(Key key) {
return Iterables.concat(
getExplicitMultibindingContributions(key),
getDelegateMultibindingContributions(key));
return getExplicitMultibindings(key);
}
});
}
Expand Down Expand Up @@ -676,25 +666,36 @@ private ImmutableList<Resolver> getResolverLineage() {
* this and all ancestor resolvers.
*/
private ImmutableSet<ContributionBinding> getExplicitBindings(Key requestKey) {
ImmutableSet.Builder<ContributionBinding> explicitBindingsForKey = ImmutableSet.builder();
ImmutableSet.Builder<ContributionBinding> 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<ContributionBinding> getExplicitMultibindingContributions(
Key requestKey) {
ImmutableSet.Builder<ContributionBinding> explicitMultibindingsForKey =
ImmutableSet.builder();
private ImmutableSet<ContributionBinding> getExplicitMultibindings(Key requestKey) {
ImmutableSet.Builder<ContributionBinding> 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<K, V> requests. All
// @IntoMap requests must be for Map<K, Framework<V>>.
multibindings.addAll(
createDelegateBindings(
resolver.delegateMultibindingDeclarations.get(delegateDeclarationKey)));
}
}
return explicitMultibindingsForKey.build();
return multibindings.build();
}

/**
Expand All @@ -710,37 +711,6 @@ private ImmutableSet<MultibindingDeclaration> getMultibindingDeclarations(Key ke
return multibindingDeclarations.build();
}

private ImmutableSet<ContributionBinding> getDelegateBindings(Key requestKey) {
Key delegateDeclarationKey = keyFactory.convertToDelegateKey(requestKey);
ImmutableSet.Builder<ContributionBinding> 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<ContributionBinding> getDelegateMultibindingContributions(
Key requestKey) {
if (MapType.isMap(requestKey) && !MapType.from(requestKey).valuesAreFrameworkType()) {
// There are no @Binds @IntoMap delegate declarations for Map<K, V> requests. All @IntoMap
// requests must be for Map<K, Framework<V>>.
return ImmutableSet.of();
}
Key delegateDeclarationKey = keyFactory.convertToDelegateKey(requestKey);
ImmutableSet.Builder<ContributionBinding> delegateMultibindings = ImmutableSet.builder();
for (Resolver resolver : getResolverLineage()) {
delegateMultibindings.addAll(
createDelegateBindings(
resolver.delegateMultibindingDeclarations.get(delegateDeclarationKey)));
}
return delegateMultibindings.build();
}

private Optional<ResolvedBindings> getPreviouslyResolvedBindings(
final BindingKey bindingKey) {
Optional<ResolvedBindings> result = Optional.fromNullable(resolvedBindings.get(bindingKey));
Expand Down Expand Up @@ -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));
}
}
}
Expand Down

0 comments on commit d697327

Please sign in to comment.