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

Jump to bottom

[SR-15204] UseShorthandTypeNames should not replace Optional<X> with X? on stored property declarations #304

Closed
karwa opened this issue Sep 16, 2021 · 0 comments · Fixed by #398
Closed

[SR-15204] UseShorthandTypeNames should not replace Optional<X> with X? on stored property declarations #304

karwa opened this issue Sep 16, 2021 · 0 comments · Fixed by #398
Assignees
@allevato
Labels
bug Something isn't working swift-format

Comments

@karwa
Copy link
Contributor

karwa commented Sep 16, 2021

Previous ID SR-15204
Radar None
Original Reporter @karwa
Type Bug
Additional Detail from JIRA
Votes 0
Component/s swift-format
Labels Bug
Assignee None
Priority Medium

md5: 640847747f502d1efb6fc7028d0cd127

Issue Description:

It isn't always just a shorthand; it can have meaningfully different behaviour. See: SR-11777 7 and discussion at https://forums.swift.org/t/what-is-a-variable-initialization-expression/52132

Currently, the only alternative is to disable this rule entirely, for all types, everywhere. That's less than ideal: I'd like to enable this rule and to use shorthand names when they really are just shorthands.
@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 9, 2022
allevato added a commit to allevato/swift-format that referenced this issue Sep 16, 2022
@allevato
Don't use shorthand syntax for Optional<T> if all of the following …
d65362e 
…are true:

*   It is the type of a variable (file-, function-, or type-scoped)
*   It is mutable (`var`, not `let`)
*   It is not a computed property
*   It does not have an initializer expression

If all of these are true, shortening it to `T?` would introduce a useless
nil-initialization that can be problematic in situations that want to
optimize performance (see https://forums.swift.org/t/what-is-a-variable-initialization-expression/52132).

Fixes apple#304.
allevato added a commit to allevato/swift-format that referenced this issue Sep 16, 2022
@allevato
Don't use shorthand syntax for Optional<T> if all of the following …
2f1223d 
…are true:

*   It is the type of a variable (file-, function-, or type-scoped)
*   It is mutable (`var`, not `let`)
*   It is not a computed property
*   It does not have an initializer expression

If all of these are true, shortening it to `T?` would introduce a useless
nil-initialization that can be problematic in situations that want to
optimize performance (see https://forums.swift.org/t/what-is-a-variable-initialization-expression/52132).

Fixes apple#304.
allevato added a commit to allevato/swift-format that referenced this issue Sep 16, 2022
@allevato
Don't use shorthand syntax for Optional<T> if all of the following …
9501173 
…are true:

*   It is the type of a variable (file-, function-, or type-scoped)
*   It is mutable (`var`, not `let`)
*   It is not a computed property
*   It does not have an initializer expression

If all of these are true, shortening it to `T?` would introduce a useless
nil-initialization that can be problematic in situations that want to
optimize performance (see https://forums.swift.org/t/what-is-a-variable-initialization-expression/52132).

Fixes apple#304.
@allevato allevato mentioned this issue Sep 16, 2022
Merged
@allevato allevato self-assigned this Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@allevato allevato

Labels
bug Something isn't working swift-format
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't transform Optional<T> -> T? if it would cause a performance issue allevato/swift-format
2 participants
@allevato @karwa