Skip to content

Commit

Permalink
De-dupe assisted parameter names and field names to avoid naming conf…
Browse files Browse the repository at this point in the history
…licts.

Fixes #2570

RELNOTES=De-dupe assisted parameter names and field names to avoid naming conflicts. #2570
PiperOrigin-RevId: 382425214
  • Loading branch information
java-team-github-bot authored and Dagger Team committed Jul 1, 2021
1 parent 7af40df commit 3d93625
Show file tree
Hide file tree
Showing 10 changed files with 743 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,11 @@ Expression getDependencyExpression(ClassName requestingClass) {
// An assisted factory binding should have a single request for an assisted injection type.
DependencyRequest assistedInjectionRequest = getOnlyElement(binding.provisionDependencies());
Expression assistedInjectionExpression =
componentBindingExpressions.getDependencyExpression(
BindingRequest.bindingRequest(assistedInjectionRequest.key(), RequestKind.INSTANCE),
// This is kind of gross because the anonymous class doesn't really have a name we can
// reference. The requesting class name is really only needed to determine if we need to
// append "OwningClass.this." to the method call or not.
// TODO(bcorso): We should probably use a non-anonymous class here instead so that we
// actually have a proper class name.
requestingClass.peerClass(""));
((AssistedPrivateMethodBindingExpression)
componentBindingExpressions.getBindingExpression(
BindingRequest.bindingRequest(
assistedInjectionRequest.key(), RequestKind.INSTANCE)))
.getAssistedDependencyExpression(requestingClass.peerClass(""));
return Expression.create(
assistedInjectionExpression.type(),
CodeBlock.of("$L", anonymousfactoryImpl(assistedInjectionExpression)));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Copyright (C) 2017 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.internal.codegen.writing;

import static com.google.auto.common.MoreElements.asExecutable;
import static com.google.auto.common.MoreTypes.asDeclared;
import static com.google.auto.common.MoreTypes.asExecutable;
import static com.google.common.base.Preconditions.checkArgument;

import com.google.common.collect.ImmutableList;
import com.squareup.javapoet.ParameterSpec;
import com.squareup.javapoet.TypeName;
import dagger.internal.codegen.binding.AssistedInjectionAnnotations;
import dagger.internal.codegen.binding.Binding;
import dagger.internal.codegen.langmodel.DaggerTypes;
import dagger.internal.codegen.writing.ComponentImplementation.ShardImplementation;
import dagger.spi.model.BindingKind;
import java.util.List;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.ExecutableType;
import javax.lang.model.type.TypeMirror;

/** Utility class for generating unique assisted parameter names for a component shard. */
final class AssistedInjectionParameters {
/**
* Returns the list of assisted parameters as {@link ParameterSpec}s.
*
* <p>The type of each parameter will be the resolved type given by the binding key, and the name
* of each parameter will be the name given in the {@link
* dagger.assisted.AssistedInject}-annotated constructor.
*/
public static ImmutableList<ParameterSpec> assistedParameterSpecs(
Binding binding, DaggerTypes types, ShardImplementation shardImplementation) {
checkArgument(binding.kind() == BindingKind.ASSISTED_INJECTION);
ExecutableElement constructor = asExecutable(binding.bindingElement().get());
ExecutableType constructorType =
asExecutable(types.asMemberOf(asDeclared(binding.key().type().java()), constructor));
return assistedParameterSpecs(
constructor.getParameters(), constructorType.getParameterTypes(), shardImplementation);
}

private static ImmutableList<ParameterSpec> assistedParameterSpecs(
List<? extends VariableElement> paramElements,
List<? extends TypeMirror> paramTypes,
ShardImplementation shardImplementation) {
ImmutableList.Builder<ParameterSpec> assistedParameterSpecs = ImmutableList.builder();
for (int i = 0; i < paramElements.size(); i++) {
VariableElement paramElement = paramElements.get(i);
TypeMirror paramType = paramTypes.get(i);
if (AssistedInjectionAnnotations.isAssistedParameter(paramElement)) {
assistedParameterSpecs.add(
ParameterSpec.builder(
TypeName.get(paramType),
shardImplementation.getUniqueFieldNameForAssistedParam(paramElement))
.build());
}
}
return assistedParameterSpecs.build();
}

private AssistedInjectionParameters() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
* Copyright (C) 2021 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.internal.codegen.writing;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.squareup.javapoet.MethodSpec.methodBuilder;
import static dagger.internal.codegen.binding.AssistedInjectionAnnotations.assistedParameterSpecs;
import static dagger.internal.codegen.javapoet.CodeBlocks.parameterNames;
import static dagger.internal.codegen.writing.AssistedInjectionParameters.assistedParameterSpecs;
import static dagger.internal.codegen.writing.ComponentImplementation.MethodSpecKind.PRIVATE_METHOD;
import static javax.lang.model.element.Modifier.PRIVATE;

import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.CodeBlock;
import com.squareup.javapoet.TypeName;
import dagger.assisted.Assisted;
import dagger.assisted.AssistedFactory;
import dagger.assisted.AssistedInject;
import dagger.internal.codegen.binding.BindingRequest;
import dagger.internal.codegen.binding.ContributionBinding;
import dagger.internal.codegen.compileroption.CompilerOptions;
import dagger.internal.codegen.javapoet.Expression;
import dagger.internal.codegen.langmodel.DaggerTypes;
import dagger.internal.codegen.writing.ComponentImplementation.ShardImplementation;
import dagger.spi.model.BindingKind;
import dagger.spi.model.RequestKind;
import javax.lang.model.type.TypeMirror;

/** A binding expression that wraps private method call for assisted fatory creation. */
final class AssistedPrivateMethodBindingExpression extends MethodBindingExpression {
private final ShardImplementation shardImplementation;
private final ContributionBinding binding;
private final BindingRequest request;
private final BindingExpression wrappedBindingExpression;
private final CompilerOptions compilerOptions;
private final DaggerTypes types;
private String methodName;

@AssistedInject
AssistedPrivateMethodBindingExpression(
@Assisted BindingRequest request,
@Assisted ContributionBinding binding,
@Assisted BindingExpression wrappedBindingExpression,
ComponentImplementation componentImplementation,
DaggerTypes types,
CompilerOptions compilerOptions) {
super(componentImplementation.shardImplementation(binding), types);
checkArgument(binding.kind() == BindingKind.ASSISTED_INJECTION);
checkArgument(request.requestKind() == RequestKind.INSTANCE);
this.binding = checkNotNull(binding);
this.request = checkNotNull(request);
this.wrappedBindingExpression = checkNotNull(wrappedBindingExpression);
this.shardImplementation = componentImplementation.shardImplementation(binding);
this.compilerOptions = compilerOptions;
this.types = types;
}

Expression getAssistedDependencyExpression(ClassName requestingClass) {
return Expression.create(
returnType(),
requestingClass.equals(shardImplementation.name())
? CodeBlock.of(
"$N($L)", methodName(), parameterNames(assistedParameterSpecs(binding, types)))
: CodeBlock.of(
"$L.$N($L)",
shardImplementation.shardFieldReference(),
methodName(),
parameterNames(assistedParameterSpecs(binding, types))));
}

@Override
protected CodeBlock methodCall() {
throw new IllegalStateException("This should not be accessed");
}

@Override
protected TypeMirror returnType() {
return types.accessibleType(binding.contributedType(), shardImplementation.name());
}

private String methodName() {
if (methodName == null) {
// Have to set methodName field before implementing the method in order to handle recursion.
methodName = shardImplementation.getUniqueMethodName(request);

// TODO(bcorso): Fix the order that these generated methods are written to the component.
shardImplementation.addMethod(
PRIVATE_METHOD,
methodBuilder(methodName)
.addModifiers(PRIVATE)
.addParameters(assistedParameterSpecs(binding, types, shardImplementation))
.returns(TypeName.get(returnType()))
.addStatement(
"return $L",
wrappedBindingExpression
.getDependencyExpression(shardImplementation.name())
.codeBlock())
.build());
}
return methodName;
}

@AssistedFactory
static interface Factory {
AssistedPrivateMethodBindingExpression create(
BindingRequest request,
ContributionBinding binding,
BindingExpression wrappedBindingExpression);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ public final class ComponentBindingExpressions {
private final ImmediateFutureBindingExpression.Factory immediateFutureBindingExpressionFactory;
private final MembersInjectionBindingExpression.Factory membersInjectionBindingExpressionFactory;
private final PrivateMethodBindingExpression.Factory privateMethodBindingExpressionFactory;
private final AssistedPrivateMethodBindingExpression.Factory
assistedPrivateMethodBindingExpressionFactory;
private final ProducerNodeInstanceBindingExpression.Factory
producerNodeInstanceBindingExpressionFactory;
private final ProviderInstanceBindingExpression.Factory providerInstanceBindingExpressionFactory;
Expand Down Expand Up @@ -106,6 +108,7 @@ public final class ComponentBindingExpressions {
ImmediateFutureBindingExpression.Factory immediateFutureBindingExpressionFactory,
MembersInjectionBindingExpression.Factory membersInjectionBindingExpressionFactory,
PrivateMethodBindingExpression.Factory privateMethodBindingExpressionFactory,
AssistedPrivateMethodBindingExpression.Factory assistedPrivateMethodBindingExpressionFactory,
ProducerNodeInstanceBindingExpression.Factory producerNodeInstanceBindingExpressionFactory,
ProviderInstanceBindingExpression.Factory providerInstanceBindingExpressionFactory,
UnscopedDirectInstanceBindingExpressionFactory unscopedDirectInstanceBindingExpressionFactory,
Expand Down Expand Up @@ -137,6 +140,8 @@ public final class ComponentBindingExpressions {
this.types = types;
this.compilerOptions = compilerOptions;
this.switchingProviders = new SwitchingProviders(componentImplementation, types);
this.assistedPrivateMethodBindingExpressionFactory =
assistedPrivateMethodBindingExpressionFactory;
}

/**
Expand Down Expand Up @@ -428,6 +433,12 @@ private BindingExpression instanceBindingExpression(ContributionBinding binding)
Optional<BindingExpression> maybeDirectInstanceExpression =
unscopedDirectInstanceBindingExpressionFactory.create(binding);
if (maybeDirectInstanceExpression.isPresent()) {
BindingExpression directInstanceExpression = maybeDirectInstanceExpression.get();
if (binding.kind() == BindingKind.ASSISTED_INJECTION) {
BindingRequest request = bindingRequest(binding.key(), RequestKind.INSTANCE);
return assistedPrivateMethodBindingExpressionFactory.create(
request, binding, directInstanceExpression);
}
boolean isDefaultModeAssistedFactory =
binding.kind() == BindingKind.ASSISTED_FACTORY && !isFastInit();

Expand All @@ -439,7 +450,6 @@ private BindingExpression instanceBindingExpression(ContributionBinding binding)
// using a field in the component similar to Provider requests. This should also be the case
// in FastInit, but it hasn't been implemented yet.
if (!needsCaching(binding) && !isDefaultModeAssistedFactory) {
BindingExpression directInstanceExpression = maybeDirectInstanceExpression.get();
return directInstanceExpression.requiresMethodEncapsulation()
? wrapInMethod(binding, RequestKind.INSTANCE, directInstanceExpression)
: directInstanceExpression;
Expand Down
17 changes: 11 additions & 6 deletions java/dagger/internal/codegen/writing/ComponentImplementation.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,6 @@
import dagger.internal.codegen.kotlin.KotlinMetadataUtil;
import dagger.internal.codegen.langmodel.DaggerElements;
import dagger.internal.codegen.langmodel.DaggerTypes;
import dagger.internal.codegen.writing.ComponentImplementation.FieldSpecKind;
import dagger.internal.codegen.writing.ComponentImplementation.MethodSpecKind;
import dagger.internal.codegen.writing.ComponentImplementation.ShardImplementation;
import dagger.internal.codegen.writing.ComponentImplementation.TypeSpecKind;
import dagger.spi.model.BindingGraph.Node;
import dagger.spi.model.Key;
import dagger.spi.model.RequestKind;
Expand All @@ -99,6 +95,7 @@
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.Modifier;
import javax.lang.model.element.TypeElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.DeclaredType;
import javax.lang.model.type.TypeMirror;

Expand Down Expand Up @@ -251,7 +248,6 @@ private static ImmutableList<ImmutableList<Binding>> bindingPartitions(
componentCreatorImplementationFactoryProvider;
private final BindingGraph graph;
private final ComponentNames componentNames;
private final CompilerOptions compilerOptions;
private final DaggerElements elements;
private final DaggerTypes types;
private final KotlinMetadataUtil metadataUtil;
Expand All @@ -277,7 +273,6 @@ private static ImmutableList<ImmutableList<Binding>> bindingPartitions(
componentCreatorImplementationFactoryProvider;
this.graph = graph;
this.componentNames = componentNames;
this.compilerOptions = compilerOptions;
this.elements = elements;
this.types = types;
this.metadataUtil = metadataUtil;
Expand Down Expand Up @@ -423,6 +418,7 @@ public final class ShardImplementation {
private final UniqueNameSet componentMethodNames = new UniqueNameSet();
private final List<CodeBlock> initializations = new ArrayList<>();
private final Map<Key, CodeBlock> cancellations = new LinkedHashMap<>();
private final Map<VariableElement, String> uniqueAssistedName = new LinkedHashMap<>();
private final List<CodeBlock> componentRequirementInitializations = new ArrayList<>();
private final ImmutableMap<ComponentRequirement, ParameterSpec> constructorParameters;
private final ListMultimap<FieldSpecKind, FieldSpec> fieldSpecsMap =
Expand Down Expand Up @@ -569,6 +565,15 @@ String getUniqueFieldName(String name) {
return componentFieldNames.getUniqueName(name);
}

public String getUniqueFieldNameForAssistedParam(VariableElement element) {
if (uniqueAssistedName.containsKey(element)) {
return uniqueAssistedName.get(element);
}
String name = getUniqueFieldName(element.getSimpleName().toString());
uniqueAssistedName.put(element, name);
return name;
}

/** Returns a new, unique method name for the component based on the given name. */
public String getUniqueMethodName(String name) {
return componentMethodNames.getUniqueName(name);
Expand Down
26 changes: 18 additions & 8 deletions java/dagger/internal/codegen/writing/FactoryGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import static dagger.internal.codegen.binding.SourceFiles.generateBindingFieldsForDependencies;
import static dagger.internal.codegen.binding.SourceFiles.generatedClassNameForBinding;
import static dagger.internal.codegen.binding.SourceFiles.parameterizedGeneratedTypeNameForBinding;
import static dagger.internal.codegen.extension.DaggerStreams.toImmutableList;
import static dagger.internal.codegen.extension.DaggerStreams.toImmutableMap;
import static dagger.internal.codegen.javapoet.AnnotationSpecs.Suppression.RAWTYPES;
import static dagger.internal.codegen.javapoet.AnnotationSpecs.Suppression.UNCHECKED;
import static dagger.internal.codegen.javapoet.AnnotationSpecs.suppressWarnings;
Expand Down Expand Up @@ -67,11 +67,13 @@
import dagger.spi.model.BindingKind;
import dagger.spi.model.DependencyRequest;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import javax.annotation.processing.Filer;
import javax.inject.Inject;
import javax.lang.model.SourceVersion;
import javax.lang.model.element.Element;
import javax.lang.model.element.VariableElement;

/**
* Generates {@link Factory} implementations from {@link ProvisionBinding} instances for {@link
Expand Down Expand Up @@ -223,28 +225,36 @@ private void addCreateMethod(ProvisionBinding binding, TypeSpec.Builder factoryB
}

private MethodSpec getMethod(ProvisionBinding binding) {
UniqueNameSet uniqueFieldNames = new UniqueNameSet();
ImmutableMap<DependencyRequest, FieldSpec> frameworkFields = frameworkFields(binding);
frameworkFields.values().forEach(field -> uniqueFieldNames.claim(field.name));
Map<VariableElement, ParameterSpec> assistedParameters =
assistedParameters(binding).stream()
.collect(
toImmutableMap(
element -> element,
element ->
ParameterSpec.builder(
TypeName.get(element.asType()),
uniqueFieldNames.getUniqueName(element.getSimpleName()))
.build()));
TypeName providedTypeName = providedTypeName(binding);
MethodSpec.Builder getMethod =
methodBuilder("get")
.addModifiers(PUBLIC)
.returns(providedTypeName)
.addParameters(
// The 'get' method for an assisted injection type takes in the assisted parameters.
assistedParameters(binding).stream()
.map(ParameterSpec::get)
.collect(toImmutableList()));
.addParameters(assistedParameters.values());

if (factoryTypeName(binding).isPresent()) {
getMethod.addAnnotation(Override.class);
}

ImmutableMap<DependencyRequest, FieldSpec> frameworkFields = frameworkFields(binding);
CodeBlock invokeNewInstance =
ProvisionMethod.invoke(
binding,
request ->
frameworkTypeUsageStatement(
CodeBlock.of("$N", frameworkFields.get(request)), request.kind()),
param -> assistedParameters.get(param).name,
generatedClassNameForBinding(binding),
moduleParameter(binding).map(module -> CodeBlock.of("$N", module)),
compilerOptions,
Expand Down
Loading

0 comments on commit 3d93625

Please sign in to comment.