Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[O# Parity] Dedupe build and live diagnostics #5728

Closed
Tracked by #5951
dibarbet opened this issue Jun 8, 2023 · 7 comments
Closed
Tracked by #5951

[O# Parity] Dedupe build and live diagnostics #5728

dibarbet opened this issue Jun 8, 2023 · 7 comments

Comments

@dibarbet
Copy link
Member

dibarbet commented Jun 8, 2023

When you run a build, problems will be added to the problem window. However once the error is fixed, the errors won't disappear until a new build is run. We should remove these build errors when the next live diagnostics request.

Roughly how this might work in vscode

The recommended way on how to implement something like this in VS Code is the following:

extension one provides extension API to set sort of filter or problems piping
extension two plugs in such a filter into extension one.
In the concrete setup the implementation would be as follows:

devkit extension uses custom tasks and provides such an extension API
Roslyn extension uses the API of devkit to filter the problems or alternatively even deliver them to VS Code on behalf of the devkit extension. The Roslyn extension could also use a custom middleware to filter the problems provided by the Roslyn server or uses custom message to tell the server to not generate them at all.
@dibarbet dibarbet added this to the GA milestone Jun 8, 2023
@mikadumont mikadumont mentioned this issue Jun 8, 2023
10 tasks
@arunchndr
Copy link
Member

Moving to Becca since this in the LSP layer. This will be a GA blocker.

@arunchndr arunchndr modified the milestones: GA, Preview 2 Jun 26, 2023
@arunchndr arunchndr added the Bug label Jun 26, 2023
@beccamc
Copy link
Contributor

beccamc commented Jul 10, 2023

The problem here is that DevKit extension reports the msbuild problems, and C# extension reports the Roslyn problems. VS Code doesn't allow the Roslyn extension to overwrite results from the DevKit extension.

The DevKit extension uses the mscompile problem matcher to report problems to VS Code. There isn't a way for us to tap into the results of this problem matcher. This will require us to write a custom task via VS Code's API for MSBuild that exposes the problem matcher

@arunchndr arunchndr modified the milestones: Preview 2, GA Jul 10, 2023
@arunchndr
Copy link
Member

This is O# parity problem. So our plan is to focus on long term solution here instead of a short term woraround. So pushing out milestone.

@mavasani
Copy link
Contributor

mavasani commented Aug 1, 2023

Discussed this offline with @beccamc and we believe the following work is required here:

  1. Rosyln LSP server: Expose an API that either exposes all build-only diagnostic IDs OR given a diagnostic ID, answers whether the diagnostic ID is build-only or not. The exact API shape and implementation would depend on the client requirements, with performance considerations in mind.

  2. C# Devkit extension:

    1. Write a custom task/problem matcher to intercept build diagnostics, instead of using VSCode's msCompile problem matcher.
    2. Call into a new "build started" API added into the C# extension when the build command is invoked, just before the MSBuild task to run the build is invoked.
    3. Call into a new "build completed" API into C# extension (see 3i. below) and pass all the build reported diagnostics to it.
  3. C# extension:

    1. Expose new APIs mentioned in 2ii. and 2iii. above, so the C# devkit extension to call into it to clear diagnostics prior to build start and then report diagnostics from build at build completion.
    2. Maintain 2 separate diagnostic sources in the extension: one for build-only diagnostics and one for live diagnostics.
    3. On "build started" API invocation, clear all the prior diagnostics reported from both the above sources, to clear out the problems window and start with a clean state. Also drop all the cached diagnostics in both the above sources.
    4. On "build completed" API invocation with build reported diagnostics, bucketize these diagnostics into build-only and rest using the new API added in the Roslyn LSP server in 1. above.
    5. Report the build-only diagnostics to VSCode from the build-only diagnostic source OR build-only diagnostic identifier or some such.
    6. Group the rest of the diagnostics from ii. above by document and project, and report these as live diagnostics.

Hopefully, above design should be doable to address this issue.

I did have one question for @dibarbet @beccamc - is it possible to implement the custom task/problem matcher mentioned in 2. above in the C# extension itself? If so, we can move bulk of all the work into the C# extension, and just need to expose a simple "build command invoked" eventing API from it. C# devkit extension would invoke this API with the given project or solution name/node and let the C# extension do everything required.

@beccamc
Copy link
Contributor

beccamc commented Aug 1, 2023

This is great, thanks @mavasani! From my understanding, the custom task/problem matcher will need to be in the C# Devkit extension. However, moving it to the C# extension is a great idea and I will look into it. David please chime in if you have any other info.

@dibarbet dibarbet modified the milestones: Post GA, Next Aug 2, 2023
mavasani added a commit to mavasani/roslyn that referenced this issue Aug 11, 2023
Work towards dotnet/vscode-csharp#5728

Exposes an LSP API to fetch build-only diagnostic IDs for the given solution snapshot. This API will be invoked from VSCode C# extension on execution of explicit build/rebuild commands to separate out build-only and live diagnostics. See dotnet/vscode-csharp#5728 (comment) for details.
mavasani added a commit to mavasani/roslyn that referenced this issue Aug 11, 2023
Work towards dotnet/vscode-csharp#5728

Exposes an LSP API to fetch build-only diagnostic IDs for the given solution snapshot. This API will be invoked from VSCode C# extension on execution of explicit build/rebuild commands to separate out build-only and live diagnostics. See dotnet/vscode-csharp#5728 (comment) for details.
mavasani added a commit to mavasani/vscode-csharp that referenced this issue Aug 11, 2023
Work towards dotnet#5728. Follow-up to dotnet/roslyn#69475.

C# extension can now invoke `getBuildOnlyDiagnosticIds` API after each explicit build/rebuild command to identify build-only diagnostic IDs.
beccamc pushed a commit to dotnet/roslyn that referenced this issue Sep 21, 2023
* Add BuildOnlyDiagnosticIds LSP request handler

Work towards dotnet/vscode-csharp#5728

Exposes an LSP API to fetch build-only diagnostic IDs for the given solution snapshot. This API will be invoked from VSCode C# extension on execution of explicit build/rebuild commands to separate out build-only and live diagnostics. See dotnet/vscode-csharp#5728 (comment) for details.

* Add build-only compiler diagnostics only for the project languages in the solution + address feedback

* Add unit tests
@beccamc
Copy link
Contributor

beccamc commented Sep 22, 2023

Implemented a custom task in Devkit that parses the build results into a format that can be easily turned into Diagnostic objects (see Devkit dev/beccam/dedupeDiagnostics). These results need to be passed to C# via brokered service. Then bucketized via the new getBuildIdsOnly api and added into a new diagnostic collection.

@beccamc
Copy link
Contributor

beccamc commented Oct 10, 2023

This item requires updates to multiple components. It is in progress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants