Skip to content

Commit

Permalink
Remove BindingType from BindingGraphFactory resolution logic.
Browse files Browse the repository at this point in the history 
This CL makes a number of changes:

  * Makes `Binding`'s binding type optional at creation time.
  * Makes `BindingFactory` set `Optional.empty()` if a binding type cannot be determined from the binding element.
  * Adds `BindingNode#withBindingType(BindingType)` to allow setting the binding type for certain bindings after they've been created.
  * Implements `BindingGraphTransformations#withFixedBindingTypes(MutableNetwork)` to fix missing binding types after the network is fully constructed.
  * Removes the `BindingGraphFactory.Resolver#createDelegateBindings` logic and replace it with `BindingFactory#delegateBinding`.

RELNOTES=N/A
PiperOrigin-RevId: 684097253
  • Loading branch information
@bcorso
bcorso authored and Dagger Team committed Oct 9, 2024
1 parent e62fb19 commit a03263c
Show file tree
Hide file tree
Showing 23 changed files with 301 additions and 120 deletions.
5 changes: 3 additions & 2 deletions java/dagger/internal/codegen/binding/AssistedFactoryBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import dagger.internal.codegen.model.Key;
import dagger.internal.codegen.model.RequestKind;
import dagger.internal.codegen.xprocessing.Nullability;
import java.util.Optional;

/** A binding for a {@link BindingKind#ASSISTED_FACTORY}. */
@CheckReturnValue
Expand All @@ -37,8 +38,8 @@ public BindingKind kind() {
}

