Refactor ResolvedBindings and LegacyBindingGraph to include the ComponentPath.

Including the ComponentPath in ResolvedBindings avoids having to create new objects
of type ResolvedBindingsWithPath to satisfy our Map key. In addition, passing the ComponentPath while creating LegacyBindingGraph avoids having to recreate it from scratch during BindingGraphConverter phase.

RELNOTES=N/A
PiperOrigin-RevId: 570137117

Author: bcorso

java/dagger/internal/codegen/binding/BindingGraphConverter.java

@@ -19,7 +19,6 @@
 import static com.google.common.base.Verify.verify;
 import static dagger.internal.codegen.binding.BindingRequest.bindingRequest;
 import static dagger.internal.codegen.extension.DaggerGraphs.unreachableNodes;
-import static dagger.internal.codegen.extension.DaggerStreams.toImmutableList;
 import static dagger.internal.codegen.model.BindingKind.SUBCOMPONENT_CREATOR;
 
 import androidx.room.compiler.processing.XMethodElement;
@@ -29,11 +28,9 @@
 import com.google.auto.value.extension.memoized.Memoized;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Iterators;
 import com.google.common.graph.ImmutableNetwork;
 import com.google.common.graph.MutableNetwork;
-import com.google.common.graph.Network;
 import com.google.common.graph.NetworkBuilder;
 import dagger.internal.codegen.binding.BindingGraph.TopLevelBindingGraph;
 import dagger.internal.codegen.binding.ComponentDescriptor.ComponentMethodDescriptor;
