Remove 'state set' concept for diagnostic analysis subsystem. #77121
Conversation
dotnet-issue-labeler
bot
added
Area-Analyzers
untriaged
...guageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.Executor.cs
Outdated
Show resolved
Hide resolved
…nosticIncrementalAnalyzer.Executor.cs
…slyn into removeStateSet
|
|
||
| private sealed class HostAnalyzerStateSets | ||
| private sealed class HostAnalyzerInfo |
There was a problem hiding this comment.
view with whitespace off. this moved up one level of nesting so other callers could talk directly to this.
| // return existing state sets | ||
| // No need to use _projectAnalyzerStateMapGuard during reads of _projectAnalyzerStateMap | ||
| return _projectAnalyzerStateMap.Values.SelectManyAsArray(e => e.StateSetMap.Values); | ||
| } |
|
@dibarbet ptal |
|
@dibarbet @JoeRobich This is ready for review. |
|
@ToddGrun ptal |
| @@ -59,8 +61,8 @@ internal partial class DiagnosticAnalyzerService | |||
| // </summary> | |||
| async Task<CompilationWithAnalyzersPair?> CreateCompilationWithAnalyzersAsync() | |||
| { | |||
| var projectAnalyzers = stateSets.SelectAsArray(s => !s.IsHostAnalyzer, s => s.Analyzer); | |||
| var hostAnalyzers = stateSets.SelectAsArray(s => s.IsHostAnalyzer, s => s.Analyzer); | |||
| var projectAnalyzers = analyzers.WhereAsArray(static (s, info) => !info.IsHostAnalyzer(s), hostAnalyzerInfo); | |||
There was a problem hiding this comment.
var projectAnalyzers = analyzers.WhereAsArray(static (s, info) => !info.IsHostAnalyzer(s), hostAnalyzerInfo);
mentioned this after one of the earlier PRs had already been merged, but the projectAnalyzers/hostAnalyzers arrays appear to be intermediary arrays used only once, and the WhereAsArray conditions to create the filtered versions could be slightly modified to not need the intermediary arrays.
There was a problem hiding this comment.
yup. can likely go through improve more here.
src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs
Show resolved
Hide resolved
...rotocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs
Show resolved
Hide resolved
...Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer.cs
Show resolved
Hide resolved
...Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer.cs
Show resolved
Hide resolved
| /// Creates a new project state sets. | ||
| /// </summary> | ||
| private ProjectAnalyzerStateSets CreateProjectStateSets(Project project) | ||
| private ProjectAnalyzerInfo CreateProjectAnalyzerInfo(Project project) |
There was a problem hiding this comment.
nit - maybe male a local function inside UpdateProjectAnalyzerInfoAsync?
There was a problem hiding this comment.
yup. i have a followup that changes a lot of these cases to local functions.
...eServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.cs
Show resolved
Hide resolved
Followup to #77113
This was just a class wrapping a DiagnosticAnalyzer and a bool saying if it was a host-analyzer or not. It's just much simpler and cleaner to pass around DiagnosticAnalyzers and occasionally call to a separate helper to determine if it is a host analyzer or not.