Skip to content

Commit

Permalink
Fixed the incorrect logic from ProductionBindingRepresentation which …
Browse files Browse the repository at this point in the history
…could have resulted in type casting error.

In the existing logic, if someone is requesting a production binding with Provider, Dagger will end up generating a Producer typed expression.

This cl changes the behavior, so that if a binding request kind for a production binding is PROVIDER, then use derivedFromFrameworkInstanceRequestRepresentation to derive the expression from the corresponding ProducerNodeInstanceRequestRepresentation. This will still fail, but with better error message "cannot request a PROVIDER from PRODUCER_NODE". We can follow up to decide whether we intend to support the behavior.

RELNOTES=n/a
PiperOrigin-RevId: 623929516
  • Loading branch information
wanyingd1996 authored and Dagger Team committed Apr 19, 2024
1 parent 4077ea0 commit 5043bcc
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 8 deletions.
2 changes: 2 additions & 0 deletions java/dagger/internal/codegen/binding/FrameworkType.java
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ public static Optional<FrameworkType> forRequestKind(RequestKind requestKind) {
switch (requestKind) {
case PROVIDER:
return Optional.of(FrameworkType.PROVIDER);
case PRODUCER:
return Optional.of(FrameworkType.PRODUCER_NODE);
default:
return Optional.empty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import dagger.internal.codegen.model.BindingGraph.DependencyEdge;
import dagger.internal.codegen.model.BindingGraph.Node;
import dagger.internal.codegen.model.DiagnosticReporter;
import dagger.internal.codegen.model.RequestKind;
import dagger.internal.codegen.validation.ValidationBindingGraphPlugin;
import java.util.stream.Stream;
import javax.inject.Inject;
Expand All @@ -48,7 +49,7 @@ public String pluginName() {

@Override
public void visitGraph(BindingGraph bindingGraph, DiagnosticReporter diagnosticReporter) {
provisionDependenciesOnProductionBindings(bindingGraph)
provisionDependenciesOnProductionBindings(bindingGraph, diagnosticReporter)
.forEach(
provisionDependent ->
diagnosticReporter.reportDependency(
Expand All @@ -60,11 +61,18 @@ public void visitGraph(BindingGraph bindingGraph, DiagnosticReporter diagnosticR
}

private Stream<DependencyEdge> provisionDependenciesOnProductionBindings(
BindingGraph bindingGraph) {
BindingGraph bindingGraph, DiagnosticReporter diagnosticReporter) {
return bindingGraph.bindings().stream()
.filter(binding -> binding.isProduction())
.flatMap(binding -> incomingDependencies(binding, bindingGraph))
.filter(edge -> !dependencyCanUseProduction(edge, bindingGraph));
.filter(
edge -> {
if (edge.dependencyRequest().kind().equals(RequestKind.PROVIDER)) {
diagnosticReporter.reportDependency(
ERROR, edge, "Cannot request a producer binding with Provider");
}
return !dependencyCanUseProduction(edge, bindingGraph);
});
}

/** Returns the dependencies on {@code binding}. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import dagger.internal.codegen.binding.BindingRequest;
import dagger.internal.codegen.binding.FrameworkType;
import dagger.internal.codegen.binding.ProductionBinding;
import dagger.internal.codegen.model.RequestKind;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
Expand All @@ -38,7 +39,7 @@ final class ProductionBindingRepresentation implements BindingRepresentation {
private final ProductionBinding binding;
private final DerivedFromFrameworkInstanceRequestRepresentation.Factory
derivedFromFrameworkInstanceRequestRepresentationFactory;
private final RequestRepresentation frameworkInstanceRequestRepresentation;
private final RequestRepresentation producerNodeInstanceRequestRepresentation;
private final Map<BindingRequest, RequestRepresentation> requestRepresentations = new HashMap<>();

@AssistedInject
Expand Down Expand Up @@ -66,7 +67,7 @@ final class ProductionBindingRepresentation implements BindingRepresentation {
? bindingRepresentations.scope(
binding, unscopedFrameworkInstanceCreationExpressionFactory.create(binding))
: unscopedFrameworkInstanceCreationExpressionFactory.create(binding));
this.frameworkInstanceRequestRepresentation =
this.producerNodeInstanceRequestRepresentation =
producerNodeInstanceRequestRepresentationFactory.create(binding, frameworkInstanceSupplier);
}

Expand All @@ -77,11 +78,11 @@ public RequestRepresentation getRequestRepresentation(BindingRequest request) {
}

private RequestRepresentation getRequestRepresentationUncached(BindingRequest request) {
return request.frameworkType().isPresent()
? frameworkInstanceRequestRepresentation
return request.requestKind().equals(RequestKind.PRODUCER)
? producerNodeInstanceRequestRepresentation
: derivedFromFrameworkInstanceRequestRepresentationFactory.create(
binding,
frameworkInstanceRequestRepresentation,
producerNodeInstanceRequestRepresentation,
request.requestKind(),
FrameworkType.PRODUCER_NODE);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package dagger.internal.codegen;


import androidx.room.compiler.processing.util.Source;
import com.google.common.collect.ImmutableMap;
import dagger.testing.compile.CompilerTests;
Expand Down Expand Up @@ -415,4 +416,68 @@ public void productionScope_injectConstructor() throws Exception {
subject.generatedSource(goldenFileRule.goldenSource("test/DaggerParent"));
});
}

@Test
public void requestProducerNodeWithProvider_failsWithNotSupportedError() {
Source moduleFile =
CompilerTests.javaSource(
"test.ExecutorModule",
"package test;",
"",
"import com.google.common.util.concurrent.MoreExecutors;",
"import dagger.Module;",
"import dagger.Provides;",
"import dagger.producers.Production;",
"import java.util.concurrent.Executor;",
"",
"@Module",
"final class ExecutorModule {",
" @Provides @Production Executor executor() {",
" return MoreExecutors.directExecutor();",
" }",
"}");
Source producerModuleFile =
CompilerTests.javaSource(
"test.SimpleModule",
"package test;",
"",
"import dagger.producers.ProducerModule;",
"import dagger.producers.Produces;",
"import javax.inject.Provider;",
"import java.util.concurrent.Executor;",
"import dagger.producers.Production;",
"",
"@ProducerModule",
"final class SimpleModule {",
" @Produces String str(Provider<Integer> num) {",
" return \"\";",
" }",
" @Produces Integer num() { return 1; }",
"}");
Source componentFile =
CompilerTests.javaSource(
"test.SimpleComponent",
"package test;",
"",
"import com.google.common.util.concurrent.ListenableFuture;",
"import dagger.producers.ProductionComponent;",
"",
"@ProductionComponent(modules = {ExecutorModule.class, SimpleModule.class})",
"interface SimpleComponent {",
" ListenableFuture<String> str();",
"",
" @ProductionComponent.Builder",
" interface Builder {",
" SimpleComponent build();",
" }",
"}");

CompilerTests.daggerCompiler(moduleFile, producerModuleFile, componentFile)
.withProcessingOptions(compilerMode.processorOptions())
.compile(
subject -> {
subject.hasErrorCount(1);
subject.hasErrorContaining("request a producer binding with Provider");
});
}
}

0 comments on commit 5043bcc

Please sign in to comment.