@Override
public BindingType bindingType() {
return BindingType.PROVISION;
public Optional<BindingType> optionalBindingType() {
return Optional.of(BindingType.PROVISION);
}

@Override
Expand Down
5 changes: 3 additions & 2 deletions java/dagger/internal/codegen/binding/AssistedInjectionBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import dagger.internal.codegen.model.BindingKind;
import dagger.internal.codegen.model.DependencyRequest;
import dagger.internal.codegen.xprocessing.Nullability;
import java.util.Optional;

/** A binding for a {@link BindingKind#ASSISTED_INJECTION}. */
@CheckReturnValue
Expand All @@ -39,8 +40,8 @@ public BindingKind kind() {
}

@Override
public BindingType bindingType() {
return BindingType.PROVISION;
public Optional<BindingType> optionalBindingType() {
return Optional.of(BindingType.PROVISION);
}

@Override
Expand Down
10 changes: 9 additions & 1 deletion java/dagger/internal/codegen/binding/Binding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package dagger.internal.codegen.binding;

import dagger.internal.codegen.model.DependencyRequest;
import java.util.Optional;

/**
* An abstract type for classes representing a Dagger binding. Particularly, contains the element
Expand All @@ -25,9 +26,16 @@
* subtypes.
*/
public abstract class Binding extends BindingDeclaration {
/** Returns the optional {@link BindingType}. */
abstract Optional<BindingType> optionalBindingType();

/** The {@link BindingType} of this binding. */
public abstract BindingType bindingType();
public final BindingType bindingType() {
if (optionalBindingType().isPresent()) {
return optionalBindingType().get();
}
throw new AssertionError("bindingType() is not set: " + this);
}

/** The {@link FrameworkType} of this binding. */
public final FrameworkType frameworkType() {
Expand Down
103 changes: 65 additions & 38 deletions java/dagger/internal/codegen/binding/BindingFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,7 @@ public ProductionBinding producesMethodBinding(XMethodElement method, XTypeEleme
public MultiboundMapBinding multiboundMap(
Key key, Iterable<ContributionBinding> multibindingContributions) {
return MultiboundMapBinding.builder()
.bindingType(
multibindingRequiresProduction(key, multibindingContributions)
? BindingType.PRODUCTION
: BindingType.PROVISION)
.optionalBindingType(multibindingBindingType(key, multibindingContributions))
.key(key)
.dependencies(
dependencyRequestFactory.forMultibindingContributions(key, multibindingContributions))
Expand All @@ -257,29 +254,36 @@ public MultiboundMapBinding multiboundMap(
public MultiboundSetBinding multiboundSet(
Key key, Iterable<ContributionBinding> multibindingContributions) {
return MultiboundSetBinding.builder()
.bindingType(
multibindingRequiresProduction(key, multibindingContributions)
? BindingType.PRODUCTION
: BindingType.PROVISION)
.optionalBindingType(multibindingBindingType(key, multibindingContributions))
.key(key)
.dependencies(
dependencyRequestFactory.forMultibindingContributions(key, multibindingContributions))
.build();
}

private boolean multibindingRequiresProduction(
private Optional<BindingType> multibindingBindingType(
Key key, Iterable<ContributionBinding> multibindingContributions) {
if (MapType.isMap(key)) {
MapType mapType = MapType.from(key);
if (mapType.valuesAreTypeOf(TypeNames.PRODUCER)
|| mapType.valuesAreTypeOf(TypeNames.PRODUCED)) {
return true;
return Optional.of(BindingType.PRODUCTION);
}
} else if (SetType.isSet(key) && SetType.from(key).elementsAreTypeOf(TypeNames.PRODUCED)) {
return true;
return Optional.of(BindingType.PRODUCTION);
}
if (Iterables.any(
multibindingContributions,
binding -> binding.optionalBindingType().equals(Optional.of(BindingType.PRODUCTION)))) {
return Optional.of(BindingType.PRODUCTION);
}
return Iterables.any(
multibindingContributions, binding -> binding.bindingType().equals(BindingType.PRODUCTION));
multibindingContributions,
binding -> binding.optionalBindingType().isEmpty())
// If a dependency is missing a BindingType then we can't determine the BindingType of this
// binding yet since it may end up depending on a production type.
? Optional.empty()
: Optional.of(BindingType.PROVISION);
}

/**
Expand Down Expand Up @@ -380,6 +384,11 @@ SubcomponentCreatorBinding subcomponentCreatorBinding(
return SubcomponentCreatorBinding.builder().key(subcomponentDeclaration.key()).build();
}

/** Returns a {@link BindingKind#DELEGATE} binding. */
DelegateBinding delegateBinding(DelegateDeclaration delegateDeclaration) {
return delegateBinding(delegateDeclaration, Optional.empty());
}

/**
* Returns a {@link BindingKind#DELEGATE} binding.
*
Expand All @@ -388,23 +397,31 @@ SubcomponentCreatorBinding subcomponentCreatorBinding(
*/
DelegateBinding delegateBinding(
DelegateDeclaration delegateDeclaration, ContributionBinding actualBinding) {
return delegateBinding(delegateDeclaration, Optional.of(actualBinding));
return delegateBinding(delegateDeclaration, delegateBindingType(Optional.of(actualBinding)));
}

private DelegateBinding delegateBinding(
DelegateDeclaration delegateDeclaration, Optional<ContributionBinding> actualBinding) {
BindingType bindingType = delegateBindingType(actualBinding);
DelegateDeclaration delegateDeclaration, Optional<BindingType> optionalBindingType) {
return DelegateBinding.builder()
.contributionType(delegateDeclaration.contributionType())
.bindingElement(delegateDeclaration.bindingElement().get())
.contributingModule(delegateDeclaration.contributingModule().get())
.delegateRequest(delegateDeclaration.delegateRequest())
.nullability(Nullability.of(delegateDeclaration.bindingElement().get()))
.bindingType(bindingType)
.optionalBindingType(optionalBindingType)
.key(
bindingType == BindingType.PRODUCTION
? keyFactory.forDelegateBinding(delegateDeclaration, TypeNames.PRODUCER)
: keyFactory.forDelegateBinding(delegateDeclaration, TypeNames.PROVIDER))
optionalBindingType.isEmpty()
// This is used by BindingGraphFactory which passes in an empty optionalBindingType.
// In this case, multibound map contributions will always return the key type
// without framework types, i.e. Map<K,V>.
? delegateDeclaration.key()
// This is used by LegacyBindingGraphFactory, which passes in a non-empty
// optionalBindingType. Then, KeyFactory decides whether or not multibound map
// contributions should include the factory type based on the compiler flag,
// -Adagger.useFrameworkTypeInMapMultibindingContributionKey.
: optionalBindingType.get() == BindingType.PRODUCTION
? keyFactory.forDelegateBinding(delegateDeclaration, TypeNames.PRODUCER)
: keyFactory.forDelegateBinding(delegateDeclaration, TypeNames.PROVIDER))
.scope(injectionAnnotations.getScope(delegateDeclaration.bindingElement().get()))
.build();
}
Expand All @@ -414,44 +431,54 @@ private DelegateBinding delegateBinding(
* no binding that satisfies the {@code @Binds} declaration.
*/
public DelegateBinding unresolvedDelegateBinding(DelegateDeclaration delegateDeclaration) {
return delegateBinding(delegateDeclaration, Optional.empty());
return delegateBinding(delegateDeclaration, Optional.of(BindingType.PROVISION));
}

private BindingType delegateBindingType(Optional<ContributionBinding> actualBinding) {
private Optional<BindingType> delegateBindingType(Optional<ContributionBinding> actualBinding) {
if (actualBinding.isEmpty()) {
return BindingType.PROVISION;
return Optional.empty();
}
checkArgument(actualBinding.get().bindingType() != BindingType.MEMBERS_INJECTION);
return actualBinding.get().bindingType();
return Optional.of(actualBinding.get().bindingType());
}

/**
* Returns an {@link BindingKind#OPTIONAL} binding for {@code key}.
*
* @param requestKind the kind of request for the optional binding
* @param underlyingKeyBindings the possibly empty set of bindings that exist in the component for
* the underlying (non-optional) key
*/
/** Returns an {@link BindingKind#OPTIONAL} present binding for {@code key}. */
OptionalBinding syntheticPresentOptionalDeclaration(
Key key, ImmutableCollection<Binding> optionalContributions) {
checkArgument(!optionalContributions.isEmpty());
RequestKind requestKind = getRequestKind(OptionalType.from(key).valueType());
boolean isProduction =
optionalContributions.stream()
.anyMatch(binding -> binding.bindingType() == BindingType.PRODUCTION)
|| requestKind.equals(RequestKind.PRODUCER) // handles producerFromProvider cases
|| requestKind.equals(RequestKind.PRODUCED); // handles producerFromProvider cases
return OptionalBinding.builder()
.bindingType(isProduction ? BindingType.PRODUCTION : BindingType.PROVISION)
.optionalBindingType(presentOptionalBindingType(key, optionalContributions))
.key(key)
.delegateRequest(dependencyRequestFactory.forSyntheticPresentOptionalBinding(key))
.build();
}

private Optional<BindingType> presentOptionalBindingType(
Key key, ImmutableCollection<Binding> optionalContributions) {
RequestKind requestKind = getRequestKind(OptionalType.from(key).valueType());
if (requestKind.equals(RequestKind.PRODUCER) // handles producerFromProvider cases
|| requestKind.equals(RequestKind.PRODUCED)) { // handles producerFromProvider cases
return Optional.of(BindingType.PRODUCTION);
}
if (optionalContributions.stream()
.filter(binding -> binding.optionalBindingType().isPresent())
.anyMatch(binding -> binding.bindingType() == BindingType.PRODUCTION)) {
return Optional.of(BindingType.PRODUCTION);
}
return optionalContributions.stream()
.anyMatch(binding -> binding.optionalBindingType().isEmpty())
// If a dependency is missing a BindingType then we can't determine the BindingType of this
// binding yet since it may end up depending on a production type.
? Optional.empty()
: Optional.of(BindingType.PROVISION);
}

/** Returns an {@link BindingKind#OPTIONAL} absent binding for {@code key}. */
OptionalBinding syntheticAbsentOptionalDeclaration(Key key) {
return OptionalBinding.builder().key(key).bindingType(BindingType.PROVISION).build();
return OptionalBinding.builder()
.key(key)
.optionalBindingType(Optional.of(BindingType.PROVISION))
.build();
}

/** Returns a {@link BindingKind#MEMBERS_INJECTOR} binding. */
Expand Down
43 changes: 2 additions & 41 deletions java/dagger/internal/codegen/binding/BindingGraphFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import dagger.internal.codegen.model.Key;
import dagger.internal.codegen.model.Scope;
import java.util.ArrayDeque;
import java.util.Deque;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
Expand Down Expand Up @@ -122,6 +121,7 @@ private BindingGraph createBindingGraph(
unreachableNodes(network.asGraph(), resolver.componentNode).forEach(network::removeNode);
}

network = BindingGraphTransformations.withFixedBindingTypes(network);
return BindingGraph.create(
ImmutableNetwork.copyOf(network),
createFullBindingGraph);
Expand All @@ -137,7 +137,6 @@ private final class Resolver {
final Map<Key, ResolvedBindings> resolvedContributionBindings = new LinkedHashMap<>();
final Map<Key, ResolvedBindings> resolvedMembersInjectionBindings = new LinkedHashMap<>();
final RequiresResolutionChecker requiresResolutionChecker = new RequiresResolutionChecker();
final Deque<Key> cycleStack = new ArrayDeque<>();
final Queue<ComponentDescriptor> subcomponentsToResolve = new ArrayDeque<>();

Resolver(ComponentDescriptor componentDescriptor) {
Expand Down Expand Up @@ -383,48 +382,10 @@ private ImmutableSet<ContributionBinding> createDelegateBindings(
ImmutableSet<DelegateDeclaration> delegateDeclarations) {
ImmutableSet.Builder<ContributionBinding> builder = ImmutableSet.builder();
for (DelegateDeclaration delegateDeclaration : delegateDeclarations) {
builder.add(createDelegateBinding(delegateDeclaration));
builder.add(bindingFactory.delegateBinding(delegateDeclaration));
}
return builder.build();
}

/**
* Creates one (and only one) delegate binding for a delegate declaration, based on the resolved
* bindings of the right-hand-side of a {@link dagger.Binds} method. If there are duplicate
* bindings for the dependency key, there should still be only one binding for the delegate key.
*/
private ContributionBinding createDelegateBinding(DelegateDeclaration delegateDeclaration) {
Key delegateKey = delegateDeclaration.delegateRequest().key();
if (cycleStack.contains(delegateKey)) {
return bindingFactory.unresolvedDelegateBinding(delegateDeclaration);
}

ResolvedBindings resolvedDelegate;
try {
cycleStack.push(delegateKey);
resolvedDelegate = lookUpBindings(delegateKey);
} finally {
cycleStack.pop();
}
if (resolvedDelegate.bindings().isEmpty()) {
// This is guaranteed to result in a missing binding error, so it doesn't matter if the
// binding is a Provision or Production, except if it is a @IntoMap method, in which
// case the key will be of type Map<K, Provider<V>>, which will be "upgraded" into a
// Map<K, Producer<V>> if it's requested in a ProductionComponent. This may result in a
// strange error, that the RHS needs to be provided with an @Inject or @Provides
// annotated method, but a user should be able to figure out if a @Produces annotation
// is needed.
// TODO(gak): revisit how we model missing delegates if/when we clean up how we model
// binding declarations
return bindingFactory.unresolvedDelegateBinding(delegateDeclaration);
}
// It doesn't matter which of these is selected, since they will later on produce a
// duplicate binding error.
ContributionBinding explicitDelegate =
(ContributionBinding) resolvedDelegate.bindings().iterator().next();
return bindingFactory.delegateBinding(delegateDeclaration, explicitDelegate);
}

/**
* Returns a {@link BindingNode} for the given binding that is owned by an ancestor component,
* if one exists. Otherwise returns {@link Optional#empty()}.
Expand Down
Loading

0 comments on commit a03263c

Please sign in to comment.