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

Alternate staged builders that handle optional, etc. #180

Merged
merged 1 commit into from
Jun 8, 2024

Conversation

Randgalt
Copy link
Owner

@Randgalt Randgalt commented Apr 11, 2024

A variant of staged builders is available that only stages required record components. Any optional components (when addConcreteSettersForOptional is enabled) are not staged and are added to the final stage. Additionally, if Collection options are enabled, those too are added to the final stage.

Closes #170

attn: @olsavmic

Reviewers - please test various use cases. I'm not very familiar with this feature and did what I thought was right.

@Randgalt Randgalt added this to the v42 milestone Apr 11, 2024
@Randgalt Randgalt force-pushed the jordanz/optional-handling-for-staged-builders branch 4 times, most recently from 7b12bce to 5e372a2 Compare April 11, 2024 12:23
@olsavmic
Copy link

olsavmic commented Jun 4, 2024

This is perfect! I'm going to test it on our codebase and report possible issues in the next few days.

I don't expect any though, this is the design I envisioned, thank you!

@Randgalt
Copy link
Owner Author

Randgalt commented Jun 4, 2024

Let me know and I'll merge.

@olsavmic
Copy link

olsavmic commented Jun 7, 2024

There is a bug when immutableCollections are enabled together with STAGED_REQUIRED_ONLY option. The code does not compile, as the stage for the collection is detected as NOT REQUIRED yet the builder is not generated.

See #182 for a reproducer.

@olsavmic
Copy link

olsavmic commented Jun 7, 2024

Fix for the bug is in 9632f41.


The configurations around collections and nullability are getting pretty complicated :/
I'll test the different combinations, but I'll mainly try to come up with a description of the sane mutual configurations for these options

useImmutableCollections / useUnmodifiableCollections (changes behavior with defaults)
allowNullableCollections
interpretNotNulls
nullablePattern

@olsavmic
Copy link

olsavmic commented Jun 7, 2024

The changes in #182 make it work for us, but as I said above, the behaviour with nullability is now not uniform.

I'd consider merging this change and then discuss the following proposal:

"1st" layer:

  • Define whether fields are @Null by default or @NotNull by default - that information is then combined together with annotations, allowing them to override the defaults.

"2nd" layer:

  • interpretNotNulls is configuring just the runtime checks behavior

    • When a field is resolved as NotNull, a runtime check is generated (regardless of whether it's a collection or not)
  • allowNullableCollections

    • false -> Automatically set to empty collection
    • true -> may be NULL (but the runtime check still applies, meaning that NotNull field would throw if it's not set and interpretNotNulls is true)
  • Anytime field is resolved as NotNull, it is required in the STAGED_REQUIRED_ONLY

@Randgalt
Copy link
Owner Author

Randgalt commented Jun 8, 2024

@olsavmic I agree some of the options have gotten out of hand. Too many cooks and not enough quality. I'd like to deprecate a lot of it and re-work it.

A variant of staged builders is available that only stages required record components. Any optional components
(when `addConcreteSettersForOptional` is enabled) are not staged and are added to the final stage. Additionally, if Collection options are enabled, those too are
added to the final stage.

Closes #170
@Randgalt Randgalt force-pushed the jordanz/optional-handling-for-staged-builders branch from 5e372a2 to 919a567 Compare June 8, 2024 08:26
@Randgalt Randgalt merged commit 62011fe into master Jun 8, 2024
2 checks passed
@Randgalt Randgalt deleted the jordanz/optional-handling-for-staged-builders branch June 8, 2024 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow nullable arguments to be optional in StagedBuilders, allowing them to initialize during the final stage
2 participants