diff --git a/dagger-compiler/main/java/dagger/internal/codegen/processingstep/ModuleProcessingStep.java b/dagger-compiler/main/java/dagger/internal/codegen/processingstep/ModuleProcessingStep.java index 922283f436e..fd22f2777cf 100644 --- a/dagger-compiler/main/java/dagger/internal/codegen/processingstep/ModuleProcessingStep.java +++ b/dagger-compiler/main/java/dagger/internal/codegen/processingstep/ModuleProcessingStep.java @@ -25,13 +25,16 @@ import androidx.room3.compiler.processing.XProcessingEnv; import androidx.room3.compiler.processing.XTypeElement; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Sets; import dagger.internal.codegen.base.SourceFileGenerator; import dagger.internal.codegen.base.ValidationReport; import dagger.internal.codegen.binding.BindingFactory; +import dagger.internal.codegen.binding.BindingGraph; +import dagger.internal.codegen.binding.BindingGraphFactory; +import dagger.internal.codegen.binding.ComponentDescriptor; import dagger.internal.codegen.binding.ContributionBinding; import dagger.internal.codegen.binding.DelegateDeclaration; import dagger.internal.codegen.binding.ProductionBinding; +import dagger.internal.codegen.validation.BindingGraphValidator; import dagger.internal.codegen.validation.ModuleValidator; import dagger.internal.codegen.writing.InaccessibleMapKeyProxyGenerator; import dagger.internal.codegen.writing.ModuleGenerator; @@ -52,7 +55,9 @@ final class ModuleProcessingStep extends TypeCheckingProcessingStep moduleConstructorProxyGenerator; private final InaccessibleMapKeyProxyGenerator inaccessibleMapKeyProxyGenerator; private final DelegateDeclaration.Factory delegateDeclarationFactory; - private final Set processedModuleElements = Sets.newLinkedHashSet(); + private final ComponentDescriptor.Factory componentDescriptorFactory; + private final BindingGraphFactory bindingGraphFactory; + private final BindingGraphValidator bindingGraphValidator; @Inject ModuleProcessingStep( @@ -62,7 +67,10 @@ final class ModuleProcessingStep extends TypeCheckingProcessingStep producerFactoryGenerator, @ModuleGenerator SourceFileGenerator moduleConstructorProxyGenerator, InaccessibleMapKeyProxyGenerator inaccessibleMapKeyProxyGenerator, - DelegateDeclaration.Factory delegateDeclarationFactory) { + DelegateDeclaration.Factory delegateDeclarationFactory, + ComponentDescriptor.Factory componentDescriptorFactory, + BindingGraphFactory bindingGraphFactory, + BindingGraphValidator bindingGraphValidator) { this.moduleValidator = moduleValidator; this.bindingFactory = bindingFactory; this.factoryGenerator = factoryGenerator; @@ -70,6 +78,9 @@ final class ModuleProcessingStep extends TypeCheckingProcessingStep process( @Override protected void process(XTypeElement module, ImmutableSet annotations) { - if (processedModuleElements.contains(module)) { - return; - } // For backwards compatibility, we allow a companion object to be annotated with @Module even // though it's no longer required. However, we skip processing the companion object itself // because it will now be processed when processing the companion object's enclosing class. @@ -104,14 +112,23 @@ protected void process(XTypeElement module, ImmutableSet annotations } ValidationReport report = moduleValidator.validate(module); report.printMessagesTo(messager); - if (report.isClean()) { - generateForMethodsIn(module); - module.getEnclosedTypeElements().stream() - .filter(XTypeElement::isCompanionObject) - .collect(toOptional()) - .ifPresent(this::generateForMethodsIn); + if (!report.isClean()) { + return; + } + if (bindingGraphValidator.shouldDoFullBindingGraphValidation(module)) { + BindingGraph bindingGraph = + bindingGraphFactory.create( + componentDescriptorFactory.moduleComponentDescriptor(module), + /* createFullBindingGraph= */ true); + if (!bindingGraphValidator.isValid(bindingGraph.topLevelBindingGraph())) { + return; + } } - processedModuleElements.add(module); + generateForMethodsIn(module); + module.getEnclosedTypeElements().stream() + .filter(XTypeElement::isCompanionObject) + .collect(toOptional()) + .ifPresent(this::generateForMethodsIn); } private void generateForMethodsIn(XTypeElement module) { diff --git a/dagger-compiler/main/java/dagger/internal/codegen/validation/ModuleValidator.java b/dagger-compiler/main/java/dagger/internal/codegen/validation/ModuleValidator.java index 213e0bbb36c..5a187376fbd 100644 --- a/dagger-compiler/main/java/dagger/internal/codegen/validation/ModuleValidator.java +++ b/dagger-compiler/main/java/dagger/internal/codegen/validation/ModuleValidator.java @@ -59,12 +59,9 @@ import dagger.internal.codegen.base.DaggerSuperficialValidation; import dagger.internal.codegen.base.ModuleKind; import dagger.internal.codegen.base.ValidationReport; -import dagger.internal.codegen.binding.BindingGraphFactory; -import dagger.internal.codegen.binding.ComponentDescriptor; import dagger.internal.codegen.binding.ComponentRequirement; import dagger.internal.codegen.binding.InjectionAnnotations; import dagger.internal.codegen.binding.MethodSignatureFormatter; -import dagger.internal.codegen.model.BindingGraph; import dagger.internal.codegen.model.Scope; import dagger.internal.codegen.xprocessing.XElements; import dagger.internal.codegen.xprocessing.XTypeNames; @@ -111,9 +108,6 @@ public final class ModuleValidator { private final AnyBindingMethodValidator anyBindingMethodValidator; private final MethodSignatureFormatter methodSignatureFormatter; - private final ComponentDescriptor.Factory componentDescriptorFactory; - private final BindingGraphFactory bindingGraphFactory; - private final BindingGraphValidator bindingGraphValidator; private final InjectionAnnotations injectionAnnotations; private final DaggerSuperficialValidation superficialValidation; private final XProcessingEnv processingEnv; @@ -124,17 +118,11 @@ public final class ModuleValidator { ModuleValidator( AnyBindingMethodValidator anyBindingMethodValidator, MethodSignatureFormatter methodSignatureFormatter, - ComponentDescriptor.Factory componentDescriptorFactory, - BindingGraphFactory bindingGraphFactory, - BindingGraphValidator bindingGraphValidator, InjectionAnnotations injectionAnnotations, DaggerSuperficialValidation superficialValidation, XProcessingEnv processingEnv) { this.anyBindingMethodValidator = anyBindingMethodValidator; this.methodSignatureFormatter = methodSignatureFormatter; - this.componentDescriptorFactory = componentDescriptorFactory; - this.bindingGraphFactory = bindingGraphFactory; - this.bindingGraphValidator = bindingGraphValidator; this.injectionAnnotations = injectionAnnotations; this.superficialValidation = superficialValidation; this.processingEnv = processingEnv; @@ -214,11 +202,6 @@ private ValidationReport validateUncached(XTypeElement module, Set .collect(toOptional()) .ifPresent(companionModule -> validateCompanionModule(companionModule, builder)); - if (builder.build().isClean() - && bindingGraphValidator.shouldDoFullBindingGraphValidation(module)) { - validateModuleBindings(module, builder); - } - return builder.build(); } @@ -634,19 +617,6 @@ private void validateCompanionModule( } } - private void validateModuleBindings(XTypeElement module, ValidationReport.Builder report) { - BindingGraph bindingGraph = - bindingGraphFactory - .create(componentDescriptorFactory.moduleComponentDescriptor(module), true) - .topLevelBindingGraph(); - if (!bindingGraphValidator.isValid(bindingGraph)) { - // Since the validator uses a DiagnosticReporter to report errors, the ValdiationReport won't - // have any Items for them. We have to tell the ValidationReport that some errors were - // reported for the subject. - report.markDirty(); - } - } - private static String formatListForErrorMessage(List things) { switch (things.size()) { case 0: diff --git a/javatests/dagger/internal/codegen/DuplicateBindingsValidationTest.java b/javatests/dagger/internal/codegen/DuplicateBindingsValidationTest.java index a28b906e1c4..0d1a25ba208 100644 --- a/javatests/dagger/internal/codegen/DuplicateBindingsValidationTest.java +++ b/javatests/dagger/internal/codegen/DuplicateBindingsValidationTest.java @@ -739,15 +739,18 @@ public void childBindingConflictsWithParent() { " @Provides Object test.B.BModule.abConflict()"); if (fullBindingGraphValidation) { subject.hasErrorCount(2); - subject.hasErrorContaining("test.A.AModule has errors") + subject + .hasErrorContaining(errorMessage) .onSource(aComponent) - .onLineContaining("@Component("); - subject.hasErrorContaining(errorMessage) + .onLineContaining("interface A {"); + subject + .hasErrorContaining(errorMessage) .onSource(aComponent) .onLineContaining("class AModule"); } else { subject.hasErrorCount(1); - subject.hasErrorContaining(errorMessage) + subject + .hasErrorContaining(errorMessage) .onSource(aComponent) .onLineContaining("interface A {"); } @@ -834,15 +837,18 @@ public void grandchildBindingConflictsWithGrandparent() { " @Provides Object test.C.CModule.acConflict()"); if (fullBindingGraphValidation) { subject.hasErrorCount(2); - subject.hasErrorContaining("test.A.AModule has errors") + subject + .hasErrorContaining(errorMessage) .onSource(aComponent) - .onLineContaining("@Component("); - subject.hasErrorContaining(errorMessage) + .onLineContaining("interface A {"); + subject + .hasErrorContaining(errorMessage) .onSource(aComponent) .onLineContaining("class AModule"); } else { subject.hasErrorCount(1); - subject.hasErrorContaining(errorMessage) + subject + .hasErrorContaining(errorMessage) .onSource(aComponent) .onLineContaining("interface A {"); } @@ -923,16 +929,23 @@ public void grandchildBindingConflictsWithChild() { " @Provides Object test.B.BModule.bcConflict()", " @Provides Object test.C.CModule.bcConflict()"); if (fullBindingGraphValidation) { - subject.hasErrorCount(2); - subject.hasErrorContaining("test.B.BModule has errors") + subject.hasErrorCount(3); + subject + .hasErrorContaining(errorMessage) + .onSource(aComponent) + .onLineContaining("interface A {"); + subject + .hasErrorContaining(errorMessage) .onSource(bComponent) - .onLineContaining("@Subcomponent(modules = B.BModule.class)"); - subject.hasErrorContaining(errorMessage) + .onLineContaining("interface B {"); + subject + .hasErrorContaining(errorMessage) .onSource(bComponent) .onLineContaining("class BModule"); } else { subject.hasErrorCount(1); - subject.hasErrorContaining(errorMessage) + subject + .hasErrorContaining(errorMessage) .onSource(aComponent) .onLineContaining("interface A {"); } diff --git a/javatests/dagger/internal/codegen/FullBindingGraphValidationTest.java b/javatests/dagger/internal/codegen/FullBindingGraphValidationTest.java index 50ad0a6e014..5988b4ed903 100644 --- a/javatests/dagger/internal/codegen/FullBindingGraphValidationTest.java +++ b/javatests/dagger/internal/codegen/FullBindingGraphValidationTest.java @@ -140,12 +140,14 @@ public void includesModuleWithErrors_validationTypeError() { .compile( subject -> { subject.hasErrorCount(2); - subject.hasErrorContainingMatch(MODULE_WITH_ERRORS_MESSAGE.pattern()) + subject + .hasErrorContainingMatch(MODULE_WITH_ERRORS_MESSAGE.pattern()) .onSource(MODULE_WITH_ERRORS) .onLineContaining("interface ModuleWithErrors"); - subject.hasErrorContaining("ModuleWithErrors has errors") + subject + .hasErrorContainingMatch(INCLUDES_MODULE_WITH_ERRORS_MESSAGE.pattern()) .onSource(INCLUDES_MODULE_WITH_ERRORS) - .onLineContaining("ModuleWithErrors.class"); + .onLineContaining("interface IncludesModuleWithErrors"); }); }