-
Notifications
You must be signed in to change notification settings - Fork 414
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
Prevent Unrelated Casts on Child Choice Node #2184
Prevent Unrelated Casts on Child Choice Node #2184
Conversation
4b40c68
to
810c405
Compare
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.
Hmm, this does significantly reduce the readability in many cases. I didn’t consider that you couldn’t easily chain if case
…
After reading this, I think I changed my mind and we should offer non-deprecated casting methods to any option that the SyntaxChildChoices
can have. And if one of that option is a base kind like DeclSyntax
, we should offer a casting method to any DeclSyntaxProtocol
.
I completely agree. That was my main concern with value binding in the first place. I haven't yet found the time to go through this pitch: https://forums.swift.org/t/pitch-is-case-expressions/64185, so I'm not sure why it hasn't gone through yet. The issue definitely should be addressed—it's a day-to-day problem for many folks.
Sounds good to me. I'll try cook something |
I haven’t read the pitch completely either but I would imagine it’s just a matter of someone writing the official proposal and creating an implementation for it. |
Base on last John's post https://forums.swift.org/t/pitch-is-case-expressions/64185/69 the whole pitch is abandoned 😢 |
810c405
to
9e7c4ce
Compare
54e0aaf
to
d60a3e1
Compare
CodeGeneration/Sources/generate-swift-syntax/templates/swiftsyntax/SyntaxNodeCasting.swift
Show resolved
Hide resolved
@ahoppen how about now? Usage is cleaner, but there is plenty of code more on the implementation part :/ |
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.
Looks good to me in general
CodeGeneration/Sources/generate-swift-syntax/templates/swiftsyntax/SyntaxNodeCasting.swift
Show resolved
Hide resolved
2d6331e
to
3590c97
Compare
3590c97
to
a68216c
Compare
@swift-ci Please test |
@swift-ci please test windows |
@swift-ci Please test Windows |
Resolves part of #2092