-
Notifications
You must be signed in to change notification settings - Fork 345
Internal SwiftUI views should use synthesized memberwise initializer #347
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
Conversation
4f3a2d5 to
7935b44
Compare
7935b44 to
e1779e7
Compare
|
|
||
| </details> | ||
|
|
||
| - <a id='omit-internal-keyword'></a>(<a href='#omit-internal-keyword'>link</a>) **Omit the `internal` keyword** when defining types, properties, or functions with an internal access control level. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reorganized the access control rules to all be next to eachother
|
|
||
| </details> | ||
|
|
||
| - <a id='omit-redundant-public'></a>(<a href='#omit-redundant-public'>link</a>) **Avoid using `public` access control in `internal` types.** In this case the `public` modifier is redundant and has no effect. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged this rule into the "Access control should be at the strictest level possible" rule -- the wording of that rule already covered this behavior
| **[⬆ back to top](#table-of-contents)** | ||
|
|
||
| ## Objective-C Interoperability | ||
| ## SwiftUI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New top-level SwiftUI section!
|
|
||
| **[⬆ back to top](#table-of-contents)** | ||
|
|
||
| ## Objective-C Interoperability |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this below the "Testing" and "SwiftUI" sections. Realistically we may just want to remove it, but can do that separatety.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm glad we moved this further down 👍
bachand
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @calda . LGTM!
| 1. [Patterns](#patterns) | ||
| 1. [File Organization](#file-organization) | ||
| 1. [Objective-C Interoperability](#objective-c-interoperability) | ||
| 1. [SwiftUI](#swiftui) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| #### Why? | ||
|
|
||
| Removing redundant memberwise initializers reduces boilerplate and makes the code more concise. The compiler-synthesized initializers are equivalent to the explicit ones, so there's no functional difference. | ||
| Removing redundant memberwise initializers reduces boilerplate and makes it easier to add more properties in the future. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| <details> | ||
|
|
||
| [](https://github.com/nicklockwood/SwiftFormat/blob/main/Rules.md#redundantFileprivate) | ||
| [](https://github.com/nicklockwood/SwiftFormat/blob/main/Rules.md#redundantFileprivate) [](https://github.com/nicklockwood/SwiftFormat/blob/main/Rules.md#redundantPublic) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| } | ||
| ``` | ||
|
|
||
| However, you can use `internal` access control instead of `private` access control to enable the use of the [compiler-synthesized memberwise initializer](#omit-redundant-memberwise-init). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| } | ||
| ``` | ||
|
|
||
| This doesn't apply to SwiftUI dynamic properties, which should always be left private. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
|
||
| **[⬆ back to top](#table-of-contents)** | ||
|
|
||
| ## Objective-C Interoperability |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm glad we moved this further down 👍
Summary
This PR updates the redundant memberwise init rule to also apply to structs with private properties. For
internalSwiftUI views, prefer defininginternalproperties rather thanprivateproperties so the synthesized memberwise initializer can be used.Autocorrect is implemented in the new
--prefer-synthesized-init-for-internal-structsoption added in nicklockwood/SwiftFormat#2291.Reasoning
This reduces boilerplate and makes it easier to add additional properties in the future.
This doesn't apply to SwiftUI dynamic properties, which should always be left
private.