17.10: Cherry-pick fix for scenario where lightbulbs weren't being displayed #74760
Conversation
Addresses https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2087100 The recent change to remove solution crawler has apparently caused some regressions in lightbulb functionality and performance. This PR attempts to address at least one functional issue introduced as part of that change. Previously, during solution load, solution crawler would cause the analyzer state sets to update via IncrementalAnalyzerProcessor.LowPriorityProcessor.ProcessProjectAsync's call to DiagnosticIncrementalAnalyzer.AnalyzeProjectAsync. Similarly, document updates would update the analyzer states the crawler's idle processing. Instead of trying to hook back in these calls, this PR instead changes the diagnostic analyzer service to no longer offer a TryGetDiagnosticsForSpanAsync, instead forcing users to use the GetDiagnosticsForSpanAsync instead. The only caller of TryGetDiagnosticsForSpanAsync was inside the CodeFixService, and it seems like that code should just go ahead and force correct results. Thus, there is no concept of a non-upToDate call into the diagnostic service. I'm going to separately investigate an issue with the StateManager's statesets not getting updated. I'm unsure why the code doesn't update the statesets upon calculation, but the fact that we are now recreating this on every lightbulb request is at least partly responsible for the recent lightbulb perf regression.
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
|
Tagging @arkalyanms. There were a couple merge conflicts. I'm fairly confident in that I resolved them correclty, but it would be nice to get some verification that things are working as I haven't done any validation other than manual code inspection and building. |
|
@arkalyanms We coudl take this, but my preference would be to just say: "this is fixed in 17.11. sorry, please move to that if this is affecting you". |
|
@arkalyanms -- test failures, even on rerun, that don't appear to be related. Are tests on ci for the 17.10 branch in a working state? |
17.10 is LTS and support last 2 years longer than 17.11, so we need the port. |
|
This is a servicing branch, there isn't much of an expectation that integration tests will run here successfully given that they're running on VS latest, not 17.10. Generally we ignore them in servicing if we reasonably don't think they're related. |
Same tests fail in dummy PR: #74821 |
Thanks for checking, merging PR. |
Cherry-pick c5e8624.
Tracked for 17.10 by https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2136195