Skip to content

Commit

Permalink
Add string qualifier to @assisted annotation to disambiguate duplicat…
Browse files Browse the repository at this point in the history
…e assisted parameter types.

Fixes #2281

RELNOTES=Fixes #2281: Force duplicate assisted types to use @assisted("...") to disambiguate.
PiperOrigin-RevId: 353061817
  • Loading branch information
bcorso authored and Dagger Team committed Feb 1, 2021
1 parent 12a8f39 commit 87fb27d
Show file tree
Hide file tree
Showing 10 changed files with 785 additions and 97 deletions.
50 changes: 49 additions & 1 deletion java/dagger/assisted/Assisted.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,52 @@
@Documented
@Retention(RUNTIME)
@Target(PARAMETER)
public @interface Assisted {}
public @interface Assisted {

/**
* Returns an identifier for an {@link Assisted} parameter.
*
* <p>Within an {@link AssistedInject} constructor, each {@link Assisted} parameter must be
* uniquely defined by the combination of its identifier and type. If no identifier is specified,
* the default identifier is an empty string. Thus, the following parameters are equivalent within
* an {@link AssistedInject} constructor:
*
* <ul>
* <li> {@code @Assisted Foo foo}
* <li> {@code @Assisted("") Foo foo}
* </ul>
*
* <p>Within an {@link AssistedFactory} method, each parameter must match an {@link Assisted}
* parameter in the associated {@link AssistedInject} constructor (i.e. identifier + type).
* A parameter with no {@code @Assisted} annotation will be assigned the default identifier. Thus,
* the following parameters are equivalent within an {@link AssistedFactory} method:
*
* <ul>
* <li> {@code Foo foo}
* <li> {@code @Assisted Foo foo}
* <li> {@code @Assisted("") Foo foo}
* </ul>
*
* <p>Example:
*
* <pre><code>
* final class DataService {
* {@literal @}AssistedInject
* DataService(
* DataFetcher dataFetcher,
* {@literal @}Assisted String name,
* {@literal @}Assisted("id") String id,
* {@literal @}Assisted("repo") String repo) {}
* }
*
* {@literal @}AssistedFactory
* interface DataServiceFactory {
* DataService create(
* String name,
* {@literal @}Assisted("id") String id,
* {@literal @}Assisted("repo") String repo);
* }
* </code></pre>
*/
String value() default "";
}
118 changes: 42 additions & 76 deletions java/dagger/internal/codegen/AssistedFactoryProcessingStep.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@
package dagger.internal.codegen;

import static com.google.auto.common.MoreElements.asType;
import static com.google.auto.common.MoreElements.isAnnotationPresent;
import static com.google.auto.common.MoreTypes.asDeclared;
import static com.google.auto.common.MoreTypes.asExecutable;
import static com.google.auto.common.MoreTypes.asTypeElement;
import static com.google.common.collect.Iterables.getOnlyElement;
import static dagger.internal.codegen.binding.AssistedInjectionAnnotations.assistedInjectedConstructors;
Expand All @@ -36,8 +34,6 @@
import static javax.lang.model.element.Modifier.STATIC;

import com.google.auto.common.MoreElements;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.CodeBlock;
Expand All @@ -48,20 +44,22 @@
import com.squareup.javapoet.TypeName;
import com.squareup.javapoet.TypeSpec;
import com.squareup.javapoet.TypeVariableName;
import dagger.assisted.Assisted;
import dagger.assisted.AssistedFactory;
import dagger.internal.codegen.base.SourceFileGenerationException;
import dagger.internal.codegen.base.SourceFileGenerator;
import dagger.internal.codegen.binding.AssistedInjectionAnnotations;
import dagger.internal.codegen.binding.AssistedInjectionAnnotations.AssistedFactoryMetadata;
import dagger.internal.codegen.binding.AssistedInjectionAnnotations.AssistedParameter;
import dagger.internal.codegen.binding.BindingFactory;
import dagger.internal.codegen.binding.ProvisionBinding;
import dagger.internal.codegen.langmodel.DaggerElements;
import dagger.internal.codegen.langmodel.DaggerTypes;
import dagger.internal.codegen.validation.TypeCheckingProcessingStep;
import dagger.internal.codegen.validation.ValidationReport;
import java.lang.annotation.Annotation;
import java.util.Map;
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;
import javax.annotation.processing.Filer;
import javax.annotation.processing.Messager;
import javax.inject.Inject;
Expand All @@ -70,7 +68,6 @@
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.TypeElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.DeclaredType;
import javax.lang.model.type.ExecutableType;
import javax.lang.model.type.TypeKind;
Expand Down Expand Up @@ -179,36 +176,38 @@ ValidationReport<TypeElement> validate(TypeElement factory) {
return report.build();
}

