When modifiable framework instances are in fact modified, ensure that superclass implementations get the correct provider instance during initialization.
DelegateFactory is used to solve this cycle (though not a binding cycle, it is a "codegen" cycle of sorts), and a DelegateProducer is added for the same purpose but for producers.
In order for the delegate factories to be initialized properly, initialization is moved out of the constructors of abstract subcomponent implementations and into a method that can be called after super(); calls in the final implementation.
This is a rollforward of 218dd448dfd1d791480f13e507eed567fdabae43, which relied on Guava's functional types extending the java.util.function types (which continue to bite us in bizarre ways). I've verified that the broken targets now build
RELNOTES=n/a
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=221828464
diff --git a/java/dagger/internal/codegen/CodeBlocks.java b/java/dagger/internal/codegen/CodeBlocks.java
index bc8fccd..c1ca32d 100644
--- a/java/dagger/internal/codegen/CodeBlocks.java
+++ b/java/dagger/internal/codegen/CodeBlocks.java
@@ -28,6 +28,7 @@
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.CodeBlock;
import com.squareup.javapoet.MethodSpec;
+import com.squareup.javapoet.ParameterSpec;
import com.squareup.javapoet.TypeName;
import java.util.stream.Collector;
import javax.lang.model.element.ExecutableElement;
@@ -56,6 +57,17 @@
}
/**
+ * Returns a comma-separated {@link CodeBlock} using the name of every parameter in {@code
+ * parameters}.
+ */
+ static CodeBlock parameterNames(Iterable<ParameterSpec> parameters) {
+ // TODO(ronshapiro): Add DaggerStreams.stream(Iterable)
+ return stream(parameters.spliterator(), false)
+ .map(p -> CodeBlock.of("$N", p))
+ .collect(toParametersCodeBlock());
+ }
+
+ /**
* Returns one unified {@link CodeBlock} which joins each item in {@code codeBlocks} with a
* newline.
*/
diff --git a/java/dagger/internal/codegen/ComponentImplementation.java b/java/dagger/internal/codegen/ComponentImplementation.java
index 56885c9..b570122 100644
--- a/java/dagger/internal/codegen/ComponentImplementation.java
+++ b/java/dagger/internal/codegen/ComponentImplementation.java
@@ -87,6 +87,13 @@
/** The component constructor. */
CONSTRUCTOR,
+ /**
+ * In ahead-of-time subcomponents, this method coordinates the invocation of {@link
+ * #INITIALIZE_METHOD initialization methods} instead of constructors.
+ */
+ // TODO(b/117833324): try to merge this with other initialize() methods so it looks more natural
+ CONFIGURE_INITIALIZATION_METHOD,
+
/** A builder method for the component. (Only used by the root component.) */
BUILDER_METHOD,
@@ -160,6 +167,7 @@
private final SetMultimap<BindingRequest, DependencyRequest> multibindingContributionsMade =
HashMultimap.create();
private ImmutableList<ParameterSpec> constructorParameters;
+ private Optional<MethodSpec> configureInitializationMethod = Optional.empty();
ComponentImplementation(
ComponentDescriptor componentDescriptor,
@@ -241,6 +249,42 @@
}
/**
+ * Returns the {@link #configureInitializationMethod()} of the nearest supertype that defines one,
+ * if any.
+ *
+ * <p>Only returns a present value in {@link CompilerOptions#aheadOfTimeSubcomponents()}.
+ */
+ Optional<MethodSpec> superConfigureInitializationMethod() {
+ for (Optional<ComponentImplementation> currentSuper = superclassImplementation;
+ currentSuper.isPresent();
+ currentSuper = currentSuper.get().superclassImplementation) {
+ if (currentSuper.get().configureInitializationMethod.isPresent()) {
+ return currentSuper.get().configureInitializationMethod;
+ }
+ }
+ return Optional.empty();
+ }
+
+ /**
+ * Returns the {@link MethodSpecKind#CONFIGURE_INITIALIZATION_METHOD} of this implementation if
+ * there is one.
+ *
+ * <p>Only returns a present value in {@link CompilerOptions#aheadOfTimeSubcomponents()}.
+ */
+ Optional<MethodSpec> configureInitializationMethod() {
+ return configureInitializationMethod;
+ }
+
+ /**
+ * Set's this component implementation's {@code configureInitialization()} method and {@linkplain
+ * #addMethod(MethodSpecKind, MethodSpec) adds the method}.
+ */
+ void setConfigureInitializationMethod(MethodSpec method) {
+ configureInitializationMethod = Optional.of(method);
+ addMethod(MethodSpecKind.CONFIGURE_INITIALIZATION_METHOD, method);
+ }
+
+ /**
* Returns the name of the builder class for this component. It will be a sibling of this
* generated class unless this is a top-level component, in which case it will be nested.
*/
diff --git a/java/dagger/internal/codegen/ComponentImplementationFactory.java b/java/dagger/internal/codegen/ComponentImplementationFactory.java
index d5cd976..af7acf5 100644
--- a/java/dagger/internal/codegen/ComponentImplementationFactory.java
+++ b/java/dagger/internal/codegen/ComponentImplementationFactory.java
@@ -23,6 +23,7 @@
import static com.squareup.javapoet.MethodSpec.methodBuilder;
import static dagger.internal.codegen.AnnotationSpecs.Suppression.UNCHECKED;
import static dagger.internal.codegen.BindingRequest.bindingRequest;
+import static dagger.internal.codegen.CodeBlocks.parameterNames;
import static dagger.internal.codegen.CodeBlocks.toParametersCodeBlock;
import static dagger.internal.codegen.ComponentGenerator.componentName;
import static dagger.internal.codegen.ComponentImplementation.MethodSpecKind.BUILDER_METHOD;
@@ -289,7 +290,7 @@
for (List<CodeBlock> partition : partitions) {
String methodName = componentImplementation.getUniqueMethodName("cancelProducers");
MethodSpec method =
- MethodSpec.methodBuilder(methodName)
+ methodBuilder(methodName)
.addModifiers(PRIVATE)
.addParameter(boolean.class, MAY_INTERRUPT_IF_RUNNING)
.addCode(CodeBlocks.concat(partition))
@@ -439,15 +440,30 @@
constructor.addStatement(
CodeBlock.of(
"super($L)",
- superclassImplementation.constructorParameters().stream()
- .map(param -> CodeBlock.of("$N", param))
- .collect(toParametersCodeBlock()))));
+ parameterNames(superclassImplementation.constructorParameters()))));
+
+ Optional<MethodSpec.Builder> configureInitialization =
+ partitions.isEmpty() || !componentImplementation.isAbstract()
+ ? Optional.empty()
+ : Optional.of(configureInitializationMethodBuilder(constructorParameters));
+
+ if (componentImplementation.superConfigureInitializationMethod().isPresent()) {
+ MethodSpec superConfigureInitializationMethod =
+ componentImplementation.superConfigureInitializationMethod().get();
+ CodeBlock superInvocation =
+ CodeBlock.of(
+ "$N($L)",
+ superConfigureInitializationMethod,
+ parameterNames(superConfigureInitializationMethod.parameters));
+ if (configureInitialization.isPresent()) {
+ configureInitialization.get().addStatement("super.$L", superInvocation);
+ } else if (!componentImplementation.isAbstract()) {
+ constructor.addStatement(superInvocation);
+ }
+ }
ImmutableList<ParameterSpec> initializeParameters = initializeParameters();
- CodeBlock initializeParametersCodeBlock =
- constructorParameters.stream()
- .map(param -> CodeBlock.of("$N", param))
- .collect(toParametersCodeBlock());
+ CodeBlock initializeParametersCodeBlock = parameterNames(constructorParameters);
for (List<CodeBlock> partition : partitions) {
String methodName = componentImplementation.getUniqueMethodName("initialize");
@@ -461,10 +477,47 @@
.addAnnotation(AnnotationSpecs.suppressWarnings(UNCHECKED))
.addCode(CodeBlocks.concat(partition));
initializeMethod.addParameters(initializeParameters);
- constructor.addStatement("$L($L)", methodName, initializeParametersCodeBlock);
+ configureInitialization
+ .orElse(constructor)
+ .addStatement("$L($L)", methodName, initializeParametersCodeBlock);
componentImplementation.addMethod(INITIALIZE_METHOD, initializeMethod.build());
}
componentImplementation.addMethod(CONSTRUCTOR, constructor.build());
+ configureInitialization.ifPresent(
+ method -> componentImplementation.setConfigureInitializationMethod(method.build()));
+ }
+
+ /**
+ * Returns a {@link MethodSpec.Builder} for the {@link
+ * ComponentImplementation#configureInitializationMethod()}.
+ */
+ private MethodSpec.Builder configureInitializationMethodBuilder(
+ ImmutableList<ParameterSpec> initializationMethodParameters) {
+ String methodName = componentImplementation.getUniqueMethodName("configureInitialization");
+ MethodSpec.Builder configureInitialization =
+ methodBuilder(methodName)
+ .addModifiers(PROTECTED)
+ .addParameters(initializationMethodParameters);
+
+ // Checks all super configureInitialization() methods to see if they have the same signature
+ // as this one, and if so, adds as an @Override annotation
+ for (Optional<ComponentImplementation> currentSuperImplementation =
+ componentImplementation.superclassImplementation();
+ currentSuperImplementation.isPresent();
+ currentSuperImplementation =
+ currentSuperImplementation.get().superclassImplementation()) {
+ Optional<MethodSpec> superConfigureInitializationMethod =
+ currentSuperImplementation.get().configureInitializationMethod();
+ if (superConfigureInitializationMethod
+ .filter(superMethod -> superMethod.name.equals(methodName))
+ .filter(superMethod -> superMethod.parameters.equals(initializationMethodParameters))
+ .isPresent()) {
+ configureInitialization.addAnnotation(Override.class);
+ break;
+ }
+ }
+
+ return configureInitialization;
}
/** Returns the list of {@link ParameterSpec}s for the initialize methods. */
diff --git a/java/dagger/internal/codegen/FrameworkFieldInitializer.java b/java/dagger/internal/codegen/FrameworkFieldInitializer.java
index 159d5bb..a95b461 100644
--- a/java/dagger/internal/codegen/FrameworkFieldInitializer.java
+++ b/java/dagger/internal/codegen/FrameworkFieldInitializer.java
@@ -26,6 +26,7 @@
import com.squareup.javapoet.FieldSpec;
import com.squareup.javapoet.TypeName;
import dagger.internal.DelegateFactory;
+import dagger.producers.internal.DelegateProducer;
import java.util.Optional;
/**
@@ -106,7 +107,22 @@
CodeBlock fieldInitialization = frameworkInstanceCreationExpression.creationExpression();
CodeBlock initCode = CodeBlock.of("this.$N = $L;", getOrCreateField(), fieldInitialization);
- if (fieldInitializationState == InitializationState.DELEGATED) {
+ if (isReplacingSuperclassFrameworkInstance()) {
+ // TODO(ronshapiro): can we have DELEGATED share this branch? If we allow the FieldSpec
+ // to be modified in the ComponentImplementation, we can give it the same initializer to a
+ // delegate factory
+ CodeBlock delegateFactoryVariable = CodeBlock.of("$NDelegate", fieldSpec);
+ // TODO(ronshapiro): Use a type parameter here. Or even better, can a static method that
+ // accepts the delegate factory and the delegated instance infer the type parameters?
+ // And then we also don't need a cast.
+ codeBuilder
+ .add("$1T $2L = ($1T) $3N;", delegateType(), delegateFactoryVariable, fieldSpec)
+ .add(
+ "$L.$N($L);",
+ delegateFactoryVariable,
+ setDelegateMethodName(),
+ fieldInitialization);
+ } else if (fieldInitializationState == InitializationState.DELEGATED) {
// If we were recursively invoked, set the delegate factory as part of our initialization
CodeBlock delegateFactoryVariable = CodeBlock.of("$NDelegate", fieldSpec);
codeBuilder
@@ -152,13 +168,16 @@
resolvedBindings, frameworkInstanceCreationExpression.alternativeFrameworkClass());
TypeName fieldType;
- if (!fieldInitializationState.equals(InitializationState.DELEGATED)
+ boolean rawTypeUsed = false;
+ if (!isReplacingSuperclassFrameworkInstance()
+ && !fieldInitializationState.equals(InitializationState.DELEGATED)
&& specificType().isPresent()) {
// For some larger components, this causes javac to compile much faster by getting the
// field type to exactly match the type of the expression being assigned to it.
fieldType = specificType().get();
} else if (useRawType) {
fieldType = contributionBindingField.type().rawType;
+ rawTypeUsed = true;
} else {
fieldType = contributionBindingField.type();
}
@@ -167,15 +186,58 @@
FieldSpec.builder(
fieldType, componentImplementation.getUniqueFieldName(contributionBindingField.name()));
contributionField.addModifiers(PRIVATE);
- if (useRawType && !specificType().isPresent()) {
+ if (rawTypeUsed) {
contributionField.addAnnotation(AnnotationSpecs.suppressWarnings(RAWTYPES));
}
+
+ if (isReplacingSuperclassFrameworkInstance()) {
+ // If a binding is modified in a subclass, the framework instance will be replaced in the
+ // subclass implementation. The superclass framework instance initialization will run first,
+ // however, and may refer to the modifiable binding method returning this type's modified
+ // framework instance before it is initialized, so we use a delegate factory as a placeholder
+ // until it has properly been initialized.
+ contributionField.initializer("new $T<>()", delegateType());
+ }
+
fieldSpec = contributionField.build();
componentImplementation.addField(FRAMEWORK_FIELD, fieldSpec);
return fieldSpec;
}
+ /**
+ * Returns true if this framework field is replacing a superclass's implementation of the
+ * framework field.
+ */
+ private boolean isReplacingSuperclassFrameworkInstance() {
+ return componentImplementation
+ .superclassImplementation()
+ .flatMap(
+ superclassImplementation ->
+ // TODO(b/117833324): can we constrain this further?
+ superclassImplementation.getModifiableBindingMethod(
+ BindingRequest.bindingRequest(
+ resolvedBindings.key(),
+ FrameworkType.forBindingType(resolvedBindings.bindingType()))))
+ .isPresent();
+ }
+
+ private Class<?> delegateType() {
+ return isProvider() ? DelegateFactory.class : DelegateProducer.class;
+ }
+
+ private String setDelegateMethodName() {
+ return isProvider() ? "setDelegatedProvider" : "setDelegatedProducer";
+ }
+
+ private boolean isProvider() {
+ return resolvedBindings.bindingType().equals(BindingType.PROVISION)
+ && frameworkInstanceCreationExpression
+ .alternativeFrameworkClass()
+ .map(TypeNames.PROVIDER::equals)
+ .orElse(true);
+ }
+
/** Returns the type of the instance when it is a specific factory type. */
@Override
public Optional<TypeName> specificType() {
diff --git a/java/dagger/internal/codegen/SetFactoryCreationExpression.java b/java/dagger/internal/codegen/SetFactoryCreationExpression.java
index 5eb3d40..d69f4a2 100644
--- a/java/dagger/internal/codegen/SetFactoryCreationExpression.java
+++ b/java/dagger/internal/codegen/SetFactoryCreationExpression.java
@@ -62,7 +62,9 @@
Optional<CodeBlock> superContributions = superContributions();
if (superContributions.isPresent()) {
// TODO(b/117833324): consider decomposing the Provider<Set<Provider>> and adding the
- // individual contributions separately from the collection contributions
+ // individual contributions separately from the collection contributions. Though this may
+ // actually not be doable/desirable if the super provider instance is a DelegateFactory or
+ // another internal type that is not SetFactory
builderMethodCalls.add(".addCollection$N($L)", methodNameSuffix, superContributions.get());
setProviders++;
}
diff --git a/java/dagger/producers/internal/DelegateProducer.java b/java/dagger/producers/internal/DelegateProducer.java
new file mode 100644
index 0000000..896c366
--- /dev/null
+++ b/java/dagger/producers/internal/DelegateProducer.java
@@ -0,0 +1,88 @@
+/*
+ * Copyright (C) 2018 The Dagger Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package dagger.producers.internal;
+
+import com.google.common.util.concurrent.ListenableFuture;
+import dagger.internal.DoubleCheck;
+import dagger.producers.Producer;
+import javax.inject.Provider;
+
+/**
+ * A DelegateProducer that is used to stitch Producer indirection during initialization across
+ * partial subcomponent implementations.
+ */
+public final class DelegateProducer<T> implements CancellableProducer<T> {
+ private CancellableProducer<T> delegate;
+
+ @Override
+ public ListenableFuture<T> get() {
+ return delegate.get();
+ }
+
+ public void setDelegatedProducer(Producer<T> delegate) {
+ if (delegate == null) {
+ throw new IllegalArgumentException();
+ }
+ if (this.delegate != null) {
+ throw new IllegalStateException();
+ }
+ this.delegate = (CancellableProducer<T>) (CancellableProducer) delegate;
+ }
+
+ @Override
+ public void cancel(boolean mayInterruptIfRunning) {
+ delegate.cancel(mayInterruptIfRunning);
+ }
+
+ @Override
+ public Producer<T> newDependencyView() {
+ return new ProducerView<T>() {
+ @Override
+ Producer<T> createDelegate() {
+ return delegate.newDependencyView();
+ }
+ };
+ }
+
+ @Override
+ public Producer<T> newEntryPointView(final CancellationListener cancellationListener) {
+ return new ProducerView<T>() {
+ @Override
+ Producer<T> createDelegate() {
+ return delegate.newEntryPointView(cancellationListener);
+ }
+ };
+ }
+
+ private abstract static class ProducerView<T> implements Producer<T> {
+ private final Provider<Producer<T>> delegate =
+ DoubleCheck.provider(
+ new Provider<Producer<T>>() {
+ @Override
+ public Producer<T> get() {
+ return createDelegate();
+ }
+ });
+
+ abstract Producer<T> createDelegate();
+
+ @Override
+ public ListenableFuture<T> get() {
+ return delegate.get().get();
+ }
+ }
+}