Support roslyn analyzers and code fixes about vscode-csharp HOT 41 CLOSED

Pull request is now waiting for review (finally), heres few gifs about current behavior.

Analyzers / codefixes (with roslynator project):

Rulesets:

from vscode-csharp.

@savpek Thank you for doing this. I just wanted to drop a note of appreciation instead of something like "Seriously, when can we have this?" as I can see how hard you're following up in (OmniSharp/omnisharp-roslyn#1076) over the last 1 year.

Hope you know that there are those of us who appreciate the work and understand that it takes time.

from vscode-csharp.

Reviews pending now. I am waiting to continue with fadeout #2646 and fix all issues feature (microsoft/vscode#62110, https://github.com/dotnet/roslyn/blob/master/docs/analyzers/FixAllProvider.md) which can be made top of this 🙂

from vscode-csharp.

PR merged to omnisharp-roslyn master.

You can now test this feature with "omnisharp.path": "latest" in vscode and set omnisharp.json with:

{
    "RoslynExtensionsOptions": {
        "EnableAnalyzersSupport": true,
        "LocationPaths": [
        ]
    }
}

There will be shortcut in upcoming vscode-omnisharp release to enable this feature: #2836

If you want to test global analyzer/code action support with Roslynator project see OmniSharp/omnisharp-vim#451 (comment) with one trick which must be done before roslynator can be used as global extension.

from vscode-csharp.

Seriously, when can we have this?

from vscode-csharp.

Any progress on this lately?
Are we waiting on your work @savpek ? 👍

from vscode-csharp.

Happy holidays, i managed to get it working

However its early prototype with dirty hacks like statics on state management, i refactor it and create PR after some refactoring. Maybe even during 2017!

Its very likely that this version doesn't support yet things like loading analyzers from projects directly, currently it loads analyzers from ExternalFeaturesHostServicesProvider.

from vscode-csharp.

OmniSharp/omnisharp-roslyn#1076

from vscode-csharp.

One way is:

• Clone my current fork https://github.com/savpek/omnisharp-roslyn
• Checkout branch roslyn-analyzers
• dotnet build on branch
• Add configuration (modify path to where fork clone was built) "omnisharp.path": "C:\\Github\\omnisharp-roslyn\\bin\\Debug\\OmniSharp.Stdio.Driver\\net461\\Omnisharp.exe", to your workspace or user configuration in vscode.
• Reload vscode.

You can also add new remote to omnisharp-roslyn repo that points to my fork and checkout from there.

from vscode-csharp.

For bigger C# projects, Visual Studio Code(with omnisharp) is not a good choice. It still misses a lot of features. My recommendation is to stick with Visual Studio IDE when you're on Windows or Mac or if you're on Linux there's Rider.

I believe we will be able to replace heavyweight IDEs one day but it's not today.

If you thumbed down this comment and you have something to share I'd be happy to know. This is just my opinion based on my experience. I just think it's much much faster to work with VS or Rider(at this time). I still use VS Code for other types of projects though.

from vscode-csharp.

It is on our backlog, but there isn't a large team working on this extension. If you're passionate about this feature, we would welcome your contribution. That said, it'll require public API changes in Roslyn that I have not had the time to work on yet.

from vscode-csharp.

+1

from vscode-csharp.

great idea, we demoed this in Dallas at .NET Unboxed last year and it seemed to be a big hit back then.. now that omnisharp has moved to .NET CLI we can restart the efforts in that direction!

cc @david-driscoll

from vscode-csharp.

I will close this as it was implemented in OmniSharp/omnisharp-roslyn#1076 and shipped in 1.19.0 of the VS Code extension.
Other outstanding issues can be handled separately.

from vscode-csharp.

... if you did this, and the dotnet-cli actually stabilized, I could stop using Visual Studio 2015 Entercrap Edition with Update 1. The name itself sounds non-productive. 👍

from vscode-csharp.

Folks using vscode are now asking me for this. I might take a shot at it this weekend; depending on how painful it is — any tips on where to get started? I’m looking to go the reflection route pending updates to Roslyn to expose what’s needed in public api.

from vscode-csharp.

@savpek Thank you for the response. I tried modifying the file as suggested:

