Skip to content

Fix renaming enum case parameters with unnamed associated arguments #2678

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

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Jun 9, 2024

Linked to and swiftlang/swift#74241 and swiftlang/sourcekit-lsp#1471

We treated enum case parameters the same way as function parameters and weren’t considering that they can be unlabeled. That caused us to insert eg. _ in front of the case’s type, producing case myCase(_ String), which is invalid. Report enumCaseParameters as a special parameter kind so that sourcekitd can work with it.

swiftlang/sourcekit-lsp#1228

@ahoppen
Copy link
Member Author

ahoppen commented Jun 9, 2024

@@ -102,6 +102,12 @@ public struct DeclNameLocation: Equatable {
/// The parameter of a function declaration, like `func foo(a b: Int)`
case parameters([Argument])

/// An enum ase declaration like `case myCase(label: String)`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// An enum ase declaration like `case myCase(label: String)`.
/// An enum case declaration like `case myCase(label: String)`.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I swear I made this type 10 times while writing the PR. One of them seems to have slipped through 😆

We treated enum case parameters the same way as function parameters and weren’t considering that they can be unlabeled. That caused us to insert eg. `_ ` in front of the case’s type, producing `case myCase(_ String)`, which is invalid. Report `enumCaseParameters` as a special parameter kind so that sourcekitd can work with it.

swiftlang/sourcekit-lsp#1228
@ahoppen ahoppen force-pushed the enum-case-rename branch from 91c6bbe to b2180c4 Compare June 12, 2024 02:21
@ahoppen
Copy link
Member Author

ahoppen commented Jun 12, 2024

@ahoppen
Copy link
Member Author

ahoppen commented Jun 12, 2024

@ahoppen ahoppen merged commit 52f0fb1 into swiftlang:main Jun 12, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants