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

Don’t offer text edits from inlay hints if they are syntactically invalid #770

Merged
merged 1 commit into from Jul 18, 2023

Conversation

ahoppen
Copy link
Collaborator

@ahoppen ahoppen commented Jul 17, 2023

Don’t offer any text edits for inlay hints displayed in closure paramters or enum case items.

Fixes #759
rdar://111559715

@ahoppen ahoppen requested a review from bnbarham July 17, 2023 19:06
@ahoppen
Copy link
Collaborator Author

ahoppen commented Jul 17, 2023

@swift-ci Please test


self.range = startIndex..<endIndex
self.printedType = printedType
self.hasExplicitType = hasExplicitType
self.canBeFollowedByTypeAnnotation = tokenAtOffset?.canBeFollowedByTypeAnnotation ?? true
Copy link
Contributor

Choose a reason for hiding this comment

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

Would a false default be better here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not really. canBeFollowedByTypeAnnotation is a blacklist-based method that also defaults to true. That’s why I think we should also default to true here for consistency.

Sources/SourceKitLSP/Swift/VariableTypeInfo.swift Outdated Show resolved Hide resolved
var node = Syntax(self)
LOOP: while let parent = node.parent {
switch parent.kind {
case .caseItem, .closureParam:
Copy link
Contributor

Choose a reason for hiding this comment

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

I was going to ask if this handles the case that the param is in (). But apparently it does, because closureParam is only the non-signature case. x in is a ClosureParamListSyntax where as (x) in is a ClosureParameterClauseSyntax. The former has ClosureParams and the latter ClosureParameters 😅.

Not something that needs fixing here, but worth knowing. I don't have a great name for the former case 🤔.

…alid

Don’t offer any text edits for inlay hints displayed in closure paramters or enum case items.

Fixes apple#759
rdar://111559715
@ahoppen
Copy link
Collaborator Author

ahoppen commented Jul 18, 2023

@swift-ci Please test

@ahoppen
Copy link
Collaborator Author

ahoppen commented Jul 18, 2023

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 8c7c32d into apple:main Jul 18, 2023
3 checks passed
@ahoppen ahoppen deleted the ahoppen/inlayhint-edits branch July 18, 2023 23:32
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.

InlayHint textEdits are not always correct
2 participants