Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add rule to infer property types from the right-hand-side value rather than writing the type explicitly on the left-hand side #263
Add rule to infer property types from the right-hand-side value rather than writing the type explicitly on the left-hand side #263
Changes from 1 commit
d343fb2
3750e2c
5a39cdc
39f921f
e3c339c
0425be7
0f4aed8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 spent a bit of time thinking about this case in more detail, that
let myShape1: ShapeStyle = .saturnOutline
is allowed butlet myShape1: = ShapeStyle.saturnOutline
isn't. I couldn't understand why this wasn't allowed.This syntax was introduced in SE-0299. This question came up briefly in the review thread, and Pavel mentioned that there was more discussion in the pitch thread. A found the most compelling discussion here in the pitch thread.
They intentionally don't frame this as adding a static member to the protocol type, but rather a shorthand syntax that lets you omit the name of the concrete type.
Basically, the thinking is that this code:
is actually implicit shorthand for this code:
not shorthand for this code:
This is subtle but distinct, and is a reasonable explanation for why
ShapeStyle.saturnOutline
isn't allowed.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 problem should also go away eventually, once
any
is required in a future language mode (just not Swift 6).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.
By the way -- for this we have the ability to tell SwiftFormat a list of symbols to exclude from this rule (
--preservesymbols ShapeStyle,OtherType,etc
). Inside the apps repo we could exclude any type or property name where we frequently use this syntax. Two relevant examples are:UnresolvedShapeStyle
any UnresolvedShapeStyle
so this is already handled properlycanonicalFake
let experimentsService: ExperimentsService = .canonicalFake
(etc). It seems like a good idea to exclude this using--preservesymbols canonicalFake
. Updating the callsites tolet experimentsService: any ExperimentsService = .canonicalFake
also works.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.
Thanks for flagging this @calda . I was wondering about this... My recollection is that eventually it's going to be required that we use
any
for any existential/protocol types but that this requirement is being slowly rolled out. Is your understanding also that eventually all existential/protocol type will need to be preceded byany
? If so, I think it's less problematic if we need to work around certain situations like the above ones you highlighted for the time being.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 am trying to understand if there would be cases when we still need
--preservesymbols
if/when the language requires that all existential/protocol types are preceded byany
.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 saw on the Swift Evolution forums that they decided to not make
any
mandatory in Swift 6 mode after all. Although after a quick search on the forums I can't actually find where they said that.This doesn't necessarily confirm it, but if you build code like this using a nightly Swift 6 build it doesn't require
any
.If all call sites used the existential
any
syntax, we wouldn't need to use--preservesymbols
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.
Found the quote: https://forums.swift.org/t/progress-toward-the-swift-6-language-mode/68315
It sounds like they didn't rule out this direction, but rather just deferred it for longer (probably a very long time, if we have to wait for Swift 7!)
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.
In the apps repo PR that I merged today, I included
--preservesymbols canonicalFake
. This is a common and reasonable pattern that I think we should continue supporting. It's not common to use theany ExperimentService
spelling today, so it felt too weird to require it in this specific case due to this limitation of the linter.