Remove all suppressable warnings: repeated-module and dependency-cycle.

------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=116672601
google · Mar 9, 2016 · 8f14c79 · 8f14c79
1 parent 447a25b
commit 8f14c79
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 277 deletions.
diff --git a/...nctional-tests/src/main/java/test/subcomponent/repeat/SubcomponentWithRepeatedModule.java b/...nctional-tests/src/main/java/test/subcomponent/repeat/SubcomponentWithRepeatedModule.java
@@ -30,7 +30,6 @@ interface SubcomponentWithRepeatedModule {
 
   @Subcomponent.Builder
   interface Builder {
-    @SuppressWarnings("repeated-module")
     Builder repeatedModule(RepeatedModule repeatedModule);
 
     SubcomponentWithRepeatedModule build();

diff --git a/compiler/src/main/java/dagger/internal/codegen/BindingGraphValidator.java b/compiler/src/main/java/dagger/internal/codegen/BindingGraphValidator.java
@@ -44,7 +44,6 @@
 import dagger.internal.codegen.SourceElement.HasSourceElement;
 import dagger.producers.ProductionComponent;
 import java.util.ArrayDeque;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Deque;
 import java.util.Formatter;
@@ -67,7 +66,6 @@
 import javax.lang.model.util.Elements;
 import javax.lang.model.util.SimpleTypeVisitor6;
 import javax.lang.model.util.Types;
-import javax.tools.Diagnostic;
 
 import static com.google.auto.common.MoreElements.getAnnotationMirror;
 import static com.google.auto.common.MoreTypes.asDeclared;
@@ -89,6 +87,7 @@
 import static dagger.internal.codegen.ContributionBinding.Kind.IS_SYNTHETIC_KIND;
 import static dagger.internal.codegen.ContributionBinding.Kind.SYNTHETIC_MULTIBOUND_MAP;
 import static dagger.internal.codegen.ContributionType.indexByContributionType;
+import static dagger.internal.codegen.ErrorMessages.CONTAINS_DEPENDENCY_CYCLE_FORMAT;
 import static dagger.internal.codegen.ErrorMessages.DUPLICATE_SIZE_LIMIT;
 import static dagger.internal.codegen.ErrorMessages.INDENT;
 import static dagger.internal.codegen.ErrorMessages.MEMBERS_INJECTION_WITH_UNBOUNDED_TYPE;
@@ -102,7 +101,6 @@
 import static dagger.internal.codegen.ErrorMessages.stripCommonTypePrefixes;
 import static dagger.internal.codegen.Util.componentCanMakeNewInstances;
 import static javax.tools.Diagnostic.Kind.ERROR;
-import static javax.tools.Diagnostic.Kind.WARNING;
 
 public class BindingGraphValidator {
 
@@ -1057,21 +1055,14 @@ private void reportCycle(
       Element rootRequestElement = requestPath.get(0).requestElement();
       ImmutableList<DependencyRequest> cycle =
           requestPath.subList(indexOfDuplicatedKey, requestPath.size());
-      ImmutableSet<DependencyRequest> providersBreakingCycle = providersBreakingCycle(cycle);
-      Diagnostic.Kind kind = providersBreakingCycle.isEmpty() ? ERROR : WARNING;
-      if (kind == WARNING
-          && (suppressCycleWarnings(rootRequestElement)
-              || suppressCycleWarnings(rootRequestElement.getEnclosingElement())
-              || suppressCycleWarnings(providersBreakingCycle))) {
+      if (!providersBreakingCycle(cycle).isEmpty()) {
         return;
       }
       // TODO(cgruber): Provide a hint for the start and end of the cycle.
       TypeElement componentType = MoreElements.asType(rootRequestElement.getEnclosingElement());
       reportBuilder.addItem(
           String.format(
-              kind == WARNING
-                  ? ErrorMessages.CONTAINS_DEPENDENCY_CYCLE_WARNING_FORMAT
-                  : ErrorMessages.CONTAINS_DEPENDENCY_CYCLE_ERROR_FORMAT,
+              CONTAINS_DEPENDENCY_CYCLE_FORMAT,
               componentType.getQualifiedName(),
               rootRequestElement.getSimpleName(),
               Joiner.on("\n")
@@ -1080,7 +1071,7 @@ private void reportCycle(
                           .transform(dependencyRequestFormatter)
                           .filter(not(equalTo("")))
                           .skip(1))),
-          kind,
+          ERROR,
           rootRequestElement);
     }
 
@@ -1140,20 +1131,6 @@ private boolean isImplicitProviderMapForValueMap(
     }
   }
 
-  private boolean suppressCycleWarnings(Element requestElement) {
-    SuppressWarnings suppressions = requestElement.getAnnotation(SuppressWarnings.class);
-    return suppressions != null && Arrays.asList(suppressions.value()).contains("dependency-cycle");
-  }
-
-  private boolean suppressCycleWarnings(Iterable<DependencyRequest> dependencyRequests) {
-    for (DependencyRequest dependencyRequest : dependencyRequests) {
-      if (suppressCycleWarnings(dependencyRequest.requestElement())) {
-        return true;
-      }
-    }
-    return false;
-  }
-
   ValidationReport<TypeElement> validate(BindingGraph subject) {
     Validation validation = new Validation(subject);
     validation.validateSubgraph();

diff --git a/compiler/src/main/java/dagger/internal/codegen/ComponentHierarchyValidator.java b/compiler/src/main/java/dagger/internal/codegen/ComponentHierarchyValidator.java
@@ -15,21 +15,16 @@
  */
 package dagger.internal.codegen;
 
-import com.google.auto.common.MoreElements;
 import com.google.auto.common.MoreTypes;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
-import dagger.internal.codegen.ComponentDescriptor.BuilderSpec;
 import dagger.internal.codegen.ComponentDescriptor.ComponentMethodDescriptor;
 import java.util.Map;
-import javax.lang.model.element.Element;
-import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.element.VariableElement;
 
 import static com.google.common.base.Functions.constant;
-import static java.util.Arrays.asList;
 
 /**
  * Validates the relationships between parent components and subcomponents.
@@ -73,41 +68,14 @@ private ValidationReport<TypeElement> validateSubcomponentMethods(
             }
           }
           break;
+
         case SUBCOMPONENT_BUILDER:
         case PRODUCTION_SUBCOMPONENT_BUILDER:
-          BuilderSpec subcomponentBuilderSpec = subcomponentDescriptor.builderSpec().get();
-          for (Map.Entry<TypeElement, ExecutableElement> builderMethodEntry :
-              subcomponentBuilderSpec.methodMap().entrySet()) {
-            TypeElement moduleType = builderMethodEntry.getKey();
-            TypeElement originatingComponent = existingModuleToOwners.get(moduleType);
-            /* A subcomponent builder allows you to pass a module that is already present in the
-             * parent.  This can't be an error because it might be valid in _other_ components, so
-             * we warn here, unless the warning is suppressed on the subcomponent method or the
-             * builder method. */
-            ExecutableElement builderMethodElement = builderMethodEntry.getValue();
-            if (originatingComponent != null
-                && !repeatedModuleWarningsSuppressed(subcomponentMethodDescriptor.methodElement())
-                && !repeatedModuleWarningsSuppressed(builderMethodElement)) {
-              /* TODO(gak): consider putting this on the builder method directly if it's in the
-               * component being compiled */
-              reportBuilder.addWarning(
-                  String.format(
-                      "%1$s is installed in %2$s. A subcomponent cannot use an instance of a "
-                          + "module that differs from its parent. The implementation of %4$s "
-                          + "in %5$s will throw %6$s. To suppress this warning, annotate "
-                          + "either %4$s, %3$s, %5$s.%7$s, or %5$s with "
-                          + "@SuppressWarnings(\"repeated-module\").",
-                      moduleType.getSimpleName(),
-                      originatingComponent.getQualifiedName(),
-                      subcomponentBuilderSpec.builderDefinitionType().getQualifiedName(),
-                      builderMethodElement.getSimpleName(),
-                      componentDescriptor.componentDefinitionType().getQualifiedName(),
-                      UnsupportedOperationException.class.getSimpleName(),
-                      subcomponentMethodDescriptor.methodElement().getSimpleName()),
-                  builderMethodElement);
-            }
-          }
+          /* A subcomponent builder allows you to pass a module that is already present in the
+           * parent.  This can't be an error because it might be valid in _other_ components. Don't
+           * bother warning, because there's nothing to do except suppress the warning. */
           break;
+
         default:
           throw new AssertionError();
       }
@@ -126,19 +94,4 @@ private ValidationReport<TypeElement> validateSubcomponentMethods(
     }
     return reportBuilder.build();
   }
-
-  private boolean repeatedModuleWarningsSuppressed(Element element) {
-    while (true) {
-      // TODO(dpb): Extract a method to check whether a warning is suppressed on an element.
-      SuppressWarnings suppressWarnings = element.getAnnotation(SuppressWarnings.class);
-      if (suppressWarnings != null
-          && asList(suppressWarnings.value()).contains("repeated-module")) {
-        return true;
-      }
-      if (MoreElements.isType(element)) {
-        return false;
-      }
-      element = element.getEnclosingElement();
-    }
-  }
 }
