search

gohanlon / swift-memberwise-init-macro

Swift Macro for enhanced automatic inits.
MIT License
122 stars 8 forks source link

Add `package` ACL #5

Closed davdroman closed 1 year ago

gohanlon commented 1 year ago

Awesome, thanks for the PR!

Pretty straightforward. The only thing I want to consider is the default behavior of optionalsDefaultNil when providing an initializer with package access. In the meantime, I added a tracking issue (#6).

davdroman commented 1 year ago

My 2 cents, personally I've always found the behavior of the built-in synthesized memberwise inits in Swift annoying and unpredictable when it comes to defaulting optional params to nil — I talked about it here.

And there has even been a popular forum discussion advocating for the removal of implicit nil assignments altogether.

So, if it were up to me, optionalsDefaultNil wouldn't even be an option, but if it had to be, it would be always false across all access levels unless the user opted in explicitly.

gohanlon commented 1 year ago

I too lament that Swift provides initializers defaulting optionals to nil. A design tenet of MemberwiseInit has been to strive to be a pure superset of Swift's memberwise initializer, and this approach has largely served it well. But in this case, I'm now convinced that it makes sense to deviate from Swift 5 and always set optionalsDefaultNil to false, regardless of access level.

I've created #7 to track and discuss that!

I'm leaning towards keeping optionalsDefaultNil as an option to allow for closer parity with Swift's behavior.

gohanlon commented 1 year ago

Thanks for your contribution! I've merged your changes with some adjustments into PR #9 with some minor tweaks and tests. Closing this PR as we move forward with PR #9.