Skip to content

Commit 8f14c79

Browse files
netdpbronshapiro
netdpb
authored and
ronshapiro
committed
Remove all suppressable warnings: repeated-module and dependency-cycle.
------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=116672601
1 parent 447a25b commit 8f14c79

File tree

5 files changed

+10
-277
lines changed

5 files changed

+10
-277
lines changed

‎compiler/src/it/functional-tests/src/main/java/test/subcomponent/repeat/SubcomponentWithRepeatedModule.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ interface SubcomponentWithRepeatedModule {
3030

3131
@Subcomponent.Builder
3232
interface Builder {
33-
@SuppressWarnings("repeated-module")
3433
Builder repeatedModule(RepeatedModule repeatedModule);
3534

3635
SubcomponentWithRepeatedModule build();

‎compiler/src/main/java/dagger/internal/codegen/BindingGraphValidator.java

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import dagger.internal.codegen.SourceElement.HasSourceElement;
4545
import dagger.producers.ProductionComponent;
4646
import java.util.ArrayDeque;
47-
import java.util.Arrays;
4847
import java.util.Collection;
4948
import java.util.Deque;
5049
import java.util.Formatter;
@@ -67,7 +66,6 @@
6766
import javax.lang.model.util.Elements;
6867
import javax.lang.model.util.SimpleTypeVisitor6;
6968
import javax.lang.model.util.Types;
70-
import javax.tools.Diagnostic;
7169

7270
import static com.google.auto.common.MoreElements.getAnnotationMirror;
7371
import static com.google.auto.common.MoreTypes.asDeclared;
@@ -89,6 +87,7 @@
8987
import static dagger.internal.codegen.ContributionBinding.Kind.IS_SYNTHETIC_KIND;
9088
import static dagger.internal.codegen.ContributionBinding.Kind.SYNTHETIC_MULTIBOUND_MAP;
9189
import static dagger.internal.codegen.ContributionType.indexByContributionType;
90+
import static dagger.internal.codegen.ErrorMessages.CONTAINS_DEPENDENCY_CYCLE_FORMAT;
9291
import static dagger.internal.codegen.ErrorMessages.DUPLICATE_SIZE_LIMIT;
9392
import static dagger.internal.codegen.ErrorMessages.INDENT;
9493
import static dagger.internal.codegen.ErrorMessages.MEMBERS_INJECTION_WITH_UNBOUNDED_TYPE;
@@ -102,7 +101,6 @@
102101
import static dagger.internal.codegen.ErrorMessages.stripCommonTypePrefixes;
103102
import static dagger.internal.codegen.Util.componentCanMakeNewInstances;
104103
import static javax.tools.Diagnostic.Kind.ERROR;
105-
import static javax.tools.Diagnostic.Kind.WARNING;
106104

107105
public class BindingGraphValidator {
108106

@@ -1057,21 +1055,14 @@ private void reportCycle(
10571055
Element rootRequestElement = requestPath.get(0).requestElement();
10581056
ImmutableList<DependencyRequest> cycle =
10591057
requestPath.subList(indexOfDuplicatedKey, requestPath.size());
1060-
ImmutableSet<DependencyRequest> providersBreakingCycle = providersBreakingCycle(cycle);
1061-
Diagnostic.Kind kind = providersBreakingCycle.isEmpty() ? ERROR : WARNING;
1062-
if (kind == WARNING
1063-
&& (suppressCycleWarnings(rootRequestElement)
1064-
|| suppressCycleWarnings(rootRequestElement.getEnclosingElement())
1065-
|| suppressCycleWarnings(providersBreakingCycle))) {
1058+
if (!providersBreakingCycle(cycle).isEmpty()) {
10661059
return;
10671060
}
10681061
// TODO(cgruber): Provide a hint for the start and end of the cycle.
10691062
TypeElement componentType = MoreElements.asType(rootRequestElement.getEnclosingElement());
10701063
reportBuilder.addItem(
10711064
String.format(
1072-
kind == WARNING
1073-
? ErrorMessages.CONTAINS_DEPENDENCY_CYCLE_WARNING_FORMAT
1074-
: ErrorMessages.CONTAINS_DEPENDENCY_CYCLE_ERROR_FORMAT,
1065+
CONTAINS_DEPENDENCY_CYCLE_FORMAT,
10751066
componentType.getQualifiedName(),
10761067
rootRequestElement.getSimpleName(),
10771068
Joiner.on("\n")
@@ -1080,7 +1071,7 @@ private void reportCycle(
10801071
.transform(dependencyRequestFormatter)
10811072
.filter(not(equalTo("")))
10821073
.skip(1))),
1083-
kind,
1074+
ERROR,
10841075
rootRequestElement);
10851076
}
10861077

@@ -1140,20 +1131,6 @@ private boolean isImplicitProviderMapForValueMap(
11401131
}
11411132
}
11421133

1143-
private boolean suppressCycleWarnings(Element requestElement) {
1144-
SuppressWarnings suppressions = requestElement.getAnnotation(SuppressWarnings.class);
1145-
return suppressions != null && Arrays.asList(suppressions.value()).contains("dependency-cycle");
1146-
}
1147-
1148-
private boolean suppressCycleWarnings(Iterable<DependencyRequest> dependencyRequests) {
1149-
for (DependencyRequest dependencyRequest : dependencyRequests) {
1150-
if (suppressCycleWarnings(dependencyRequest.requestElement())) {
1151-
return true;
1152-
}
1153-
}
1154-
return false;
1155-
}
1156-
11571134
ValidationReport<TypeElement> validate(BindingGraph subject) {
11581135
Validation validation = new Validation(subject);
11591136
validation.validateSubgraph();

‎compiler/src/main/java/dagger/internal/codegen/ComponentHierarchyValidator.java

Lines changed: 5 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,16 @@
1515
*/
1616
package dagger.internal.codegen;
1717

18-
import com.google.auto.common.MoreElements;
1918
import com.google.auto.common.MoreTypes;
2019
import com.google.common.collect.ImmutableMap;
2120
import com.google.common.collect.Maps;
2221
import com.google.common.collect.Sets;
23-
import dagger.internal.codegen.ComponentDescriptor.BuilderSpec;
2422
import dagger.internal.codegen.ComponentDescriptor.ComponentMethodDescriptor;
2523
import java.util.Map;
26-
import javax.lang.model.element.Element;
27-
import javax.lang.model.element.ExecutableElement;
2824
import javax.lang.model.element.TypeElement;
2925
import javax.lang.model.element.VariableElement;
3026

3127
import static com.google.common.base.Functions.constant;
32-
import static java.util.Arrays.asList;
3328

3429
/**
3530
* Validates the relationships between parent components and subcomponents.
@@ -73,41 +68,14 @@ private ValidationReport<TypeElement> validateSubcomponentMethods(
7368
}
7469
}
7570
break;
71+
7672
case SUBCOMPONENT_BUILDER:
7773
case PRODUCTION_SUBCOMPONENT_BUILDER:
78-
BuilderSpec subcomponentBuilderSpec = subcomponentDescriptor.builderSpec().get();
79-
for (Map.Entry<TypeElement, ExecutableElement> builderMethodEntry :
80-
subcomponentBuilderSpec.methodMap().entrySet()) {
81-
TypeElement moduleType = builderMethodEntry.getKey();
82-
TypeElement originatingComponent = existingModuleToOwners.get(moduleType);
83-
/* A subcomponent builder allows you to pass a module that is already present in the
84-
* parent. This can't be an error because it might be valid in _other_ components, so
85-
* we warn here, unless the warning is suppressed on the subcomponent method or the
86-
* builder method. */
87-
ExecutableElement builderMethodElement = builderMethodEntry.getValue();
88-
if (originatingComponent != null
89-
&& !repeatedModuleWarningsSuppressed(subcomponentMethodDescriptor.methodElement())
90-
&& !repeatedModuleWarningsSuppressed(builderMethodElement)) {
91-
/* TODO(gak): consider putting this on the builder method directly if it's in the
92-
* component being compiled */
93-
reportBuilder.addWarning(
94-
String.format(
95-
"%1$s is installed in %2$s. A subcomponent cannot use an instance of a "
96-
+ "module that differs from its parent. The implementation of %4$s "
97-
+ "in %5$s will throw %6$s. To suppress this warning, annotate "
98-
+ "either %4$s, %3$s, %5$s.%7$s, or %5$s with "
99-
+ "@SuppressWarnings(\"repeated-module\").",
100-
moduleType.getSimpleName(),
101-
originatingComponent.getQualifiedName(),
102-
subcomponentBuilderSpec.builderDefinitionType().getQualifiedName(),
103-
builderMethodElement.getSimpleName(),
104-
componentDescriptor.componentDefinitionType().getQualifiedName(),
105-
UnsupportedOperationException.class.getSimpleName(),
106-
subcomponentMethodDescriptor.methodElement().getSimpleName()),
107-
builderMethodElement);
108-
}
109-
}
74+
/* A subcomponent builder allows you to pass a module that is already present in the
75+
* parent. This can't be an error because it might be valid in _other_ components. Don't
76+
* bother warning, because there's nothing to do except suppress the warning. */
11077
break;
78+
11179
default:
11280
throw new AssertionError();
11381
}
@@ -126,19 +94,4 @@ private ValidationReport<TypeElement> validateSubcomponentMethods(
12694
}
12795
return reportBuilder.build();
12896
}
129-
130-
private boolean repeatedModuleWarningsSuppressed(Element element) {
131-
while (true) {
132-
// TODO(dpb): Extract a method to check whether a warning is suppressed on an element.
133-
SuppressWarnings suppressWarnings = element.getAnnotation(SuppressWarnings.class);
134-
if (suppressWarnings != null
135-
&& asList(suppressWarnings.value()).contains("repeated-module")) {
136-
return true;
137-
}
138-
if (MoreElements.isType(element)) {
139-
return false;
140-
}
141-
element = element.getEnclosingElement();
142-
}
143-
}
14497
}