@@ -70,7 +67,7 @@ final class BindingGraphConverter {
    */
   BindingGraph convert(LegacyBindingGraph legacyBindingGraph, boolean isFullBindingGraph) {
     MutableNetwork<Node, Edge> network = asNetwork(legacyBindingGraph);
-    ComponentNode rootNode = rootComponentNode(network);
+    ComponentNode rootNode = legacyBindingGraph.componentNode();
 
     // When bindings are copied down into child graphs because they transitively depend on local
     // multibindings or optional bindings, the parent-owned binding is still there. If that
@@ -92,47 +89,19 @@ private MutableNetwork<Node, Edge> asNetwork(LegacyBindingGraph graph) {
     return converter.network;
   }
 
-  // TODO(dpb): Example of BindingGraph logic applied to derived networks.
-  private ComponentNode rootComponentNode(Network<Node, Edge> network) {
-    return (ComponentNode)
-        Iterables.find(
-            network.nodes(),
-            node -> node instanceof ComponentNode && node.componentPath().atRoot());
-  }
-
-  /**
-   * Used as a cache key to make sure resolved bindings are cached per component path.
-   * This is required so that binding nodes are not reused across different branches of the
-   * graph since the ResolvedBindings class only contains the component and not the path.
-   */
-  @AutoValue
-  abstract static class ResolvedBindingsWithPath {
-    abstract ResolvedBindings resolvedBindings();
-    abstract ComponentPath componentPath();
-
-    static ResolvedBindingsWithPath create(
-        ResolvedBindings resolvedBindings, ComponentPath componentPath) {
-      return new AutoValue_BindingGraphConverter_ResolvedBindingsWithPath(
-          resolvedBindings, componentPath);
-    }
-  }
-
   private final class Converter {
     /** The path from the root graph to the currently visited graph. */
     private final Deque<LegacyBindingGraph> bindingGraphPath = new ArrayDeque<>();
 
-    /** The {@link ComponentPath} for each component in {@link #bindingGraphPath}. */
-    private final Deque<ComponentPath> componentPaths = new ArrayDeque<>();
-
     private final MutableNetwork<Node, Edge> network =
         NetworkBuilder.directed().allowsParallelEdges(true).allowsSelfLoops(true).build();
     private final Set<BindingNode> bindings = new HashSet<>();
 
-    private final Map<ResolvedBindingsWithPath, ImmutableSet<BindingNode>> resolvedBindingsMap =
+    private final Map<ResolvedBindings, ImmutableSet<BindingNode>> resolvedBindingsMap =
         new HashMap<>();
 
     private void visitRootComponent(LegacyBindingGraph graph) {
-      visitComponent(graph, null);
+      visitComponent(graph);
     }
 
     /**
@@ -145,30 +114,20 @@ private void visitRootComponent(LegacyBindingGraph graph) {
      *       {@link #visitSubcomponentFactoryMethod(ComponentNode, ComponentNode, XMethodElement)}.
      *   <li>For each entry point in the component, calls {@link #visitEntryPoint(ComponentNode,
      *       DependencyRequest)}.
-     *   <li>For each child component, calls {@link #visitComponent(LegacyBindingGraph,
-     *       ComponentNode)}, updating the traversal state.
+     *   <li>For each child component, calls {@link #visitComponent(LegacyBindingGraph)},
+     *       updating the traversal state.
      * </ol>
      *
      * @param graph the currently visited graph
      */
-    private void visitComponent(LegacyBindingGraph graph, ComponentNode parentComponent) {
+    private void visitComponent(LegacyBindingGraph graph) {
       bindingGraphPath.addLast(graph);
-      ComponentPath graphPath =
-          ComponentPath.create(
-              bindingGraphPath.stream()
-                  .map(LegacyBindingGraph::componentDescriptor)
-                  .map(ComponentDescriptor::typeElement)
-                  .map(DaggerTypeElement::from)
-                  .collect(toImmutableList()));
-      componentPaths.addLast(graphPath);
-      ComponentNode currentComponent =
-          ComponentNodeImpl.create(componentPath(), graph.componentDescriptor());
-
-      network.addNode(currentComponent);
+
+      network.addNode(graph.componentNode());
 
       for (ComponentMethodDescriptor entryPointMethod :
           graph.componentDescriptor().entryPointMethods()) {
-        visitEntryPoint(currentComponent, entryPointMethod.dependencyRequest().get());
+        visitEntryPoint(graph.componentNode(), entryPointMethod.dependencyRequest().get());
       }
 
       for (ResolvedBindings resolvedBindings : graph.resolvedBindings()) {
@@ -180,7 +139,7 @@ private void visitComponent(LegacyBindingGraph graph, ComponentNode parentCompon
             }
           }
           if (binding.kind().equals(SUBCOMPONENT_CREATOR)
-              && binding.componentPath().equals(currentComponent.componentPath())) {
+              && binding.componentPath().equals(graph.componentPath())) {
             network.addEdge(
                 binding,
                 subcomponentNode(binding.key().type().xprocessing(), graph),
@@ -191,22 +150,23 @@ private void visitComponent(LegacyBindingGraph graph, ComponentNode parentCompon
       }
 
       if (bindingGraphPath.size() > 1) {
-        LegacyBindingGraph parent = Iterators.get(bindingGraphPath.descendingIterator(), 1);
-        parent
+        LegacyBindingGraph parentGraph = Iterators.get(bindingGraphPath.descendingIterator(), 1);
+        parentGraph
             .componentDescriptor()
             .getFactoryMethodForChildComponent(graph.componentDescriptor())
             .ifPresent(
                 childFactoryMethod ->
                     visitSubcomponentFactoryMethod(
-                        parentComponent, currentComponent, childFactoryMethod.methodElement()));
+                        parentGraph.componentNode(),
+                        graph.componentNode(),
+                        childFactoryMethod.methodElement()));
       }
 
       for (LegacyBindingGraph child : graph.subgraphs()) {
-        visitComponent(child, currentComponent);
+        visitComponent(child);
       }
 
       verify(bindingGraphPath.removeLast().equals(graph));
-      verify(componentPaths.removeLast().equals(graphPath));
     }
 
     /**
@@ -242,17 +202,17 @@ private void visitSubcomponentFactoryMethod(
      * component.
      */
     private ComponentPath componentPath() {
-      return componentPaths.getLast();
+      return bindingGraphPath.getLast().componentPath();
     }
 
     /**
      * Returns the subpath from the root component to the matching {@code ancestor} of the current
      * component.
      */
     private ComponentPath pathFromRootToAncestor(XTypeElement ancestor) {
-      for (ComponentPath componentPath : componentPaths) {
-        if (componentPath.currentComponent().xprocessing().equals(ancestor)) {
-          return componentPath;
+      for (LegacyBindingGraph graph : bindingGraphPath) {
+        if (graph.componentDescriptor().typeElement().equals(ancestor)) {
+          return graph.componentPath();
         }
       }
       throw new IllegalArgumentException(
@@ -325,23 +285,18 @@ private ResolvedBindings resolvedDependencies(
     }
 
     private ImmutableSet<BindingNode> bindingNodes(ResolvedBindings resolvedBindings) {
-      ResolvedBindingsWithPath resolvedBindingsWithPath =
-          ResolvedBindingsWithPath.create(resolvedBindings, componentPath());
-      return resolvedBindingsMap.computeIfAbsent(
-          resolvedBindingsWithPath, this::uncachedBindingNodes);
+      return resolvedBindingsMap.computeIfAbsent(resolvedBindings, this::uncachedBindingNodes);
     }
 
-    private ImmutableSet<BindingNode> uncachedBindingNodes(
-        ResolvedBindingsWithPath resolvedBindingsWithPath) {
+    private ImmutableSet<BindingNode> uncachedBindingNodes(ResolvedBindings resolvedBindings) {
       ImmutableSet.Builder<BindingNode> bindingNodes = ImmutableSet.builder();
-      resolvedBindingsWithPath.resolvedBindings()
+      resolvedBindings
           .allBindings()
           .asMap()
           .forEach(
               (component, bindings) -> {
                 for (Binding binding : bindings) {
-                  bindingNodes.add(
-                      bindingNode(resolvedBindingsWithPath.resolvedBindings(), binding, component));
+                  bindingNodes.add(bindingNode(resolvedBindings, binding, component));
                 }
               });
       return bindingNodes.build();

java/dagger/internal/codegen/binding/BindingGraphFactory.java

@@ -51,6 +51,8 @@
 import dagger.internal.codegen.base.OptionalType;
 import dagger.internal.codegen.compileroption.CompilerOptions;
 import dagger.internal.codegen.javapoet.TypeNames;
+import dagger.internal.codegen.model.ComponentPath;
+import dagger.internal.codegen.model.DaggerTypeElement;
 import dagger.internal.codegen.model.DependencyRequest;
 import dagger.internal.codegen.model.Key;
 import dagger.internal.codegen.model.Scope;
@@ -190,8 +192,14 @@ private LegacyBindingGraph createLegacyBindingGraph(
       optionalsBuilder.addAll(moduleDescriptor.optionalDeclarations());
     }
 
+    DaggerTypeElement component = DaggerTypeElement.from(componentDescriptor.typeElement());
+    ComponentPath componentPath =
+        parentResolver.isPresent()
+            ? parentResolver.get().componentPath.childPath(component)
+            : ComponentPath.create(ImmutableList.of(component));
     final Resolver requestResolver =
         new Resolver(
+            componentPath,
             parentResolver,
             componentDescriptor,
             indexBindingDeclarationsByKey(explicitBindingsBuilder.build()),
@@ -237,6 +245,7 @@ private LegacyBindingGraph createLegacyBindingGraph(
     }
 
     return new LegacyBindingGraph(
+        componentPath,
         componentDescriptor,
         ImmutableMap.copyOf(requestResolver.getResolvedContributionBindings()),
         ImmutableMap.copyOf(requestResolver.getResolvedMembersInjectionBindings()),
@@ -300,6 +309,7 @@ public void clearCache() {
   }
 
   private final class Resolver {
+    final ComponentPath componentPath;
     final Optional<Resolver> parentResolver;
     final ComponentDescriptor componentDescriptor;
     final ImmutableSetMultimap<Key, ContributionBinding> explicitBindings;
@@ -318,13 +328,15 @@ private final class Resolver {
     final Queue<ComponentDescriptor> subcomponentsToResolve = new ArrayDeque<>();
 
     Resolver(
+        ComponentPath componentPath,
         Optional<Resolver> parentResolver,
         ComponentDescriptor componentDescriptor,
         ImmutableSetMultimap<Key, ContributionBinding> explicitBindings,
         ImmutableSetMultimap<Key, MultibindingDeclaration> multibindingDeclarations,
         ImmutableSetMultimap<Key, SubcomponentDeclaration> subcomponentDeclarations,
         ImmutableSetMultimap<Key, DelegateDeclaration> delegateDeclarations,
         ImmutableSetMultimap<Key, OptionalBindingDeclaration> optionalBindingDeclarations) {
+      this.componentPath = componentPath;
       this.parentResolver = parentResolver;
       this.componentDescriptor = checkNotNull(componentDescriptor);
       this.explicitBindings = checkNotNull(explicitBindings);
@@ -428,6 +440,7 @@ && isAssistedFactoryType(requestKey.type().xprocessing().getTypeElement())) {
       }
 
       return ResolvedBindings.forContributionBindings(
+          componentPath,
           requestKey,
           Multimaps.index(bindings, binding -> getOwningComponent(requestKey, binding)),
           multibindingDeclarations,
@@ -466,8 +479,8 @@ ResolvedBindings lookUpMembersInjectionBinding(Key requestKey) {
           injectBindingRegistry.getOrFindMembersInjectionBinding(requestKey);
       return binding.isPresent()
           ? ResolvedBindings.forMembersInjectionBinding(
-              requestKey, componentDescriptor, binding.get())
-          : ResolvedBindings.noBindings(requestKey);
+              componentPath, requestKey, componentDescriptor, binding.get())
+          : ResolvedBindings.noBindings(componentPath, requestKey);
     }
 
     /**

java/dagger/internal/codegen/binding/LegacyBindingGraph.java

@@ -22,6 +22,8 @@
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Multimaps;
+import dagger.internal.codegen.model.BindingGraph.ComponentNode;
+import dagger.internal.codegen.model.ComponentPath;
 import dagger.internal.codegen.model.Key;
 import dagger.internal.codegen.model.RequestKind;
 import java.util.Collection;
@@ -30,22 +32,33 @@
 // TODO(bcorso): Remove the LegacyBindingGraph after we've migrated to the new BindingGraph.
 /** The canonical representation of a full-resolved graph. */
 final class LegacyBindingGraph {
+  private final ComponentNode componentNode;
   private final ComponentDescriptor componentDescriptor;
   private final ImmutableMap<Key, ResolvedBindings> contributionBindings;
   private final ImmutableMap<Key, ResolvedBindings> membersInjectionBindings;
   private final ImmutableList<LegacyBindingGraph> subgraphs;
 
   LegacyBindingGraph(
+      ComponentPath componentPath,
       ComponentDescriptor componentDescriptor,
       ImmutableMap<Key, ResolvedBindings> contributionBindings,
       ImmutableMap<Key, ResolvedBindings> membersInjectionBindings,
       ImmutableList<LegacyBindingGraph> subgraphs) {
+    this.componentNode = ComponentNodeImpl.create(componentPath, componentDescriptor);
     this.componentDescriptor = componentDescriptor;
     this.contributionBindings = contributionBindings;
     this.membersInjectionBindings = membersInjectionBindings;
     this.subgraphs = checkForDuplicates(subgraphs);
   }
 
+  ComponentNode componentNode() {
+    return componentNode;
+  }
+
+  ComponentPath componentPath() {
+    return componentNode.componentPath();
+  }
+
   ComponentDescriptor componentDescriptor() {
     return componentDescriptor;
   }

java/dagger/internal/codegen/binding/ResolvedBindings.java

@@ -27,6 +27,7 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.collect.Multimap;
+import dagger.internal.codegen.model.ComponentPath;
 import dagger.internal.codegen.model.Key;
 
 /**
@@ -40,6 +41,9 @@
  */
 @AutoValue
 abstract class ResolvedBindings {
+  /** The component path for the resolved bindings. */
+  abstract ComponentPath componentPath();
+
   /** The binding key for which the {@link #bindings()} have been resolved. */
   abstract Key key();
 
@@ -127,12 +131,14 @@ final XTypeElement owningComponent(ContributionBinding binding) {
 
   /** Creates a {@link ResolvedBindings} for contribution bindings. */
   static ResolvedBindings forContributionBindings(
+      ComponentPath componentPath,
       Key key,
       Multimap<XTypeElement, ContributionBinding> contributionBindings,
       Iterable<MultibindingDeclaration> multibindings,
       Iterable<SubcomponentDeclaration> subcomponentDeclarations,
       Iterable<OptionalBindingDeclaration> optionalBindingDeclarations) {
     return new AutoValue_ResolvedBindings(
+        componentPath,
         key,
         ImmutableSetMultimap.copyOf(contributionBindings),
         ImmutableMap.of(),
@@ -145,10 +151,12 @@ static ResolvedBindings forContributionBindings(
    * Creates a {@link ResolvedBindings} for members injection bindings.
    */
   static ResolvedBindings forMembersInjectionBinding(
+      ComponentPath componentPath,
       Key key,
       ComponentDescriptor owningComponent,
       MembersInjectionBinding ownedMembersInjectionBinding) {
     return new AutoValue_ResolvedBindings(
+        componentPath,
         key,
         ImmutableSetMultimap.of(),
         ImmutableMap.of(owningComponent.typeElement(), ownedMembersInjectionBinding),
@@ -160,8 +168,9 @@ static ResolvedBindings forMembersInjectionBinding(
   /**
    * Creates a {@link ResolvedBindings} appropriate for when there are no bindings for the key.
    */
-  static ResolvedBindings noBindings(Key key) {
+  static ResolvedBindings noBindings(ComponentPath componentPath, Key key) {
     return new AutoValue_ResolvedBindings(
+        componentPath,
         key,
         ImmutableSetMultimap.of(),
         ImmutableMap.of(),