Randgalt / record-builder

Record builder generator for Java records
Apache License 2.0
723 stars 51 forks source link

Alternate staged builders that handle optional, etc. #180

Closed Randgalt closed 3 months ago

Randgalt commented 5 months ago

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.

olsavmic commented 3 months ago

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 commented 3 months ago

Let me know and I'll merge.

olsavmic commented 3 months ago

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 https://github.com/Randgalt/record-builder/pull/182 for a reproducer.

olsavmic commented 3 months ago

Fix for the bug is in https://github.com/Randgalt/record-builder/pull/182/commits/9632f4133b36431db5b5fa217426b0448a4fca60.


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 commented 3 months ago

The changes in https://github.com/Randgalt/record-builder/pull/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:

"2nd" layer:

Randgalt commented 3 months ago

@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.