Skip to content

Commit

Permalink
[Refactor]: Move members injection optimization into its RequestRepre…
Browse files Browse the repository at this point in the history
…sentation.

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
  • Loading branch information
bcorso authored and Dagger Team committed Dec 2, 2024
1 parent 841d765 commit 369bbc6
Show file tree
Hide file tree
Showing 5 changed files with 170 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,14 @@
import static androidx.room.compiler.processing.XTypeKt.isVoid;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.Iterables.getOnlyElement;
import static dagger.internal.codegen.base.Util.reentrantComputeIfAbsent;
import static dagger.internal.codegen.binding.BindingRequest.bindingRequest;
import static dagger.internal.codegen.javapoet.CodeBlocks.makeParametersCodeBlock;
import static dagger.internal.codegen.langmodel.Accessibility.isRawTypeAccessible;
import static dagger.internal.codegen.langmodel.Accessibility.isTypeAccessibleFrom;
import static dagger.internal.codegen.xprocessing.MethodSpecs.overriding;
import static dagger.internal.codegen.xprocessing.XElements.getSimpleName;
import static dagger.internal.codegen.xprocessing.XProcessingEnvs.isPreJava8SourceVersion;

import androidx.room.compiler.processing.XMethodElement;
import androidx.room.compiler.processing.XProcessingEnv;
import androidx.room.compiler.processing.XType;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -185,47 +182,26 @@ && isRawTypeAccessible(dependencyType, requestingClass.packageName())) {

/** Returns the implementation of a component method. */
public MethodSpec getComponentMethod(ComponentMethodDescriptor componentMethod) {
checkArgument(componentMethod.dependencyRequest().isPresent());
BindingRequest request = bindingRequest(componentMethod.dependencyRequest().get());
return overriding(componentMethod.methodElement(), graph.componentTypeElement().getType())
.addCode(
request.isRequestKind(RequestKind.MEMBERS_INJECTION)
? getMembersInjectionComponentMethodImplementation(request, componentMethod)
: getContributionComponentMethodImplementation(request, componentMethod))
.addCode(getComponentMethodCodeBlock(componentMethod))
.build();
}

private CodeBlock getMembersInjectionComponentMethodImplementation(
BindingRequest request, ComponentMethodDescriptor componentMethod) {
checkArgument(request.isRequestKind(RequestKind.MEMBERS_INJECTION));
XMethodElement methodElement = componentMethod.methodElement();
RequestRepresentation requestRepresentation = getRequestRepresentation(request);
MembersInjectionBinding binding =
((MembersInjectionRequestRepresentation) requestRepresentation).binding();
if (binding.injectionSites().isEmpty()) {
// If there are no injection sites either do nothing (if the return type is void) or return
// the input instance as-is.
return isVoid(methodElement.getReturnType())
? CodeBlock.of("")
: CodeBlock.of(
"return $L;", getSimpleName(getOnlyElement(methodElement.getParameters())));
private CodeBlock getComponentMethodCodeBlock(ComponentMethodDescriptor componentMethod) {
Expression expression = getComponentMethodExpression(componentMethod);
if (isVoid(componentMethod.methodElement().getReturnType())) {
return expression.codeBlock().isEmpty()
? expression.codeBlock()
: CodeBlock.of("$L;", expression.codeBlock());
}
Expression expression = getComponentMethodExpression(requestRepresentation, componentMethod);
return isVoid(methodElement.getReturnType())
? CodeBlock.of("$L;", expression.codeBlock())
: CodeBlock.of("return $L;", expression.codeBlock());
}

private CodeBlock getContributionComponentMethodImplementation(
BindingRequest request, ComponentMethodDescriptor componentMethod) {
checkArgument(!request.isRequestKind(RequestKind.MEMBERS_INJECTION));
Expression expression =
getComponentMethodExpression(getRequestRepresentation(request), componentMethod);
return CodeBlock.of("return $L;", expression.codeBlock());
}

private Expression getComponentMethodExpression(
RequestRepresentation requestRepresentation, ComponentMethodDescriptor componentMethod) {
private Expression getComponentMethodExpression(ComponentMethodDescriptor componentMethod) {
checkArgument(componentMethod.dependencyRequest().isPresent());
BindingRequest request = bindingRequest(componentMethod.dependencyRequest().get());
RequestRepresentation requestRepresentation = getRequestRepresentation(request);

Expression expression =
requestRepresentation.getDependencyExpressionForComponentMethod(
componentMethod, componentImplementation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package dagger.internal.codegen.writing;

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

import androidx.room.compiler.processing.XExecutableParameterElement;
Expand Down Expand Up @@ -54,6 +55,15 @@ protected Expression getDependencyExpressionForComponentMethod(
ComponentMethodDescriptor componentMethod, ComponentImplementation component) {
XMethodElement methodElement = componentMethod.methodElement();
XExecutableParameterElement parameter = getOnlyElement(methodElement.getParameters());
if (binding.injectionSites().isEmpty()) {
// If there are no injection sites either do nothing (if the return type is void) or return
// the input instance as-is.
return Expression.create(
methodElement.getReturnType(),
isVoid(methodElement.getReturnType())
? CodeBlock.of("")
: CodeBlock.of("$L", parameter.getJvmName()));
}
return membersInjectionMethods.getInjectExpression(
binding.key(), CodeBlock.of("$L", parameter.getJvmName()), component.name());
}
Expand Down
32 changes: 32 additions & 0 deletions javatests/dagger/internal/codegen/MembersInjectionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,38 @@ public void kotlinNullableFieldInjection() {
});
}

@Test
public void testMembersInjectionBindingWithNoInjectionSites() throws Exception {
Source component =
CompilerTests.javaSource(
"test.MyComponent",
"package test;",
"",
"import dagger.Component;",
"",
"@Component",
"public interface MyComponent {",
" void inject(Foo foo);",
"",
" Foo injectAndReturn(Foo foo);",
"}");

Source foo =
CompilerTests.javaSource(
"test.Foo",
"package test;",
"",
"class Foo {}");

CompilerTests.daggerCompiler(component, foo)
.withProcessingOptions(compilerMode.processorOptions())
.compile(
subject -> {
subject.hasErrorCount(0);
subject.generatedSource(goldenFileRule.goldenSource("test/DaggerMyComponent"));
});
}

private Source stripJetbrainsNullable(Source source) {
return CompilerTests.javaSource(
((Source.JavaSource) source).getQName(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package test;

import dagger.internal.DaggerGenerated;
import javax.annotation.processing.Generated;

@DaggerGenerated
@Generated(
value = "dagger.internal.codegen.ComponentProcessor",
comments = "https://dagger.dev"
)
@SuppressWarnings({
"unchecked",
"rawtypes",
"KotlinInternal",
"KotlinInternalInJava",
"cast",
"deprecation",
"nullness:initialization.field.uninitialized"
})
public final class DaggerMyComponent {
private DaggerMyComponent() {
}

public static Builder builder() {
return new Builder();
}

public static MyComponent create() {
return new Builder().build();
}

public static final class Builder {
private Builder() {
}

public MyComponent build() {
return new MyComponentImpl();
}
}

private static final class MyComponentImpl implements MyComponent {
private final MyComponentImpl myComponentImpl = this;

private MyComponentImpl() {


}

@Override
public void inject(Foo foo) {
}

@Override
public Foo injectAndReturn(Foo foo) {
return foo;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package test;

import dagger.internal.DaggerGenerated;
import javax.annotation.processing.Generated;

@DaggerGenerated
@Generated(
value = "dagger.internal.codegen.ComponentProcessor",
comments = "https://dagger.dev"
)
@SuppressWarnings({
"unchecked",
"rawtypes",
"KotlinInternal",
"KotlinInternalInJava",
"cast",
"deprecation",
"nullness:initialization.field.uninitialized"
})
public final class DaggerMyComponent {
private DaggerMyComponent() {
}

public static Builder builder() {
return new Builder();
}

public static MyComponent create() {
return new Builder().build();
}

public static final class Builder {
private Builder() {
}

public MyComponent build() {
return new MyComponentImpl();
}
}

private static final class MyComponentImpl implements MyComponent {
private final MyComponentImpl myComponentImpl = this;

private MyComponentImpl() {


}

@Override
public void inject(Foo foo) {
}

@Override
public Foo injectAndReturn(Foo foo) {
return foo;
}
}
}

0 comments on commit 369bbc6

Please sign in to comment.