$ cat ~/.omnisharp/omnisharp.json 
{
    "RoslynExtensionsOptions": {
        "EnableAnalyzersSupport": true,
        "LocationPaths": [
        ]
    }

No warnings showing still. I did notice that VS Code updated as well as the OmniSharp extension appeared to update today. I'm not actually sure what version has this functionality in it?

OmniSharp server started.
    Path: /Users/EJ/.vscode/extensions/ms-vscode.csharp-1.18.0/.omnisharp/1.32.16/run
    PID: 77688

Other interesting section of the log. Puma is located in the list. However, it also references the 2.6.1 version of CodeAnalysis. Puma is referencing CodeAnalysis 2.9. Could that be causing the problem?

"Analyzers": [
    "/usr/local/share/dotnet/sdk/NuGetFallbackFolder/microsoft.aspnetcore.mvc.analyzers/2.2.0/analyzers/dotnet/cs/Microsoft.AspNetCore.Mvc.Analyzers.dll",
    "/Users/EJ/.nuget/packages/microsoft.codeanalysis.analyzers/2.6.1/analyzers/dotnet/cs/Microsoft.CodeAnalysis.Analyzers.dll",
    "/Users/EJ/.nuget/packages/microsoft.codeanalysis.analyzers/2.6.1/analyzers/dotnet/cs/Microsoft.CodeAnalysis.CSharp.Analyzers.dll",
    "/usr/local/share/dotnet/sdk/NuGetFallbackFolder/microsoft.entityframeworkcore.analyzers/2.2.0/analyzers/dotnet/cs/Microsoft.EntityFrameworkCore.Analyzers.dll",
    "/Users/EJ/.nuget/packages/puma.security.rules/2.1.0/analyzers/dotnet/cs/Microsoft.Web.XmlTransform.dll",
    "/Users/EJ/.nuget/packages/puma.security.rules/2.1.0/analyzers/dotnet/cs/Puma.Security.Rules.dll"
  ],

Happy to provide the full trace log if that helps.

from vscode-csharp.

The link is in a comment just a little above: #43 (comment)

from vscode-csharp.

Good idea !

This type of feature will help us during the refactoring and code fixes and many other things we need to use visual studio because it is much easier.

from vscode-csharp.

+1

from vscode-csharp.

+1, currently biggest reason to stay on Visual sStudio is lack of refactoring and analyzer tools in omnisharp/vscode combo.

This opens good way to community participate because creating analyzers and refactoring tools is easy and well documented feature of Roslyn. However if i try create new refactoring tool to Omnisharp core i don't where to start.

Another thing is that personally i prefer refactoring tooling and analyzers via plugins. Or bundle of plugins if set of them are good and usefull. As example Resharper has currently problem that it has like ten thousand refactoring/analytics bundled to it. Its very good tool indeed but because it contains all of them as default it slows it down greatly. Another problem is that they are all available at same time and realisticly one developer usually uses more like dozen of them.

from vscode-csharp.

I assume this will be revisited again once the Roslyn Project System is fully baked for .NET Core 1.1

from vscode-csharp.

To me, this issue has 3 aspects:

• global refactorings - equivalent to installing Roslyn refactoring(s) VSIX into Visual Studio
• global analyzers/code fixes - equivalent to installing Roslyn analyzer(s)/code fixe(s) VSIX into Visual Studio
• project scoped analyzers/code fixes - equivalent to installing Roslyn analyzer(s)/code fixe(s) Nuget into a project

The first part of it was implemented recently into OmniSharp and can be used with C# for VS Code already, the latter two items are still pending.

from vscode-csharp.

Is anyone working on this?

from vscode-csharp.

Is there possibility to prototype this feature without official public api with reflection from roslyn? I understood from dotnet/roslyn#19908 that current code fixes are surfaced same way without official api from roslyn.

from vscode-csharp.

@savpek: You're definitely welcome to give it a shot over in the omnisharp-roslyn repo.

from vscode-csharp.

I digged down in this issue littlebit this morning.

First i tried to load analyzer assemblies based on two year old commit from OmniSharp/omnisharp-roslyn@6618a62, however after digging deeper down and after written own version of that implementation against current omnisharp-roslyn version i started think that those non used roslyn workspace commands OnAnalyzerReferenceAdded are required if analyzers are added project basis and not globally. Am i right on this? So i abandoned this way at this point.

After that i started digging how custom code actions are loaded currently, i found that ExternalFeaturesHostServicesProvider loads them and it actually loads analyzers too. However i have no idea is this assembly load enough. Does further actions required like OnAnalyzerReferenceAdded on workspace?

Code actions are loaded and surfaced from BaseCodeActionService by reflection. I currently think that if can implement same kind of implementation to CodeCheckService they should be surfaced to editors as expexted? However at this point rabbit hole begins and need to figure out what apis i need to use / manually expose from roslyn to make this work. Anyone can throw directions for this?

Another question is that is there anywhere explained how core team develops omnisharp-roslyn in practical manner? I have to restart vscode between iterations because it locks down omnisharp dll:s and prevents building omnisharp after change. Is there some kind of shadow copy magic that can be easily used on this or how you guys archieve it? 😄

from vscode-csharp.

Am I understanding this correctly if I said this issue is what's holding back CodeAnalysis warnings from showing in vscode? I would really love to see that. Getting nice output in our console but would be extra good if it were integrated as part of the Problems panel and inline with the code as you wrote it, just like in Visual Studio 👍

from vscode-csharp.

@savpek I just mainly restart vscode or something similar. Sometimes just copy the current working version to another folder for use. It's a little clunky and could certainly be improved.

from vscode-csharp.

LOVE - YOUR - WORK !

from vscode-csharp.

@savpek <3 <3 <3 <3

from vscode-csharp.

@savpek

Superb work. This is going to boost editor (especially VSCode) capabilities from advanced editor / light IDE
closer to heavy IDE's without performance degradation.

A medal for ya! 👍

from vscode-csharp.

@savpek and where's the pull request?

from vscode-csharp.

@savpek How far is this from being added, sir?

from vscode-csharp.

@savpek How can I test your work on my VSCode instance? I tried running the latest OmniSharp build, but that did not work for some reason...

from vscode-csharp.

I'm attempting to test this in the latest version of the extension. Here's what I have done thus far:

• Configured these two switches in the settings.json file:

"omnisharp.path": "latest",
"omnisharp.loggingLevel": "trace",

• Added the Puma.Security.Rules v2.1.0 package reference to the project, restored, and confirmed the dll exists in the following location:

<PackageReference Include="Puma.Security.Rules" Version="2.1.0" />

$ ls ~/.nuget/packages/puma.security.rules/2.1.0/analyzers/dotnet/cs/
Microsoft.Web.XmlTransform.dll  Puma.Security.Rules.dll

• Created a omnisharp.json file in the home directory:

$ ls ~/.omnisharp/
omnisharp.json

• The following content added to the omnisharp.json file:

$ cat ~/.omnisharp/omnisharp.json 
{
    "RoslynExtensionsOptions": {
        "EnableAnalyzersSupport": true,
        "LocationPaths": [
            "/Users/EJ/.nuget/packages/puma.security.rules/2.1.0/analyzers/dotnet/cs"
        ]
    }
}

• Confirmed that a dotnet build produces the following warning:

Infrastructure/Startup/SeedData.cs(35,26): warning SEC0115: System.Random does not provide cryptographically random numbers. Consider using the System.Security.Cryptography.RNGCryptoServiceProvider for random values used in a security context. [/Users/EJ/git/devsecops-workshop/src/app/api/Sans.CreditUnion.API.csproj]

• No intellisense warnings are showing up in the code editor on that line of code. I found this in the Omnisharp Log data, not sure if this has anything to do with it:

[dbug]: OmniSharp.MSBuild.Discovery.MSBuildLocator
        Attempting to resolve 'Puma.Security.Rules.resources, Version=2.1.0.0, Culture=en-US, PublicKeyToken=null'
[dbug]: OmniSharp.MSBuild.Discovery.MSBuildLocator
        FAILURE: Could not locate '/Users/EJ/.vscode/extensions/ms-vscode.csharp-1.18.0/.omnisharp/3.5.0-beta.2193/omnisharp/msbuild/Current/Bin/Puma.Security.Rules.resources.dll'.
[dbug]: OmniSharp.MSBuild.Discovery.MSBuildLocator
        Attempting to resolve 'Puma.Security.Rules.resources, Version=2.1.0.0, Culture=en, PublicKeyToken=null'
[dbug]: OmniSharp.MSBuild.Discovery.MSBuildLocator
        FAILURE: Could not locate '/Users/EJ/.vscode/extensions/ms-vscode.csharp-1.18.0/.omnisharp/3.5.0-beta.2193/omnisharp/msbuild/Current/Bin/Puma.Security.Rules.resources.dll'.

Any ideas on something I may have missed for testing this?

from vscode-csharp.

@ejohn20 haven't got time to test by myself but one thing comes in mind that

        "LocationPaths": [
            "/Users/EJ/.nuget/packages/puma.security.rules/2.1.0/analyzers/dotnet/cs"
        ]

seems strange, can you replace it with

        "LocationPaths": [
        ]

If analyzers are included in nuget packages at csproj they should be automatically activated in project scope.

LocationPaths are for global analyzers/codefixes/refactorings like kind of vsix extensions in visual studio. However i don't see reason why package cannot be activated from nuget folder globally 🤔 there may be something strange going on.

I will try check this closer this week if that doesn't do the trick.

from vscode-csharp.

@savpek Another interesting note. Opened VSCode this morning and it updated again. Now shows this version:

OmniSharp server started.
    Path: /Users/EJ/.vscode/extensions/ms-vscode.csharp-1.18.0/.omnisharp/3.5.0-beta.2210/run
    PID: 10508

Still no warnings displaying in the code documents though.

from vscode-csharp.

ejohn20 yes, the same problem, we have the custom ruleset package based on stylecop and no warnings/errors in the code. I did all the recommended actions from this thread.

from vscode-csharp.

@ejohn20 moved issue to OmniSharp/omnisharp-roslyn#1472, i can repro it so just need to check with debugger whats wrong.

from vscode-csharp.

@savpek Thank you for looking into this! I'll move conversations over to the new issue.

from vscode-csharp.