Skip to content

Conversation

@mavasani
Copy link
Contributor

Fixes AB#1815692
Fixes #68797

Currently, enabling FSA for either compiler or analyzer diagnostics computes and reports full solution diagnostics for both compiler and analyzers. We need to trigger FSA for selected analyzers (compiler and/or other analyzers) based on both options. For optimal performance, this is done as a pre-filtering of analyzers to execute rather than post-filtering of diagnostics reported from both compiler and analyzers.

NOTE: dotnet/vscode-csharp#5872 tracks corrupt diagnostics being reported for non-source documents when full solution analysis is enabled.

With this PR + dotnet/vscode-csharp#5868:

LSP_FSA_Fix

…oth compiler and analyzer diagnostics

Fixes [AB#1815692](https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1815692)
Fixes dotnet#68797

Currently, enabling FSA for either compiler or analyzer diagnostics computes and reports full solution diagnostics for both compiler and analyzers. We need to trigger FSA for selected analyzers (compiler and/or other analyzers) based on both options. For optimal performance, this is done as a pre-filtering of analyzers to execute rather than post-filtering of diagnostics reported from both compiler and analyzers.
@mavasani mavasani requested a review from a team as a code owner June 28, 2023 15:48
@ghost ghost added the untriaged Issues and PRs which have not yet been triaged by a lead label Jun 28, 2023
Comment on lines +305 to +308
if (_shouldIncludeAnalyzer != null && !_shouldIncludeAnalyzer(stateSet.Analyzer))
{
return false;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the core fix. Most of the other changes in the PR are just threading through this func.

Comment on lines +192 to +193
Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer = !compilerFullSolutionAnalysisEnabled || !analyzersFullSolutionAnalysisEnabled
? ShouldIncludeAnalyzer : null;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only place where the delegate is passed in to skip analyzers.

mavasani added 3 commits June 28, 2023 22:00
…d analyzer diagnostic scope options. Default the compiler diagostic scope to match the provided analyzer diagnostic scope, with the tests being able to provide an explicit compiler diagnostic scope.
@mavasani mavasani merged commit 50df09d into dotnet:main Jun 30, 2023
@mavasani mavasani deleted the LspPullDiagsFSA branch June 30, 2023 16:31
@ghost ghost added this to the Next milestone Jun 30, 2023
@allisonchou allisonchou modified the milestones: Next, 17.8 P1 Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WorkspacePullDiagnosticHandler should handle background analysis scope options for both compiler and analyzer diagnostics

3 participants