// Given the previous checks, we can be sure we have a
// single factory method with a valid return type.
ExecutableElement factoryMethod = getOnlyElement(abstractFactoryMethods);
ExecutableType factoryMethodType =
types.resolveExecutableType(factoryMethod, factory.asType());
DeclaredType returnType = asDeclared(factoryMethodType.getReturnType());
AssistedFactoryMetadata metadata =
AssistedFactoryMetadata.create(factory.asType(), elements, types);

ImmutableList<TypeMirror> assistedParameterTypes =
assistedInjectConstructorParameterMap(returnType).entrySet().stream()
.filter(e -> isAnnotationPresent(e.getKey(), Assisted.class))
.map(Map.Entry::getValue)
.collect(toImmutableList());

ImmutableList<TypeMirror> factoryMethodParameterTypes =
ImmutableList.copyOf(factoryMethodType.getParameterTypes());
// Note: We check uniqueness of the @AssistedInject constructor parameters in
// AssistedInjectProcessingStep. We need to check uniqueness for here too because we may
// have resolved some type parameters that were not resolved in the @AssistedInject type.
Set<AssistedParameter> uniqueAssistedParameters = new HashSet<>();
for (AssistedParameter assistedParameter : metadata.assistedFactoryAssistedParameters()) {
if (!uniqueAssistedParameters.add(assistedParameter)) {
report.addError(
"@AssistedFactory method has duplicate @Assisted types: " + assistedParameter,
assistedParameter.variableElement());
}
}

if (!typesAssignableTo(factoryMethodParameterTypes, assistedParameterTypes)) {
if (!ImmutableSet.copyOf(metadata.assistedInjectAssistedParameters())
.equals(ImmutableSet.copyOf(metadata.assistedFactoryAssistedParameters()))) {
report.addError(
String.format(
"The parameters of the factory method must be assignable to the list of "
+ "@Assisted parameters in %s."
"The parameters in the factory method must match the @Assisted parameters in %s."
+ "\n Actual: %s#%s"
+ "\n Expected: %s#%s(%s)",
returnType,
factory.getQualifiedName(),
factoryMethod,
factory.getQualifiedName(),
factoryMethod.getSimpleName(),
assistedParameterTypes.stream().map(Object::toString).collect(joining(", "))),
factoryMethod);
metadata.assistedInjectType(),
metadata.factory().getQualifiedName(),
metadata.factoryMethod(),
metadata.factory().getQualifiedName(),
metadata.factoryMethod().getSimpleName(),
metadata.assistedInjectAssistedParameters().stream()
.map(AssistedParameter::type)
.map(Object::toString)
.collect(joining(", "))),
metadata.factoryMethod());
}

return report.build();
Expand All @@ -218,39 +217,6 @@ private boolean isAssistedInjectionType(TypeMirror type) {
return type.getKind() == TypeKind.DECLARED
&& AssistedInjectionAnnotations.isAssistedInjectionType(asTypeElement(type));
}

/** Returns {@code true} if {@code types1} are all assignable to {@code types2}. */
private boolean typesAssignableTo(
ImmutableList<TypeMirror> types1, ImmutableList<TypeMirror> types2) {
if (types1.size() != types2.size()) {
return false;
}

for (int i = 0; i < types1.size(); i++) {
if (!types.isAssignable(types1.get(i), types2.get(i))) {
return false;
}
}
return true;
}

private ImmutableMap<VariableElement, TypeMirror> assistedInjectConstructorParameterMap(
DeclaredType assistedType) {
// We keep track of the constructor both as an ExecutableElement to access @Assisted
// parameters and as an ExecutableType to access the resolved parameter types.
ExecutableElement assistedConstructor =
getOnlyElement(assistedInjectedConstructors(asTypeElement(assistedType)));
ExecutableType assistedConstructorType =
asExecutable(types.asMemberOf(assistedType, assistedConstructor));

ImmutableMap.Builder<VariableElement, TypeMirror> builder = ImmutableMap.builder();
for (int i = 0; i < assistedConstructor.getParameters().size(); i++) {
builder.put(
assistedConstructor.getParameters().get(i),
assistedConstructorType.getParameterTypes().get(i));
}
return builder.build();
}
}

/** Generates an implementation of the {@link dagger.assisted.AssistedFactory}-annotated class. */
Expand Down Expand Up @@ -320,15 +286,12 @@ public Optional<TypeSpec.Builder> write(ProvisionBinding binding) {
builder.superclass(factory.asType());
}

