Skip to content

Commit

Permalink
Roll forward of CL/363689284: Make nested subcomponent and switching …
Browse files Browse the repository at this point in the history
…provider impls static within the generated component class.

RELNOTES=Make nested subcomponent and switching provider impls static within the generated component class.
PiperOrigin-RevId: 374203747
  • Loading branch information
bcorso authored and Dagger Team committed May 17, 2021
1 parent 8acc433 commit d9f08aa
Show file tree
Hide file tree
Showing 30 changed files with 555 additions and 403 deletions.
11 changes: 10 additions & 1 deletion java/dagger/internal/codegen/binding/SourceFiles.java
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,16 @@ public static ImmutableList<TypeVariableName> bindingTypeElementTypeVariableName
*/
// TODO(gak): maybe this should be a function of TypeMirrors instead of Elements?
public static String simpleVariableName(TypeElement typeElement) {
String candidateName = UPPER_CAMEL.to(LOWER_CAMEL, typeElement.getSimpleName().toString());
return simpleVariableName(ClassName.get(typeElement));
}

/**
* Returns a name to be used for variables of the given {@linkplain ClassName}. Prefer
* semantically meaningful variable names, but if none can be derived, this will produce something
* readable.
*/
public static String simpleVariableName(ClassName className) {
String candidateName = UPPER_CAMEL.to(LOWER_CAMEL, className.simpleName());
String variableName = protectAgainstKeywords(candidateName);
verify(isName(variableName), "'%s' was expected to be a valid variable name");
return variableName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.squareup.javapoet.MethodSpec.constructorBuilder;
import static com.squareup.javapoet.MethodSpec.methodBuilder;
import static com.squareup.javapoet.TypeSpec.classBuilder;
import static dagger.internal.codegen.binding.SourceFiles.simpleVariableName;
Expand Down Expand Up @@ -58,6 +57,7 @@
import dagger.internal.codegen.writing.ModuleProxies;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Stream;
import javax.inject.Inject;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.Modifier;
Expand Down Expand Up @@ -105,7 +105,8 @@ Optional<ComponentCreatorImplementation> create() {

/** Base class for building a creator implementation. */
private abstract class Builder {
final ComponentImplementation componentImplementation;
private final UniqueNameSet fieldNames = new UniqueNameSet();
private final ComponentImplementation componentImplementation;
final ClassName className;
final TypeSpec.Builder classBuilder;

Expand All @@ -121,8 +122,8 @@ private abstract class Builder {
ComponentCreatorImplementation build() {
setModifiers();
setSupertype();
this.fields = addFields();
addConstructor();
this.fields = addFields();
addSetterMethods();
addFactoryMethod();
return ComponentCreatorImplementation.create(classBuilder.build(), className, fields);
Expand Down Expand Up @@ -168,10 +169,7 @@ private Set<ComponentRequirement> neededUserSettableRequirements() {

private void setModifiers() {
visibility().ifPresent(classBuilder::addModifiers);
if (!componentImplementation.isNested()) {
classBuilder.addModifiers(STATIC);
}
classBuilder.addModifiers(FINAL);
classBuilder.addModifiers(STATIC, FINAL);
}

/** Returns the visibility modifier the generated class should have, if any. */
Expand All @@ -181,11 +179,22 @@ private void setModifiers() {
protected abstract void setSupertype();

/** Adds a constructor for the creator type, if needed. */
protected abstract void addConstructor();
protected void addConstructor() {
MethodSpec.Builder constructor = MethodSpec.constructorBuilder().addModifiers(PRIVATE);
componentImplementation
.creatorComponentFields()
.forEach(
field -> {
fieldNames.claim(field.name);
classBuilder.addField(field);
constructor.addParameter(field.type, field.name);
constructor.addStatement("this.$1N = $1N", field);
});
classBuilder.addMethod(constructor.build());
}

private ImmutableMap<ComponentRequirement, FieldSpec> addFields() {
// Fields in an abstract creator class need to be visible from subclasses.
UniqueNameSet fieldNames = new UniqueNameSet();
ImmutableMap<ComponentRequirement, FieldSpec> result =
Maps.toMap(
Sets.intersection(neededUserSettableRequirements(), setterMethods()),
Expand Down Expand Up @@ -346,17 +355,20 @@ private void addNullHandlingForParameter(

private CodeBlock componentConstructorArgs(
ImmutableMap<ComponentRequirement, String> factoryMethodParameters) {
return componentConstructorRequirements().stream()
.map(
requirement -> {
if (fields.containsKey(requirement)) {
return CodeBlock.of("$N", fields.get(requirement));
} else if (factoryMethodParameters.containsKey(requirement)) {
return CodeBlock.of("$L", factoryMethodParameters.get(requirement));
} else {
return newModuleInstance(requirement);
}
})
return Stream.concat(
componentImplementation.creatorComponentFields().stream()
.map(field -> CodeBlock.of("$N", field)),
componentConstructorRequirements().stream()
.map(
requirement -> {
if (fields.containsKey(requirement)) {
return CodeBlock.of("$N", fields.get(requirement));
} else if (factoryMethodParameters.containsKey(requirement)) {
return CodeBlock.of("$L", factoryMethodParameters.get(requirement));
} else {
return newModuleInstance(requirement);
}
}))
.collect(toParametersCodeBlock());
}

Expand Down Expand Up @@ -394,7 +406,9 @@ protected void setSupertype() {

@Override
protected void addConstructor() {
// Just use the implicit no-arg public constructor.
if (!componentImplementation.creatorComponentFields().isEmpty()) {
super.addConstructor();
}
}

@Override
Expand Down Expand Up @@ -490,11 +504,6 @@ protected void setSupertype() {
// There's never a supertype for a root component auto-generated builder type.
}

@Override
protected void addConstructor() {
classBuilder.addMethod(constructorBuilder().addModifiers(PRIVATE).build());
}

@Override
protected ImmutableSet<ComponentRequirement> setterMethods() {
return componentDescriptor().dependenciesAndConcreteModules();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static com.google.common.base.Preconditions.checkState;
import static com.squareup.javapoet.MethodSpec.methodBuilder;
import static dagger.internal.codegen.binding.ComponentCreatorKind.BUILDER;
import static dagger.internal.codegen.extension.DaggerStreams.toImmutableList;
import static dagger.internal.codegen.javapoet.CodeBlocks.parameterNames;
import static dagger.internal.codegen.writing.ComponentImplementation.MethodSpecKind.BUILDER_METHOD;
import static dagger.internal.codegen.writing.ComponentImplementation.MethodSpecKind.COMPONENT_METHOD;
Expand All @@ -29,6 +30,7 @@
import static javax.lang.model.element.Modifier.STATIC;

import com.google.auto.common.MoreTypes;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
Expand Down Expand Up @@ -243,10 +245,18 @@ private void createSubcomponentFactoryMethod(ExecutableElement factoryMethod) {
checkState(parent.isPresent());
Collection<ParameterSpec> params = getFactoryMethodParameters(graph).values();
MethodSpec.Builder method = MethodSpec.overriding(factoryMethod, parentType(), types);
params.forEach(
param -> method.addStatement("$T.checkNotNull($N)", Preconditions.class, param));
params.forEach(param -> method.addStatement("$T.checkNotNull($N)", Preconditions.class, param));
method.addStatement(
"return new $T($L)", componentImplementation.name(), parameterNames(params));
"return new $T($L)",
componentImplementation.name(),
parameterNames(
ImmutableList.<ParameterSpec>builder()
.addAll(
componentImplementation.creatorComponentFields().stream()
.map(field -> ParameterSpec.builder(field.type, field.name).build())
.collect(toImmutableList()))
.addAll(params)
.build()));

parent.get().componentImplementation.addMethod(COMPONENT_METHOD, method.build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,11 @@ private FrameworkInstanceCreationExpression frameworkInstanceCreationExpression(
case COMPONENT:
// The cast can be removed when we drop java 7 source support
return new InstanceFactoryCreationExpression(
() -> CodeBlock.of("($T) this", binding.key().type()));
() ->
CodeBlock.of(
"($T) $L",
binding.key().type(),
componentImplementation.componentFieldReference()));

case BOUND_INSTANCE:
return instanceFactoryCreationExpression(
Expand Down Expand Up @@ -512,7 +516,7 @@ private Optional<BindingExpression> unscopedDirectInstanceExpression(

case COMPONENT:
return Optional.of(
new ComponentInstanceBindingExpression(binding, componentImplementation.name()));
new ComponentInstanceBindingExpression(componentImplementation, binding));

case COMPONENT_DEPENDENCY:
return Optional.of(
Expand All @@ -531,8 +535,7 @@ private Optional<BindingExpression> unscopedDirectInstanceExpression(

case SUBCOMPONENT_CREATOR:
return Optional.of(
new SubcomponentCreatorBindingExpression(
binding, componentImplementation.getSubcomponentCreatorSimpleName(binding.key())));
new SubcomponentCreatorBindingExpression(componentImplementation, binding));

case MULTIBOUND_SET:
return Optional.of(
Expand Down

0 comments on commit d9f08aa

Please sign in to comment.