Remove 'synchronize with build' internal functionality #72802
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
|
@mavasani ptal, and let me know if my understanding of these capabilities is correct. |
| var projectStateSets = project.SupportsCompilation | ||
| ? GetOrUpdateProjectStateSets(project) | ||
| : ProjectAnalyzerStateSets.Default; | ||
| var hostStateSets = GetOrCreateHostStateSets(project, projectStateSets); |
There was a problem hiding this comment.
aside, i'd love to know what the different state sets all mean.
There was a problem hiding this comment.
HostStateSets - state sets corresponding to analyzers coming from installed analyzer VSIXes, which are applicable for any project opened in the VS Session. The most common use case is for our built-in IDE code style analyzers that ship in Features.dll inside Roslyn VSIX installed with VS.
ProjectStateSets - state sets corresponding to analyzers coming from installed analyzer NuGet packages, which are applicable only to the specific project.
There was a problem hiding this comment.
Oh. That's very helpful. Thanks!!
| @@ -212,8 +214,6 @@ async ValueTask ClearErrorsCoreAsync(ProjectId projectId, Solution solution, InP | |||
| ProcessAndRaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); | |||
| } | |||
|
|
|||
| await SetLiveErrorsForProjectAsync(projectId, ImmutableArray<DiagnosticData>.Empty, GetApplicableCancellationToken(state)).ConfigureAwait(false); | |||
There was a problem hiding this comment.
this method only existed to forward to the diagservice, and so it got removed.
| } | ||
|
|
||
| private async ValueTask SetLiveErrorsForProjectAsync(ProjectId projectId, InProgressState state) | ||
| private static void SetLiveErrorsForProject(ProjectId projectId, InProgressState state) |
There was a problem hiding this comment.
Is it possible to now remove this method completely? Is there anything related to live errors that needs to be implemented in this type now?
There was a problem hiding this comment.
will do in followup :)
We do not want external errors flowing in and us having to manage their state through the DiagAnalyzerService.
Note: the external errors still flow in and are managed by the ExternalErrorDiagnosticUpdateSource. This allows features like allowing the user to suppress build-diagnostics, but can be done without the DiagAnalyzerService being involved. This pushes us to have entirely separate systems for live diags (DiagAnalyzerService) and external build diagnostics (ExternalErrorDiagnosticUpdateSource).
In an LSP-pull world, this works nicely. The build system and Roslyn are already communicating diagnostics with the host and the host owns things like squiggles and task list. The host also owns dedupe and superseding of live diags over build diags.
This will also allow us to trim down ExternalErrorDiagnosticUpdateSource and ProjectExternalErrorReporter in the future. Those systems have a complex eventing based approach that nothing in roslyn currently listens to. So we can remove those events in teh future, and just use these as a buffer of build-errors for the rare features that need to know this (currently, only the ability to suppress a build-only diagnostic).