// Define all types associated with the @AssistedFactory before generating the implementation.
DeclaredType factoryType = asDeclared(binding.key().type());
ExecutableElement factoryMethod =
AssistedInjectionAnnotations.assistedFactoryMethod(factory, elements, types);
ExecutableType factoryMethodType = asExecutable(types.asMemberOf(factoryType, factoryMethod));
DeclaredType returnType = asDeclared(factoryMethodType.getReturnType());
TypeElement returnElement = asTypeElement(returnType);
AssistedFactoryMetadata metadata =
AssistedFactoryMetadata.create(asDeclared(factory.asType()), elements, types);
ParameterSpec delegateFactoryParam =
ParameterSpec.builder(delegateFactoryTypeName(returnType), "delegateFactory").build();
ParameterSpec.builder(
delegateFactoryTypeName(metadata.assistedInjectType()), "delegateFactory")
.build();
builder
.addField(
FieldSpec.builder(delegateFactoryParam.type, delegateFactoryParam.name)
Expand All @@ -340,11 +303,14 @@ public Optional<TypeSpec.Builder> write(ProvisionBinding binding) {
.addStatement("this.$1N = $1N", delegateFactoryParam)
.build())
.addMethod(
MethodSpec.overriding(factoryMethod, asDeclared(factory.asType()), types)
MethodSpec.overriding(metadata.factoryMethod(), metadata.factoryType(), types)
.addStatement(
"return $N.get($L)",
delegateFactoryParam,
factoryMethod.getParameters().stream()
// Use the order of the parameters from the @AssistedInject constructor but
// use the parameter names of the @AssistedFactory method.
metadata.assistedInjectAssistedParameters().stream()
.map(metadata.assistedFactoryAssistedParametersMap()::get)
.map(param -> CodeBlock.of("$L", param.getSimpleName()))
.collect(toParametersCodeBlock()))
.build())
Expand All @@ -353,7 +319,7 @@ public Optional<TypeSpec.Builder> write(ProvisionBinding binding) {
.addModifiers(PUBLIC, STATIC)
.addParameter(delegateFactoryParam)
.addTypeVariables(
returnElement.getTypeParameters().stream()
metadata.assistedInjectElement().getTypeParameters().stream()
.map(TypeVariableName::get)
.collect(toImmutableList()))
.returns(providerOf(TypeName.get(factory.asType())))
Expand All @@ -362,7 +328,7 @@ public Optional<TypeSpec.Builder> write(ProvisionBinding binding) {
INSTANCE_FACTORY,
// Java 7 type inference requires the method call provide the exact type here.
sourceVersion.compareTo(SourceVersion.RELEASE_7) <= 0
? CodeBlock.of("<$T>", types.accessibleType(factoryType, name))
? CodeBlock.of("<$T>", types.accessibleType(metadata.factoryType(), name))
: CodeBlock.of(""),
name,
delegateFactoryParam)
Expand Down
87 changes: 87 additions & 0 deletions java/dagger/internal/codegen/AssistedInjectProcessingStep.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* Copyright (C) 2020 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;

import static com.google.auto.common.MoreTypes.asDeclared;
import static com.google.common.base.Preconditions.checkState;
import static dagger.internal.codegen.langmodel.DaggerElements.closestEnclosingTypeElement;

import com.google.auto.common.MoreElements;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import dagger.assisted.AssistedInject;
import dagger.internal.codegen.binding.AssistedInjectionAnnotations;
import dagger.internal.codegen.binding.AssistedInjectionAnnotations.AssistedParameter;
import dagger.internal.codegen.langmodel.DaggerTypes;
import dagger.internal.codegen.validation.TypeCheckingProcessingStep;
import dagger.internal.codegen.validation.ValidationReport;
import java.lang.annotation.Annotation;
import java.util.HashSet;
import java.util.Set;
import javax.annotation.processing.Messager;
import javax.inject.Inject;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.type.DeclaredType;

/** An annotation processor for {@link dagger.assisted.AssistedInject}-annotated elements. */
final class AssistedInjectProcessingStep extends TypeCheckingProcessingStep<ExecutableElement> {
private final DaggerTypes types;
private final Messager messager;

@Inject
AssistedInjectProcessingStep(DaggerTypes types, Messager messager) {
super(MoreElements::asExecutable);
this.types = types;
this.messager = messager;
}

@Override
public ImmutableSet<Class<? extends Annotation>> annotations() {
return ImmutableSet.of(AssistedInject.class);
}

@Override
protected void process(
ExecutableElement assistedInjectElement,
ImmutableSet<Class<? extends Annotation>> annotations) {
new AssistedInjectValidator().validate(assistedInjectElement).printMessagesTo(messager);
}

private final class AssistedInjectValidator {
ValidationReport<ExecutableElement> validate(ExecutableElement constructor) {
checkState(constructor.getKind() == ElementKind.CONSTRUCTOR);
ValidationReport.Builder<ExecutableElement> report = ValidationReport.about(constructor);

DeclaredType assistedInjectType =
asDeclared(closestEnclosingTypeElement(constructor).asType());
ImmutableList<AssistedParameter> assistedParameters =
AssistedInjectionAnnotations.assistedInjectAssistedParameters(assistedInjectType, types);

Set<AssistedParameter> uniqueAssistedParameters = new HashSet<>();
for (AssistedParameter assistedParameter : assistedParameters) {
if (!uniqueAssistedParameters.add(assistedParameter)) {
report.addError(
"@AssistedInject constructor has duplicate @Assisted type: " + assistedParameter,
assistedParameter.variableElement());
}
}

return report.build();
}
}
}
Loading

0 comments on commit 87fb27d

Please sign in to comment.