Print component name for each binding along dependency path when requ…

…esting binding from wrong component.

RELNOTES="Print component name for each binding along dependency path when requesting binding from wrong component."
PiperOrigin-RevId: 378977829

java/dagger/internal/codegen/bindinggraphvalidation/MissingBindingValidator.java

@@ -17,22 +17,33 @@
 package dagger.internal.codegen.bindinggraphvalidation;
 
 import static com.google.common.base.Verify.verify;
+import static com.google.common.collect.Iterables.getLast;
 import static dagger.internal.codegen.base.Keys.isValidImplicitProvisionKey;
 import static dagger.internal.codegen.base.Keys.isValidMembersInjectionKey;
 import static dagger.internal.codegen.base.RequestKinds.canBeSatisfiedByProductionBinding;
+import static dagger.internal.codegen.binding.DependencyRequestFormatter.DOUBLE_INDENT;
 import static dagger.internal.codegen.extension.DaggerStreams.instancesOf;
 import static javax.tools.Diagnostic.Kind.ERROR;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import dagger.internal.codegen.binding.DependencyRequestFormatter;
 import dagger.internal.codegen.binding.InjectBindingRegistry;
 import dagger.internal.codegen.langmodel.DaggerTypes;
+import dagger.internal.codegen.validation.DiagnosticMessageGenerator;
+import dagger.model.Binding;
 import dagger.model.BindingGraph;
 import dagger.model.BindingGraph.ComponentNode;
 import dagger.model.BindingGraph.DependencyEdge;
+import dagger.model.BindingGraph.Edge;
 import dagger.model.BindingGraph.MissingBinding;
 import dagger.model.BindingGraph.Node;
+import dagger.model.ComponentPath;
 import dagger.model.Key;
 import dagger.spi.BindingGraphPlugin;
 import dagger.spi.DiagnosticReporter;
+import java.util.List;
+import java.util.stream.Collectors;
 import javax.inject.Inject;
 import javax.lang.model.type.TypeKind;
 
