Skip to content

Commit

Permalink
Generalize support for property builders.
Browse files Browse the repository at this point in the history 
Now, in addition to being able to say immutableListBuilder() for a property of type ImmutableList<T>, you can say fooBuilder() for a property of an arbitrary type that has a builder following certain conventions. In particular, you can do this if the type of foo() is itself an @autovalue class with a builder.

API discussion: https://docs.google.com/document/d/1F-mP_pgLVIfaqm2A24Kt-2bkDkAVBQv88vSzAHZ7NOw/edit# (starting on page 5).

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=140584023
  • Loading branch information
@eamonnmcmanus @ronshapiro
eamonnmcmanus authored and ronshapiro committed Dec 6, 2016
1 parent fa03168 commit c8c00a9
Show file tree
Hide file tree
Showing 9 changed files with 838 additions and 208 deletions.
148 changes: 148 additions & 0 deletions value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
Expand Down Expand Up @@ -2107,6 +2109,152 @@ public void testOneTwoThreeFour() {
.isEqualTo("OneTwoThreeFourImpl{one=one, two=two, three=false, four=4}");
}

@AutoValue
abstract static class OuterWithBuilder {
abstract String foo();
abstract InnerWithBuilder inner();
abstract Builder toBuilder();

static Builder builder() {
return new AutoValue_AutoValueTest_OuterWithBuilder.Builder();
}

@AutoValue.Builder
abstract static class Builder {
abstract Builder foo(String x);
abstract Builder inner(InnerWithBuilder x);
abstract InnerWithBuilder.Builder innerBuilder();

abstract OuterWithBuilder build();
}
}

@AutoValue
abstract static class InnerWithBuilder {
abstract int bar();
abstract Builder toBuilder();

static Builder builder() {
return new AutoValue_AutoValueTest_InnerWithBuilder.Builder();
}

@AutoValue.Builder
abstract static class Builder {
abstract Builder setBar(int x);

abstract InnerWithBuilder build();
}
}

public void testBuilderWithinBuilder() {
OuterWithBuilder x = OuterWithBuilder.builder()
.inner(InnerWithBuilder.builder()
.setBar(23)
.build())
.foo("yes")
.build();
assertThat(x.toString()).isEqualTo("OuterWithBuilder{foo=yes, inner=InnerWithBuilder{bar=23}}");

OuterWithBuilder.Builder xBuilder = x.toBuilder();
xBuilder.innerBuilder().setBar(17);
OuterWithBuilder y = xBuilder.build();
assertThat(y.toString()).isEqualTo("OuterWithBuilder{foo=yes, inner=InnerWithBuilder{bar=17}}");
}

public static class MyMap<K, V> extends HashMap<K, V> {
public MyMap() {}

public MyMap(Map<K, V> map) {
super(map);
}

public MyMapBuilder<K, V> toBuilder() {
return new MyMapBuilder<K, V>(this);
}
}

public static class MyMapBuilder<K, V> extends LinkedHashMap<K, V> {
public MyMapBuilder() {}

public MyMapBuilder(Map<K, V> map) {
super(map);
}

public MyMap<K, V> build() {
return new MyMap<K, V>(this);
}
}

@AutoValue
abstract static class BuildMyMap<K, V> {
abstract MyMap<K, V> map();

static <K, V> Builder<K, V> builder() {
return new AutoValue_AutoValueTest_BuildMyMap.Builder<K, V>();
}

@AutoValue.Builder
abstract static class Builder<K, V> {
abstract MyMapBuilder<K, V> mapBuilder();
abstract BuildMyMap<K, V> build();
}
}

public void testMyMapBuilder() {
BuildMyMap.Builder<String, Integer> builder = BuildMyMap.builder();
MyMapBuilder<String, Integer> mapBuilder = builder.mapBuilder();
mapBuilder.put("23", 23);
BuildMyMap<String, Integer> built = builder.build();
assertThat(built.map()).containsExactly("23", 23);
}

public static class MyStringMap<V> extends MyMap<String, V> {
public MyStringMap() {}

public MyStringMap(Map<String, V> map) {
super(map);
}

@Override public MyStringMapBuilder<V> toBuilder() {
return new MyStringMapBuilder<V>(this);
}
}

public static class MyStringMapBuilder<V> extends MyMapBuilder<String, V> {
public MyStringMapBuilder() {}

public MyStringMapBuilder(Map<String, V> map) {
super(map);
}

@Override public MyStringMap<V> build() {
return new MyStringMap<V>(this);
}
}

@AutoValue
abstract static class BuildMyStringMap<V> {
abstract MyStringMap<V> map();

static <V> Builder<V> builder() {
return new AutoValue_AutoValueTest_BuildMyStringMap.Builder<V>();
}

@AutoValue.Builder
abstract static class Builder<V> {
abstract MyStringMapBuilder<V> mapBuilder();
abstract BuildMyStringMap<V> build();
}
}

