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

New analyzer API (DiagnosticSuppressor) is not supported #1711

Closed
sailro opened this issue Feb 12, 2020 · 27 comments · Fixed by #2116 or #2182
Closed

New analyzer API (DiagnosticSuppressor) is not supported #1711

sailro opened this issue Feb 12, 2020 · 27 comments · Fixed by #2116 or #2182

Comments

@sailro
Copy link

sailro commented Feb 12, 2020

I'm using a pack of Roslyn Analyzers built from:
https://github.com/microsoft/Microsoft.Unity.Analyzers

On a Unity project (Game engine) available here:
https://learn.unity.com/project/roll-a-ball-tutorial

When using Visual Studio:
image

When using VSCode:
image

Omnisharp is able to correctly consume regular Analyzers (see UNT0009). But Diagnostic suppressors are not supported:

"Private member 'ReadmeEditor.Awake' is unused. [Assembly-CSharp-Editor]"

This message is correctly suppressed in the VS context, but not within VSCode.

The code for this suppressor is here:
https://github.com/microsoft/Microsoft.Unity.Analyzers/blob/master/src/Microsoft.Unity.Analyzers/UnusedMessageSuppressor.cs

The Roslyn team added a new API regarding programmatic suppression of diagnostics:
dotnet/roslyn#36067

@filipw
Copy link
Member

filipw commented Feb 12, 2020

thanks for the issue, this is indeed currently not supported

@sailro
Copy link
Author

sailro commented Feb 12, 2020

@mavasani did this great job of adding this new DiagnosticSuppressor API. Perhaps supporting this in OmniSharp is not that complicated (host initialization?). -I say this without knowing, I suspect that it's not all that simple, but I just hope :) -

@loligans
Copy link

@sailro, @filipw I think I may have gotten around this issue by enabling roslyn analyzers and editorconfig support. You can configure the rules you want enabled or disabled globally within your .editorconfig file.

Note: After enabling roslyn analyzers and editorconfig support, Omnisharp will require to be restarted in order for the changes to take effect. In addition, OmniSharp needs to be restarted after you make any changes to your .editorconfig file.

Aside from the inconvenience of having to reload OmniSharp everytime I make a changes to my .editorconfig, one annoyance I have with this setup is that I am unable to disable analysis of specific files or directories.

As a user sometimes I would like to be able to exclude specific files or directories from analysis. An example would be auto generated files from an EF Core migration. @savpek do you know if specific files or directories can be excluded from OmniSharp file analysis?

@savpek
Copy link
Contributor

savpek commented Feb 22, 2020

@loligans have you tested suppres message attributes where specifically following:

namespaceanddescendants - (Requires compiler version 3.x or higher and Visual Studio 2019) This scope suppresses warnings in a namespace and all its descendant symbols. The namespaceanddescendants value is ignored by legacy analysis.

Which seems roslyn way to do similar thing. However i have never tested those with omnisharp so they may or may not work. If they don't that is very good feature to add 🤔

@loligans
Copy link

@savpek Sorry, I'm not sure how to do what you're suggesting.

What I have tried is to disable the analyzers that pollute the VSCode Problems Pane by using a nested .editorconfig inside the Migrations directory. Unfortunately, that had no effect on Omnisharp's analysis.

@mavasani
Copy link

mavasani commented Apr 22, 2020

@loligans have you tried https://docs.microsoft.com/en-us/visualstudio/code-quality/use-roslyn-analyzers?view=vs-2019#set-rule-severity-of-multiple-analyzer-rules-at-once-in-an-editorconfig-file?

Dropping an editorconfig with the following contents in any folder should ensure no analyzer diagnostics reported within it, unless some parent directory is explicitly enabling some rules.

[*.cs]
dotnet_analyzer_diagnostic.severity = none

Another approach is trying out .editorconfig with following contents, which will treat the entire folder and sub-folders as generated code, which should filter almost all analyzer diagnostics (except the critical ones like some security rules which explicitly want to flag generated code)

