Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AutoValue] NPE since v1.10 when using builder getters and nested builders #1386

Closed
jmesny opened this issue Oct 24, 2022 · 1 comment · Fixed by #1387
Closed

[AutoValue] NPE since v1.10 when using builder getters and nested builders #1386

jmesny opened this issue Oct 24, 2022 · 1 comment · Fixed by #1387
Assignees

Comments

@jmesny
Copy link

jmesny commented Oct 24, 2022

Hi,

Since version 1.10 it is no longer possible to use builder getters and nested builders at the same time.

Here is an example describing the issue.

When the nested builder from Bar is exposed, the generated bar() builder getter in Foo does not work as expected.
The reason is that the generated implementation does not handle the case where the bar value would be missing, as stated by the Optional return type.
Calling this bar() method would lead to a NullPointerException being thrown.

The code below works with version 1.9.

import com.google.auto.value.AutoValue;

import java.util.Optional;

public class Issue {

    @AutoValue
    public static abstract class Foo {
        public abstract Bar bar();

        public static Foo.Builder builder() {
            return new AutoValue_Issue_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();

            // Generated code for the above `bar()` method:

            // v1.9
            //    @Override
            //    Optional<Bar> bar() {
            //      if (bar == null) {
            //        return Optional.empty();
            //      } else {
            //        return Optional.of(bar);
            //      }
            //    }

            // v1.10
            //    @Override
            //    Optional<Bar> bar() {
            //      return Optional.of(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 Foo autoBuild();

            public Foo build() {
                if (bar().isEmpty()) {
                    // ^ NullPointerException when calling `bar()` with v1.10
                }
                return autoBuild();
            }
        }
    }

    @AutoValue
    public static abstract class Bar {
        public abstract Bar.Builder toBuilder();

        public static Bar.Builder builder() {
            return new AutoValue_Issue_Bar.Builder();
        }

        @AutoValue.Builder
        public abstract static class Builder {
            public abstract Bar build();
        }
    }


    public static void main(String[] args) {
        Foo.builder()
                .build();
    }
}

Thank you!

@eamonnmcmanus
Copy link
Member

Thanks, we will investigate.

copybara-service bot pushed a commit that referenced this issue Oct 24, 2022
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
copybara-service bot pushed a commit that referenced this issue Oct 24, 2022
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
copybara-service bot pushed a commit that referenced this issue Oct 25, 2022
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants