Skip to content

Commit

Permalink
Generated code optimizations (#156)
Browse files Browse the repository at this point in the history
* Opportunistically update a few dependencies

This version of autovalue finally fixes the weird invalid jar issue that would cause some extensions to not run (an issue I had a lot with this project locally). Also uses latest gradle and some misc deps

* Make person have a nullable address for demo purposes

* If property is nullable, check first before writing

This is a minor optimization that saves space on the wire. By not writing null values, we don't have to write their keys either.

* Add test to make sure null nullable props are omitted

* Reuse adapters for types rather than look up duplicate adapters

* Update tests for new optimizations

* Remove nullable property checks

* Update tests after removing nullability checks

* Shorten typenames to simple names

* Update tests

* Fix test
  • Loading branch information
ZacSweers authored and rharter committed Dec 7, 2017
1 parent 21e5d13 commit a3d7096
Show file tree
Hide file tree
Showing 8 changed files with 277 additions and 321 deletions.
4 changes: 2 additions & 2 deletions auto-value-gson/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ dependencies {
api 'com.google.auto:auto-common:0.8'

testCompile 'junit:junit:4.12'
testCompile 'com.google.truth:truth:0.33'
testCompile 'com.google.testing.compile:compile-testing:0.11'
testCompile 'com.google.truth:truth:0.36'
testCompile 'com.google.testing.compile:compile-testing:0.13'
testCompile files(Jvm.current().getToolsJar())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
import com.google.auto.common.MoreTypes;
import com.google.auto.service.AutoService;
import com.google.auto.value.extension.AutoValueExtension;
import com.google.common.base.CaseFormat;
import com.google.common.base.Defaults;
import com.google.common.base.Joiner;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.common.primitives.Primitives;
import com.google.gson.Gson;
import com.google.gson.TypeAdapter;
Expand All @@ -19,11 +20,13 @@
import com.google.gson.stream.JsonToken;
import com.google.gson.stream.JsonWriter;
import com.squareup.javapoet.AnnotationSpec;
import com.squareup.javapoet.ArrayTypeName;
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.CodeBlock;
import com.squareup.javapoet.FieldSpec;
import com.squareup.javapoet.JavaFile;
import com.squareup.javapoet.MethodSpec;
import com.squareup.javapoet.NameAllocator;
import com.squareup.javapoet.ParameterSpec;
import com.squareup.javapoet.ParameterizedTypeName;
import com.squareup.javapoet.TypeName;
Expand All @@ -41,6 +44,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import javax.annotation.Generated;
import javax.annotation.processing.Messager;
import javax.annotation.processing.ProcessingEnvironment;
Expand All @@ -56,6 +60,8 @@
import javax.lang.model.util.Types;
import javax.tools.Diagnostic;

import static com.google.common.base.CaseFormat.LOWER_CAMEL;
import static com.google.common.base.CaseFormat.UPPER_CAMEL;
import static javax.lang.model.element.Modifier.ABSTRACT;
import static javax.lang.model.element.Modifier.FINAL;
import static javax.lang.model.element.Modifier.PRIVATE;
Expand Down Expand Up @@ -297,23 +303,62 @@ public List<Property> readProperties(Map<String, ExecutableElement> properties)
return values;
}

ImmutableMap<Property, FieldSpec> createFields(List<Property> properties) {
ImmutableMap.Builder<Property, FieldSpec> fields = ImmutableMap.builder();
ImmutableMap<TypeName, FieldSpec> createFields(List<Property> properties) {
ImmutableMap.Builder<TypeName, FieldSpec> fields = ImmutableMap.builder();

ClassName jsonAdapter = ClassName.get(TypeAdapter.class);
Set<TypeName> seenTypes = Sets.newHashSet();
NameAllocator nameAllocator = new NameAllocator();
for (Property property : properties) {
if (!property.shouldDeserialize() && !property.shouldSerialize()) {
continue;
}
TypeName type = property.type.isPrimitive() ? property.type.box() : property.type;
ParameterizedTypeName adp = ParameterizedTypeName.get(jsonAdapter, type);
fields.put(property,
FieldSpec.builder(adp, property.humanName + "Adapter", PRIVATE, FINAL).build());
if (!seenTypes.contains(property.type)) {
fields.put(property.type,
FieldSpec.builder(adp,
nameAllocator.newName(simpleName(property.type)) + "_adapter", PRIVATE, FINAL)
.build());
seenTypes.add(property.type);
}
}

return fields.build();
}

private static String simpleName(TypeName typeName) {
if (typeName instanceof ClassName) {
return UPPER_CAMEL.to(LOWER_CAMEL, ((ClassName) typeName).simpleName());
} else if (typeName instanceof ParameterizedTypeName) {
ParameterizedTypeName parameterizedTypeName = (ParameterizedTypeName) typeName;
return UPPER_CAMEL.to(LOWER_CAMEL, parameterizedTypeName.rawType.simpleName())
+ (parameterizedTypeName.typeArguments.isEmpty() ? "" : "__")
+ simpleName(parameterizedTypeName.typeArguments);
} else if (typeName instanceof ArrayTypeName) {
return "array__" + simpleName(((ArrayTypeName) typeName).componentType);
} else if (typeName instanceof WildcardTypeName) {
WildcardTypeName wildcardTypeName = (WildcardTypeName) typeName;
return "wildcard__"
+ simpleName(ImmutableList.<TypeName>builder().addAll(wildcardTypeName.lowerBounds)
.addAll(wildcardTypeName.upperBounds)
.build());
} else if (typeName instanceof TypeVariableName) {
TypeVariableName variable = (TypeVariableName) typeName;
return variable.name
+ (variable.bounds.isEmpty() ? "" : "__")
+ simpleName(variable.bounds);
} else {
return typeName.toString();
}
}

private static String simpleName(List<TypeName> typeNames) {
return Joiner.on("_").join(typeNames.stream()
.map(AutoValueGsonExtension::simpleName)
.collect(Collectors.toList()));
}

private Map<Property, FieldSpec> createDefaultValueFields(List<Property> properties) {
ImmutableMap.Builder<Property, FieldSpec> builder = ImmutableMap.builder();
for (Property prop : properties) {
Expand All @@ -330,7 +375,7 @@ private Map<Property, FieldSpec> createDefaultValueFields(List<Property> propert
}

private String upperCamelizeHumanName(Property prop) {
return CaseFormat.LOWER_CAMEL.to(CaseFormat.UPPER_CAMEL, prop.humanName);
return LOWER_CAMEL.to(UPPER_CAMEL, prop.humanName);
}

MethodSpec generateConstructor(List<Property> properties, Map<String, TypeName> types) {
Expand Down Expand Up @@ -404,17 +449,18 @@ public TypeSpec createTypeAdapter(Context context, ClassName className, ClassNam
if (defaultSetters) {
defaultValueFields = createDefaultValueFields(properties);
}
ImmutableMap<Property, FieldSpec> adapters = createFields(properties);
ImmutableMap<TypeName, FieldSpec> adapters = createFields(properties);
Set<FieldSpec> initializedFields = Sets.newHashSet();
for (Property prop : properties) {
if (defaultSetters && !prop.shouldDeserialize() && !prop.nullable()) {
// Property should be ignored for deserialization but is not marked as nullable - we require a default value
constructor.addParameter(prop.type, "default" + upperCamelizeHumanName(prop));
constructor.addStatement("this.$N = default$L", defaultValueFields.get(prop), upperCamelizeHumanName(prop));
}
if (!prop.shouldDeserialize() && !prop.shouldSerialize()) {
FieldSpec field = adapters.get(prop.type);
if ((!prop.shouldDeserialize() && !prop.shouldSerialize()) || initializedFields.contains(field)) {
continue;
}
FieldSpec field = adapters.get(prop);
if (prop.typeAdapter != null) {
if (typeUtils.isAssignable(prop.typeAdapter, typeAdapterFactory)) {
if (prop.type instanceof ParameterizedTypeName || prop.type instanceof TypeVariableName) {
Expand All @@ -434,6 +480,7 @@ public TypeSpec createTypeAdapter(Context context, ClassName className, ClassNam
TypeName type = prop.type.isPrimitive() ? prop.type.box() : prop.type;
constructor.addStatement("this.$N = $N.getAdapter($T.class)", field, gsonParam, type);
}
initializedFields.add(field);
}

ClassName gsonTypeAdapterName = className.nestedClass("GsonTypeAdapter");
Expand All @@ -444,7 +491,7 @@ public TypeSpec createTypeAdapter(Context context, ClassName className, ClassNam
.superclass(superClass)
.addFields(adapters.values())
.addMethod(constructor.build())
.addMethod(createWriteMethod(autoValueTypeName, adapters))
.addMethod(createWriteMethod(autoValueTypeName, properties, adapters))
.addMethod(createReadMethod(className, autoValueTypeName, properties, adapters));

if (defaultSetters) {
Expand Down Expand Up @@ -474,7 +521,8 @@ public List<MethodSpec> createDefaultMethods(ClassName gsonTypeAdapterName, List
}

public MethodSpec createWriteMethod(TypeName autoValueClassName,
ImmutableMap<Property, FieldSpec> adapters) {
List<Property> properties,
ImmutableMap<TypeName, FieldSpec> adapters) {
ParameterSpec jsonWriter = ParameterSpec.builder(JsonWriter.class, "jsonWriter").build();
ParameterSpec annotatedParam = ParameterSpec.builder(autoValueClassName, "object").build();
MethodSpec.Builder writeMethod = MethodSpec.methodBuilder("write")
Expand All @@ -490,13 +538,11 @@ public MethodSpec createWriteMethod(TypeName autoValueClassName,
writeMethod.endControlFlow();

writeMethod.addStatement("$N.beginObject()", jsonWriter);
for (Map.Entry<Property, FieldSpec> entry : adapters.entrySet()) {
Property prop = entry.getKey();
for (Property prop : properties) {
if (!prop.shouldSerialize()) {
continue;
}
FieldSpec field = entry.getValue();

FieldSpec field = adapters.get(prop.type);
writeMethod.addStatement("$N.name($S)", jsonWriter, prop.serializedName());
writeMethod.addStatement("$N.write($N, $N.$N())", field, jsonWriter, annotatedParam, prop.methodName);
}
Expand All @@ -508,7 +554,7 @@ public MethodSpec createWriteMethod(TypeName autoValueClassName,
public MethodSpec createReadMethod(ClassName className,
TypeName autoValueClassName,
List<Property> properties,
ImmutableMap<Property, FieldSpec> adapters) {
ImmutableMap<TypeName, FieldSpec> adapters) {
ParameterSpec jsonReader = ParameterSpec.builder(JsonReader.class, "jsonReader").build();
MethodSpec.Builder readMethod = MethodSpec.methodBuilder("read")
.addAnnotation(Override.class)
Expand Down Expand Up @@ -568,7 +614,7 @@ public MethodSpec createReadMethod(ClassName className,
readMethod.addCode("case $S:\n", alternate);
}
readMethod.beginControlFlow("case $S:", prop.serializedName());
readMethod.addStatement("$N = $N.read($N)", field, adapters.get(prop), jsonReader);
readMethod.addStatement("$N = $N.read($N)", field, adapters.get(prop.type), jsonReader);
readMethod.addStatement("break");
readMethod.endControlFlow();
}
Expand Down
Loading

0 comments on commit a3d7096

Please sign in to comment.