Fix exception for non-cs/vb document #72677
Conversation
src/Features/LanguageServer/Protocol/Features/CodeFixes/CodeFixService.cs
Outdated
Show resolved
Hide resolved
| var syntaxFacts = sourceDocument.GetRequiredLanguageService<ISyntaxFactsService>(); | ||
| var containingMethod = syntaxFacts.GetContainingMethodDeclaration(root, range.Start, useFullSpan: false); | ||
| range = containingMethod?.Span ?? range; | ||
| if (sourceDocument.SupportsSyntaxTree) |
There was a problem hiding this comment.
i'd just move this above, and bail out immediately.
There was a problem hiding this comment.
this is in case we want to support non cs/vb in roslyn workspace
There was a problem hiding this comment.
so, i'm wary about thsi currently as i don't understand the semantics. specifically, what is hte impact of us passing in the full range of hte document into the methods you're calling.
Also, CodeFixService is firmly lang nutral. Any lang specific checks (like getting syntax facts, or asking about things like "the containzing method") need to move into a lang-specific api.
That way a langauge like xaml could do the same sort of thing. they need to be able to say "here's the span i want to limit myself to looking at".
There was a problem hiding this comment.
Make sense. How about we get this fix in to unblock insertion. I will follow up with a proper fix next.
Fixed in ed85d66
|
/azp run roslyn-CI |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| foreach (var diagnostic in diagnostics) | ||
| { | ||
| if (diagnostic.Location.SourceSpan.IntersectsWith(span.Value)) | ||
| filteredDiagnostics.Add(diagnostic); |
There was a problem hiding this comment.
fine for now. but we really need this to be done through an ILangService so tha tthe language decides what span to be using instead. right now this is really just not the right way to encode this sort of concept into a feature.
No description provided.