public void testMyStringMapBuilder() {
BuildMyStringMap.Builder<Integer> builder = BuildMyStringMap.builder();
MyStringMapBuilder<Integer> mapBuilder = builder.mapBuilder();
mapBuilder.put("23", 23);
BuildMyStringMap<Integer> built = builder.build();
assertThat(built.map()).containsExactly("23", 23);
}

static class VersionId {}

static class ItemVersionId extends VersionId {}
Expand Down
6 changes: 4 additions & 2 deletions value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static com.google.common.collect.Sets.union;

import com.google.auto.common.MoreElements;
import com.google.auto.common.MoreTypes;
import com.google.auto.service.AutoService;
import com.google.auto.value.AutoValue;
import com.google.auto.value.extension.AutoValueExtension;
Expand Down Expand Up @@ -740,6 +741,7 @@ private TypeSimplifier defineVarsForType(
ImmutableSet<ExecutableElement> toBuilderMethods,
ImmutableSet<ExecutableElement> propertyMethods,
Optional<BuilderSpec.Builder> builder) {
DeclaredType declaredType = MoreTypes.asDeclared(type.asType());
Set<TypeMirror> types = new TypeMirrorSet();
types.addAll(returnTypesOf(propertyMethods));
if (builder.isPresent()) {
Expand All @@ -762,7 +764,7 @@ private TypeSimplifier defineVarsForType(
allMethodExcludedAnnotations(propertyMethods);
types.addAll(allMethodAnnotationTypes(propertyMethods, excludedAnnotationsMap));
String pkg = TypeSimplifier.packageNameOf(type);
TypeSimplifier typeSimplifier = new TypeSimplifier(typeUtils, pkg, types, type.asType());
TypeSimplifier typeSimplifier = new TypeSimplifier(typeUtils, pkg, types, declaredType);
vars.imports = typeSimplifier.typesToImport();
vars.generated = generatedTypeElement == null
? ""
Expand All @@ -775,7 +777,7 @@ private TypeSimplifier defineVarsForType(
List<Property> props = new ArrayList<Property>();
EclipseHack eclipseHack = eclipseHack();
ImmutableMap<ExecutableElement, TypeMirror> returnTypes =
eclipseHack.methodReturnTypes(propertyMethods, type);
eclipseHack.methodReturnTypes(propertyMethods, declaredType);
for (ExecutableElement method : propertyMethods) {
TypeMirror returnType = returnTypes.get(method);
String propertyType = typeSimplifier.simplify(returnType);
Expand Down
8 changes: 5 additions & 3 deletions value/src/main/java/com/google/auto/value/processor/AutoValueTemplateVars.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.google.auto.value.processor;

import com.google.auto.value.processor.PropertyBuilderClassifier.PropertyBuilder;
import com.google.auto.value.processor.escapevelocity.Template;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -157,10 +158,11 @@ class AutoValueTemplateVars extends TemplateVars {
/**
* A map from property names to information about the associated property builder. A property
* called foo (defined by a method foo() or getFoo()) can have a property builder called
* fooBuilder(). The type of foo must be an immutable Guava type, like ImmutableSet, and
* fooBuilder() must return the corresponding builder, like ImmutableSet.Builder.
* fooBuilder(). The type of foo must be a type that has an associated builder following
* certain conventions. Guava immutable types such as ImmutableList follow those conventions,
* as do many {@code @AutoValue} types.
*/
ImmutableMap<String, BuilderSpec.PropertyBuilder> builderPropertyBuilders =
ImmutableMap<String, PropertyBuilder> builderPropertyBuilders =
ImmutableMap.of();

/**
Expand Down
67 changes: 21 additions & 46 deletions value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import com.google.auto.common.MoreElements;
import com.google.auto.common.MoreTypes;
import com.google.auto.value.processor.PropertyBuilderClassifier.PropertyBuilder;
import com.google.common.base.Equivalence;
import com.google.common.base.Optional;
import com.google.common.collect.ImmutableBiMap;
Expand All @@ -40,6 +41,7 @@
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.util.ElementFilter;
import javax.lang.model.util.Elements;
import javax.lang.model.util.Types;

/**
Expand All @@ -52,6 +54,7 @@ class BuilderMethodClassifier {

private final ErrorReporter errorReporter;
private final Types typeUtils;
private final Elements elementUtils;
private final TypeElement autoValueClass;
private final TypeElement builderType;
private final ImmutableBiMap<ExecutableElement, String> getterToPropertyName;
Expand All @@ -65,8 +68,10 @@ class BuilderMethodClassifier {
LinkedListMultimap.create();
private final Multimap<String, ExecutableElement> propertyNameToUnprefixedSetters =
LinkedListMultimap.create();
private final Map<String, ExecutableElement> propertyNameToPropertyBuilder =
private final Map<String, PropertyBuilder> propertyNameToPropertyBuilder =
Maps.newLinkedHashMap();
private final EclipseHack eclipseHack;

private boolean settersPrefixed;

private BuilderMethodClassifier(
Expand All @@ -78,6 +83,7 @@ private BuilderMethodClassifier(
TypeSimplifier typeSimplifier) {
this.errorReporter = errorReporter;
this.typeUtils = processingEnv.getTypeUtils();
this.elementUtils = processingEnv.getElementUtils();
this.autoValueClass = autoValueClass;
this.builderType = builderType;
this.getterToPropertyName = getterToPropertyName;
Expand All @@ -88,6 +94,7 @@ private BuilderMethodClassifier(
}
this.getterNameToGetter = getterToPropertyNameBuilder.build();
this.typeSimplifier = typeSimplifier;
this.eclipseHack = new EclipseHack(processingEnv);
}

/**
Expand Down Expand Up @@ -136,7 +143,7 @@ ImmutableMultimap<String, ExecutableElement> propertyNameToSetters() {
settersPrefixed ? propertyNameToPrefixedSetters : propertyNameToUnprefixedSetters);
}

Map<String, ExecutableElement> propertyNameToPropertyBuilder() {
Map<String, PropertyBuilder> propertyNameToPropertyBuilder() {
return propertyNameToPropertyBuilder;
}

Expand Down Expand Up @@ -240,7 +247,17 @@ private boolean classifyMethodNoArgs(ExecutableElement method) {
if (methodName.endsWith("Builder")) {
String property = methodName.substring(0, methodName.length() - "Builder".length());
if (getterToPropertyName.containsValue(property)) {
return classifyPropertyBuilder(method, property);
PropertyBuilderClassifier propertyBuilderClassifier = new PropertyBuilderClassifier(
errorReporter, typeUtils, elementUtils, this, getterToPropertyName, typeSimplifier,
eclipseHack);
Optional<PropertyBuilder> propertyBuilder =
propertyBuilderClassifier.makePropertyBuilder(method, property);
if (propertyBuilder.isPresent()) {
propertyNameToPropertyBuilder.put(property, propertyBuilder.get());
return true;
} else {
return false;
}
}
}

Expand Down Expand Up @@ -295,48 +312,6 @@ private boolean classifyGetter(
return false;
}

// Construct this string so it won't be found by Maven shading and renamed, which is not what
// we want.
private static final String COM_GOOGLE_COMMON_COLLECT_IMMUTABLE =
"com.".concat("google.common.collect.Immutable");

private boolean classifyPropertyBuilder(ExecutableElement method, String property) {
TypeMirror builderTypeMirror = builderMethodReturnType(method);
TypeElement builderTypeElement = MoreTypes.asTypeElement(builderTypeMirror);
String builderTypeString = builderTypeElement.getQualifiedName().toString();
boolean isGuavaBuilder = (builderTypeString.startsWith(COM_GOOGLE_COMMON_COLLECT_IMMUTABLE)
&& builderTypeString.endsWith(".Builder"));
if (!isGuavaBuilder) {
errorReporter.reportError("Method looks like a property builder, but its return type "
+ "is not a builder for an immutable type in com.google.common.collect", method);
return false;
}
// Given, e.g. ImmutableSet.Builder<String>, construct ImmutableSet<String> and check that
// it is indeed the type of the property.
DeclaredType builderTypeDeclared = MoreTypes.asDeclared(builderTypeMirror);
TypeMirror[] builderTypeArgs =
builderTypeDeclared.getTypeArguments().toArray(new TypeMirror[0]);
if (builderTypeArgs.length == 0) {
errorReporter.reportError("Property builder type cannot be raw (missing <...>)", method);
return false;
}
TypeElement enclosingTypeElement =
MoreElements.asType(builderTypeElement.getEnclosingElement());
TypeMirror expectedPropertyType =
typeUtils.getDeclaredType(enclosingTypeElement, builderTypeArgs);
TypeMirror actualPropertyType = getterToPropertyName.inverse().get(property).getReturnType();
if (!TYPE_EQUIVALENCE.equivalent(expectedPropertyType, actualPropertyType)) {
String error = String.format(
"Return type of property-builder method implies a property of type %s, but property "
+ "%s has type %s",
expectedPropertyType, property, actualPropertyType);
errorReporter.reportError(error, method);
return false;
}
propertyNameToPropertyBuilder.put(property, method);
return true;
}

/**
* Classifies a method given that it has one argument. Currently, a method with one argument can
* only be a setter, meaning that it must look like {@code foo(T)} or {@code setFoo(T)}, where
Expand Down Expand Up @@ -528,7 +503,7 @@ private ImmutableList<ExecutableElement> copyOfMethods(TypeMirror targetType) {
* {@code ParentBuilder} from an interface to an abstract class to make it work, but you'll often
* need to do that anyway.
*/
private TypeMirror builderMethodReturnType(ExecutableElement builderMethod) {
TypeMirror builderMethodReturnType(ExecutableElement builderMethod) {
DeclaredType builderTypeMirror = MoreTypes.asDeclared(builderType.asType());
TypeMirror methodMirror;
try {
Expand Down
Loading

0 comments on commit c8c00a9

Please sign in to comment.