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

Ctor null checks omitted when builder is present, but ctor still package-visible. #345

Closed
JakeWharton opened this issue Jun 13, 2016 · 2 comments
Assignees

Comments

@JakeWharton
Copy link
Contributor

Given a model

@AutoValue
public abstract class Whatever {
  public abstract String whatever();

  @AutoValue.Builder
  interface Builder {
    Builder whatever(String whatever);
    Whatever build();
  }
}

AutoValue 1.3-rc1 will generate:

@Generated("com.google.auto.value.processor.AutoValueProcessor")
 abstract class $AutoValue_Whatever extends Whatever {

  private final String whatever;

  $AutoValue_Whatever(
      String whatever) {
    this.whatever = whatever;
  }

  // omitted

  static final class Builder implements Whatever.Builder {
    private String whatever;
    Builder() {
    }
    Builder(Whatever source) {
      this.whatever = source.whatever();
    }
    @Override
    public Whatever.Builder whatever(String whatever) {
      this.whatever = whatever;
      return this;
    }
    @Override
    public Whatever build() {
      String missing = "";
      if (whatever == null) {
        missing += " whatever";
      }
      if (!missing.isEmpty()) {
        throw new IllegalStateException("Missing required properties:" + missing);
      }
      return new AutoValue_Whatever(
          this.whatever);
    }
  }

}

As you can see the constructor is still package scoped which the application code could use to insert a null value for whatever. This constructor is also used by extensions which are not null-checking their values since they rely on the constructor to validate the nullability constraints.

@eamonnmcmanus eamonnmcmanus self-assigned this Jun 13, 2016
@eamonnmcmanus
Copy link
Member

Crud, yes, in the absence of extensions the constructor is private so omitting the null checks is valid, but if there is an extension then the constructor has to be package-private so that the extension-generated subclass can call it. I think that means we have to put the null checks back, at least for that case.

eamonnmcmanus added a commit to eamonnmcmanus/auto that referenced this issue Jun 15, 2016
…ve null checks if there are extensions, because in that case the constructor is package-private rather than private. Fixes google#345.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=124847338
@JakeWharton
Copy link
Contributor Author

Thanks!

On Wed, Jun 15, 2016 at 8:17 PM Éamonn McManus notifications@github.com
wrote:

Closed #345 #345 via #346
#346.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#345 (comment), or mute the
thread
https://github.com/notifications/unsubscribe/AAEEEV7U_dobDBb4xLvteJ_Kc_DDohUwks5qMJYRgaJpZM4I0tbM
.

eamonnmcmanus added a commit to eamonnmcmanus/auto that referenced this issue Aug 30, 2016
…ve null checks if there are extensions, because in that case the constructor is package-private rather than private. Fixes google#345.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=124847338
bestreviewsbookssoftware12 added a commit to bestreviewsbookssoftware12/auto that referenced this issue Aug 24, 2024
…ve null checks if there are extensions, because in that case the constructor is package-private rather than private. Fixes google/auto#345.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=124847338
bestreviewsbookssoftware12 added a commit to bestreviewsbookssoftware12/auto that referenced this issue Aug 24, 2024
…ve null checks if there are extensions, because in that case the constructor is package-private rather than private. Fixes google/auto#345.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=124847338
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants