diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java b/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java index 333e6ec3c9..4fe9cd596b 100644 --- a/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java +++ b/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java @@ -1851,6 +1851,91 @@ public void testBuilderWithPropertyBuilders() { } } + @AutoValue + public abstract static class BuilderWithOptionalPropertyBuilders { + public abstract com.google.common.base.Optional getFoo(); + + public abstract BuilderWithOptionalPropertyBuilders.Builder toBuilder(); + + public static Builder builder() { + return new AutoValue_AutoValueTest_BuilderWithOptionalPropertyBuilders.Builder(); + } + + @AutoValue.Builder + public abstract static class Builder { + public abstract com.google.common.base.Optional getFoo(); + + public abstract Builder foo(com.google.common.base.Optional foo); + + public abstract BuilderWithSetAndGet.Builder fooBuilder(); + + public abstract BuilderWithOptionalPropertyBuilders build(); + } + } + + @Test + public void testBuilderWithOptionalPropertyBuilders() { + BuilderWithOptionalPropertyBuilders a = + BuilderWithOptionalPropertyBuilders.builder() + .build(); + + assertThat(a.getFoo()).isAbsent(); + + BuilderWithSetAndGet foo = BuilderWithSetAndGet.builder() + .setAnInt(0) + .setAList(ImmutableList.of(1)) + .build(); + BuilderWithOptionalPropertyBuilders b = + BuilderWithOptionalPropertyBuilders.builder() + .foo(com.google.common.base.Optional.of(foo)) + .build(); + + assertThat(b.getFoo()).hasValue(foo); + + BuilderWithOptionalPropertyBuilders c = + BuilderWithOptionalPropertyBuilders.builder() + .foo(com.google.common.base.Optional.absent()) + .build(); + + assertThat(c.getFoo()).isAbsent(); + + BuilderWithOptionalPropertyBuilders.Builder buildD = + BuilderWithOptionalPropertyBuilders.builder() + .foo(com.google.common.base.Optional.of(foo)); + + // get fooBuilder to trigger toBuilder logic + buildD.fooBuilder(); + BuilderWithOptionalPropertyBuilders d = buildD.build(); + + assertThat(d.getFoo()).hasValue(foo); + + BuilderWithOptionalPropertyBuilders.Builder buildE = + BuilderWithOptionalPropertyBuilders.builder() + .foo(com.google.common.base.Optional.of(foo)); + + buildE.fooBuilder().setAnInt(1); + BuilderWithOptionalPropertyBuilders e = buildE.build(); + + assertThat(e.getFoo()).hasValue(foo.toBuilder().setAnInt(1).build()); + + BuilderWithOptionalPropertyBuilders.Builder buildF = + BuilderWithOptionalPropertyBuilders.builder(); + + buildF.fooBuilder().setAList(ImmutableList.of(1)).setAnInt(0); + BuilderWithOptionalPropertyBuilders f = buildF.build(); + + assertThat(f.getFoo()).hasValue(foo); + + try { + BuilderWithOptionalPropertyBuilders.builder().foo(null).build(); + fail("Did not get expected exception"); + } catch (RuntimeException expected) { + // We don't specify whether you get the exception on foo(null) or on + // build(), nor + // which exception it is exactly. + } + } + @AutoValue public abstract static class BuilderWithExoticPropertyBuilders> { diff --git a/value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java index 4709b74981..8d4cf2aed9 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java @@ -349,6 +349,29 @@ public boolean isNullable() { return !getNullableAnnotation().isEmpty(); } + /** + * Returns an expression to check if the property is able to be retrieved as a non-null value + * using {@link #getValueRetrieval()}. + */ + public String getPresenseCheck() { + if (optional == null) { + return identifier + " != null"; + } else { + return identifier + ".isPresent()"; + } + } + + /** + * Returns an expression to get the value of the property, unwrapping it if it is an Optional. + */ + public String getValueRetrieval() { + if (optional == null) { + return identifier; + } else { + return identifier + ".get()"; + } + } + public String getAccess() { return access(method); } diff --git a/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java b/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java index 3c472feba0..00651fb519 100644 --- a/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java +++ b/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java @@ -197,7 +197,16 @@ private boolean classifyMethods( } for (Map.Entry getterEntry : getterToPropertyName.entrySet()) { String property = getterEntry.getValue(); - String propertyType = typeSimplifier.simplify(getterEntry.getKey().getReturnType()); + + TypeMirror propertyTypeMirror = getterEntry.getKey().getReturnType(); + // Handle `Optional` by replacing the type mirror with the contained type + Optionalish propertyOptionalish = Optionalish.createIfOptional(propertyTypeMirror, + typeSimplifier.simplifyRaw(propertyTypeMirror)); + if (propertyOptionalish != null) { + propertyTypeMirror = propertyOptionalish.getContainedType(typeUtils); + } + + String propertyType = typeSimplifier.simplify(propertyTypeMirror); boolean hasSetter = propertyNameToSetter.containsKey(property); PropertyBuilder propertyBuilder = propertyNameToPropertyBuilder.get(property); boolean hasBuilder = propertyBuilder != null; diff --git a/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java b/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java index 7b678a6f5b..a07463702b 100644 --- a/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java +++ b/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java @@ -87,6 +87,7 @@ public static class PropertyBuilder { private final String initDefault; private final String builtToBuilder; private final String copyAll; + private final Optionalish optional; PropertyBuilder( ExecutableElement propertyBuilderMethod, @@ -95,7 +96,8 @@ public static class PropertyBuilder { String beforeInitDefault, String initDefault, String builtToBuilder, - String copyAll) { + String copyAll, + Optionalish optional) { this.propertyBuilderMethod = propertyBuilderMethod; this.name = propertyBuilderMethod.getSimpleName() + "$"; this.builderType = builderType; @@ -104,6 +106,7 @@ public static class PropertyBuilder { this.initDefault = initDefault; this.builtToBuilder = builtToBuilder; this.copyAll = copyAll; + this.optional = optional; } /** The property builder method, for example {@code barBuilder()}. */ @@ -167,6 +170,24 @@ public String getBuiltToBuilder() { public String getCopyAll() { return copyAll; } + + /** + * The {@link Optionalish} that was created for the property, if applicable. + */ + public Optionalish getOptional() { + return optional; + } + + /** + * An expression to build the property, wrapping it using Optionalish if needed. + */ + public String getBuild() { + if (optional == null) { + return name + ".build()"; + } else { + return optional.getRawType() + ".of(" + name + ".build())"; + } + } } // Construct this string so it won't be found by Maven shading and renamed, which is not what @@ -181,6 +202,8 @@ public String getCopyAll() { // `BarBuilder` type are: // (1) It must have an instance method called `build()` that returns `Bar`. If the type of // `bar()` is `Bar` then the type of `build()` must be `Bar`. + // The property may be `Optional bar()`, in which case the type of `build()` must + // still be `Bar`, and it still cannot return `null`. // (2) `BarBuilder` must have a public no-arg constructor, or `Bar` must have a static method // `builder()` or `newBuilder()` that returns `BarBuilder`. // (3) `Bar` must have an instance method `BarBuilder toBuilder()`, or `BarBuilder` must be a @@ -202,6 +225,14 @@ Optional makePropertyBuilder(ExecutableElement method, String p ExecutableElement barGetter = getterToPropertyName.inverse().get(property); TypeMirror barTypeMirror = barGetter.getReturnType(); + + // Handle `Optional` by replacing the type mirror with the contained type + Optionalish barOptionalish = + Optionalish.createIfOptional(barTypeMirror, typeSimplifier.simplifyRaw(barTypeMirror)); + if (barOptionalish != null) { + barTypeMirror = barOptionalish.getContainedType(typeUtils); + } + if (barTypeMirror.getKind() != TypeKind.DECLARED) { errorReporter.reportError("Method looks like a property builder, but the type of property " + property + " is not a class or interface", method); @@ -292,7 +323,8 @@ Optional makePropertyBuilder(ExecutableElement method, String p beforeInitDefault, initDefault, builtToBuilder, - copyAll); + copyAll, + barOptionalish); return Optional.of(propertyBuilder); } diff --git a/value/src/main/java/com/google/auto/value/processor/autovalue.vm b/value/src/main/java/com/google/auto/value/processor/autovalue.vm index 1a5ed34b75..892180c2b3 100644 --- a/value/src/main/java/com/google/auto/value/processor/autovalue.vm +++ b/value/src/main/java/com/google/auto/value/processor/autovalue.vm @@ -289,13 +289,10 @@ $a #else - if ($p == null) { - ${propertyBuilder.name} = ${propertyBuilder.initializer}; - } else { - + if (${p.presenseCheck}) { #if (${propertyBuilder.builtToBuilder}) - ${propertyBuilder.name} = ${p}.${propertyBuilder.builtToBuilder}(); + ${propertyBuilder.name} = ${p.valueRetrieval}.${propertyBuilder.builtToBuilder}(); #else @@ -305,6 +302,8 @@ $a #end $p = null; + } else { + ${propertyBuilder.name} = ${propertyBuilder.initializer}; } #end @@ -341,13 +340,16 @@ $a #if ($propertyBuilder) if (${propertyBuilder.name} != null) { - return ${propertyBuilder.name}.build(); + return ${propertyBuilder.build}; } + #if (!${p.optional}) + if ($p == null) { ${propertyBuilder.beforeInitDefault} $p = ${propertyBuilder.initDefault}; } + #end #end return $p; @@ -367,11 +369,11 @@ $a #if ($propertyBuilder) if (${propertyBuilder.name} != null) { - this.$p = ${propertyBuilder.name}.build(); - } else if (this.$p == null) { + this.$p = ${propertyBuilder.build}; + } #if (!${p.optional}) else if (this.$p == null) { ${propertyBuilder.beforeInitDefault} this.$p = ${propertyBuilder.initDefault}; - } + } #end #end #end diff --git a/value/src/test/java/com/google/auto/value/processor/CompilationTest.java b/value/src/test/java/com/google/auto/value/processor/CompilationTest.java index 5392a6597f..054c3ce21e 100644 --- a/value/src/test/java/com/google/auto/value/processor/CompilationTest.java +++ b/value/src/test/java/com/google/auto/value/processor/CompilationTest.java @@ -602,6 +602,7 @@ public void correctBuilder() throws Exception { " public abstract ImmutableList anImmutableList();", " public abstract Optional anOptionalString();", " public abstract NestedAutoValue aNestedAutoValue();", + " public abstract Optional> anOptionalNestedAutoValue();", "", " public abstract Builder toBuilder();", "", @@ -616,6 +617,7 @@ public void correctBuilder() throws Exception { " public abstract Builder anOptionalString(Optional s);", " public abstract Builder anOptionalString(String s);", " public abstract NestedAutoValue.Builder aNestedAutoValueBuilder();", + " public abstract NestedAutoValue.Builder anOptionalNestedAutoValueBuilder();", "", " public Builder aList(ArrayList x) {", // ArrayList should not be imported in the generated class. @@ -675,6 +677,7 @@ public void correctBuilder() throws Exception { " private final ImmutableList anImmutableList;", " private final Optional anOptionalString;", " private final NestedAutoValue aNestedAutoValue;", + " private final Optional> anOptionalNestedAutoValue;", "", " private AutoValue_Baz(", " int anInt,", @@ -683,7 +686,8 @@ public void correctBuilder() throws Exception { " List aList,", " ImmutableList anImmutableList,", " Optional anOptionalString,", - " NestedAutoValue aNestedAutoValue) {", + " NestedAutoValue aNestedAutoValue,", + " Optional> anOptionalNestedAutoValue) {", " this.anInt = anInt;", " this.aByteArray = aByteArray;", " this.aNullableIntArray = aNullableIntArray;", @@ -691,6 +695,7 @@ public void correctBuilder() throws Exception { " this.anImmutableList = anImmutableList;", " this.anOptionalString = anOptionalString;", " this.aNestedAutoValue = aNestedAutoValue;", + " this.anOptionalNestedAutoValue = anOptionalNestedAutoValue;", " }", "", " @Override public int anInt() {", @@ -724,6 +729,10 @@ public void correctBuilder() throws Exception { " return aNestedAutoValue;", " }", "", + " @Override public Optional> anOptionalNestedAutoValue() {", + " return anOptionalNestedAutoValue;", + " }", + "", " @Override public String toString() {", " return \"Baz{\"", " + \"anInt=\" + anInt + \", \"", @@ -732,7 +741,8 @@ public void correctBuilder() throws Exception { " + \"aList=\" + aList + \", \"", " + \"anImmutableList=\" + anImmutableList + \", \"", " + \"anOptionalString=\" + anOptionalString + \", \"", - " + \"aNestedAutoValue=\" + aNestedAutoValue", + " + \"aNestedAutoValue=\" + aNestedAutoValue + \", \"", + " + \"anOptionalNestedAutoValue=\" + anOptionalNestedAutoValue", " + \"}\";", " }", "", @@ -752,7 +762,8 @@ public void correctBuilder() throws Exception { " && (this.aList.equals(that.aList()))", " && (this.anImmutableList.equals(that.anImmutableList()))", " && (this.anOptionalString.equals(that.anOptionalString()))", - " && (this.aNestedAutoValue.equals(that.aNestedAutoValue()));", + " && (this.aNestedAutoValue.equals(that.aNestedAutoValue()))", + " && (this.anOptionalNestedAutoValue.equals(that.anOptionalNestedAutoValue()));", " }", " return false;", " }", @@ -773,6 +784,8 @@ public void correctBuilder() throws Exception { " h ^= this.anOptionalString.hashCode();", " h *= 1000003;", " h ^= this.aNestedAutoValue.hashCode();", + " h *= 1000003;", + " h ^= this.anOptionalNestedAutoValue.hashCode();", " return h;", " }", "", @@ -790,6 +803,8 @@ public void correctBuilder() throws Exception { " private Optional anOptionalString = Optional.absent();", " private NestedAutoValue.Builder aNestedAutoValueBuilder$;", " private NestedAutoValue aNestedAutoValue;", + " private NestedAutoValue.Builder anOptionalNestedAutoValueBuilder$;", + " private Optional> anOptionalNestedAutoValue = Optional.absent();", "", " Builder() {", " }", @@ -802,6 +817,7 @@ public void correctBuilder() throws Exception { " this.anImmutableList = source.anImmutableList();", " this.anOptionalString = source.anOptionalString();", " this.aNestedAutoValue = source.aNestedAutoValue();", + " this.anOptionalNestedAutoValue = source.anOptionalNestedAutoValue();", " }", "", " @Override", @@ -867,12 +883,12 @@ public void correctBuilder() throws Exception { " @Override", " public ImmutableList.Builder anImmutableListBuilder() {", " if (anImmutableListBuilder$ == null) {", - " if (anImmutableList == null) {", - " anImmutableListBuilder$ = ImmutableList.builder();", - " } else {", + " if (anImmutableList != null) {", " anImmutableListBuilder$ = ImmutableList.builder();", " anImmutableListBuilder$.addAll(anImmutableList);", " anImmutableList = null;", + " } else {", + " anImmutableListBuilder$ = ImmutableList.builder();", " }", " }", " return anImmutableListBuilder$;", @@ -910,17 +926,30 @@ public void correctBuilder() throws Exception { " @Override", " public NestedAutoValue.Builder aNestedAutoValueBuilder() {", " if (aNestedAutoValueBuilder$ == null) {", - " if (aNestedAutoValue == null) {", - " aNestedAutoValueBuilder$ = NestedAutoValue.builder();", - " } else {", + " if (aNestedAutoValue != null) {", " aNestedAutoValueBuilder$ = aNestedAutoValue.toBuilder();", " aNestedAutoValue = null;", + " } else {", + " aNestedAutoValueBuilder$ = NestedAutoValue.builder();", " }", " }", " return aNestedAutoValueBuilder$;", " }", "", " @Override", + " public NestedAutoValue.Builder anOptionalNestedAutoValueBuilder() {", + " if (anOptionalNestedAutoValueBuilder$ == null) {", + " if (anOptionalNestedAutoValue.isPresent()) {", + " anOptionalNestedAutoValueBuilder$ = anOptionalNestedAutoValue.get().toBuilder();", + " anOptionalNestedAutoValue = null;", + " } else {", + " anOptionalNestedAutoValueBuilder$ = NestedAutoValue.builder();", + " }", + " }", + " return anOptionalNestedAutoValueBuilder$;", + " }", + "", + " @Override", " public Baz build() {", " if (anImmutableListBuilder$ != null) {", " this.anImmutableList = anImmutableListBuilder$.build();", @@ -933,6 +962,9 @@ public void correctBuilder() throws Exception { " NestedAutoValue.Builder aNestedAutoValue$builder = NestedAutoValue.builder();", " this.aNestedAutoValue = aNestedAutoValue$builder.build();", " }", + " if (anOptionalNestedAutoValueBuilder$ != null) {", + " this.anOptionalNestedAutoValue = Optional.of(anOptionalNestedAutoValueBuilder$.build());", + " }", " String missing = \"\";", " if (this.anInt == null) {", " missing += \" anInt\";", @@ -953,7 +985,8 @@ public void correctBuilder() throws Exception { " this.aList,", " this.anImmutableList,", " this.anOptionalString,", - " this.aNestedAutoValue);", + " this.aNestedAutoValue,", + " this.anOptionalNestedAutoValue);", " }", " }", "}");