@@ -41,12 +52,19 @@ final class MissingBindingValidator implements BindingGraphPlugin {
 
   private final DaggerTypes types;
   private final InjectBindingRegistry injectBindingRegistry;
+  private final DependencyRequestFormatter dependencyRequestFormatter;
+  private final DiagnosticMessageGenerator.Factory diagnosticMessageGeneratorFactory;
 
   @Inject
   MissingBindingValidator(
-      DaggerTypes types, InjectBindingRegistry injectBindingRegistry) {
+      DaggerTypes types,
+      InjectBindingRegistry injectBindingRegistry,
+      DependencyRequestFormatter dependencyRequestFormatter,
+      DiagnosticMessageGenerator.Factory diagnosticMessageGeneratorFactory) {
     this.types = types;
     this.injectBindingRegistry = injectBindingRegistry;
+    this.dependencyRequestFormatter = dependencyRequestFormatter;
+    this.diagnosticMessageGeneratorFactory = diagnosticMessageGeneratorFactory;
   }
 
   @Override
@@ -68,8 +86,23 @@ public void visitGraph(BindingGraph graph, DiagnosticReporter diagnosticReporter
 
   private void reportMissingBinding(
       MissingBinding missingBinding, BindingGraph graph, DiagnosticReporter diagnosticReporter) {
-    diagnosticReporter.reportBinding(
-        ERROR, missingBinding, missingBindingErrorMessage(missingBinding, graph));
+    List<ComponentPath> alternativeComponents =
+        graph.bindings(missingBinding.key()).stream()
+            .map(Binding::componentPath)
+            .distinct()
+            .collect(Collectors.toList());
+    // Print component name for each binding along the dependency path if the missing binding
+    // exists in a different component than expected
+    if (alternativeComponents.isEmpty()) {
+      diagnosticReporter.reportBinding(
+          ERROR, missingBinding, missingBindingErrorMessage(missingBinding, graph));
+    } else {
+      diagnosticReporter.reportComponent(
+          ERROR,
+          graph.componentNode(missingBinding.componentPath()).get(),
+          missingBindingErrorMessage(missingBinding, graph)
+              + wrongComponentErrorMessage(missingBinding, alternativeComponents, graph));
+    }
   }
 
   private String missingBindingErrorMessage(MissingBinding missingBinding, BindingGraph graph) {
@@ -91,17 +124,63 @@ private String missingBindingErrorMessage(MissingBinding missingBinding, Binding
       errorMessage.append(
           " This type supports members injection but cannot be implicitly provided.");
     }
-    graph.bindings(key).stream()
-        .map(binding -> binding.componentPath().currentComponent())
-        .distinct()
-        .forEach(
-            component ->
-                errorMessage
-                    .append("\nA binding with matching key exists in component: ")
-                    .append(component.getQualifiedName()));
     return errorMessage.toString();
   }
 
+  private String wrongComponentErrorMessage(
+      MissingBinding missingBinding,
+      List<ComponentPath> alternativeComponentPath,
+      BindingGraph graph) {
+    ImmutableSet<DependencyEdge> entryPoints =
+        graph.entryPointEdgesDependingOnBinding(missingBinding);
+    DiagnosticMessageGenerator generator = diagnosticMessageGeneratorFactory.create(graph);
+    ImmutableList<DependencyEdge> dependencyTrace =
+        generator.dependencyTrace(missingBinding, entryPoints);
+    StringBuilder message =
+        graph.isFullBindingGraph()
+            ? new StringBuilder()
+            : new StringBuilder(dependencyTrace.size() * 100 /* a guess heuristic */);
+    // Check in which component the missing binding is requested. This can be different from the
+    // component the missing binding is in because we'll try to search up the parent components for
+    // a binding which makes missing bindings end up at the root component. This is different from
+    // the place we are logically requesting the binding from. Note that this is related to the
+    // particular dependency trace being shown and so is not necessarily stable.
+    String missingComponentName =
+        getComponentFromDependencyEdge(dependencyTrace.get(0), graph, false);
+    boolean hasSameComponentName = false;
+    for (ComponentPath component : alternativeComponentPath) {
+      message.append("\nA binding for ").append(missingBinding.key()).append(" exists in ");
+      if (component.currentComponent().getQualifiedName().contentEquals(missingComponentName)) {
+        hasSameComponentName = true;
+        message.append("[").append(component).append("]");
+      } else {
+        message.append(component.currentComponent().getQualifiedName());
+      }
+      message.append(":");
+    }
+    for (DependencyEdge edge : dependencyTrace) {
+      String line = dependencyRequestFormatter.format(edge.dependencyRequest());
+      if (line.isEmpty()) {
+        continue;
+      }
+      // If we ran into a rare case where the component names collide and we need to show the full
+      // path, only show the full path for the first dependency request. This is guaranteed to be
+      // the component in question since the logic for checking for a collision uses the first
+      // edge in the trace. Do not expand subsequent component paths to reduce spam.
+      String componentName =
+          String.format("[%s] ", getComponentFromDependencyEdge(edge, graph, hasSameComponentName));
+      hasSameComponentName = false;
+      message.append("\n").append(line.replace(DOUBLE_INDENT, DOUBLE_INDENT + componentName));
+    }
+    if (!dependencyTrace.isEmpty()) {
+      generator.appendComponentPathUnlessAtRoot(message, source(getLast(dependencyTrace), graph));
+    }
+    message.append(
+        generator.getRequestsNotInTrace(
+            dependencyTrace, generator.requests(missingBinding), entryPoints));
+    return message.toString();
+  }
+
   private boolean allIncomingDependenciesCanUseProduction(
       MissingBinding missingBinding, BindingGraph graph) {
     return graph.network().inEdges(missingBinding).stream()
@@ -129,4 +208,16 @@ private boolean typeHasInjectionSites(Key key) {
         .map(binding -> !binding.injectionSites().isEmpty())
         .orElse(false);
   }
+
+  private static String getComponentFromDependencyEdge(
+      DependencyEdge edge, BindingGraph graph, boolean completePath) {
+    ComponentPath componentPath = graph.network().incidentNodes(edge).source().componentPath();
+    return completePath
+        ? componentPath.toString()
+        : componentPath.currentComponent().getQualifiedName().toString();
+  }
+
+  private Node source(Edge edge, BindingGraph graph) {
+    return graph.network().incidentNodes(edge).source();
+  }
 }

java/dagger/internal/codegen/validation/DiagnosticMessageGenerator.java

@@ -178,7 +178,15 @@ private String getMessageInternal(
         appendComponentPathUnlessAtRoot(message, source(getLast(dependencyTrace)));
       }
     }
+    message.append(getRequestsNotInTrace(dependencyTrace, requests, entryPoints));
+    return message.toString();
+  }
 