diff --git a/compiler/src/main/java/dagger/internal/codegen/ErrorMessages.java b/compiler/src/main/java/dagger/internal/codegen/ErrorMessages.java
@@ -242,13 +242,7 @@ static String provisionMayNotDependOnProducerType(TypeMirror type) {
   static final String MEMBERS_INJECTION_WITH_UNBOUNDED_TYPE =
       "Type parameters must be bounded for members injection. %s required by %s, via:\n%s";
 
-  static final String CONTAINS_DEPENDENCY_CYCLE_ERROR_FORMAT =
-      "%s.%s() contains a dependency cycle:\n%s";
-
-  static final String CONTAINS_DEPENDENCY_CYCLE_WARNING_FORMAT =
-      "%s.%s() contains a dependency cycle. "
-          + "You can suppress this warning by annotating the component method, the component, or "
-          + "any dependency request in the cycle with @SuppressWarnings(\"dependency-cycle\"):\n%s";
+  static final String CONTAINS_DEPENDENCY_CYCLE_FORMAT = "%s.%s() contains a dependency cycle:\n%s";
 
   static final String MALFORMED_MODULE_METHOD_FORMAT =
       "Cannot generated a graph because method %s on module %s was malformed";

diff --git a/compiler/src/test/java/dagger/internal/codegen/GraphValidationTest.java b/compiler/src/test/java/dagger/internal/codegen/GraphValidationTest.java
@@ -384,196 +384,6 @@ public void falsePositiveCyclicDependencyIndirectionDetected() {
         .onLine(28);
   }
 
-  @Test
-  public void cyclicDependencySimpleProviderIndirectionWarning() {
-    JavaFileObject component =
-        JavaFileObjects.forSourceLines(
-            "test.Outer",
-            "package test;",
-            "",
-            "import dagger.Component;",
-            "import dagger.Module;",
-            "import dagger.Provides;",
-            "import javax.inject.Inject;",
-            "import javax.inject.Provider;",
-            "",
-            "final class Outer {",
-            "  static class A {",
-            "    @Inject A(B bParam) {}",
-            "  }",
-            "",
-            "  static class B {",
-            "    @Inject B(C cParam, D dParam) {}",
-            "  }",
-            "",
-            "  static class C {",
-            "    @Inject C(Provider<A> aParam) {}",
-            "  }",
-            "",
-            "  static class D {",
-            "    @Inject D() {}",
-            "  }",
-            "",
-            "  @Component()",
-            "  interface CComponent {",
-            "    C get();",
-            "  }",
-            "}");
-
-    String expectedWarning =
-        Joiner.on('\n')
-            .join(
-                "test.Outer.CComponent.get() contains a dependency cycle. "
-                    + "You can suppress this warning by annotating the component method, the "
-                    + "component, or any dependency request in the cycle with "
-                    + "@SuppressWarnings(\"dependency-cycle\"):",
-                "      test.Outer.C.<init>(javax.inject.Provider<test.Outer.A> aParam)",
-                "          [parameter: javax.inject.Provider<test.Outer.A> aParam]",
-                "      test.Outer.A.<init>(test.Outer.B bParam)",
-                "          [parameter: test.Outer.B bParam]",
-                "      test.Outer.B.<init>(test.Outer.C cParam, test.Outer.D dParam)",
-                "          [parameter: test.Outer.C cParam]");
-    assertAbout(javaSource())
-        .that(component)
-        .withCompilerOptions("-Xlint:-processing", "-Xlint:-rawtypes")
-        .processedWith(new ComponentProcessor())
-        .compilesWithoutError()
-        .withWarningContaining(expectedWarning)
-        .in(component)
-        .onLine(28);
-  }
-
-  @Test
-  public void cyclicDependencySimpleProviderIndirectionWarningSuppressed() {
-    JavaFileObject component =
-        JavaFileObjects.forSourceLines(
-            "test.Outer",
-            "package test;",
-            "",
-            "import dagger.Component;",
-            "import dagger.Module;",
-            "import dagger.Provides;",
-            "import javax.inject.Inject;",
-            "import javax.inject.Provider;",
-            "",
-            "final class Outer {",
-            "  static class A {",
-            "    @Inject A(B bParam) {}",
-            "  }",
-            "",
-            "  static class B {",
-            "    @Inject B(C cParam, D dParam) {}",
-            "  }",
-            "",
-            "  static class C {",
-            "    @Inject C(Provider<A> aParam) {}",
-            "  }",
-            "",
-            "  static class D {",
-            "    @Inject D() {}",
-            "  }",
-            "",
-            "  @SuppressWarnings(\"dependency-cycle\")",
-            "  @Component()",
-            "  interface CComponent {",
-            "    C get();",
-            "  }",
-            "}");
-
-    assertAbout(javaSource())
-        .that(component)
-        .withCompilerOptions("-Xlint:-processing", "-Xlint:-rawtypes")
-        .processedWith(new ComponentProcessor())
-        .compilesWithoutWarnings();
-  }
-
-  @Test
-  public void cyclicDependencySimpleProviderIndirectionWarningSuppressed_atDependencyRequest() {
-    JavaFileObject component =
-        JavaFileObjects.forSourceLines(
-            "test.Outer",
-            "package test;",
-            "",
-            "import dagger.Component;",
-            "import dagger.Module;",
-            "import dagger.Provides;",
-            "import javax.inject.Inject;",
-            "import javax.inject.Provider;",
-            "",
-            "final class Outer {",
-            "  static class A {",
-            "    @Inject A(B bParam) {}",
-            "  }",
-            "",
-            "  static class B {",
-            "    @Inject B(C bParam, D dParam) {}",
-            "  }",
-            "",
-            "  static class C {",
-            "    @Inject C(@SuppressWarnings(\"dependency-cycle\") Provider<A> aParam) {}",
-            "  }",
-            "",
-            "  static class D {",
-            "    @Inject D() {}",
-            "  }",
-            "",
-            "  @Component()",
-            "  interface CComponent {",
-            "    C get();",
-            "  }",
-            "}");
-
-    assertAbout(javaSource())
-        .that(component)
-        .withCompilerOptions("-Xlint:-processing", "-Xlint:-rawtypes")
-        .processedWith(new ComponentProcessor())
-        .compilesWithoutWarnings();
-  }
-
-  @Test
-  public void cyclicDependencySimpleProviderIndirectionWarningNotSuppressed_atDependencyRequest() {
-    JavaFileObject component =
-        JavaFileObjects.forSourceLines(
-            "test.Outer",
-            "package test;",
-            "",
-            "import dagger.Component;",
-            "import dagger.Module;",
-            "import dagger.Provides;",
-            "import javax.inject.Inject;",
-            "import javax.inject.Provider;",
-            "",
-            "final class Outer {",
-            "  static class A {",
-            "    @Inject A(B bParam) {}",
-            "  }",
-            "",
-            "  static class B {",
-            "    @Inject B(@SuppressWarnings(\"dependency-cycle\") C cParam, D dParam) {}",
-            "  }",
-            "",
-            "  static class C {",
-            "    @Inject C(Provider<A> aParam) {}",
-            "  }",
-            "",
-            "  static class D {",
-            "    @Inject D() {}",
-            "  }",
-            "",
-            "  @Component()",
-            "  interface CComponent {",
-            "    C get();",
-            "  }",
-            "}");
-
-    assertAbout(javaSource())
-        .that(component)
-        .withCompilerOptions("-Xlint:-processing", "-Xlint:-rawtypes")
-        .processedWith(new ComponentProcessor())
-        .compilesWithoutError()
-        .withWarningContaining("dependency cycle");
-  }
-
   @Test public void duplicateExplicitBindings_ProvidesAndComponentProvision() {
     JavaFileObject component = JavaFileObjects.forSourceLines("test.Outer",
         "package test;",