[*.cs]
generated_code = true

@mavasani
Copy link

mavasani commented Apr 23, 2020

NOTE: The suggested features above needs "Visual Studio 2019 version 16.5 or later"

@loligans
Copy link

loligans commented Apr 23, 2020

Thank you for the help @mavasani. Applying the suggested configuration unfortunately did not solve the issue. I should note that I am running on Linux, so I do not have Visual Studio 2019 installed.

VSCode Version: 1.44.2
Omnisharp: 1.35.0

Here is my Omnisharp log if that helps with diagnostics:

[info]: OmniSharp.Stdio.Host
        Starting OmniSharp on arch 0.0 (x64)
[info]: OmniSharp.Services.DotNetCliService
        DotNetPath set to dotnet
[info]: OmniSharp.MSBuild.Discovery.MSBuildLocator
        Located 1 MSBuild instance(s)
            1: StandAlone 16.4 - "/home/loligans/.vscode/extensions/ms-dotnettools.csharp-1.21.17/.omnisharp/1.35.0/omnisharp/.msbuild/Current/Bin"
[info]: OmniSharp.MSBuild.Discovery.MSBuildLocator
        MSBUILD_EXE_PATH environment variable set to '/home/loligans/.vscode/extensions/ms-dotnettools.csharp-1.21.17/.omnisharp/1.35.0/omnisharp/.msbuild/Current/Bin/MSBuild.dll'
[info]: OmniSharp.MSBuild.Discovery.MSBuildLocator
        Registered MSBuild instance: StandAlone 16.4 - "/home/loligans/.vscode/extensions/ms-dotnettools.csharp-1.21.17/.omnisharp/1.35.0/omnisharp/.msbuild/Current/Bin"
            CscToolExe = csc.exe
            MSBuildToolsPath = /home/loligans/.vscode/extensions/ms-dotnettools.csharp-1.21.17/.omnisharp/1.35.0/omnisharp/.msbuild/Current/Bin
            CscToolPath = /home/loligans/.vscode/extensions/ms-dotnettools.csharp-1.21.17/.omnisharp/1.35.0/omnisharp/.msbuild/Current/Bin/Roslyn
            BypassFrameworkInstallChecks = true
            MSBuildExtensionsPath = /home/loligans/.vscode/extensions/ms-dotnettools.csharp-1.21.17/.omnisharp/1.35.0/omnisharp/.msbuild

@mavasani
Copy link

What version of Roslyn compiler do you have? This is a compiler feature added in version 3.5.0 of the compiler, so should work even in other hosts.

@loligans
Copy link

It's not clear to me exactly how to get the Roslyn compiler version. However, when I run dotnet build I get the following:

Microsoft (R) Build Engine version 16.4.0+e901037fe for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

Is 16.4.0+e901037fe close to what you're looking for?

@mavasani
Copy link

16.4.0+e901037fe

That indicates you are on MSBuild toolset that shipped with VS2019 16.4. So you will need 16.5 version of it...

@loligans
Copy link

I can confirm the issue I was experiencing is resolved using the latest build of the vscode extension (v1.21.19-beta1). The suggestion below is what worked for me.

[*.cs]
generated_code = true

@mavasani
Copy link

@alexrp
Copy link

alexrp commented May 21, 2020

Worth noting that DiagnosticSuppressor support is important for Microsoft's Unity analyzers.

@heshuimu
Copy link

heshuimu commented Oct 5, 2020

+1 for Unity Analyzers. This could especially be useful to silence invalid warnings from other analyzers on Unity methods:

// CA1822: Member 'Awake' does not access instance data and can be marked as static. 
// IDE0051 : Private member 'TouchOnScreenTest.Awake' is unused. 
private void Awake()
{
	Application.targetFrameRate = 120;
}

@marcospgp
Copy link

marcospgp commented Dec 11, 2020

So is there no way currently to enable the Unity analyzers on VSCode?

