Remove duplicate missing binding messages.

A missing binding was being reported for each component/subcomponent in the graph. This leads to error spam.

This CL changes the way missing bindings are included into the BindingGraph. In particular, we create a single missing binding node in the root component rather than 1 in every component. This change should be equally as valid as the previous choice because we don't know where a missing binding will be provided anyway. However, choosing the root component has two benefits: it produces much more succinct error messages, and it simplifies the BindingGraph for plugins.

RELNOTES=Fix duplicate missing binding error messages.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=264182374

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

@@ -23,6 +23,7 @@
 import static dagger.internal.codegen.extension.DaggerStreams.toImmutableList;
 import static dagger.model.BindingKind.SUBCOMPONENT_CREATOR;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Iterators;
@@ -326,10 +327,11 @@ private BindingNode bindingNode(
     }
 
     private MissingBinding missingBindingNode(ResolvedBindings dependencies) {
-      // TODO(ronshapiro): Revisit whether missing binding nodes should all use the root component
-      // path, and the component(s) that have the missing bindings should be determined by the
-      // predecessors of missing binding nodes.
-      return BindingGraphProxies.missingBindingNode(componentPath(), dependencies.key());
+      // Put all missing binding nodes in the root component. This simplifies the binding graph
+      // and produces better error messages for users since all dependents point to the same node.
+      return BindingGraphProxies.missingBindingNode(
+          ComponentPath.create(ImmutableList.of(componentPath().rootComponent())),
+          dependencies.key());
     }
 
     private ComponentNode subcomponentNode(TypeMirror subcomponentBuilderType, BindingGraph graph) {

javatests/dagger/internal/codegen/MissingBindingSuggestionsTest.java

@@ -18,6 +18,7 @@
 
 import static com.google.testing.compile.CompilationSubject.assertThat;
 import static dagger.internal.codegen.Compilers.daggerCompiler;
+import static dagger.internal.codegen.TestUtils.message;
 
 import com.google.testing.compile.Compilation;
 import com.google.testing.compile.JavaFileObjects;
@@ -94,6 +95,7 @@ private static JavaFileObject emptyInterface(String interfaceName) {
     Compilation compilation =
         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: test.BarComponent");
   }
@@ -154,7 +156,164 @@ private static JavaFileObject emptyInterface(String interfaceName) {
         daggerCompiler()
             .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: test.BazComponent");
   }
+
+  @Test
+  public void missingBindingInParentComponent() {
+    JavaFileObject parent =
+        JavaFileObjects.forSourceLines(
+            "Parent",
+            "import dagger.Component;",
+            "",
+            "@Component",
+            "interface Parent {",
+            "  Foo foo();",
+            "  Bar bar();",
+            "  Child child();",
+            "}");
+    JavaFileObject child =
+        JavaFileObjects.forSourceLines(
+            "Child",
+            "import dagger.Subcomponent;",
+            "",
+            "@Subcomponent(modules=BazModule.class)",
+            "interface Child {",
+            "  Foo foo();",
+            "  Baz baz();",
+            "}");
+    JavaFileObject foo =
+        JavaFileObjects.forSourceLines(
+            "Foo",
+            "import javax.inject.Inject;",
+            "",
+            "class Foo {",
+            "  @Inject Foo(Bar bar) {}",
+            "}");
+    JavaFileObject bar =
+        JavaFileObjects.forSourceLines(
+            "Bar",
+            "import javax.inject.Inject;",
+            "",
+            "class Bar {",
+            "  @Inject Bar(Baz baz) {}",
+            "}");
+    JavaFileObject baz = JavaFileObjects.forSourceLines("Baz", "class Baz {}");
+    JavaFileObject bazModule = JavaFileObjects.forSourceLines(
+        "BazModule",
+        "import dagger.Module;",
+        "import dagger.Provides;",
+        "import javax.inject.Inject;",
+        "",
+        "@Module",
+        "final class BazModule {",
+        "  @Provides Baz provideBaz() {return new Baz();}",
+        "}");
+
+    Compilation compilation = daggerCompiler().compile(parent, child, foo, bar, baz, bazModule);
+    assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
+    assertThat(compilation)
+        .hadErrorContaining(
+            message(
+                "[Dagger/MissingBinding] Baz cannot be provided without an @Inject constructor or "
+                    + "an @Provides-annotated method.",
+                "A binding with matching key exists in component: Child",
+                "    Baz is injected at",
+                "        Bar(baz)",
+                "    Bar is provided at",
+                "        Parent.bar()",
+                "The following other entry points also depend on it:",
+                "    Parent.foo()",
+                "    Child.foo() [Parent → Child]"))
+        .inFile(parent)
+        .onLineContaining("interface Parent");
+  }
+
+  @Test
+  public void missingBindingInSiblingComponent() {
+    JavaFileObject parent =
+        JavaFileObjects.forSourceLines(
+            "Parent",
+            "import dagger.Component;",
+            "",
+            "@Component",
+            "interface Parent {",
+            "  Foo foo();",
+            "  Bar bar();",
+            "  Child1 child1();",
+            "  Child2 child2();",
+            "}");
+    JavaFileObject child1 =
+        JavaFileObjects.forSourceLines(
+            "Child1",
+            "import dagger.Subcomponent;",
+            "",
+            "@Subcomponent",
+            "interface Child1 {",
+            "  Foo foo();",
+            "  Baz baz();",
+            "}");
+    JavaFileObject child2 =
+        JavaFileObjects.forSourceLines(
+            "Child2",
+            "import dagger.Subcomponent;",
+            "",
+            "@Subcomponent(modules = BazModule.class)",
+            "interface Child2 {",
+            "  Foo foo();",
+            "  Baz baz();",
+            "}");
+    JavaFileObject foo =
+        JavaFileObjects.forSourceLines(
+            "Foo",
+            "import javax.inject.Inject;",
+            "",
+            "class Foo {",
+            "  @Inject Foo(Bar bar) {}",
+            "}");
+    JavaFileObject bar =
+        JavaFileObjects.forSourceLines(
+            "Bar",
+            "import javax.inject.Inject;",
+            "",
+            "class Bar {",
+            "  @Inject Bar(Baz baz) {}",
+            "}");
+    JavaFileObject baz = JavaFileObjects.forSourceLines("Baz", "class Baz {}");
+    JavaFileObject bazModule = JavaFileObjects.forSourceLines(
+        "BazModule",
+        "import dagger.Module;",
+        "import dagger.Provides;",
+        "import javax.inject.Inject;",
+        "",
+        "@Module",
+        "final class BazModule {",
+        "  @Provides Baz provideBaz() {return new Baz();}",
+        "}");
+
+    Compilation compilation =
+        daggerCompiler().compile(parent, child1, child2, foo, bar, baz, bazModule);
+    assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
+    assertThat(compilation)
+        .hadErrorContaining(
+            message(
+                "[Dagger/MissingBinding] Baz cannot be provided without an @Inject constructor or "
+                    + "an @Provides-annotated method.",
+                "A binding with matching key exists in component: Child2",
+                "    Baz is injected at",
+                "        Bar(baz)",
+                "    Bar is provided at",
+                "        Parent.bar()",
+                "The following other entry points also depend on it:",
+                "    Parent.foo()",
+                "    Child1.foo() [Parent → Child1]",
+                "    Child2.foo() [Parent → Child2]",
+                "    Child1.baz() [Parent → Child1]"))
+        .inFile(parent)
+        .onLineContaining("interface Parent");
+  }
 }

