Skip to content

Commit

Permalink
Somewhat hacky fix for issue with missing paramters in AOTS configure…
Browse files Browse the repository at this point in the history
…Initialization/initialize methods.

RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=229232022
  • Loading branch information
cgdecker authored and ronshapiro committed Jan 15, 2019
1 parent b6b9dca commit fed0de5
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 22 deletions.
32 changes: 22 additions & 10 deletions java/dagger/internal/codegen/ComponentImplementation.java
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ static ConfigureInitializationMethod create(
private final UniqueNameSet componentFieldNames = new UniqueNameSet();
private final UniqueNameSet componentMethodNames = new UniqueNameSet();
private final List<CodeBlock> initializations = new ArrayList<>();
private final Map<ComponentRequirement, CodeBlock> componentRequirementInitializations =
new LinkedHashMap<>();
private final Set<ComponentRequirement> componentRequirementParameters = new HashSet<>();
private final List<CodeBlock> componentRequirementInitializations = new ArrayList<>();
private final Map<ComponentRequirement, String> componentRequirementParameterNames =
new HashMap<>();
private final Set<Key> cancellableProducerKeys = new LinkedHashSet<>();
Expand Down Expand Up @@ -476,12 +476,24 @@ void addInitialization(CodeBlock codeBlock) {
}

/**
* Adds the given code block that initializes the given {@link ComponentRequirement} to the
* component implementation.
* Adds the given component requirement as one that should have a parameter in the component's
* initialization methods.
*/
void addComponentRequirementInitialization(
ComponentRequirement requirement, CodeBlock codeBlock) {
componentRequirementInitializations.put(requirement, codeBlock);
void addComponentRequirementParameter(ComponentRequirement requirement) {
componentRequirementParameters.add(requirement);
}

/**
* The set of component requirements that have parameters in the component's initialization
* methods.
*/
ImmutableSet<ComponentRequirement> getComponentRequirementParameters() {
return ImmutableSet.copyOf(componentRequirementParameters);
}

/** Adds the given code block that initializes a {@link ComponentRequirement}. */
void addComponentRequirementInitialization(CodeBlock codeBlock) {
componentRequirementInitializations.add(codeBlock);
}

/**
Expand Down Expand Up @@ -542,7 +554,7 @@ ImmutableList<CodeBlock> getInitializations() {
}

/**
* Returns the map of {@link ComponentRequirement}s to {@link CodeBlock}s that initialize them.
* Returns a list of {@link CodeBlock}s for initializing {@link ComponentRequirement}s.
*
* <p>These initializations are kept separate from {@link #getInitializations()} because they must
* be executed before the initializations of any framework instance initializations in a
Expand All @@ -551,8 +563,8 @@ ImmutableList<CodeBlock> getInitializations() {
* {@link dagger.producers.internal.DelegateProducer} since the types of these initialized fields
* have no interface type that we can write a proxy for.
*/
ImmutableMap<ComponentRequirement, CodeBlock> getComponentRequirementInitializations() {
return ImmutableMap.copyOf(componentRequirementInitializations);
ImmutableList<CodeBlock> getComponentRequirementInitializations() {
return ImmutableList.copyOf(componentRequirementInitializations);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -388,8 +388,7 @@ private void implementInitializationMethod(
ImmutableMap<ComponentRequirement, ParameterSpec> initializationParameters) {
initializationMethod.addParameters(initializationParameters.values());
initializationMethod.addCode(
CodeBlocks.concat(
componentImplementation.getComponentRequirementInitializations().values()));
CodeBlocks.concat(componentImplementation.getComponentRequirementInitializations()));
componentImplementation
.superConfigureInitializationMethod()
.ifPresent(
Expand Down Expand Up @@ -587,14 +586,14 @@ private final ImmutableMap<ComponentRequirement, ParameterSpec> initializationPa
* {@code configureInitialization} method.
*/
private ImmutableSet<ComponentRequirement> configureInitializationRequirements() {
ImmutableSet<ComponentRequirement> initializationRequirements =
componentImplementation.getComponentRequirementInitializations().keySet();
ImmutableSet<ComponentRequirement> initializationParameters =
componentImplementation.getComponentRequirementParameters();
ImmutableSet<ComponentRequirement> superConfigureInitializationRequirements =
componentImplementation
.superConfigureInitializationMethod()
.map(ConfigureInitializationMethod::parameters)
.orElse(ImmutableSet.of());
return Sets.union(initializationRequirements, superConfigureInitializationRequirements)
return Sets.union(initializationParameters, superConfigureInitializationRequirements)
.immutableCopy();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,9 @@ private ComponentRequirementExpression createField(ComponentRequirement requirem
}

private abstract static class AbstractField implements ComponentRequirementExpression {
private final ComponentRequirement componentRequirement;
protected final ComponentImplementation componentImplementation;
protected final String fieldName;
final ComponentRequirement componentRequirement;
final ComponentImplementation componentImplementation;
final String fieldName;
private final Supplier<MemberSelect> field = memoize(this::addField);

private AbstractField(
Expand Down Expand Up @@ -179,8 +179,7 @@ public CodeBlock getExpression(ClassName requestingClass) {
private MemberSelect addField() {
FieldSpec field = createField();
componentImplementation.addField(COMPONENT_REQUIREMENT_FIELD, field);
componentImplementation.addComponentRequirementInitialization(
componentRequirement, fieldInitialization(field));
componentImplementation.addComponentRequirementInitialization(fieldInitialization(field));
return MemberSelect.localField(componentImplementation.name(), fieldName);
}

Expand Down Expand Up @@ -232,6 +231,7 @@ private ComponentParameterField(
ComponentImplementation componentImplementation,
String name) {
super(componentRequirement, componentImplementation);
componentImplementation.addComponentRequirementParameter(componentRequirement);
// Get the name that the component implementation will use for its parameter for the
// requirement. If the given name is different than the name of the field created for the
// requirement (as may be the case when the parameter name is derived from a user-written
Expand All @@ -242,8 +242,7 @@ private ComponentParameterField(
// weird where the component actually has fields named, say, "foo" and "fooParam".
this.name =
componentImplementation.getParameterName(
componentRequirement,
name.equals(fieldName) ? name + "Param" : name);
componentRequirement, name.equals(fieldName) ? name + "Param" : name);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4974,6 +4974,67 @@ public void multipleComponentMethodsForSameBindingRequest() {
.containsElementsIn(generatedRoot);
}

@Test
public void boundInstanceUsedOnlyInInitialize() {
JavaFileObject subcomponent =
JavaFileObjects.forSourceLines(
"test.Sub",
"package test;",
"",
"import dagger.BindsInstance;",
"import dagger.Subcomponent;",
"import javax.inject.Provider;",
"",
"@Subcomponent",
"interface Sub {",
" Provider<String> stringProvider();",
"",
" @Subcomponent.Builder",
" interface Builder {",
" @BindsInstance",
" Builder string(String string);",
" Sub build();",
" }",
"}");

JavaFileObject generated =
JavaFileObjects.forSourceLines(
"test.Sub",
"package test;",
"",
"import dagger.internal.InstanceFactory;",
"import dagger.internal.Preconditions;",
IMPORT_GENERATED_ANNOTATION,
"import javax.inject.Provider;",
"",
GENERATED_ANNOTATION,
"public abstract class DaggerSub implements Sub {",
" private Provider<String> stringProvider;",
"",
" protected DaggerSub() {}",
"",
" protected void configureInitialization(String stringParam) {",
" initialize(stringParam);",
" }",
"",
" @SuppressWarnings(\"unchecked\")",
" private void initialize(final String stringParam) {",
" this.stringProvider = InstanceFactory.create(stringParam);",
" }",
"",
" @Override",
" public Provider<String> stringProvider() {",
" return stringProvider;",
" }",
"}");

Compilation compilation = compile(ImmutableList.of(subcomponent));
assertThat(compilation).succeededWithoutWarnings();
assertThat(compilation)
.generatedSourceFile("test.DaggerSub")
.containsElementsIn(generated);
}

// TODO(ronshapiro): remove copies from AheadOfTimeSubcomponents*Test classes
private void createAncillaryClasses(
ImmutableList.Builder<JavaFileObject> filesBuilder, String... ancillaryClasses) {
Expand Down

0 comments on commit fed0de5

Please sign in to comment.