From 00dbd4534ffaaa79afac0603c69ce64275fea395 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Mon, 24 Oct 2022 14:07:32 -0700 Subject: [PATCH] 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 https://github.com/google/auto/issues/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: 483480779 --- .../google/auto/value/AutoValueJava8Test.java | 60 +++++++++++++++++++ .../google/auto/value/processor/builder.vm | 2 +- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueJava8Test.java b/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueJava8Test.java index 4a4f1b3ff7..80086841aa 100644 --- a/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueJava8Test.java +++ b/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueJava8Test.java @@ -42,6 +42,7 @@ import java.util.Arrays; import java.util.List; import java.util.Optional; +import java.util.OptionalDouble; import java.util.Set; import java.util.function.Predicate; import javax.annotation.processing.AbstractProcessor; @@ -862,4 +863,63 @@ public void testOptionalExtends() { OptionalExtends t = OptionalExtends.builder().setPredicate(predicate).build(); assertThat(t.predicate()).hasValue(predicate); } + + @AutoValue + public abstract static class Foo { + public abstract Bar bar(); + + public abstract double baz(); + + public static Foo.Builder builder() { + return new AutoValue_AutoValueJava8Test_Foo.Builder(); + } + + @AutoValue.Builder + public abstract static class Builder { + // https://github.com/google/auto/blob/master/value/userguide/builders-howto.md#normalize + abstract Optional bar(); + + abstract OptionalDouble baz(); + + public abstract Builder bar(Bar bar); + + public abstract Builder baz(double baz); + + // https://github.com/google/auto/blob/master/value/userguide/builders-howto.md#nested_builders + public abstract Bar.Builder barBuilder(); + + abstract Foo autoBuild(); + + public Foo build() { + if (bar().isEmpty()) { + bar(Bar.builder().build()); + } + if (baz().isEmpty()) { + baz(0.0); + } + return autoBuild(); + } + } + } + + @AutoValue + public abstract static class Bar { + public abstract Bar.Builder toBuilder(); + + public static Bar.Builder builder() { + return new AutoValue_AutoValueJava8Test_Bar.Builder(); + } + + @AutoValue.Builder + public abstract static class Builder { + public abstract Bar build(); + } + } + + @Test + public void nestedOptionalGetter() { + Foo foo = Foo.builder().build(); + assertThat(foo.bar()).isNotNull(); + assertThat(foo.baz()).isEqualTo(0.0); + } } diff --git a/value/src/main/java/com/google/auto/value/processor/builder.vm b/value/src/main/java/com/google/auto/value/processor/builder.vm index 5c4f4d5dbd..d032cda98f 100644 --- a/value/src/main/java/com/google/auto/value/processor/builder.vm +++ b/value/src/main/java/com/google/auto/value/processor/builder.vm @@ -192,7 +192,7 @@ class ${builderName}${builderFormalTypes} ## if ($noValueToGetCondition) { return $builderGetter.optional.empty; } - #elseif ($p.nullable) + #else if ($p == null) { return $builderGetter.optional.empty; }