Skip to content

Commit

Permalink
Rework the way AutoValue generates imports.
Browse files Browse the repository at this point in the history 
Previously we attempted to construct the set of all referenced types, before generating anything, and we then used the TypeSimplifier class to generate appropriate imports and respell classes appropriately. Now, we have a two-pass system. Initially we use backquotes to wrap every class name in the text, so java.lang.Number will be `java.lang.Number` and java.util.List<? extends java.lang.Number> might be `java.util.List`<? extends `java.lang.Number`>. The complete source program is generated with class names encoded that way. Then we rescan the source to find which classes are actually referenced in it. We compute the appropriate imports, and we replace the backquoted names with the appropriate spellings given those imports.

This makes the code much simpler and less error-prone. In particular, we don't need to have two kinds of TypeVisitor, one for finding referenced classes and another for converting types to strings. The act of converting to a string includes recording the referenced classes.

RELNOTES=Reworked the logic for imports in AutoValue.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=178278647
  • Loading branch information
@eamonnmcmanus @ronshapiro
eamonnmcmanus authored and ronshapiro committed Dec 8, 2017
1 parent c9fd48f commit 39b9987
Show file tree
Hide file tree
Showing 23 changed files with 1,141 additions and 1,001 deletions.
10 changes: 5 additions & 5 deletions value/src/it/functional/src/test/java/com/google/auto/value/gwt/GwtCompilationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,11 @@ public void testBasic() {
"foo.bar.AutoValue_Baz_CustomFieldSerializer",
"package foo.bar;",
"",
"import javax.annotation.Generated;",
"import com.google.gwt.user.client.rpc.CustomFieldSerializer;",
"import com.google.gwt.user.client.rpc.SerializationException;",
"import com.google.gwt.user.client.rpc.SerializationStreamReader;",
"import com.google.gwt.user.client.rpc.SerializationStreamWriter;",
"import javax.annotation.Generated;",
"",
"@Generated(\"com.google.auto.value.processor.AutoValueProcessor\")",
"public final class AutoValue_Baz_CustomFieldSerializer"
Expand Down Expand Up @@ -161,12 +161,12 @@ public void testSuppressWarnings() {
"foo.bar.AutoValue_Baz_CustomFieldSerializer",
"package foo.bar;",
"",
"import java.util.List;",
"import javax.annotation.Generated;",
"import com.google.gwt.user.client.rpc.CustomFieldSerializer;",
"import com.google.gwt.user.client.rpc.SerializationException;",
"import com.google.gwt.user.client.rpc.SerializationStreamReader;",
"import com.google.gwt.user.client.rpc.SerializationStreamWriter;",
"import java.util.List;",
"import javax.annotation.Generated;",
"",
"@Generated(\"com.google.auto.value.processor.AutoValueProcessor\")",
"public final class AutoValue_Baz_CustomFieldSerializer"
Expand Down Expand Up @@ -261,12 +261,12 @@ public void testBuildersAndGenerics() {
"foo.bar.AutoValue_Baz_CustomFieldSerializer",
"package foo.bar;",
"",
"import java.util.Map;",
"import javax.annotation.Generated;",
"import com.google.gwt.user.client.rpc.CustomFieldSerializer;",
"import com.google.gwt.user.client.rpc.SerializationException;",
"import com.google.gwt.user.client.rpc.SerializationStreamReader;",
"import com.google.gwt.user.client.rpc.SerializationStreamWriter;",
"import java.util.Map;",
"import javax.annotation.Generated;",
"",
"@Generated(\"com.google.auto.value.processor.AutoValueProcessor\")",
"public final class AutoValue_Baz_CustomFieldSerializer"
Expand Down
2 changes: 0 additions & 2 deletions value/src/it/gwtserializer/src/test/java/com/google/auto/value/client/GwtSerializerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
import com.google.gwt.user.client.rpc.RemoteService;
import com.google.gwt.user.client.rpc.RemoteServiceRelativePath;
import com.google.gwt.user.server.rpc.RemoteServiceServlet;
import java.lang.Override;
import java.lang.SuppressWarnings;

public class GwtSerializerTest extends GWTTestCase {

Expand Down
24 changes: 10 additions & 14 deletions value/src/main/java/com/google/auto/value/processor/AnnotationOutput.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,8 @@
*
* @author emcmanus@google.com (Éamonn McManus)
*/
class AnnotationOutput {
private final TypeSimplifier typeSimplifier;

AnnotationOutput(TypeSimplifier typeSimplifier) {
this.typeSimplifier = typeSimplifier;
}
final class AnnotationOutput {
private AnnotationOutput() {} // There are no instances of this class.

/**
* Visitor that produces a string representation of an annotation value, suitable for inclusion
Expand All @@ -49,7 +45,7 @@ class AnnotationOutput {
* for example the construction of an {@code @AutoAnnotation} class. That's why we have this
* abstract class and two concrete subclasses.
*/
private abstract class SourceFormVisitor
private abstract static class SourceFormVisitor
extends SimpleAnnotationValueVisitor8<Void, StringBuilder> {
@Override
protected Void defaultAction(Object value, StringBuilder sb) {
Expand Down Expand Up @@ -112,7 +108,7 @@ public Void visitFloat(float f, StringBuilder sb) {

@Override
public Void visitEnumConstant(VariableElement c, StringBuilder sb) {
sb.append(typeSimplifier.simplify(c.asType())).append('.').append(c.getSimpleName());
sb.append(TypeEncoder.encode(c.asType())).append('.').append(c.getSimpleName());
return null;
}

Expand All @@ -124,12 +120,12 @@ public Void visitString(String s, StringBuilder sb) {

@Override
public Void visitType(TypeMirror classConstant, StringBuilder sb) {
sb.append(typeSimplifier.simplify(classConstant)).append(".class");
sb.append(TypeEncoder.encode(classConstant)).append(".class");
return null;
}
}

private class InitializerSourceFormVisitor extends SourceFormVisitor {
private static class InitializerSourceFormVisitor extends SourceFormVisitor {
private final ProcessingEnvironment processingEnv;
private final String memberName;
private final Element context;
Expand All @@ -153,10 +149,10 @@ public Void visitAnnotation(AnnotationMirror a, StringBuilder sb) {
}
}

private class AnnotationSourceFormVisitor extends SourceFormVisitor {
private static class AnnotationSourceFormVisitor extends SourceFormVisitor {
@Override
public Void visitAnnotation(AnnotationMirror a, StringBuilder sb) {
sb.append('@').append(typeSimplifier.simplify(a.getAnnotationType()));
sb.append('@').append(TypeEncoder.encode(a.getAnnotationType()));
Map<ExecutableElement, AnnotationValue> map =
ImmutableMap.<ExecutableElement, AnnotationValue>copyOf(a.getElementValues());
if (!map.isEmpty()) {
Expand All @@ -177,7 +173,7 @@ public Void visitAnnotation(AnnotationMirror a, StringBuilder sb) {
* Returns a string representation of the given annotation value, suitable for inclusion in a Java
* source file as the initializer of a variable of the appropriate type.
*/
String sourceFormForInitializer(
static String sourceFormForInitializer(
AnnotationValue annotationValue,
ProcessingEnvironment processingEnv,
String memberName,
Expand All @@ -193,7 +189,7 @@ String sourceFormForInitializer(
* Returns a string representation of the given annotation mirror, suitable for inclusion in a
* Java source file to reproduce the annotation in source form.
*/
String sourceFormForAnnotation(AnnotationMirror annotationMirror) {
static String sourceFormForAnnotation(AnnotationMirror annotationMirror) {
StringBuilder sb = new StringBuilder();
new AnnotationSourceFormVisitor().visitAnnotation(annotationMirror, sb);
return sb.toString();
Expand Down
93 changes: 17 additions & 76 deletions value/src/main/java/com/google/auto/value/processor/AutoAnnotationProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
*/
package com.google.auto.value.processor;

import static java.util.stream.Collectors.toCollection;

import com.google.auto.common.MoreElements;
import com.google.auto.common.SuperficialValidation;
import com.google.auto.service.AutoService;
Expand All @@ -28,14 +26,12 @@
import com.google.common.primitives.Primitives;
import java.io.IOException;
import java.io.Writer;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import javax.annotation.Generated;
import javax.annotation.processing.AbstractProcessor;
import javax.annotation.processing.ProcessingEnvironment;
import javax.annotation.processing.Processor;
Expand Down Expand Up @@ -143,36 +139,25 @@ private void processMethod(ExecutableElement method) {
}

TypeElement annotationElement = getAnnotationReturnType(method);
TypeMirror annotationTypeMirror = annotationElement.asType();

Set<Class<?>> wrapperTypesUsedInCollections = wrapperTypesUsedInCollections(method);

ImmutableMap<String, ExecutableElement> memberMethods = getMemberMethods(annotationElement);
Set<TypeMirror> memberTypes = getMemberTypes(memberMethods.values());
Set<TypeMirror> referencedTypes = getReferencedTypes(
annotationTypeMirror, method, memberTypes, wrapperTypesUsedInCollections);
TypeElement methodClass = (TypeElement) method.getEnclosingElement();
String pkg = TypeSimplifier.packageNameOf(methodClass);
TypeSimplifier typeSimplifier = new TypeSimplifier(
typeUtils, pkg, referencedTypes, annotationTypeMirror);

AnnotationOutput annotationOutput = new AnnotationOutput(typeSimplifier);
ImmutableMap<String, AnnotationValue> defaultValues = getDefaultValues(annotationElement);
ImmutableMap<String, Member> members =
getMembers(method, memberMethods, typeSimplifier, annotationOutput);
ImmutableMap<String, Parameter> parameters =
getParameters(annotationElement, method, members, typeSimplifier);
ImmutableMap<String, Member> members = getMembers(method, memberMethods);
ImmutableMap<String, Parameter> parameters = getParameters(annotationElement, method, members);
validateParameters(annotationElement, method, members, parameters, defaultValues);

String generatedClassName = generatedClassName(method);

AutoAnnotationTemplateVars vars = new AutoAnnotationTemplateVars();
vars.annotationFullName = annotationElement.toString();
vars.annotationName = typeSimplifier.simplify(annotationElement.asType());
vars.annotationName = TypeEncoder.encode(annotationElement.asType());
vars.className = generatedClassName;
vars.imports = typeSimplifier.typesToImport();
vars.generated = getGeneratedTypeName(typeSimplifier);
vars.arrays = typeSimplifier.simplify(getTypeMirror(Arrays.class));
vars.generated = getGeneratedTypeName();
vars.members = members;
vars.params = parameters;
vars.pkg = pkg;
Expand All @@ -185,18 +170,19 @@ private void processMethod(ExecutableElement method) {
}
vars.invariableHashes = invariableHashes.keySet();
String text = vars.toText();
text = TypeEncoder.decode(text, processingEnv, pkg, annotationElement.asType());
text = Reformatter.fixup(text);
String fullName = fullyQualifiedName(pkg, generatedClassName);
writeSourceFile(fullName, text, methodClass);
}

private String getGeneratedTypeName(TypeSimplifier typeSimplifier) {
private String getGeneratedTypeName() {
TypeElement generatedTypeElement =
processingEnv.getElementUtils().getTypeElement("javax.annotation.Generated");
if (generatedTypeElement == null) {
return "";
}
return typeSimplifier.simplify(generatedTypeElement.asType());
return TypeEncoder.encode(generatedTypeElement.asType());
}

/**
Expand Down Expand Up @@ -301,16 +287,14 @@ private ImmutableMap<String, ExecutableElement> getMemberMethods(TypeElement ann

private ImmutableMap<String, Member> getMembers(
Element context,
ImmutableMap<String, ExecutableElement> memberMethods,
TypeSimplifier typeSimplifier,
AnnotationOutput annotationOutput) {
ImmutableMap<String, ExecutableElement> memberMethods) {
ImmutableMap.Builder<String, Member> members = ImmutableMap.builder();
for (Map.Entry<String, ExecutableElement> entry : memberMethods.entrySet()) {
ExecutableElement memberMethod = entry.getValue();
String name = memberMethod.getSimpleName().toString();
members.put(
name,
new Member(processingEnv, context, memberMethod, typeSimplifier, annotationOutput));
new Member(processingEnv, context, memberMethod));
}
return members.build();
}
Expand All @@ -328,17 +312,10 @@ private ImmutableMap<String, AnnotationValue> getDefaultValues(TypeElement annot
return defaultValues.build();
}

private Set<TypeMirror> getMemberTypes(Collection<ExecutableElement> memberMethods) {
return memberMethods.stream()
.map(m -> m.getReturnType())
.collect(toCollection(TypeMirrorSet::new));
}

private ImmutableMap<String, Parameter> getParameters(
TypeElement annotationElement,
ExecutableElement method,
Map<String, Member> members,
TypeSimplifier typeSimplifier) {
Map<String, Member> members) {
ImmutableMap.Builder<String, Parameter> parameters = ImmutableMap.builder();
boolean error = false;
for (VariableElement parameter : method.getParameters()) {
Expand All @@ -353,7 +330,7 @@ private ImmutableMap<String, Parameter> getParameters(
TypeMirror parameterType = parameter.asType();
TypeMirror memberType = member.getTypeMirror();
if (compatibleTypes(parameterType, memberType)) {
parameters.put(name, new Parameter(parameterType, typeSimplifier));
parameters.put(name, new Parameter(parameterType));
} else {
reportError(parameter,
"@AutoAnnotation method parameter '%s' has type %s but %s.%s has type %s",
Expand Down Expand Up @@ -440,40 +417,10 @@ private Set<Class<?>> wrapperTypesUsedInCollections(ExecutableElement method) {
return usedInCollections.build();
}

private Set<TypeMirror> getReferencedTypes(
TypeMirror annotation,
ExecutableElement method,
Set<TypeMirror> memberTypes,
Set<Class<?>> wrapperTypesUsedInCollections) {
Set<TypeMirror> types = new TypeMirrorSet();
types.add(annotation);
types.add(getTypeMirror(Generated.class));
for (VariableElement parameter : method.getParameters()) {
// Method parameter types are usually the same as annotation member types, but in the case of
// List<Integer> for int[] we are referencing List.
types.add(parameter.asType());
}
types.addAll(memberTypes);
if (containsArrayType(types)) {
// If there are array properties then we will be referencing java.util.Arrays.
types.add(getTypeMirror(Arrays.class));
}
if (!wrapperTypesUsedInCollections.isEmpty()) {
// If there is at least one parameter whose type is a collection of a primitive wrapper type
// (for example List<Integer>) then we will be referencing java util.Collection.
types.add(getTypeMirror(Collection.class));
}
return types;
}

private TypeMirror getTypeMirror(Class<?> c) {
return processingEnv.getElementUtils().getTypeElement(c.getName()).asType();
}

private static boolean containsArrayType(Set<TypeMirror> types) {
return types.stream().anyMatch(t -> t.getKind().equals(TypeKind.ARRAY));
}

private static boolean isGwtCompatible(TypeElement annotationElement) {
return annotationElement.getAnnotationMirrors().stream()
.map(mirror -> mirror.getAnnotationType().asElement())
Expand Down Expand Up @@ -507,20 +454,14 @@ public static class Member {
private final ProcessingEnvironment processingEnv;
private final Element context;
private final ExecutableElement method;
private final TypeSimplifier typeSimplifier;
private final AnnotationOutput annotationOutput;

Member(
ProcessingEnvironment processingEnv,
Element context,
ExecutableElement method,
TypeSimplifier typeSimplifier,
AnnotationOutput annotationDefaults) {
ExecutableElement method) {
this.processingEnv = processingEnv;
this.context = context;
this.method = method;
this.typeSimplifier = typeSimplifier;
this.annotationOutput = annotationDefaults;
}

@Override
Expand All @@ -529,13 +470,13 @@ public String toString() {
}

public String getType() {
return typeSimplifier.simplify(getTypeMirror());
return TypeEncoder.encode(getTypeMirror());
}

public String getComponentType() {
Preconditions.checkState(getTypeMirror().getKind() == TypeKind.ARRAY);
ArrayType arrayType = (ArrayType) getTypeMirror();
return typeSimplifier.simplify(arrayType.getComponentType());
return TypeEncoder.encode(arrayType.getComponentType());
}

public TypeMirror getTypeMirror() {
Expand Down Expand Up @@ -579,7 +520,7 @@ public String getDefaultValue() {
if (defaultValue == null) {
return null;
} else {
return annotationOutput.sourceFormForInitializer(
return AnnotationOutput.sourceFormForInitializer(
defaultValue, processingEnv, method.getSimpleName().toString(), context);
}
}
Expand All @@ -589,8 +530,8 @@ public static class Parameter {
private final String typeName;
private final TypeKind kind;

Parameter(TypeMirror type, TypeSimplifier typeSimplifier) {
this.typeName = typeSimplifier.simplify(type);
Parameter(TypeMirror type) {
this.typeName = TypeEncoder.encode(type);
this.kind = type.getKind();
}

Expand Down
12 changes: 1 addition & 11 deletions value/src/main/java/com/google/auto/value/processor/AutoAnnotationTemplateVars.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import com.google.auto.value.processor.escapevelocity.Template;
import java.util.Map;
import java.util.Set;
import java.util.SortedSet;

/**
* The variables to substitute into the autoannotation.vm template.
Expand All @@ -39,19 +38,10 @@ class AutoAnnotationTemplateVars extends TemplateVars {
Map<String, AutoAnnotationProcessor.Parameter> params;

/**
* The fully-qualified names of the classes to be imported in the generated class.
*/
SortedSet<String> imports;

/**
* The spelling of the {@code Generated} class: {@code Generated} or {@code
* javax.annotation.Generated}. Empty if the class is not available.
* The encoded form of the {@code Generated} class, or empty if it is not available.
*/
String generated;

/** The spelling of the java.util.Arrays class: Arrays or java.util.Arrays. */
String arrays;

/**
* The package of the class containing the {@code @AutoAnnotation} annotation, which is also the
* package where the annotation implementation will be generated.
Expand Down
Loading

0 comments on commit 39b9987

Please sign in to comment.