Add support for Nullable on Optional property builders #353
Conversation
octylFractal
force-pushed
the
feature/optional-from-nullable
branch
from
5fa891f
to
3631f76
Compare
|
Any plans on merging this in the near future? |
|
I'm reworking other aspects of builders at the moment but I will look into this when I am done. |
octylFractal
force-pushed
the
feature/optional-from-nullable
branch
from
3631f76
to
0d09533
Compare
|
I've just rebased onto the latest |
octylFractal
force-pushed
the
feature/optional-from-nullable
branch
4 times, most recently
from
d19bb33
to
6a9bfe5
Compare
|
It's been updated again to handle the latest changes. |
|
It looks as if there's still a test failure. I'm hoping to cut AutoValue 1.4 soon, but I will look at this and #444 once that is done. Thanks for putting in the work to put them together! |
|
Meanwhile, can I ask that you add test code to AutoValueTest in this PR and the other one? |
|
Let me know if you would like any other test cases for this PR. |
octylFractal
force-pushed
the
feature/optional-from-nullable
branch
from
00e3251
to
365b1c7
Compare
|
Is it planned to merge this PR in the near future? |
|
Any activity happening on this? Has this been superseded by something else? |
|
I'm surprised this hasn't been merged yet. What is the delay (apart from the conflicts which keep getting fixed)? |
|
I'd also love to see this feature get merged. It's really my only complaint with AutoValue -- right now the workaround (creating two versions of every builder method where you want to allow passing null) is very tedious. |
octylFractal
force-pushed
the
feature/optional-from-nullable
branch
from
365b1c7
to
824568f
Compare
|
I've just fixed up this PR to match |
octylFractal
force-pushed
the
feature/optional-from-nullable
branch
from
677a905
to
99438ae
Compare
|
Sorry to have dropped the ball on this. I've put together a modified version of this change and am sending it out for internal review. Thanks, @kenzierocks! |
…th a @nullable parameter. This is based on #353 by Kenzie Togami. Closes #353. RELNOTES: Allow an Optional property to be set in a builder through a method with a @nullable parameter. NOTE: As a side-effect of this change, a setter for a @nullable property must have its parameter also marked @nullable. Previously, if the @nullable was not a TYPE_USE @nullable, it was copied from the getter to the parameter of the generated setter implementation. For TYPE_USE @nullable it was already necessary to mark the setter parameter @nullable. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=195471227
…th a @nullable parameter. This is based on #353 by Kenzie Togami. Closes #353. RELNOTES: Allow an Optional property to be set in a builder through a method with a @nullable parameter. NOTE: As a side-effect of this change, a setter for a @nullable property must have its parameter also marked @nullable. Previously, if the @nullable was not a TYPE_USE @nullable, it was copied from the getter to the parameter of the generated setter implementation. For TYPE_USE @nullable it was already necessary to mark the setter parameter @nullable. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=195471227
…th a @nullable parameter. This is based on google/auto#353 by Kenzie Togami. Closes google/auto#353. RELNOTES: Allow an Optional property to be set in a builder through a method with a @nullable parameter. NOTE: As a side-effect of this change, a setter for a @nullable property must have its parameter also marked @nullable. Previously, if the @nullable was not a TYPE_USE @nullable, it was copied from the getter to the parameter of the generated setter implementation. For TYPE_USE @nullable it was already necessary to mark the setter parameter @nullable. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=195471227
This allows an
@Nullableannotation to exist on aBuilderproperty setter for anOptionalproperty, like so:Builder bar(@Nullable String bar);. It usesofNullable/fromNullableto convert the value to the correctOptionalinstance.This PR also replaces the logic in the(Edit: This doesn't work withBuilderproperty getters to useofNullable/fromNullableinstead of anif ($p == null)check.Optionalprimitives + wrapper classes.)This solves my request here: #278 (comment), and #384.