Skip to content

Commit 369bbc6

Browse files
bcorsoDagger Team
bcorso
authored and
Dagger Team
committed
[Refactor]: Move members injection optimization into its RequestRepresentation.
I've also added tests for this optimization, since nothing failed when I removed it. This CL moves the logic for a members injection optimization into its corresponding RequestRepresentation (`MembersInjectionRequestRepresentation`). The benefit of this refactor is 1. Consolidates the logic for members injection requests into a single location. 2. Simplifies `ComponentRequestRepresentations` and allows us to share more of the code between members injection and contribution component methods. RELNOTES=N/A PiperOrigin-RevId: 702018277
1 parent 841d765 commit 369bbc6

5 files changed

+170
-36
lines changed

‎java/dagger/internal/codegen/writing/ComponentRequestRepresentations.java‎

Lines changed: 12 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,14 @@
1919
import static androidx.room.compiler.processing.XTypeKt.isVoid;
2020
import static com.google.common.base.Preconditions.checkArgument;
2121
import static com.google.common.base.Preconditions.checkNotNull;
22-
import static com.google.common.collect.Iterables.getOnlyElement;
2322
import static dagger.internal.codegen.base.Util.reentrantComputeIfAbsent;
2423
import static dagger.internal.codegen.binding.BindingRequest.bindingRequest;
2524
import static dagger.internal.codegen.javapoet.CodeBlocks.makeParametersCodeBlock;
2625
import static dagger.internal.codegen.langmodel.Accessibility.isRawTypeAccessible;
2726
import static dagger.internal.codegen.langmodel.Accessibility.isTypeAccessibleFrom;
2827
import static dagger.internal.codegen.xprocessing.MethodSpecs.overriding;
29-
import static dagger.internal.codegen.xprocessing.XElements.getSimpleName;
3028
import static dagger.internal.codegen.xprocessing.XProcessingEnvs.isPreJava8SourceVersion;
3129

