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 Jan 28, 2021
1 parent 229c99f commit 524a7ab
Show file tree
Hide file tree
Showing 11 changed files with 812 additions and 108 deletions.
35 changes: 34 additions & 1 deletion java/dagger/assisted/Assisted.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,37 @@
@Documented
@Retention(RUNTIME)
@Target(PARAMETER)
public @interface Assisted {}
public @interface Assisted {
/**
* Returns an identifier for an {@link Assisted} parameter.
*
* <p>An assisted parameter should be uniquely defined within an {@link AssistedInject}
* constructor by the combination of its identifier and type.
*
* <p>In addition, each parameter in the {@link AssistedInject} constructor must have a matching
* parameter (identifier + type) in the associated {@link AssistedFactory} method that creates the
* assisted type.
*
* <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 "";
}
142 changes: 55 additions & 87 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,21 +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.Binding;
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 @@ -71,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 @@ -174,36 +170,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 @@ -213,39 +211,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 @@ -299,13 +264,6 @@ public Element originatingElement(ProvisionBinding binding) {
@Override
public Optional<TypeSpec.Builder> write(ProvisionBinding binding) {
TypeElement factory = asType(binding.bindingElement().get());
ExecutableElement factoryMethod =
AssistedInjectionAnnotations.assistedFactoryMethod(factory, elements, types);
ExecutableType factoryMethodType =
asExecutable(types.asMemberOf(asDeclared(binding.key().type()), factoryMethod));
TypeElement returnElement = asTypeElement(factoryMethodType.getReturnType());
ParameterSpec delegateFactoryParam =
ParameterSpec.builder(delegateFactoryTypeName(returnElement), "delegateFactory").build();
TypeSpec.Builder builder =
TypeSpec.classBuilder(nameGeneratedType(binding))
.addModifiers(PUBLIC, FINAL)
Expand All @@ -320,6 +278,12 @@ public Optional<TypeSpec.Builder> write(ProvisionBinding binding) {
builder.superclass(factory.asType());
}

AssistedFactoryMetadata metadata =
AssistedFactoryMetadata.create(asDeclared(factory.asType()), elements, types);
ParameterSpec delegateFactoryParam =
ParameterSpec.builder(
delegateFactoryTypeName(metadata.assistedInjectType()), "delegateFactory")
.build();
builder
.addField(
FieldSpec.builder(delegateFactoryParam.type, delegateFactoryParam.name)
Expand All @@ -331,11 +295,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 @@ -344,7 +311,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 @@ -357,19 +324,20 @@ public Optional<TypeSpec.Builder> write(ProvisionBinding binding) {
return Optional.of(builder);
}

private TypeName delegateFactoryTypeName(TypeElement returnElement) {
Binding delegateBinding =
bindingFactory.injectionBinding(
getOnlyElement(assistedInjectedConstructors(returnElement)), Optional.empty());
ImmutableList<TypeVariableName> typeParameters =
returnElement.getTypeParameters().stream()
.map(TypeVariableName::get)
.collect(toImmutableList());
return typeParameters.isEmpty()
? generatedClassNameForBinding(delegateBinding)
private TypeName delegateFactoryTypeName(DeclaredType assistedInjectType) {
ClassName generatedFactoryClassName =
generatedClassNameForBinding(
bindingFactory.injectionBinding(
getOnlyElement(assistedInjectedConstructors(asTypeElement(assistedInjectType))),
Optional.empty()));
return assistedInjectType.getTypeArguments().isEmpty()
? generatedFactoryClassName
: ParameterizedTypeName.get(
generatedClassNameForBinding(delegateBinding),
typeParameters.toArray(new TypeName[0]));
generatedFactoryClassName,
assistedInjectType.getTypeArguments().stream()
.map(TypeName::get)
.collect(toImmutableList())
.toArray(new TypeName[0]));
}
}
}
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 524a7ab

Please sign in to comment.