Randgalt / record-builder

Record builder generator for Java records
Apache License 2.0
758 stars 55 forks source link

Issue #123: support nullable collections #156

Closed hofiisek closed 1 year ago

hofiisek commented 1 year ago

Solves https://github.com/Randgalt/record-builder/issues/123

There's a new allowNullableCollections option that does the following:

All branches mentioned above should be covered by the TestNullableCollectionsBuilder test

Randgalt commented 1 year ago

Thanks for this. I'll have a look very soon

Randgalt commented 1 year ago

Thanks for this PR - it looks very good.

I tried a class that only has @RecordBuilder.Options(allowNullableCollections = true) and it's a NOP regarding specialized collections. Should we emit a warning or error? Or, should allowNullableCollections = true always imply enabling useImmutableCollections?

hofiisek commented 1 year ago

Thanks for this PR - it looks very good.

I tried a class that only has @RecordBuilder.Options(allowNullableCollections = true) and it's a NOP regarding specialized collections. Should we emit a warning or error? Or, should allowNullableCollections = true always imply enabling useImmutableCollections?

I see, didn't think of that combination before.

Since NOP is the expected behavior in this case (components and builder getters just return whatever you set to them, without any shim calls etc.), I think we can emit a warning that it will have no effect. I'd rather not enableuseImmutableCollections automatically as it changes behavior and it could possibly break someone's project (even though I would think that the majority uses unmodifiable/immutable collections anyway).

Randgalt commented 1 year ago

Since NOP is the expected behavior in this case (components and builder getters just return whatever you set to them, without any shim calls etc.), I think we can emit a warning that it will have no effect. I'd rather not enableuseImmutableCollections automatically as it changes behavior and it could possibly break someone's project (even though I would think that the majority uses unmodifiable/immutable collections anyway).

A warning (or error) would be good. I'd prefer that to doing nothing.

hofiisek commented 1 year ago

Agree. Added in the last commit :)

Randgalt commented 1 year ago

Thank you for this