javatests/dagger/internal/codegen/MissingBindingValidationTest.java

@@ -56,6 +56,7 @@ public void dependOnInterface() {
         "interface Bar {}");
     Compilation compilation = daggerCompiler().compile(component, injectable, nonInjectable);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContaining("test.Bar cannot be provided without an @Provides-annotated method.")
         .inFile(component)
@@ -81,6 +82,7 @@ public void entryPointDependsOnInterface() {
             "}");
     Compilation compilation = daggerCompiler().compile(component);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContaining(
             "[Dagger/MissingBinding] test.TestClass.A cannot be provided "
@@ -110,6 +112,7 @@ public void entryPointDependsOnQualifiedInterface() {
             "}");
     Compilation compilation = daggerCompiler().compile(component);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContaining(
             "[Dagger/MissingBinding] @test.TestClass.Q test.TestClass.A cannot be provided "
@@ -140,6 +143,7 @@ public void entryPointDependsOnQualifiedInterface() {
 
     Compilation compilation = daggerCompiler().compile(component);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContaining(
             "test.TestClass.A cannot be provided without an @Inject constructor or an "
@@ -174,6 +178,7 @@ public void entryPointDependsOnQualifiedInterface() {
 
     Compilation compilation = daggerCompiler().compile(component);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContaining(
             "test.TestClass.B cannot be provided without an @Inject constructor or an "
@@ -210,6 +215,7 @@ public void missingBindingWithSameKeyAsMembersInjectionMethod() {
 
     Compilation compilation = daggerCompiler().compile(self, component);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContaining("test.Self cannot be provided without an @Inject constructor")
         .inFile(component)
@@ -241,6 +247,7 @@ public void genericInjectClassWithWildcardDependencies() {
             "}");
     Compilation compilation = daggerCompiler().compile(component, foo);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContaining(
             "test.Foo<? extends java.lang.Number> cannot be provided "
@@ -300,6 +307,7 @@ public void genericInjectClassWithWildcardDependencies() {
 
     Compilation compilation = daggerCompiler().compile(component);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContaining(
             message(
@@ -354,6 +362,7 @@ public void bindsMethodAppearsInTrace() {
     Compilation compilation =
         daggerCompiler().compile(component, module, interfaceFile, implementationFile);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContaining(
             message(
@@ -408,6 +417,7 @@ public void bindsMethodAppearsInTrace() {
 
     Compilation compilation = daggerCompiler().compile(generic, testClass, usesTest, component);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContaining(
             message(
@@ -462,6 +472,7 @@ public void bindsMethodAppearsInTrace() {
 
     Compilation compilation = daggerCompiler().compile(generic, testClass, usesTest, component);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContaining(
             message(
@@ -524,6 +535,7 @@ public void bindingUsedOnlyInSubcomponentDependsOnBindingOnlyInSubcomponent() {
 
     Compilation compilation = daggerCompiler().compile(parent, parentModule, child, childModule);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContainingMatch(
             "(?s)\\Qjava.lang.String cannot be provided\\E.*\\QChild.needsString()\\E")
@@ -599,6 +611,7 @@ public void multibindingContributionBetweenAncestorComponentAndEntrypointCompone
     Compilation compilation =
         daggerCompiler().compile(parent, parentModule, child, childModule, grandchild);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContainingMatch(
             "(?s)\\Qjava.lang.Double cannot be provided\\E.*"
@@ -646,6 +659,7 @@ public void manyDependencies() {
             "interface NotBound {}");
     Compilation compilation = daggerCompiler().compile(component, module, notBound);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContaining(
             message(
@@ -705,6 +719,7 @@ public void tooManyRequests() {
 
     Compilation compilation = daggerCompiler().compile(foo, component);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContaining(
             message(
@@ -755,6 +770,7 @@ public void tooManyEntryPoints() {
 
     Compilation compilation = daggerCompiler().compile(component);
     assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
     assertThat(compilation)
         .hadErrorContaining(
             message(
@@ -777,4 +793,65 @@ public void tooManyEntryPoints() {
         .inFile(component)
         .onLineContaining("interface TestComponent");
   }
+
+  @Test
+  public void missingBindingInAllComponentsAndEntryPoints() {
+    JavaFileObject parent =
+        JavaFileObjects.forSourceLines(
+            "Parent",
+            "import dagger.Component;",
+            "",
+            "@Component",
+            "interface Parent {",
+            "  Foo foo();",
+            "  Bar bar();",
+            "  Child child();",
+            "}");
+    JavaFileObject child =
+        JavaFileObjects.forSourceLines(
+            "Child",
+            "import dagger.Subcomponent;",
+            "",
+            "@Subcomponent",
+            "interface Child {",
+            "  Foo foo();",
+            "  Baz baz();",
+            "}");
+    JavaFileObject foo =
+        JavaFileObjects.forSourceLines(
+            "Foo",
+            "import javax.inject.Inject;",
+            "",
+            "class Foo {",
+            "  @Inject Foo(Bar bar) {}",
+            "}");
+    JavaFileObject bar =
+        JavaFileObjects.forSourceLines(
+            "Bar",
+            "import javax.inject.Inject;",
+            "",
+            "class Bar {",
+            "  @Inject Bar(Baz baz) {}",
+            "}");
+    JavaFileObject baz = JavaFileObjects.forSourceLines("Baz", "class Baz {}");
+
+    Compilation compilation = daggerCompiler().compile(parent, child, foo, bar, baz);
+    assertThat(compilation).failed();
+    assertThat(compilation).hadErrorCount(1);
+    assertThat(compilation)
+        .hadErrorContaining(
+            message(
+                "[Dagger/MissingBinding] Baz cannot be provided without an @Inject constructor or "
+                    + "an @Provides-annotated method.",
+                "    Baz is injected at",
+                "        Bar(baz)",
+                "    Bar is provided at",
+                "        Parent.bar()",
+                "The following other entry points also depend on it:",
+                "    Parent.foo()",
+                "    Child.foo() [Parent → Child]",
+                "    Child.baz() [Parent → Child]"))
+        .inFile(parent)
+        .onLineContaining("interface Parent");
+  }
 }