Edit: You guys are talking about editor config files, but all the links talk about creating/editing them when using Visual Studio. Isn't this thread about VSCode?

Edit 2: I created a .editorconfig file in the root folder of the project and that worked, but the line generated_code = true just silenced all of my warnings. What was that supposed to solve?

@marcospgp
Copy link

marcospgp commented Dec 12, 2020

I figured this out more or less, and posted a Reddit thread explaining how to set up the Unity analyzers with Omnisharp in Visual Studio Code. Hope it helps someone because it took me 2 whole days! 😁😁

@sbaranov
Copy link

There is a problem in omnisharp support for .editorconfig. Whenever the project is updated outside of vscode, omnisharp dynamically reloads the project, but forgets to use .editorconfig at that point, so all settings gets reset - all warnings become enabled, all formatting rules switch to defaults, etc. This means that whenever you switch git branches, or make any change in Unity that affects the project such as adding a new script, you suddenly see thousands of unrelated warnings in vscode that shouldn't be there. One workaround is to restart vscode every time omnisharp lost its settings, which basically becomes every few minutes if you're actively editing.

@filipw
Copy link
Member

filipw commented Dec 12, 2020

@sbaranov this was fixed in #2028 which will be in the next release

@marcospgp
Copy link

@filipw That's good to hear! Is there an expected release date for supporting the Unity suppressors (the original intention of this issue)?

It's interesting that Unity & Microsoft are sort of partnering up to get Unity working well with VSCode, but the workhorse under the whole thing is being maintained entirely by you 😅

@filipw
Copy link
Member

filipw commented Dec 12, 2020

unfortunately the suppressors haven't been looked at yet, I am very sorry. Will try to prioritize this!
I usually put highest priority on things that I personally use 😂

It's interesting that Unity & Microsoft are sort of partnering up to get Unity working well with VSCode, but the workhorse under the whole thing is being maintained entirely by you 😅

That is very kind of you to say! but there are over 100 contributors here https://github.com/OmniSharp/omnisharp-roslyn/graphs/contributors and currently a group of 4 maintainers (the ones seen on PR reviews).

@marcospgp
Copy link

@filipw This functionality is now part of the official VSCode documentation. It would be great to get the supressors working as well!

@WeirdBeardDev
Copy link

@marcospgp

Edit 2: I created a .editorconfig file in the root folder of the project and that worked, but the line generated_code = true just silenced all of my warnings. What was that supposed to solve?

I solved this in the root .editorconfig by creating a separate header and then adding the generated code line. In this way I could turn off all warning for downloaded or imported code, e.g., TextMesh Pro.

[Assets/TextMesh Pro/**.cs]
generated_code = true

@WeirdBeardDev
Copy link

unfortunately the suppressors haven't been looked at yet, I am very sorry. Will try to prioritize this!

@filipw any updates on this issue?

@filipw
Copy link
Member

filipw commented Mar 13, 2021

no

@marcospgp
Copy link

marcospgp commented Jun 20, 2021

Seems like we are having some updates on this!! microsoft/Microsoft.Unity.Analyzers#170 (comment)

@manfred-brands
Copy link

manfred-brands commented Nov 6, 2021

@filipw I have just upgrade to vscode version 1.62.0. However, I still get issues that I know are suppressed by nunit.analyzers.
The below code does not show any error in VS2019 (16.11.5), nor when compiling with dotnet, but does show a Dereference of a possible null reference in vscode(1.62.0) with C# extension (1.23.16)

[Test]
public void Test()
{
    string? message = OptionalMessage();

    Assert.That(message, Is.Not.Null); // nunit.analyzers will suppress the possible null reference exception on the next line.
    Assert.That(message.Length, Is.GreaterThan(0));
}

private static string? OptionalMessage() => string.Empty;

I know that the nunit.analyzers are loaded as I can get violations against other rules to show up.

Is there anything I need to configure or do the analyzers need to do something different for vs-code to recognize the suppressor?

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