-
Notifications
You must be signed in to change notification settings - Fork 229
Remove LocateOwner usage from CodeActions #9037
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
Remove LocateOwner usage from CodeActions #9037
Conversation
...ft.AspNetCore.Razor.LanguageServer/CodeActions/CSharp/TypeAccessibilityCodeActionProvider.cs
Outdated
Show resolved
Hide resolved
|
|
||
| var owner = syntaxTree.Root.LocateOwner(change); | ||
| var token = syntaxTree.Root.FindToken(context.Location.AbsoluteIndex); | ||
| var owner = token?.Parent; |
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.
Can you validate that the switch on line 69 below, is still needed? Some of those quirks in tree shape might have been fixed by the FindToken API.
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.
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 checking. I suspect @code$$ { falls into the second arm now, not the first, but makes sense that the first one is needed with the current algorithm.
@333fred does Roslyn do any heuristics like "if the token in front of the caret is punctuation, use the token behind it"?
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.
Nope. The rule is very simple and straightforward: all trailing trivia, up to and not including the newline, is attached to the previous token. Any further trivia is attached to the next token (which is potentially the eof token at the end of the file). And, importantly, punctuation such as { is not trivia.
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 confirming. If this becomes important we can always create an extension method that deals with it. As it is the switch expression only deals with two cases, so its not quite at the level of "deal with compiler quirks" :)
src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Extensions/SyntaxNodeExtensions.cs
Outdated
Show resolved
Hide resolved
...spNetCore.Razor.LanguageServer/CodeActions/Razor/ComponentAccessibilityCodeActionProvider.cs
Outdated
Show resolved
Hide resolved
…tions/Razor/ComponentAccessibilityCodeActionProvider.cs Co-authored-by: Fred Silberberg <[email protected]>

No description provided.