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

Diagnostics from single file vs project are not deduped correctly #1830

Closed
filipw opened this issue Nov 2, 2017 · 11 comments
Closed

Diagnostics from single file vs project are not deduped correctly #1830

filipw opened this issue Nov 2, 2017 · 11 comments

Comments

@filipw
Copy link
Contributor

filipw commented Nov 2, 2017

I'm not sure if I simply managed to mess up my local set up but I don't think so - I can repro it on both Windows and macOS.

Environment data

VS Code version: 1.17.2
C# Extension version: 1.13.0-beta4

Steps to reproduce

Give the following code, in a .NET Core 1.1 project (project type is irrelevant, I can repro the same error i.e. with CSX)

namespace Sonova.Nephele.Shared.Web.AspNetCore
{
    public class ErrorData
    {
        public string Message { get; set; }
    }
}dddd

Hover over dddd. Then hover again, and again.

Expected behavior

Diagnostic is de-duped.

We make 2 calls to /codecheck:

  • once with current file context
  • once with no file context (against all projects)

As soon as the second one completes, the 2 diagnostic entry shows up next to the original one.

Actual behavior

The diagnostic is not deduped:

screen shot 2017-11-02 at 14 48 15

It also shows up twice in the Problems panel.

screen shot 2017-11-02 at 14 52 55

Note that the project is not multi-targeted for 2 frameworks. As mentioned, same behavior can be observed for i.e. CSX.

@peabnuts123
Copy link

Definitely seeing this. AFAIK I am running a pretty plain dotnetcore project

@cosmoddd
Copy link

Also seeing this.

@peabnuts123
Copy link

Seeing this on multiple computers and project types and also in colleagues setups. All errors are reported twice. AFAIK this is happening under any scenario, I have not actually seen a setup wherein errors are only reported once. Any idea on what is happening?

@plbpietrz
Copy link

This is a long standing issue. I saw it as far as half a year back, when I started using Code for my local C# projects.

@DustinCampbell
Copy link
Member

@rchande : This is getting hit a lot. I think you even filed a dupe at one point. Could we raise the priority on this one? It'd be great to get it fixed for 1.14.

@DustinCampbell
Copy link
Member

@TheRealPiotrP : I'm seeing a lot of these. It would be great if we could get this assigned out and fixed.

@rchande
Copy link

rchande commented Feb 6, 2018

Self-assigning.

@rchande
Copy link

rchande commented Feb 6, 2018

@DustinCampbell @mholo65 @david-driscoll

The C# extension makes 2 diagnostics calls: one with a filename, and one with no filename which is supposed to get all the diagnostics for a "project". (This actually means all the diagnostics in the O# Workspace's CurrentSolution). When we make a request with a filename, we get back the correct diagnostics. When we make a request with no filename, Omnisharp returns every diagnostic twice. The C# extension faithfully adds each diagnostic to the error list twice.

It looks like Omnisharp regressed this when we added cake support. Cake added another CodeCheckRequest handler that also uses the C# CodeCheckService. The cake code check handler properly reports that it support "cake" files and there is code in EndPointHandler.cs that sniffs the filepath to figure out what the extension is. However, if there's no file name, like in the "project" case, the C# and Cake code check handlers both get invoked, and both produce the same diagnostics, leading to duplicates.

I'm not sure what the best fix is there. I don't think adding a "language" field to the code check request would be the right thing--when we request diagnostics for all projects we probably want to include cake projects. Should we have the cake handler only provide diagnostics in cake files?

@bjorkstromm
Copy link
Contributor

bjorkstromm commented Feb 6, 2018

@rchande that's sounds good to me.

Just override Handle method here and check if the CodeCheck request contains filename or not. In case no filename is required, just return.

I can make the PR if you like.

@DustinCampbell
Copy link
Member

@rchande : Good analysis!

@rchande
Copy link

rchande commented Feb 6, 2018

@mholo65 Working on it--almost ready.

rchande pushed a commit to rchande/omnisharp-roslyn that referenced this issue Feb 7, 2018
Cake's diagnostics engine calls the generic CodeCheckService, which generates all diagnostics for the solution if called without a specific file path. This results in duplicate errors in C# projects when VS Code requests project level diagnostics. The solution is to only compute diagnostics for specified files.

Fixes dotnet/vscode-csharp#1830
@DustinCampbell DustinCampbell added this to the 1.14 milestone Feb 13, 2018
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

7 participants