32-
import androidx.room.compiler.processing.XMethodElement;
3330
import androidx.room.compiler.processing.XProcessingEnv;
3431
import androidx.room.compiler.processing.XType;
3532
import com.google.common.collect.ImmutableList;
@@ -185,47 +182,26 @@ && isRawTypeAccessible(dependencyType, requestingClass.packageName())) {
185182

186183
/** Returns the implementation of a component method. */
187184
public MethodSpec getComponentMethod(ComponentMethodDescriptor componentMethod) {
188-
checkArgument(componentMethod.dependencyRequest().isPresent());
189-
BindingRequest request = bindingRequest(componentMethod.dependencyRequest().get());
190185
return overriding(componentMethod.methodElement(), graph.componentTypeElement().getType())
191-
.addCode(
192-
request.isRequestKind(RequestKind.MEMBERS_INJECTION)
193-
? getMembersInjectionComponentMethodImplementation(request, componentMethod)
194-
: getContributionComponentMethodImplementation(request, componentMethod))
186+
.addCode(getComponentMethodCodeBlock(componentMethod))
195187
.build();
196188
}
197189

198-
private CodeBlock getMembersInjectionComponentMethodImplementation(
199-
BindingRequest request, ComponentMethodDescriptor componentMethod) {
200-
checkArgument(request.isRequestKind(RequestKind.MEMBERS_INJECTION));
201-
XMethodElement methodElement = componentMethod.methodElement();
202-
RequestRepresentation requestRepresentation = getRequestRepresentation(request);
203-
MembersInjectionBinding binding =
204-
((MembersInjectionRequestRepresentation) requestRepresentation).binding();
205-
if (binding.injectionSites().isEmpty()) {
206-
// If there are no injection sites either do nothing (if the return type is void) or return
207-
// the input instance as-is.
208-
return isVoid(methodElement.getReturnType())
209-
? CodeBlock.of("")
210-
: CodeBlock.of(
211-
"return $L;", getSimpleName(getOnlyElement(methodElement.getParameters())));
190+
private CodeBlock getComponentMethodCodeBlock(ComponentMethodDescriptor componentMethod) {
191+
Expression expression = getComponentMethodExpression(componentMethod);
192+
if (isVoid(componentMethod.methodElement().getReturnType())) {
193+
return expression.codeBlock().isEmpty()
194+
? expression.codeBlock()
195+
: CodeBlock.of("$L;", expression.codeBlock());
212196
}
213-
Expression expression = getComponentMethodExpression(requestRepresentation, componentMethod);
214-
return isVoid(methodElement.getReturnType())
215-
? CodeBlock.of("$L;", expression.codeBlock())
216-
: CodeBlock.of("return $L;", expression.codeBlock());
217-
}
218-
219-
private CodeBlock getContributionComponentMethodImplementation(
220-
BindingRequest request, ComponentMethodDescriptor componentMethod) {
221-
checkArgument(!request.isRequestKind(RequestKind.MEMBERS_INJECTION));
222-
Expression expression =
223-
getComponentMethodExpression(getRequestRepresentation(request), componentMethod);
224197
return CodeBlock.of("return $L;", expression.codeBlock());
225198
}
226199

227-
private Expression getComponentMethodExpression(
228-
RequestRepresentation requestRepresentation, ComponentMethodDescriptor componentMethod) {
200+
private Expression getComponentMethodExpression(ComponentMethodDescriptor componentMethod) {
201+
checkArgument(componentMethod.dependencyRequest().isPresent());
202+
BindingRequest request = bindingRequest(componentMethod.dependencyRequest().get());
203+
RequestRepresentation requestRepresentation = getRequestRepresentation(request);
204+
229205
Expression expression =
230206
requestRepresentation.getDependencyExpressionForComponentMethod(
231207
componentMethod, componentImplementation);

‎java/dagger/internal/codegen/writing/MembersInjectionRequestRepresentation.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package dagger.internal.codegen.writing;
1818

19+
import static androidx.room.compiler.processing.XTypeKt.isVoid;
1920
import static com.google.common.collect.Iterables.getOnlyElement;
2021

2122
import androidx.room.compiler.processing.XExecutableParameterElement;
@@ -54,6 +55,15 @@ protected Expression getDependencyExpressionForComponentMethod(
5455
ComponentMethodDescriptor componentMethod, ComponentImplementation component) {
5556
XMethodElement methodElement = componentMethod.methodElement();
5657
XExecutableParameterElement parameter = getOnlyElement(methodElement.getParameters());
58+
if (binding.injectionSites().isEmpty()) {
59+
// If there are no injection sites either do nothing (if the return type is void) or return
60+
// the input instance as-is.
61+
return Expression.create(
62+
methodElement.getReturnType(),
63+
isVoid(methodElement.getReturnType())
64+
? CodeBlock.of("")
65+
: CodeBlock.of("$L", parameter.getJvmName()));
66+
}
5767
return membersInjectionMethods.getInjectExpression(
5868
binding.key(), CodeBlock.of("$L", parameter.getJvmName()), component.name());
5969
}

‎javatests/dagger/internal/codegen/MembersInjectionTest.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,6 +1364,38 @@ public void kotlinNullableFieldInjection() {
13641364
});
13651365
}
13661366

1367+
@Test
1368+
public void testMembersInjectionBindingWithNoInjectionSites() throws Exception {
1369+
Source component =
1370+
CompilerTests.javaSource(
1371+
"test.MyComponent",
1372+
"package test;",
1373+
"",
1374+
"import dagger.Component;",
1375+
"",
1376+
"@Component",
1377+
"public interface MyComponent {",
1378+
" void inject(Foo foo);",
1379+
"",
1380+
" Foo injectAndReturn(Foo foo);",
1381+
"}");
1382+
1383+
Source foo =
1384+
CompilerTests.javaSource(
1385+
"test.Foo",
1386+
"package test;",
1387+
"",
1388+
"class Foo {}");
1389+
1390+
CompilerTests.daggerCompiler(component, foo)
1391+
.withProcessingOptions(compilerMode.processorOptions())
1392+
.compile(
1393+
subject -> {
1394+
subject.hasErrorCount(0);
1395+
subject.generatedSource(goldenFileRule.goldenSource("test/DaggerMyComponent"));
1396+
});
1397+
}
1398+
13671399
private Source stripJetbrainsNullable(Source source) {
13681400
return CompilerTests.javaSource(
13691401
((Source.JavaSource) source).getQName(),

‎javatests/dagger/internal/codegen/goldens/MembersInjectionTest_testMembersInjectionBindingWithNoInjectionSites_DEFAULT_MODE_test.DaggerMyComponent‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package test;
2+
3+
import dagger.internal.DaggerGenerated;
4+
import javax.annotation.processing.Generated;
5+
6+
@DaggerGenerated
7+
@Generated(
8+
value = "dagger.internal.codegen.ComponentProcessor",
9+
comments = "https://dagger.dev"
10+
)
11+
@SuppressWarnings({
12+
"unchecked",
13+
"rawtypes",
14+
"KotlinInternal",
15+
"KotlinInternalInJava",
16+
"cast",
17+
"deprecation",
18+
"nullness:initialization.field.uninitialized"
19+
})
20+
public final class DaggerMyComponent {
21+
private DaggerMyComponent() {
22+
}
23+
24+
public static Builder builder() {
25+
return new Builder();
26+
}
27+
28+
public static MyComponent create() {
29+
return new Builder().build();
30+
}
31+
32+
public static final class Builder {
33+
private Builder() {
34+
}
35+
36+
public MyComponent build() {
37+
return new MyComponentImpl();
38+
}
39+
}
40+
41+
private static final class MyComponentImpl implements MyComponent {
42+
private final MyComponentImpl myComponentImpl = this;
43+
44+
private MyComponentImpl() {
45+
46+
47+
}
48+
49+
@Override
50+
public void inject(Foo foo) {
51+
}
52+
53+
@Override
54+
public Foo injectAndReturn(Foo foo) {
55+
return foo;
56+
}
57+
}
58+
}

‎javatests/dagger/internal/codegen/goldens/MembersInjectionTest_testMembersInjectionBindingWithNoInjectionSites_FAST_INIT_MODE_test.DaggerMyComponent‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package test;
2+
3+
import dagger.internal.DaggerGenerated;
4+
import javax.annotation.processing.Generated;
5+
6+
@DaggerGenerated
7+
@Generated(
8+
value = "dagger.internal.codegen.ComponentProcessor",
9+
comments = "https://dagger.dev"
10+
)
11+
@SuppressWarnings({
12+
"unchecked",
13+
"rawtypes",
14+
"KotlinInternal",
15+
"KotlinInternalInJava",
16+
"cast",
17+
"deprecation",
18+
"nullness:initialization.field.uninitialized"
19+
})
20+
public final class DaggerMyComponent {
21+
private DaggerMyComponent() {
22+
}
23+
24+
public static Builder builder() {
25+
return new Builder();
26+
}
27+
28+
public static MyComponent create() {
29+
return new Builder().build();
30+
}
31+
32+
public static final class Builder {
33+
private Builder() {
34+
}
35+
36+
public MyComponent build() {
37+
return new MyComponentImpl();
38+
}
39+
}
40+
41+
private static final class MyComponentImpl implements MyComponent {
42+
private final MyComponentImpl myComponentImpl = this;
43+
44+
private MyComponentImpl() {
45+
46+
47+
}
48+
49+
@Override
50+
public void inject(Foo foo) {
51+
}
52+
53+
@Override
54+
public Foo injectAndReturn(Foo foo) {
55+
return foo;
56+
}
57+
}
58+
}

0 commit comments

Comments
 (0)