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

Support roslyn analyzers and code fixes #43

Closed
egamma opened this issue Feb 25, 2016 · 41 comments
Closed

Support roslyn analyzers and code fixes #43

egamma opened this issue Feb 25, 2016 · 41 comments

Comments

@egamma
Copy link
Contributor

egamma commented Feb 25, 2016

From @giggio on February 25, 2016 19:58

It would be nice to have Roslyn analyzers and code fixes working in VSCode.

Copied from original issue: microsoft/vscode#3468

@nicolastakashi
Copy link

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.

@filipw
Copy link
Contributor

filipw commented Mar 23, 2016

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

@dmccaffery
Copy link

... 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. 👍

@alexsandro-xpt
Copy link

+1

1 similar comment
@agoretsky
Copy link

+1

@DustinCampbell DustinCampbell added this to the future milestone Jul 21, 2016
@savpek
Copy link
Contributor

savpek commented Nov 10, 2016

+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.

@dmccaffery
Copy link

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

@filipw
Copy link
Contributor

filipw commented Jun 2, 2017

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.

@RikkiGibson
Copy link
Contributor

Is anyone working on this?

@DustinCampbell
Copy link
Member

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.

@savpek
Copy link
Contributor

savpek commented Sep 13, 2017

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.

@DustinCampbell
Copy link
Member

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

@dmccaffery
Copy link

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.

@savpek
Copy link
Contributor

savpek commented Nov 25, 2017

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? 😄

@peabnuts123
Copy link

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 👍

@david-driscoll
Copy link
Contributor

@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.

@savpek
Copy link
Contributor

savpek commented Dec 23, 2017

Happy holidays, i managed to get it working
image

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.

@savpek
Copy link
Contributor

savpek commented Dec 26, 2017

OmniSharp/omnisharp-roslyn#1076

@bugproof
Copy link

bugproof commented Jul 14, 2018

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.

@savpek
Copy link
Contributor

savpek commented Aug 5, 2018

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

Analyzers / codefixes (with roslynator project):
analyzers-codefixes

Rulesets:
rulesets

@peabnuts123
Copy link

LOVE - YOUR - WORK !

@dmccaffery
Copy link

@savpek <3 <3 <3 <3

@duki994
Copy link

duki994 commented Aug 10, 2018

@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! 👍

@bugproof
Copy link

bugproof commented Sep 4, 2018

@savpek and where's the pull request?

@amis92
Copy link

amis92 commented Sep 4, 2018

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

@pgolebiowski
Copy link

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

@loligans
Copy link

loligans commented Oct 12, 2018

@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...

@savpek
Copy link
Contributor

savpek commented Oct 12, 2018

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.

@ccjx
Copy link

ccjx commented Dec 10, 2018

@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.

@savpek
Copy link
Contributor

savpek commented Feb 19, 2019

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 🙂

@savpek
Copy link
Contributor

savpek commented Mar 19, 2019

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.

@ejohn20
Copy link

ejohn20 commented Apr 9, 2019

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?

@savpek
Copy link
Contributor

savpek commented Apr 11, 2019

@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.

@ejohn20
Copy link

ejohn20 commented Apr 11, 2019

@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.

@ejohn20
Copy link

ejohn20 commented Apr 12, 2019

@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.

@ghost
Copy link

ghost commented Apr 17, 2019

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.

@savpek
Copy link
Contributor

savpek commented Apr 18, 2019

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

@ejohn20
Copy link

ejohn20 commented Apr 19, 2019

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

@filipw
Copy link
Contributor

filipw commented Apr 30, 2019

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.

@filipw filipw closed this as completed Apr 30, 2019
dibarbet added a commit to dibarbet/vscode-csharp that referenced this issue Jun 8, 2023
* Add basic test project

* Run tests in CI

* spacing

* feedback
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