+  public String getRequestsNotInTrace(
+      ImmutableList<DependencyEdge> dependencyTrace,
+      ImmutableSet<DependencyEdge> requests,
+      ImmutableSet<DependencyEdge> entryPoints) {
+    StringBuilder message = new StringBuilder();
     // Print any dependency requests that aren't shown as part of the dependency trace.
     ImmutableSet<Element> requestsToPrint =
         requests.stream()
@@ -254,7 +262,7 @@ private static boolean isTracedRequest(
    */
   // TODO(ronshapiro): Adding a DependencyPath type to dagger.model could be useful, i.e.
   // bindingGraph.shortestPathFromEntryPoint(DependencyEdge, MaybeBindingNode)
-  ImmutableList<DependencyEdge> dependencyTrace(
+  public ImmutableList<DependencyEdge> dependencyTrace(
       MaybeBinding binding, ImmutableSet<DependencyEdge> entryPoints) {
     // Module binding graphs may have bindings unreachable from any entry points. If there are
     // no entry points for this DiagnosticInfo, don't try to print a dependency trace.
@@ -299,7 +307,7 @@ ImmutableList<DependencyEdge> dependencyTrace(
   }
 
   /** Returns all the nonsynthetic dependency requests for a binding. */
-  ImmutableSet<DependencyEdge> requests(MaybeBinding binding) {
+  public ImmutableSet<DependencyEdge> requests(MaybeBinding binding) {
     return graph.network().inEdges(binding).stream()
         .flatMap(instancesOf(DependencyEdge.class))
         .filter(edge -> edge.dependencyRequest().requestElement().isPresent())

javatests/dagger/internal/codegen/MissingBindingSuggestionsTest.java

@@ -96,8 +96,7 @@ private static JavaFileObject emptyInterface(String interfaceName) {
         daggerCompiler().compile(fooComponent, barComponent, topComponent, foo, bar, barModule);
     assertThat(compilation).failed();
     assertThat(compilation).hadErrorCount(1);
-    assertThat(compilation)
-        .hadErrorContaining("A binding with matching key exists in component: BarComponent");
+    assertThat(compilation).hadErrorContaining("A binding for Bar exists in BarComponent:");
   }
 
   @Test public void suggestsBindingInNestedSubcomponent() {
@@ -157,8 +156,7 @@ private static JavaFileObject emptyInterface(String interfaceName) {
             .compile(fooComponent, barComponent, bazComponent, topComponent, foo, baz, bazModule);
     assertThat(compilation).failed();
     assertThat(compilation).hadErrorCount(1);
-    assertThat(compilation)
-        .hadErrorContaining("A binding with matching key exists in component: BazComponent");
+    assertThat(compilation).hadErrorContaining("A binding for Baz exists in BazComponent:");
   }
 
   @Test
@@ -220,11 +218,11 @@ public void missingBindingInParentComponent() {
             message(
                 "\033[1;31m[Dagger/MissingBinding]\033[0m Baz cannot be provided without an "
                     + "@Inject constructor or an @Provides-annotated method.",
-                "A binding with matching key exists in component: Child",
+                "A binding for Baz exists in Child:",
                 "    Baz is injected at",
-                "        Bar(baz)",
+                "        [Parent] Bar(baz)",
                 "    Bar is requested at",
-                "        Parent.bar()",
+                "        [Parent] Parent.bar()",
                 "The following other entry points also depend on it:",
                 "    Parent.foo()",
                 "    Child.foo() [Parent → Child]"))
@@ -303,11 +301,11 @@ public void missingBindingInSiblingComponent() {
             message(
                 "\033[1;31m[Dagger/MissingBinding]\033[0m Baz cannot be provided without an "
                     + "@Inject constructor or an @Provides-annotated method.",
-                "A binding with matching key exists in component: Child2",
+                "A binding for Baz exists in Child2:",
                 "    Baz is injected at",
-                "        Bar(baz)",
+                "        [Parent] Bar(baz)",
                 "    Bar is requested at",
-                "        Parent.bar()",
+                "        [Parent] Parent.bar()",
                 "The following other entry points also depend on it:",
                 "    Parent.foo()",
                 "    Child1.foo() [Parent → Child1]",