Merge pull request square#459 from google/moe_sync_89_7

Moe sync 9/7

Author: ronshapiro

compiler/src/main/java/dagger/internal/codegen/BindingGraph.java

@@ -622,8 +622,19 @@ private ImmutableSet<ContributionBinding> createDelegateBindings(
        * delegate key.
        */
       private ContributionBinding createDelegateBinding(DelegateDeclaration delegateDeclaration) {
-        ResolvedBindings resolvedDelegate =
-            lookUpBindings(delegateDeclaration.delegateRequest().bindingKey());
+        BindingKey delegateBindingKey = delegateDeclaration.delegateRequest().bindingKey();
+
+        if (cycleStack.contains(delegateBindingKey)) {
+          return provisionBindingFactory.missingDelegate(delegateDeclaration);
+        }
+
+        ResolvedBindings resolvedDelegate;
+        try {
+          cycleStack.push(delegateBindingKey);
+          resolvedDelegate = lookUpBindings(delegateBindingKey);
+        } finally {
+          cycleStack.pop();
+        }
         if (resolvedDelegate.contributionBindings().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

compiler/src/test/java/dagger/internal/codegen/GraphValidationTest.java

@@ -506,6 +506,109 @@ public void falsePositiveCyclicDependencyIndirectionDetected() {
         .onLine(28);
   }
 
+  @Test
+  public void circularBindsMethods() {
+    JavaFileObject qualifier =
+        JavaFileObjects.forSourceLines(
+            "test.SomeQualifier",
+            "package test;",
+            "",
+            "import javax.inject.Qualifier;",
+            "",
+            "@Qualifier @interface SomeQualifier {}");
+    JavaFileObject module =
+        JavaFileObjects.forSourceLines(
+            "test.TestModule",
+            "package test;",
+            "",
+            "import dagger.Binds;",
+            "import dagger.Module;",
+            "",
+            "@Module",
+            "abstract class TestModule {",
+            "  @Binds abstract Object bindUnqualified(@SomeQualifier Object qualified);",
+            "  @Binds @SomeQualifier abstract Object bindQualified(Object unqualified);",
+            "}");
+    JavaFileObject component =
+        JavaFileObjects.forSourceLines(
+            "test.TestComponent",
+            "package test;",
+            "",
+            "import dagger.Component;",
+            "",
+            "@Component(modules = TestModule.class)",
+            "interface TestComponent {",
+            "  Object unqualified();",
+            "  @SomeQualifier Object qualified();",
+            "}");
+
+    assertThat(qualifier, module, component)
+        .processedWith(new ComponentProcessor())
+        .failsToCompile()
+        .withErrorContaining(
+            "test.TestComponent.unqualified() contains a dependency cycle:\n"
+                + "      java.lang.Object is injected at\n"
+                + "          test.TestModule.bindQualified(unqualified)\n"
+                + "      @test.SomeQualifier java.lang.Object is injected at\n"
+                + "          test.TestModule.bindUnqualified(qualified)\n"
+                + "      java.lang.Object is provided at\n"
+                + "          test.TestComponent.unqualified()")
+        .in(component)
+        .onLine(7)
+        .and()
+        .withErrorContaining(
+            "test.TestComponent.qualified() contains a dependency cycle:\n"
+                + "      @test.SomeQualifier java.lang.Object is injected at\n"
+                + "          test.TestModule.bindUnqualified(qualified)\n"
+                + "      java.lang.Object is injected at\n"
+                + "          test.TestModule.bindQualified(unqualified)\n"
+                + "      @test.SomeQualifier java.lang.Object is provided at\n"
+                + "          test.TestComponent.qualified()")
+        .in(component)
+        .onLine(8);
+  }
+
+  @Test
+  public void selfReferentialBinds() {
+    JavaFileObject module =
+        JavaFileObjects.forSourceLines(
+            "test.TestModule",
+            "package test;",
+            "",
+            "import dagger.Binds;",
+            "import dagger.Module;",
+            "",
+            "@Module",
+            "abstract class TestModule {",
+            "  @Binds abstract Object bindToSelf(Object sameKey);",
+            "}");
+    JavaFileObject component =
+        JavaFileObjects.forSourceLines(
+            "test.TestComponent",
+            "package test;",
+            "",
+            "import dagger.Component;",
+            "",
+            "@Component(modules = TestModule.class)",
+            "interface TestComponent {",
+            "  Object selfReferential();",
+            "}");
+
+    assertThat(module, component)
+        .processedWith(new ComponentProcessor())
+        .failsToCompile()
+        .withErrorContaining(
+            // TODO(gak): cl/126230644 produces a better error message in this case. Here it isn't
+            // unclear what is going wrong.
+            "test.TestComponent.selfReferential() contains a dependency cycle:\n"
+                + "      java.lang.Object is injected at\n"
+                + "          test.TestModule.bindToSelf(sameKey)\n"
+                + "      java.lang.Object is provided at\n"
+                + "          test.TestComponent.selfReferential()")
+        .in(component)
+        .onLine(7);
+  }
+
   @Test public void duplicateExplicitBindings_ProvidesAndComponentProvision() {
     JavaFileObject component = JavaFileObjects.forSourceLines("test.Outer",
         "package test;",

core/src/main/java/dagger/Component.java

@@ -171,9 +171,13 @@
  * inherit the <em>entire</em> binding graph from its parent when it is declared. For that reason,
  * a subcomponent isn't evaluated for completeness until it is associated with a parent.
  *
- * <p>Subcomponents are declared via a factory method on a parent component or subcomponent. The
- * method may have any name, but must return the subcomponent. The factory method's parameters may
- * be any number of the subcomponent's modules, but must at least include those without visible
+ * <p>Subcomponents are declared by listing the class in the {@link Module#subcomponents()}
+ * attribute of one of the parent component's modules. This binds the {@link Subcomponent.Builder}
+ * within the parent component.
+ *
+ * <p>Subcomponents may also be declared via a factory method on a parent component or subcomponent.
+ * The method may have any name, but must return the subcomponent. The factory method's parameters
+ * may be any number of the subcomponent's modules, but must at least include those without visible
  * no-arg constructors. The following is an example of a factory method that creates a
  * request-scoped subcomponent from a singleton-scoped parent: <pre><code>
  *   {@literal @}Singleton {@literal @}Component

core/src/main/java/dagger/Module.java

@@ -37,9 +37,11 @@
   Class<?>[] includes() default {};
 
   /**
-   * Any {@link Subcomponent}- or {@link dagger.producers.ProductionSubcomponent}-annotated classes
-   * which should be children of the component in which this module is installed. A subcomponent may
-   * be listed in more than one module in a component.
+   * Any {@link Subcomponent}- or {@code @ProductionSubcomponent}-annotated classes which should be
+   * children of the component in which this module is installed. A subcomponent may be listed in
+   * more than one module in a component.
+   *
+   * @since 2.7
    */
   @Beta
   Class<?>[] subcomponents() default {};

producers/src/main/java/dagger/producers/ProducerModule.java

@@ -46,6 +46,8 @@
    * Any {@link dagger.Subcomponent}- or {@link ProductionSubcomponent}-annotated classes which
    * should be children of the component in which this module is installed. A subcomponent may be
    * listed in more than one module in a component.
+   *
+   * @since 2.7
    */
   Class<?>[] subcomponents() default {};
 }