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: 483736037
  • Loading branch information
eamonnmcmanus authored and Google Java Core Libraries committed Oct 25, 2022
1 parent aeffb90 commit 3659a0e
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();

public abstract Builder bar(Bar bar);

// https://github.com/google/auto/blob/master/value/userguide/builders-howto.md#nested_builders
public abstract Bar.Builder barBuilder();

abstract OptionalDouble baz();

public abstract Builder baz(double baz);

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

1 comment on commit 3659a0e

@jmesny
Copy link

@jmesny jmesny commented on 3659a0e Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, and thanks for the patch.
Would it be possible to have it released in a new version?

Please sign in to comment.