Skip to content

Commit 3659a0e

Browse files
eamonnmcmanusGoogle Java Core Libraries
authored andcommitted
Fix an issue with builder getters.
In a builder, if a property `bar` has its own builder `barBuilder()` and also a getter `bar()`, then either the property must be set or `barBuilder()` must be called. If neither of those has happened, both the `bar` and `barBuilder$` fields will be null, and we should correctly report that with an empty `Optional` return from `bar()`. Previously we incorrectly simplified this logic and ended up calling `Optional.of(bar)` even though `bar` is null. Fixes #1386. Thanks to @jmesny for the bug report and regression test. RELNOTES=Fixed an issue when a builder has a property `foo` with both a getter `foo()` and a builder `fooBuilder()`. PiperOrigin-RevId: 483736037
1 parent aeffb90 commit 3659a0e

File tree

2 files changed

+61
-1
lines changed

2 files changed

+61
-1
lines changed

value/src/it/functional/src/test/java/com/google/auto/value/AutoValueJava8Test.java

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import java.util.Arrays;
4343
import java.util.List;
4444
import java.util.Optional;
45+
import java.util.OptionalDouble;
4546
import java.util.Set;
4647
import java.util.function.Predicate;
4748
import javax.annotation.processing.AbstractProcessor;
@@ -862,4 +863,63 @@ public void testOptionalExtends() {
862863
OptionalExtends t = OptionalExtends.builder().setPredicate(predicate).build();
863864
assertThat(t.predicate()).hasValue(predicate);
864865
}
866+
867+
@AutoValue
868+
public abstract static class Foo {
869+
public abstract Bar bar();
870+
871+
public abstract double baz();
872+
873+
public static Foo.Builder builder() {
874+
return new AutoValue_AutoValueJava8Test_Foo.Builder();
875+
}
876+
877+
@AutoValue.Builder
878+
public abstract static class Builder {
879+
// https://github.com/google/auto/blob/master/value/userguide/builders-howto.md#normalize
880+
abstract Optional<Bar> bar();
881+
882+
public abstract Builder bar(Bar bar);
883+
884+
// https://github.com/google/auto/blob/master/value/userguide/builders-howto.md#nested_builders
885+
public abstract Bar.Builder barBuilder();
886+
887+
abstract OptionalDouble baz();
888+
889+
public abstract Builder baz(double baz);
890+
891+
abstract Foo autoBuild();
892+
893+
public Foo build() {
894+
if (!bar().isPresent()) {
895+
bar(Bar.builder().build());
896+
}
897+
if (!baz().isPresent()) {
898+
baz(0.0);
899+
}
900+
return autoBuild();
901+
}
902+
}
903+
}
904+
905+
@AutoValue
906+
public abstract static class Bar {
907+
public abstract Bar.Builder toBuilder();
908+
909+
public static Bar.Builder builder() {
910+
return new AutoValue_AutoValueJava8Test_Bar.Builder();
911+
}
912+
913+
@AutoValue.Builder
914+
public abstract static class Builder {
915+
public abstract Bar build();
916+
}
917+
}
918+
919+
@Test
920+
public void nestedOptionalGetter() {
921+
Foo foo = Foo.builder().build();
922+
assertThat(foo.bar()).isNotNull();
923+
assertThat(foo.baz()).isEqualTo(0.0);
924+
}
865925
}

value/src/main/java/com/google/auto/value/processor/builder.vm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ class ${builderName}${builderFormalTypes} ##
192192
if ($noValueToGetCondition) {
193193
return $builderGetter.optional.empty;
194194
}
195-
#elseif ($p.nullable)
195+
#else
196196
if ($p == null) {
197197
return $builderGetter.optional.empty;
198198
}

0 commit comments

Comments
 (0)