Skip to content

Move full binding graph validation for modules from ModuleValidator to ModuleProcessingStep. #5255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
copybara-service merged 1 commit into from
Aug 18, 2026
Merged

Move full binding graph validation for modules from ModuleValidator to ModuleProcessingStep. #5255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 30 additions & 13 deletions dagger-compiler/main/java/dagger/internal/codegen/processingstep/ModuleProcessingStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -52,7 +55,9 @@ final class ModuleProcessingStep extends TypeCheckingProcessingStep<XTypeElement
private final SourceFileGenerator<XTypeElement> moduleConstructorProxyGenerator;
private final InaccessibleMapKeyProxyGenerator inaccessibleMapKeyProxyGenerator;
private final DelegateDeclaration.Factory delegateDeclarationFactory;
private final Set<XTypeElement> processedModuleElements = Sets.newLinkedHashSet();
private final ComponentDescriptor.Factory componentDescriptorFactory;
private final BindingGraphFactory bindingGraphFactory;
private final BindingGraphValidator bindingGraphValidator;

@Inject
ModuleProcessingStep(
Expand All @@ -62,14 +67,20 @@ final class ModuleProcessingStep extends TypeCheckingProcessingStep<XTypeElement
SourceFileGenerator<ProductionBinding> producerFactoryGenerator,
@ModuleGenerator SourceFileGenerator<XTypeElement> 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;
this.producerFactoryGenerator = producerFactoryGenerator;
this.moduleConstructorProxyGenerator = moduleConstructorProxyGenerator;
this.inaccessibleMapKeyProxyGenerator = inaccessibleMapKeyProxyGenerator;
this.delegateDeclarationFactory = delegateDeclarationFactory;
this.componentDescriptorFactory = componentDescriptorFactory;
this.bindingGraphFactory = bindingGraphFactory;
this.bindingGraphValidator = bindingGraphValidator;
}

@Override
Expand All @@ -91,9 +102,6 @@ public ImmutableSet<XElement> process(

@Override
protected void process(XTypeElement module, ImmutableSet<XClassName> 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.
Expand All @@ -104,14 +112,23 @@ protected void process(XTypeElement module, ImmutableSet<XClassName> 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) {
Expand Down
30 changes: 0 additions & 30 deletions dagger-compiler/main/java/dagger/internal/codegen/validation/ModuleValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -214,11 +202,6 @@ private ValidationReport validateUncached(XTypeElement module, Set<XTypeElement>
.collect(toOptional())
.ifPresent(companionModule -> validateCompanionModule(companionModule, builder));

if (builder.build().isClean()
&& bindingGraphValidator.shouldDoFullBindingGraphValidation(module)) {
validateModuleBindings(module, builder);
}

return builder.build();
}

Expand Down Expand Up @@ -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:
Expand Down
39 changes: 26 additions & 13 deletions javatests/dagger/internal/codegen/DuplicateBindingsValidationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {");
}
Expand Down Expand Up @@ -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 {");
}
Expand Down Expand Up @@ -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 {");
}
Expand Down
8 changes: 5 additions & 3 deletions javatests/dagger/internal/codegen/FullBindingGraphValidationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
}

Expand Down
Loading