‎compiler/src/main/java/dagger/internal/codegen/ErrorMessages.java

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -242,13 +242,7 @@ static String provisionMayNotDependOnProducerType(TypeMirror type) {
242242
static final String MEMBERS_INJECTION_WITH_UNBOUNDED_TYPE =
243243
"Type parameters must be bounded for members injection. %s required by %s, via:\n%s";
244244

245-
static final String CONTAINS_DEPENDENCY_CYCLE_ERROR_FORMAT =
246-
"%s.%s() contains a dependency cycle:\n%s";
247-
248-
static final String CONTAINS_DEPENDENCY_CYCLE_WARNING_FORMAT =
249-
"%s.%s() contains a dependency cycle. "
250-
+ "You can suppress this warning by annotating the component method, the component, or "
251-
+ "any dependency request in the cycle with @SuppressWarnings(\"dependency-cycle\"):\n%s";
245+
static final String CONTAINS_DEPENDENCY_CYCLE_FORMAT = "%s.%s() contains a dependency cycle:\n%s";
252246

253247
static final String MALFORMED_MODULE_METHOD_FORMAT =
254248
"Cannot generated a graph because method %s on module %s was malformed";

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

Lines changed: 0 additions & 190 deletions
Original file line numberDiff line numberDiff line change
@@ -384,196 +384,6 @@ public void falsePositiveCyclicDependencyIndirectionDetected() {
384384
.onLine(28);
385385
}
386386

387-
@Test
388-
public void cyclicDependencySimpleProviderIndirectionWarning() {
389-
JavaFileObject component =
390-
JavaFileObjects.forSourceLines(
391-
"test.Outer",
392-
"package test;",
393-
"",
394-
"import dagger.Component;",
395-
"import dagger.Module;",
396-
"import dagger.Provides;",
397-
"import javax.inject.Inject;",
398-
"import javax.inject.Provider;",
399-
"",
400-
"final class Outer {",
401-
" static class A {",
402-
" @Inject A(B bParam) {}",
403-
" }",
404-
"",
405-
" static class B {",
406-
" @Inject B(C cParam, D dParam) {}",
407-
" }",
408-
"",
409-
" static class C {",
410-
" @Inject C(Provider<A> aParam) {}",
411-
" }",
412-
"",
413-
" static class D {",
414-
" @Inject D() {}",
415-
" }",
416-
"",
417-
" @Component()",
418-
" interface CComponent {",
419-
" C get();",
420-
" }",
421-
"}");
422-
423-
String expectedWarning =
424-
Joiner.on('\n')
425-
.join(
426-
"test.Outer.CComponent.get() contains a dependency cycle. "
427-
+ "You can suppress this warning by annotating the component method, the "
428-
+ "component, or any dependency request in the cycle with "
429-
+ "@SuppressWarnings(\"dependency-cycle\"):",
430-
" test.Outer.C.<init>(javax.inject.Provider<test.Outer.A> aParam)",
431-
" [parameter: javax.inject.Provider<test.Outer.A> aParam]",
432-
" test.Outer.A.<init>(test.Outer.B bParam)",
433-
" [parameter: test.Outer.B bParam]",
434-
" test.Outer.B.<init>(test.Outer.C cParam, test.Outer.D dParam)",
435-
" [parameter: test.Outer.C cParam]");
436-
assertAbout(javaSource())
437-
.that(component)
438-
.withCompilerOptions("-Xlint:-processing", "-Xlint:-rawtypes")
439-
.processedWith(new ComponentProcessor())
440-
.compilesWithoutError()
441-
.withWarningContaining(expectedWarning)
442-
.in(component)
443-
.onLine(28);
444-
}
445-
446-
@Test
447-
public void cyclicDependencySimpleProviderIndirectionWarningSuppressed() {
448-
JavaFileObject component =
449-
JavaFileObjects.forSourceLines(
450-
"test.Outer",
451-
"package test;",
452-
"",
453-
"import dagger.Component;",
454-
"import dagger.Module;",
455-
"import dagger.Provides;",
456-
"import javax.inject.Inject;",
457-
"import javax.inject.Provider;",
458-
"",
459-
"final class Outer {",
460-
" static class A {",
461-
" @Inject A(B bParam) {}",
462-
" }",
463-
"",
464-
" static class B {",
465-
" @Inject B(C cParam, D dParam) {}",
466-
" }",
467-
"",
468-
" static class C {",
469-
" @Inject C(Provider<A> aParam) {}",
470-
" }",
471-
"",
472-
" static class D {",
473-
" @Inject D() {}",
474-
" }",
475-
"",
476-
" @SuppressWarnings(\"dependency-cycle\")",
477-
" @Component()",
478-
" interface CComponent {",
479-
" C get();",
480-
" }",
481-
"}");
482-
483-
assertAbout(javaSource())
484-
.that(component)
485-
.withCompilerOptions("-Xlint:-processing", "-Xlint:-rawtypes")
486-
.processedWith(new ComponentProcessor())
487-
.compilesWithoutWarnings();
488-
}
489-
490-
@Test
491-
public void cyclicDependencySimpleProviderIndirectionWarningSuppressed_atDependencyRequest() {
492-
JavaFileObject component =
493-
JavaFileObjects.forSourceLines(
494-
"test.Outer",
495-
"package test;",
496-
"",
497-
"import dagger.Component;",
498-
"import dagger.Module;",
499-
"import dagger.Provides;",
500-
"import javax.inject.Inject;",
501-
"import javax.inject.Provider;",
502-
"",
503-
"final class Outer {",
504-
" static class A {",
505-
" @Inject A(B bParam) {}",
506-
" }",
507-
"",
508-
" static class B {",
509-
" @Inject B(C bParam, D dParam) {}",
510-
" }",
511-
"",
512-
" static class C {",
513-
" @Inject C(@SuppressWarnings(\"dependency-cycle\") Provider<A> aParam) {}",
514-
" }",
515-
"",
516-
" static class D {",
517-
" @Inject D() {}",
518-
" }",
519-
"",
520-
" @Component()",
521-
" interface CComponent {",
522-
" C get();",
523-
" }",
524-
"}");
525-
526-
assertAbout(javaSource())
527-
.that(component)
528-
.withCompilerOptions("-Xlint:-processing", "-Xlint:-rawtypes")
529-
.processedWith(new ComponentProcessor())
530-
.compilesWithoutWarnings();
531-
}
532-
533-
@Test
534-
public void cyclicDependencySimpleProviderIndirectionWarningNotSuppressed_atDependencyRequest() {
535-
JavaFileObject component =
536-
JavaFileObjects.forSourceLines(
537-
"test.Outer",
538-
"package test;",
539-
"",
540-
"import dagger.Component;",
541-
"import dagger.Module;",
542-
"import dagger.Provides;",
543-
"import javax.inject.Inject;",
544-
"import javax.inject.Provider;",
545-
"",
546-
"final class Outer {",
547-
" static class A {",
548-
" @Inject A(B bParam) {}",
549-
" }",
550-
"",
551-
" static class B {",
552-
" @Inject B(@SuppressWarnings(\"dependency-cycle\") C cParam, D dParam) {}",
553-
" }",
554-
"",
555-
" static class C {",
556-
" @Inject C(Provider<A> aParam) {}",
557-
" }",
558-
"",
559-
" static class D {",
560-
" @Inject D() {}",
561-
" }",
562-
"",
563-
" @Component()",
564-
" interface CComponent {",
565-
" C get();",
566-
" }",
567-
"}");
568-
569-
assertAbout(javaSource())
570-
.that(component)
571-
.withCompilerOptions("-Xlint:-processing", "-Xlint:-rawtypes")
572-
.processedWith(new ComponentProcessor())
573-
.compilesWithoutError()
574-
.withWarningContaining("dependency cycle");
575-
}
576-
577387
@Test public void duplicateExplicitBindings_ProvidesAndComponentProvision() {
578388
JavaFileObject component = JavaFileObjects.forSourceLines("test.Outer",
579389
"package test;",

0 commit comments

Comments
 (0)