Skip to content

Commit

Permalink
Support revisit full binding graph from validation plugins.
Browse files Browse the repository at this point in the history
RELNOTES=n/a
PiperOrigin-RevId: 504878700
  • Loading branch information
wanyingd1996 authored and Dagger Team committed Jan 26, 2023
1 parent 9ca1213 commit bd9a25e
Show file tree
Hide file tree
Showing 18 changed files with 208 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import dagger.Provides;
import dagger.internal.codegen.compileroption.CompilerOptions;
import dagger.internal.codegen.validation.Validation;
import dagger.internal.codegen.validation.ValidationBindingGraphPlugin;
import dagger.spi.model.BindingGraphPlugin;

/** Binds the set of {@link BindingGraphPlugin}s used to implement Dagger validation. */
Expand All @@ -29,7 +30,7 @@ public interface BindingGraphValidationModule {

@Provides
@Validation
static ImmutableSet<BindingGraphPlugin> providePlugins(
static ImmutableSet<ValidationBindingGraphPlugin> providePlugins(
CompositeBindingGraphPlugin.Factory factory,
CompilerOptions compilerOptions,
DependencyCycleValidator validation1,
Expand All @@ -43,18 +44,19 @@ static ImmutableSet<BindingGraphPlugin> providePlugins(
ProvisionDependencyOnProducerBindingValidator validation9,
SetMultibindingValidator validation10,
SubcomponentFactoryMethodValidator validation11) {
ImmutableSet<BindingGraphPlugin> plugins = ImmutableSet.of(
validation1,
validation2,
validation3,
validation4,
validation5,
validation6,
validation7,
validation8,
validation9,
validation10,
validation11);
ImmutableSet<ValidationBindingGraphPlugin> plugins =
ImmutableSet.of(
validation1,
validation2,
validation3,
validation4,
validation5,
validation6,
validation7,
validation8,
validation9,
validation10,
validation11);
if (compilerOptions.experimentalDaggerErrorMessages()) {
return ImmutableSet.of(factory.create(plugins));
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import dagger.assisted.AssistedFactory;
import dagger.assisted.AssistedInject;
import dagger.internal.codegen.validation.DiagnosticMessageGenerator;
import dagger.internal.codegen.validation.ValidationBindingGraphPlugin;
import dagger.spi.model.BindingGraph;
import dagger.spi.model.BindingGraph.ChildFactoryMethodEdge;
import dagger.spi.model.BindingGraph.ComponentNode;
Expand All @@ -38,27 +39,29 @@
import dagger.spi.model.BindingGraphPlugin;
import dagger.spi.model.DaggerProcessingEnv;
import dagger.spi.model.DiagnosticReporter;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import javax.tools.Diagnostic;

/**
* Combines many {@link BindingGraphPlugin} implementations. This helps reduce spam by combining
* all of the messages that are reported on the root component.
* Combines many {@link BindingGraphPlugin} implementations. This helps reduce spam by combining all
* of the messages that are reported on the root component.
*/
final class CompositeBindingGraphPlugin implements BindingGraphPlugin {
final class CompositeBindingGraphPlugin extends ValidationBindingGraphPlugin {
@AssistedFactory
interface Factory {
CompositeBindingGraphPlugin create(ImmutableSet<BindingGraphPlugin> plugins);
CompositeBindingGraphPlugin create(ImmutableSet<ValidationBindingGraphPlugin> plugins);
}

private final ImmutableSet<BindingGraphPlugin> plugins;
private final ImmutableSet<ValidationBindingGraphPlugin> plugins;
private final DiagnosticMessageGenerator.Factory messageGeneratorFactory;
private final Map<ComponentNode, String> errorMessages = new HashMap<>();

@AssistedInject
CompositeBindingGraphPlugin(
@Assisted ImmutableSet<BindingGraphPlugin> plugins,
@Assisted ImmutableSet<ValidationBindingGraphPlugin> plugins,
DiagnosticMessageGenerator.Factory messageGeneratorFactory) {
this.plugins = plugins;
this.messageGeneratorFactory = messageGeneratorFactory;
Expand All @@ -68,10 +71,38 @@ interface Factory {
public void visitGraph(BindingGraph bindingGraph, DiagnosticReporter diagnosticReporter) {
AggregatingDiagnosticReporter aggregatingDiagnosticReporter = new AggregatingDiagnosticReporter(
bindingGraph, diagnosticReporter, messageGeneratorFactory.create(bindingGraph));
plugins.forEach(plugin -> {
aggregatingDiagnosticReporter.setCurrentPlugin(plugin.pluginName());
plugin.visitGraph(bindingGraph, aggregatingDiagnosticReporter);
});
plugins.forEach(
plugin -> {
aggregatingDiagnosticReporter.setCurrentPlugin(plugin.pluginName());
plugin.visitGraph(bindingGraph, aggregatingDiagnosticReporter);
if (plugin.visitFullGraphRequested(bindingGraph)) {
requestVisitFullGraph(bindingGraph);
}
});
if (visitFullGraphRequested(bindingGraph)) {
errorMessages.put(
bindingGraph.rootComponentNode(), aggregatingDiagnosticReporter.getMessage());
} else {
aggregatingDiagnosticReporter.report();
}
}

@Override
public void revisitFullGraph(
BindingGraph prunedGraph, BindingGraph fullGraph, DiagnosticReporter diagnosticReporter) {
AggregatingDiagnosticReporter aggregatingDiagnosticReporter =
new AggregatingDiagnosticReporter(
fullGraph,
diagnosticReporter,
errorMessages.get(prunedGraph.rootComponentNode()),
messageGeneratorFactory.create(fullGraph));
plugins.stream()
.filter(plugin -> plugin.visitFullGraphRequested(prunedGraph))
.forEach(
plugin -> {
aggregatingDiagnosticReporter.setCurrentPlugin(plugin.pluginName());
plugin.revisitFullGraph(prunedGraph, fullGraph, aggregatingDiagnosticReporter);
});
aggregatingDiagnosticReporter.report();
}

Expand Down Expand Up @@ -118,6 +149,17 @@ private static final class AggregatingDiagnosticReporter implements DiagnosticRe
this.messageGenerator = messageGenerator;
}

AggregatingDiagnosticReporter(
BindingGraph graph,
DiagnosticReporter delegate,
String baseMessage,
DiagnosticMessageGenerator messageGenerator) {
this.graph = graph;
this.delegate = delegate;
this.messageGenerator = messageGenerator;
this.messageBuilder.append(baseMessage);
}

/** Sets the currently running aggregated plugin. Used to add a diagnostic prefix. */
void setCurrentPlugin(String pluginName) {
currentPluginName = pluginName;
Expand All @@ -133,6 +175,10 @@ void report() {
}
}

String getMessage() {
return messageBuilder.toString();
}

@Override
public void reportComponent(Diagnostic.Kind diagnosticKind, ComponentNode componentNode,
String message) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@
import dagger.internal.codegen.base.OptionalType;
import dagger.internal.codegen.binding.DependencyRequestFormatter;
import dagger.internal.codegen.javapoet.TypeNames;
import dagger.internal.codegen.validation.ValidationBindingGraphPlugin;
import dagger.spi.model.Binding;
import dagger.spi.model.BindingGraph;
import dagger.spi.model.BindingGraph.ComponentNode;
import dagger.spi.model.BindingGraph.DependencyEdge;
import dagger.spi.model.BindingGraph.Node;
import dagger.spi.model.BindingGraphPlugin;
import dagger.spi.model.BindingKind;
import dagger.spi.model.DependencyRequest;
import dagger.spi.model.DiagnosticReporter;
Expand All @@ -61,7 +61,7 @@
import javax.inject.Inject;

/** Reports errors for dependency cycles. */
final class DependencyCycleValidator implements BindingGraphPlugin {
final class DependencyCycleValidator extends ValidationBindingGraphPlugin {

private final DependencyRequestFormatter dependencyRequestFormatter;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@

import dagger.internal.codegen.binding.KeyFactory;
import dagger.internal.codegen.compileroption.CompilerOptions;
import dagger.internal.codegen.validation.ValidationBindingGraphPlugin;
import dagger.spi.model.Binding;
import dagger.spi.model.BindingGraph;
import dagger.spi.model.BindingGraph.MaybeBinding;
import dagger.spi.model.BindingGraphPlugin;
import dagger.spi.model.DiagnosticReporter;
import dagger.spi.model.Key;
import javax.inject.Inject;
Expand All @@ -33,7 +33,7 @@
* Reports an error on all bindings that depend explicitly on the {@code @Production Executor} key.
*/
// TODO(dpb,beder): Validate this during @Inject/@Provides/@Produces validation.
final class DependsOnProductionExecutorValidator implements BindingGraphPlugin {
final class DependsOnProductionExecutorValidator extends ValidationBindingGraphPlugin {
private final CompilerOptions compilerOptions;
private final KeyFactory keyFactory;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@
import dagger.internal.codegen.binding.BindingNode;
import dagger.internal.codegen.binding.MultibindingDeclaration;
import dagger.internal.codegen.compileroption.CompilerOptions;
import dagger.internal.codegen.validation.ValidationBindingGraphPlugin;
import dagger.spi.model.Binding;
import dagger.spi.model.BindingGraph;
import dagger.spi.model.BindingGraph.ComponentNode;
import dagger.spi.model.BindingGraphPlugin;
import dagger.spi.model.BindingKind;
import dagger.spi.model.ComponentPath;
import dagger.spi.model.DaggerElement;
Expand All @@ -63,7 +63,7 @@
import javax.tools.Diagnostic.Kind;

/** Reports errors for conflicting bindings with the same key. */
final class DuplicateBindingsValidator implements BindingGraphPlugin {
final class DuplicateBindingsValidator extends ValidationBindingGraphPlugin {

private static final Comparator<Binding> BY_LENGTH_OF_COMPONENT_PATH =
comparing(binding -> binding.componentPath().components().size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@
import dagger.internal.codegen.binding.MethodSignatureFormatter;
import dagger.internal.codegen.compileroption.CompilerOptions;
import dagger.internal.codegen.validation.DiagnosticMessageGenerator;
import dagger.internal.codegen.validation.ValidationBindingGraphPlugin;
import dagger.spi.model.Binding;
import dagger.spi.model.BindingGraph;
import dagger.spi.model.BindingGraph.ComponentNode;
import dagger.spi.model.BindingGraphPlugin;
import dagger.spi.model.DiagnosticReporter;
import java.util.Optional;
import java.util.Set;
Expand All @@ -44,7 +44,7 @@
* Reports an error for any component that uses bindings with scopes that are not assigned to the
* component.
*/
final class IncompatiblyScopedBindingsValidator implements BindingGraphPlugin {
final class IncompatiblyScopedBindingsValidator extends ValidationBindingGraphPlugin {

private final MethodSignatureFormatter methodSignatureFormatter;
private final CompilerOptions compilerOptions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,16 @@
import static dagger.spi.model.BindingKind.INJECTION;

import dagger.internal.codegen.validation.InjectValidator;
import dagger.internal.codegen.validation.ValidationBindingGraphPlugin;
import dagger.internal.codegen.validation.ValidationReport;
import dagger.internal.codegen.validation.ValidationReport.Item;
import dagger.spi.model.Binding;
import dagger.spi.model.BindingGraph;
import dagger.spi.model.BindingGraphPlugin;
import dagger.spi.model.DiagnosticReporter;
import javax.inject.Inject;

/** Validates bindings from {@code @Inject}-annotated constructors. */
final class InjectBindingValidator implements BindingGraphPlugin {
final class InjectBindingValidator extends ValidationBindingGraphPlugin {
private final InjectValidator injectValidator;

@Inject
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@
import dagger.internal.codegen.binding.ContributionBinding;
import dagger.internal.codegen.binding.KeyFactory;
import dagger.internal.codegen.javapoet.TypeNames;
import dagger.internal.codegen.validation.ValidationBindingGraphPlugin;
import dagger.spi.model.Binding;
import dagger.spi.model.BindingGraph;
import dagger.spi.model.BindingGraphPlugin;
import dagger.spi.model.DiagnosticReporter;
import dagger.spi.model.Key;
import java.util.Set;
Expand All @@ -49,7 +49,7 @@
* Reports an error for any map binding with either more than one contribution with the same map key
* or contributions with inconsistent map key annotation types.
*/
final class MapMultibindingValidator implements BindingGraphPlugin {
final class MapMultibindingValidator extends ValidationBindingGraphPlugin {

private final BindingDeclarationFormatter bindingDeclarationFormatter;
private final KeyFactory keyFactory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@
import dagger.internal.codegen.binding.DependencyRequestFormatter;
import dagger.internal.codegen.binding.InjectBindingRegistry;
import dagger.internal.codegen.validation.DiagnosticMessageGenerator;
import dagger.internal.codegen.validation.ValidationBindingGraphPlugin;
import dagger.spi.model.Binding;
import dagger.spi.model.BindingGraph;
import dagger.spi.model.BindingGraph.ComponentNode;
import dagger.spi.model.BindingGraph.DependencyEdge;
import dagger.spi.model.BindingGraph.Edge;
import dagger.spi.model.BindingGraph.MissingBinding;
import dagger.spi.model.BindingGraph.Node;
import dagger.spi.model.BindingGraphPlugin;
import dagger.spi.model.ComponentPath;
import dagger.spi.model.DiagnosticReporter;
import dagger.spi.model.Key;
Expand All @@ -47,7 +47,7 @@
import javax.inject.Inject;

/** Reports errors for missing bindings. */
final class MissingBindingValidator implements BindingGraphPlugin {
final class MissingBindingValidator extends ValidationBindingGraphPlugin {

private final InjectBindingRegistry injectBindingRegistry;
private final DependencyRequestFormatter dependencyRequestFormatter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,18 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import dagger.internal.codegen.compileroption.CompilerOptions;
import dagger.internal.codegen.validation.ValidationBindingGraphPlugin;
import dagger.spi.model.Binding;
import dagger.spi.model.BindingGraph;
import dagger.spi.model.BindingGraph.DependencyEdge;
import dagger.spi.model.BindingGraphPlugin;
import dagger.spi.model.DiagnosticReporter;
import javax.inject.Inject;

/**
* Reports errors or warnings (depending on the {@code -Adagger.nullableValidation} value) for each
* non-nullable dependency request that is satisfied by a nullable binding.
*/
final class NullableBindingValidator implements BindingGraphPlugin {
final class NullableBindingValidator extends ValidationBindingGraphPlugin {

private final CompilerOptions compilerOptions;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@
import static dagger.internal.codegen.extension.DaggerStreams.instancesOf;
import static javax.tools.Diagnostic.Kind.ERROR;

import dagger.internal.codegen.validation.ValidationBindingGraphPlugin;
import dagger.spi.model.Binding;
import dagger.spi.model.BindingGraph;
import dagger.spi.model.BindingGraph.DependencyEdge;
import dagger.spi.model.BindingGraph.Node;
import dagger.spi.model.BindingGraphPlugin;
import dagger.spi.model.DiagnosticReporter;
import java.util.stream.Stream;
import javax.inject.Inject;
Expand All @@ -36,7 +36,7 @@
* binding.
*/
// TODO(b/29509141): Clarify the error.
final class ProvisionDependencyOnProducerBindingValidator implements BindingGraphPlugin {
final class ProvisionDependencyOnProducerBindingValidator extends ValidationBindingGraphPlugin {

@Inject
ProvisionDependencyOnProducerBindingValidator() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Multimap;
import dagger.internal.codegen.validation.ValidationBindingGraphPlugin;
import dagger.spi.model.Binding;
import dagger.spi.model.BindingGraph;
import dagger.spi.model.BindingGraphPlugin;
import dagger.spi.model.DiagnosticReporter;
import dagger.spi.model.Key;
import javax.inject.Inject;

/** Validates that there are not multiple set binding contributions to the same binding. */
final class SetMultibindingValidator implements BindingGraphPlugin {
final class SetMultibindingValidator extends ValidationBindingGraphPlugin {

@Inject
SetMultibindingValidator() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@
import com.google.common.collect.Sets.SetView;
import dagger.internal.codegen.base.Util;
import dagger.internal.codegen.binding.ComponentNodeImpl;
import dagger.internal.codegen.validation.ValidationBindingGraphPlugin;
import dagger.spi.model.BindingGraph;
import dagger.spi.model.BindingGraph.ChildFactoryMethodEdge;
import dagger.spi.model.BindingGraph.ComponentNode;
import dagger.spi.model.BindingGraphPlugin;
import dagger.spi.model.DiagnosticReporter;
import java.util.HashMap;
import java.util.Map;
Expand All @@ -43,7 +43,7 @@
import javax.inject.Inject;

/** Reports an error if a subcomponent factory method is missing required modules. */
final class SubcomponentFactoryMethodValidator implements BindingGraphPlugin {
final class SubcomponentFactoryMethodValidator extends ValidationBindingGraphPlugin {

private final Map<ComponentNode, Set<XTypeElement>> inheritedModulesCache = new HashMap<>();

Expand Down
Loading

0 comments on commit bd9a25e

Please sign in to comment.