From f4b45197f95133f286860674467b74506aefc79c Mon Sep 17 00:00:00 2001 From: Brad Corso Date: Thu, 7 Dec 2023 11:46:49 -0800 Subject: [PATCH] Internal changes RELNOTES=N/A PiperOrigin-RevId: 588861836 --- .../internal/codegen/base/TarjanSCCs.java | 7 +- .../codegen/binding/BindingGraph.java | 2 +- .../codegen/binding/BindingGraphFactory.java | 88 ++++++++++++------- 3 files changed, 59 insertions(+), 38 deletions(-) diff --git a/java/dagger/internal/codegen/base/TarjanSCCs.java b/java/dagger/internal/codegen/base/TarjanSCCs.java index ab9a0fdae79..b089333b076 100644 --- a/java/dagger/internal/codegen/base/TarjanSCCs.java +++ b/java/dagger/internal/codegen/base/TarjanSCCs.java @@ -20,6 +20,7 @@ import static java.lang.Math.min; import com.google.common.collect.ImmutableCollection; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -39,7 +40,7 @@ public final class TarjanSCCs { /** Returns the set of strongly connected components in reverse topological order. */ - public static ImmutableSet> compute( + public static ImmutableList> compute( ImmutableCollection nodes, SuccessorsFunction successorsFunction) { return new TarjanSCC<>(nodes, successorsFunction).compute(); } @@ -62,14 +63,14 @@ private static class TarjanSCC { this.lowLinks = Maps.newHashMapWithExpectedSize(nodes.size()); } - private ImmutableSet> compute() { + private ImmutableList> compute() { checkState(indexes.isEmpty(), "TarjanSCC#compute() can only be called once per instance!"); for (NodeT node : nodes) { if (!indexes.containsKey(node)) { stronglyConnect(node); } } - return ImmutableSet.copyOf(stronglyConnectedComponents); + return ImmutableList.copyOf(stronglyConnectedComponents); } private void stronglyConnect(NodeT node) { diff --git a/java/dagger/internal/codegen/binding/BindingGraph.java b/java/dagger/internal/codegen/binding/BindingGraph.java index 092f8ff9a46..0090b3d2ac2 100644 --- a/java/dagger/internal/codegen/binding/BindingGraph.java +++ b/java/dagger/internal/codegen/binding/BindingGraph.java @@ -149,7 +149,7 @@ Comparator nodeOrder() { /** Returns the set of strongly connected nodes in this graph in reverse topological order. */ @Memoized - public ImmutableSet> stronglyConnectedNodes() { + public ImmutableList> stronglyConnectedNodes() { return TarjanSCCs.compute( ImmutableSet.copyOf(network().nodes()), // NetworkBuilder does not have a stable successor order, so we have to roll our own diff --git a/java/dagger/internal/codegen/binding/BindingGraphFactory.java b/java/dagger/internal/codegen/binding/BindingGraphFactory.java index 72eb17d80c0..9410010449f 100644 --- a/java/dagger/internal/codegen/binding/BindingGraphFactory.java +++ b/java/dagger/internal/codegen/binding/BindingGraphFactory.java @@ -648,8 +648,7 @@ private ContributionBinding createDelegateBinding(DelegateDeclaration delegateDe * ResolvedBindings#owningComponent(ContributionBinding)}. */ private XTypeElement getOwningComponent(Key requestKey, ContributionBinding binding) { - if (isResolvedInParent(requestKey, binding) - && !new RequiresResolutionChecker().requiresResolution(binding)) { + if (isResolvedInParent(requestKey, binding) && !requiresResolution(binding)) { ResolvedBindings parentResolvedBindings = parentResolver.get().resolvedContributionBindings.get(requestKey); return parentResolvedBindings.owningComponent(binding); @@ -876,7 +875,7 @@ void resolve(Key key) { previouslyResolvedBindings.bindings().stream() .anyMatch(binding -> binding.kind() == BindingKind.ASSISTED_INJECTION); if (!isAssistedInjectionBinding - && !new RequiresResolutionChecker().requiresResolution(key) + && !requiresResolution(key) && getLocalExplicitBindings(key).isEmpty()) { /* Cache the inherited parent component's bindings in case resolving at the parent found * bindings in some component between this one and the previously-resolved one. */ @@ -921,7 +920,15 @@ private ResolvedBindings getResolvedMembersInjectionBindings(Key key) { return resolvedMembersInjectionBindings.get(key); } - private final class RequiresResolutionChecker { + private boolean requiresResolution(Key key) { + return new LegacyRequiresResolutionChecker().requiresResolution(key); + } + + private boolean requiresResolution(Binding binding) { + return new LegacyRequiresResolutionChecker().requiresResolution(binding); + } + + private final class LegacyRequiresResolutionChecker { private final Set cycleChecker = new HashSet<>(); /** @@ -969,9 +976,7 @@ private boolean requiresResolutionUncached(Key key) { Resolver.this, key); ResolvedBindings previouslyResolvedBindings = getPreviouslyResolvedBindings(key).get(); - if (hasLocalMultibindingContributions(key) - || hasLocalOptionalBindingContribution(previouslyResolvedBindings) - ) { + if (hasLocalBindings(previouslyResolvedBindings)) { return true; } @@ -995,35 +1000,50 @@ private boolean requiresResolutionUncached(Binding binding) { } return false; } + } - /** - * Returns {@code true} if there is at least one multibinding contribution declared within - * this component's modules that matches the key. - */ - private boolean hasLocalMultibindingContributions(Key requestKey) { - return keysMatchingRequest(requestKey) - .stream() - .anyMatch(key -> !getLocalExplicitMultibindings(key).isEmpty()); - } + private boolean hasLocalBindings(Binding binding) { + return hasLocalMultibindingContributions(binding.key()) + || hasLocalOptionalBindingContribution( + binding.key(), ImmutableSet.of((ContributionBinding) binding)); + } - /** - * Returns {@code true} if there is a contribution in this component for an {@code - * Optional} key that has not been contributed in a parent. - */ - private boolean hasLocalOptionalBindingContribution(ResolvedBindings resolvedBindings) { - if (resolvedBindings - .contributionBindings() - .stream() - .map(ContributionBinding::kind) - .anyMatch(isEqual(OPTIONAL))) { - return !getLocalExplicitBindings(keyFactory.unwrapOptional(resolvedBindings.key()).get()) - .isEmpty(); - } else { - // If a parent contributes a @Provides Optional binding and a child has a - // @BindsOptionalOf Foo method, the two should conflict, even if there is no binding for - // Foo on its own - return !getOptionalBindingDeclarations(resolvedBindings.key()).isEmpty(); - } + private boolean hasLocalBindings(ResolvedBindings resolvedBindings) { + return hasLocalMultibindingContributions(resolvedBindings.key()) + || hasLocalOptionalBindingContribution(resolvedBindings); + } + + /** + * Returns {@code true} if there is at least one multibinding contribution declared within + * this component's modules that matches the key. + */ + private boolean hasLocalMultibindingContributions(Key requestKey) { + return keysMatchingRequest(requestKey) + .stream() + .anyMatch(key -> !getLocalExplicitMultibindings(key).isEmpty()); + } + + /** + * Returns {@code true} if there is a contribution in this component for an {@code + * Optional} key that has not been contributed in a parent. + */ + private boolean hasLocalOptionalBindingContribution(ResolvedBindings resolvedBindings) { + return hasLocalOptionalBindingContribution( + resolvedBindings.key(), resolvedBindings.contributionBindings()); + } + + private boolean hasLocalOptionalBindingContribution( + Key key, ImmutableSet previousContributionBindings) { + if (previousContributionBindings.stream() + .map(ContributionBinding::kind) + .anyMatch(isEqual(OPTIONAL))) { + return !getLocalExplicitBindings(keyFactory.unwrapOptional(key).get()) + .isEmpty(); + } else { + // If a parent contributes a @Provides Optional binding and a child has a + // @BindsOptionalOf Foo method, the two should conflict, even if there is no binding for + // Foo on its own + return !getOptionalBindingDeclarations(key).isEmpty(); } } }