Skip to content

Commit

Permalink
Fix an issue with builder getters.
Browse files Browse the repository at this point in the history
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: 483480779
  • Loading branch information
eamonnmcmanus authored and Google Java Core Libraries committed Oct 24, 2022
1 parent aeffb90 commit 259e2a0
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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> 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().isPresent()) {
bar(Bar.builder().build());
}
if (!baz().isPresent()) {
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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ class ${builderName}${builderFormalTypes} ##
if ($noValueToGetCondition) {
return $builderGetter.optional.empty;
}
#elseif ($p.nullable)
#else
if ($p == null) {
return $builderGetter.optional.empty;
}
Expand Down

0 comments on commit 259e2a0

Please sign in to comment.