Skip to content

Commit

Permalink
Add flag to control LegacyBindingGraphFactory usage.
Browse files Browse the repository at this point in the history
For now, the flag's default is set to enabled to keep the functionality the same as before. However, in a future release (likely 2.55) we will flip the default to `disabled`.

The new `BindingGraphFactory` contains many bug fixes that still exist in the `LegacyBindingGraphFactory`. However, the new `BindingGraphFactory` does have some behavior changes that may affect users. The biggest change is that we no longer allow a module binding installed in a component to float to a subcomponent to satisfy missing bindings. See the javadoc in `CompilerOptions` for more details.

RELNOTES=N/A
PiperOrigin-RevId: 710221159
  • Loading branch information
bcorso authored and Dagger Team committed Dec 30, 2024
1 parent 1620e92 commit 322f084
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.base.Predicates.not;
import static dagger.internal.codegen.binding.AssistedInjectionAnnotations.isAssistedFactoryType;
import static dagger.internal.codegen.binding.LegacyBindingGraphFactory.useLegacyBindingGraphFactory;
import static dagger.internal.codegen.extension.DaggerCollectors.onlyElement;
import static dagger.internal.codegen.extension.DaggerGraphs.unreachableNodes;
import static dagger.internal.codegen.extension.DaggerStreams.toImmutableSet;
Expand Down Expand Up @@ -106,7 +107,7 @@ public final class BindingGraphFactory {
*/
public BindingGraph create(
ComponentDescriptor componentDescriptor, boolean createFullBindingGraph) {
return LegacyBindingGraphFactory.useLegacyBindingGraphFactory(componentDescriptor)
return useLegacyBindingGraphFactory(compilerOptions, componentDescriptor)
? legacyBindingGraphFactory.create(componentDescriptor, createFullBindingGraph)
: createBindingGraph(componentDescriptor, createFullBindingGraph);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,9 @@
/** A factory for {@link BindingGraph} objects. */
public final class LegacyBindingGraphFactory {

static boolean useLegacyBindingGraphFactory(ComponentDescriptor componentDescriptor) {
return true;
static boolean useLegacyBindingGraphFactory(
CompilerOptions compilerOptions, ComponentDescriptor componentDescriptor) {
return compilerOptions.useLegacyBindingGraphFactory();
}

static boolean hasStrictMultibindingsExemption(
Expand Down
20 changes: 20 additions & 0 deletions java/dagger/internal/codegen/compileroption/CompilerOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,26 @@ public final boolean doCheckForNulls() {
*/
public abstract boolean generatedClassExtendsComponent();

/**
* Returns {@code true} if Dagger should use the legacy binding graph factory.
*
* <p>Note: This flag is only intended to give users time to migrate to the new binding graph
* factory. New users should not enable this flag. This flag will be removed in a future release.
*
* <p>The legacy binding graph factory contains a number of bugs which can lead to an incorrect
* binding graph (e.g. missing multibindings), can be difficult to debug, and are often dependent
* on the ordering of bindings/dependency requests in the user's code.
*
* <p>The new binding graph factory fixes many of these issues by switching to a well known graph
* data structure and algorithms to avoid many of the subtle bugs that plagued the legacy binding
* graph factory. However, note that the new binding graph factory also has a behavior change that
* could cause issues for some users. Specifically, a module binding is no longer allowed to float
* from its installed component into one of its subcomponents in order to satisfy a missing
* dependency. Thus, any (transitive) dependencies of the module binding that are missing from the
* installed component will now be reported as an error.
*/
public abstract boolean useLegacyBindingGraphFactory();

/**
* Returns {@code true} if the key for map multibinding contributions contain a framework type.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.STRICT_MULTIBINDING_VALIDATION;
import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.STRICT_SUPERFICIAL_VALIDATION;
import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.USE_FRAMEWORK_TYPE_IN_MAP_MULTIBINDING_CONTRIBUTION_KEY;
import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.USE_LEGACY_BINDING_GRAPH_FACTORY;
import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.VALIDATE_TRANSITIVE_COMPONENT_DEPENDENCIES;
import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.WARN_IF_INJECTION_FACTORY_NOT_GENERATED_UPSTREAM;
import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.WRITE_PRODUCER_NAME_IN_TOKEN;
Expand Down Expand Up @@ -203,6 +204,11 @@ public boolean generatedClassExtendsComponent() {
return isEnabled(GENERATED_CLASS_EXTENDS_COMPONENT);
}

@Override
public boolean useLegacyBindingGraphFactory() {
return isEnabled(USE_LEGACY_BINDING_GRAPH_FACTORY);
}

@Override
public int keysPerComponentShard(XTypeElement component) {
if (options.containsKey(KEYS_PER_COMPONENT_SHARD)) {
Expand Down Expand Up @@ -238,15 +244,14 @@ private ProcessingEnvironmentCompilerOptions checkValid() {
noLongerRecognized(FLOATING_BINDS_METHODS);
noLongerRecognized(EXPERIMENTAL_AHEAD_OF_TIME_SUBCOMPONENTS);
noLongerRecognized(USE_GRADLE_INCREMENTAL_PROCESSING);
if (!isEnabled(IGNORE_PROVISION_KEY_WILDCARDS)) {
if (processingEnv.getBackend() == XProcessingEnv.Backend.KSP) {
processingEnv.getMessager().printMessage(
Diagnostic.Kind.ERROR,
String.format(
"When using KSP, you must also enable the '%s' compiler option (see %s).",
"dagger.ignoreProvisionKeyWildcards",
"https://dagger.dev/dev-guide/compiler-options#ignore-provision-key-wildcards"));
}
if (processingEnv.getBackend() == XProcessingEnv.Backend.KSP
&& !isEnabled(IGNORE_PROVISION_KEY_WILDCARDS)) {
processingEnv.getMessager().printMessage(
Diagnostic.Kind.ERROR,
String.format(
"When using KSP, you must also enable the '%s' compiler option (see %s).",
"dagger.ignoreProvisionKeyWildcards",
"https://dagger.dev/dev-guide/compiler-options#ignore-provision-key-wildcards"));
}
return this;
}
Expand Down Expand Up @@ -337,6 +342,8 @@ enum Feature implements EnumOption<FeatureStatus> {

GENERATED_CLASS_EXTENDS_COMPONENT,

USE_LEGACY_BINDING_GRAPH_FACTORY(ENABLED),

USE_FRAMEWORK_TYPE_IN_MAP_MULTIBINDING_CONTRIBUTION_KEY,

IGNORE_PROVISION_KEY_WILDCARDS(ENABLED),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ public boolean experimentalDaggerErrorMessages() {
return false;
}

@Override
public boolean useLegacyBindingGraphFactory() {
return true;
}

@Override
public boolean useFrameworkTypeInMapMultibindingContributionKey() {
return false;
Expand Down

0 comments on commit 322f084

Please sign in to comment.