Support Roslyn Analyzers and Refactorings #451

Confuset · 2019-03-13T08:56:55Z

Roslyn can be Extended to support custom Code analyzers, Code fixes and refactorings like https://github.com/JosefPihrt/Roslynator
(Installation instructions https://github.com/JosefPihrt/Roslynator/blob/master/docs/RoslynatorForVisualStudioCode.md)

But in order for this to work we would need this PR OmniSharp/omnisharp-roslyn#1076 merged in omnisharp-roslyn i guess.

nickspoons · 2019-03-13T10:15:07Z

As far as I can see, this will be fully implemented in OmniSharp-roslyn, just providing more and smarter code actions, is that right? Will anything more actually be required in this repo?

savpek · 2019-03-13T10:16:42Z

I haven't tested with omnisharp-vim, but it should not require any additional changes from this plugin once available.

Confuset · 2019-03-13T10:21:28Z

i looked stuff up here
https://microsoft.github.io/language-server-protocol/specification#textDocument_codeAction
and it states

Clients need to announce their support for code action literals and code action kinds via the corresponding client capability textDocument.codeAction.codeActionLiteralSupport.

I don't know if we are already doing this…

nickspoons · 2019-03-13T10:31:38Z

The LSP spec? I don't think that's related to this is it?

Confuset · 2019-03-13T10:35:52Z

isn't Omnisharp-roslyn implementing that? or am i way off here?

nickspoons · 2019-03-13T10:49:17Z

OmniSharp-roslyn does have an LSP implementation, but it's not complete yet. However OmniSharp-vim and OmniSharp-roslyn predate LSP and still communicate using their older protocols.

For now I suggest we close this issue and wait for the OmniSharp-roslyn PR. Hopefully OmniSharp-vim will be able to use it without any further additions, and if work is needed we can create issues around that at the time.

savpek · 2019-03-13T10:54:10Z

Can someone test omnisharp-vim with omnisharp-roslyn from https://ci.appveyor.com/project/david-driscoll/omnisharp-roslyn/builds/22739985/artifacts it could be really helpfull 🙂

nickspoons · 2019-03-13T11:00:27Z

@sevpak I can tomorrow. Any OS? Anything in particular you want tested?

savpek · 2019-03-13T11:03:40Z

I think any os will do. I am interested about basics like is it working as expected, and if not I can try to fix it since this feature should not make any API changes to omnisharp-roslyn.

Confuset · 2019-03-13T12:42:20Z

I will look into this as well on Win10x64 with gvim 8.1

Confuset · 2019-03-13T13:14:52Z

mh. just replaced my Installation of omnisharp-roslyn with the new http-build linked above.
It is runnig, but i do not get additional refactorings etc. from Roslynator i think.
I may be missing something…

C:\USERS\B\.OMNISHARP
├───omnisharp-roslyn
└───roslynator
└omnisharp.json

Thats my omnisharp.json
{ "LocationPaths": [ "C:\\Users\\B\\.omnisharp\\roslynator" ] }

savpek · 2019-03-13T13:42:25Z

You need to add following flag to omnisharp json:

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

Its mentioned in thread but there's lots of message after that one 🙂

Confuset · 2019-03-13T14:36:21Z

thanks for pointing that out. i did sort of remember that, but was unable to find it :-)

my omnisharp.json now Looks like:

{
	"RoslynExtensionsOptions": 
	{
		"EnableAnalyzersSupport": true,
		"LocationPaths": [
			"C:\\Users\\B\\.omnisharp\\roslynator"
		]
	}
}

But it Fails when trying to load Roslynator.

Application startup exception: System.Reflection.ReflectionTypeLoadException: Mindestens ein Typ in der Assembly kann nicht geladen werden. Rufen Sie die LoaderExceptions-Eigenschaft ab, wenn Sie weitere Informationen benötigen.
   bei System.Reflection.RuntimeModule.GetTypes(RuntimeModule module)
   bei System.Reflection.RuntimeAssembly.get_DefinedTypes()
   bei System.Composition.Hosting.ContainerConfiguration.<>c.<WithAssemblies>b__16_0(Assembly a)
   bei System.Linq.Enumerable.<SelectManyIterator>d__17`2.MoveNext()
   bei System.Composition.TypedParts.TypedPartExportDescriptorProvider..ctor(IEnumerable`1 types, AttributedModelProvider attributeContext)
   bei System.Composition.Hosting.ContainerConfiguration.CreateContainer()
   bei Microsoft.CodeAnalysis.Host.Mef.MefHostServices.Create(IEnumerable`1 assemblies)
   bei OmniSharp.OmniSharpWorkspace..ctor(HostServicesAggregator aggregator, ILoggerFactory loggerFactory, IFileSystemWatcher fileSystemWatcher)
   bei lambda_method(Closure , LifetimeContext , CompositionOperation )
   bei System.Composition.TypedParts.ActivationFeatures.DisposalFeature.<>c__DisplayClass0_0.<RewriteActivator>b__0(LifetimeContext c, CompositionOperation o)
   bei System.Composition.Hosting.Core.LifetimeContext.GetOrCreate(Int32 sharingId, CompositionOperation operation, CompositeActivator creator)
   bei System.Composition.Hosting.Core.CompositionOperation.Run(LifetimeContext outermostLifetimeContext, CompositeActivator compositionRootActivator)
   bei System.Composition.Hosting.Core.LifetimeContext.TryGetExport(CompositionContract contract, Object& export)
   bei System.Composition.CompositionContext.GetExport(CompositionContract contract)
   bei System.Composition.CompositionContext.GetExport[TExport](String contractName)
   bei OmniSharp.Http.Startup.Configure(IApplicationBuilder app, IServiceProvider serviceProvider, ILoggerFactory loggerFactory, HttpEnvironment httpEnvironment)
--- Ende der Stapelüberwachung vom vorhergehenden Ort, an dem die Ausnahme ausgelöst wurde ---
   bei System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   bei Microsoft.AspNetCore.Hosting.ConventionBasedStartup.Configure(IApplicationBuilder app)
   bei Microsoft.AspNetCore.Hosting.Internal.WebHost.BuildApplication()
System.Reflection.ReflectionTypeLoadException: Mindestens ein Typ in der Assembly kann nicht geladen werden. Rufen Sie die LoaderExceptions-Eigenschaft ab, wenn Sie weitere Informationen benötigen.
   bei System.Reflection.RuntimeModule.GetTypes(RuntimeModule module)
   bei System.Reflection.RuntimeAssembly.get_DefinedTypes()
   bei System.Composition.Hosting.ContainerConfiguration.<>c.<WithAssemblies>b__16_0(Assembly a)
   bei System.Linq.Enumerable.<SelectManyIterator>d__17`2.MoveNext()
   bei System.Composition.TypedParts.TypedPartExportDescriptorProvider..ctor(IEnumerable`1 types, AttributedModelProvider attributeContext)
   bei System.Composition.Hosting.ContainerConfiguration.CreateContainer()
   bei Microsoft.CodeAnalysis.Host.Mef.MefHostServices.Create(IEnumerable`1 assemblies)
   bei OmniSharp.OmniSharpWorkspace..ctor(HostServicesAggregator aggregator, ILoggerFactory loggerFactory, IFileSystemWatcher fileSystemWatcher)
   bei lambda_method(Closure , LifetimeContext , CompositionOperation )
   bei System.Composition.TypedParts.ActivationFeatures.DisposalFeature.<>c__DisplayClass0_0.<RewriteActivator>b__0(LifetimeContext c, CompositionOperation o)
   bei System.Composition.Hosting.Core.LifetimeContext.GetOrCreate(Int32 sharingId, CompositionOperation operation, CompositeActivator creator)
   bei System.Composition.Hosting.Core.CompositionOperation.Run(LifetimeContext outermostLifetimeContext, CompositeActivator compositionRootActivator)
   bei System.Composition.Hosting.Core.LifetimeContext.TryGetExport(CompositionContract contract, Object& export)
   bei System.Composition.CompositionContext.GetExport(CompositionContract contract)
   bei System.Composition.CompositionContext.GetExport[TExport](String contractName)
   bei OmniSharp.Http.Startup.Configure(IApplicationBuilder app, IServiceProvider serviceProvider, ILoggerFactory loggerFactory, HttpEnvironment httpEnvironment)
--- Ende der Stapelüberwachung vom vorhergehenden Ort, an dem die Ausnahme ausgelöst wurde ---
   bei System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   bei Microsoft.AspNetCore.Hosting.ConventionBasedStartup.Configure(IApplicationBuilder app)
   bei Microsoft.AspNetCore.Hosting.Internal.WebHost.BuildApplication()
   bei Microsoft.AspNetCore.Hosting.Internal.WebHost.<StartAsync>d__26.MoveNext()
--- Ende der Stapelüberwachung vom vorhergehenden Ort, an dem die Ausnahme ausgelöst wurde ---
   bei System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   bei System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   bei Microsoft.AspNetCore.Hosting.Internal.WebHost.Start()
   bei OmniSharp.Http.Host.Start()
   bei OmniSharp.Http.Driver.Program.<>c__DisplayClass0_1.<Main>b__1()
   bei OmniSharp.HostHelpers.Start(Func`1 action)```

without roslynator it works and do get some additional codeactions.

Confuset · 2019-03-13T15:00:11Z

mh. that now disappeared for what over reason… but i do still not see the additional stuff from roslynator

nickspoons · 2019-03-13T22:51:51Z

@savpek It works very well for me out of the box, with some additional code actions and diagnostics available.

I tried Roslynator as @Confuset did, and had the same error (well, the english version :D ) . I suspect it's a roslynator error rather than omnisharp but I don't know. In my case I built roslynator myself because the download button in the VS marketplace appears to be broken.

Confuset · 2019-03-14T09:52:43Z

@nickspoons did that error message eventually disappear?
@savpek have you tried using roslynator in omnisharp-roslyn or is this even supported by your Code? just be sure we are testing something that is supposed to work :-)

Confuset · 2019-03-14T09:54:27Z

just had an idea. roslynator can also be installed into the Project/solution using nuget. i will tried that this evening

savpek · 2019-03-14T19:33:23Z

I use roslynator daily basis but i use it via nuget in csproj. Haven't tested global analyzers with roslynator in a while. Very first version of analysis service only supported global analysers and i think i used it with roslynator at some point. But i will test it during this weekend. That error looks like theres assembly missing or cannot be loaded from roslynator dll:s. Even though my deutsch is bit rusty 😄

Confuset · 2019-03-15T23:54:17Z

ok. i tried it using the nuget variant and that is working now. So i do get the additional refactorings and analyzers.
i tested some refactorings and one has an issue:

namespace ConsoleApp1
{
    internal class Program : IFoo
    {
        public Program()
        {
        }

        static void Main(string[] args)
        {
            if (args == null)
            {
                throw new System.ArgumentNullException(nameof(args));
            }
        }

        public void Test() { }
    }

    internal interface IFoo
    {
    }
}

When i try to extract IFoo into a seperate file it fails:

Also the new IFoo.cs file falsly contains:

namespace ConsoleApp1
{
    internal class Program : IFoo
    {
        public Program()
        {
        }

        static void Main(string[] args)
        {
            if (args == null)
            {
                throw new System.ArgumentNullException(nameof(args));
            }
        }

        public void Test() { }
    }
}

I think vim fails to switch to the new buffer as the old one has changed because of the interface removal and then can not complete the moving of the interface…

Confuset · 2019-03-15T23:55:20Z

a second issue i discovered is that some refactorings are listed twice. once in english and the second one in german…

axvr · 2019-03-16T00:06:23Z

@Confuset, you should be able to resolve the issue of Vim not switching buffers with :set hidden (:h 'hidden'). Vim won't let you switch buffers if you have unsaved changes by default.

savpek · 2019-03-17T12:35:59Z

@Confuset it seems you must remove .visualstudio. files from roslynator package before it can be used as global extension. Something there tries to visual studio specific stuff that omnisharp doesn't have loaded. After i removed them omnisharp-roslyn starts as expected and roslynator (RCSxxxx) analysis is enabled.

Tested on vscode though but i don't think theres difference between vim and vscode in this matter since everything releated is done at omnisharp-roslyn side.

nickspoons · 2019-03-17T23:35:35Z

@savpek yes that appears to be correct. I now have just these in my roslynator directory and it works:

roslynator/Roslynator.CSharp.Analyzers.CodeFixes.dll
roslynator/Roslynator.CSharp.Analyzers.dll
roslynator/Roslynator.CSharp.CodeFixes.dll
roslynator/Roslynator.CSharp.Refactorings.dll
roslynator/Roslynator.CSharp.Workspaces.dll
roslynator/Roslynator.CSharp.dll
roslynator/Roslynator.Common.Workspaces.dll
roslynator/Roslynator.Common.dll
roslynator/Roslynator.Core.dll
roslynator/Roslynator.Workspaces.Core.dll

Confuset · 2019-03-20T19:58:27Z

@savpek i followed the instructions given by https://github.com/JosefPihrt/Roslynator/blob/master/docs/RoslynatorForVisualStudioCode.md so i think something else was wrong, but maybe i confuset something ;-)

Confuset · 2019-03-20T20:27:55Z

@axvr yes. thats helps. but now i have the issue that i have to switch buffers to that extracted file. then i have to first :w and then :e to get roslyn to pickup that new file.
And I get this error

"Cursor position is outside of buffer"

nickspoons · 2019-03-20T20:35:50Z

@Confuset I just tried again with roslynator. The Download is working again from the VisualStudio marketplace so I followed the instructions you linked exactly and it works for me.

One thing to note is that one of the dll names is wrong - the instructions say Roslynator.Workspaces.Common.dll but that doesn't exist - the correct filename is Roslynator.Common.Workspaces.dll.

nickspoons · 2019-03-20T20:49:48Z

@Confuset the problems regarding new buffers and invalid cursor position are not related to this issue. The existing OmniSharp-roslyn code actions already had functionality for refactoring code out to a new file, and everything you've described (having to switch to the new file (you can do it with <C-6> because it is set as the alternate buffer) and :w, invalid cursor position) are problems with our existing implementation.

It would be good to get these smoothed out though, I've created a separate issue: #452

nickspoons · 2019-03-20T20:53:45Z

Having said that ... I would have expected the new file to be recognised by OmniSharp-roslyn immediately. @savpek do you know how the internals work when a code action generates code for a new file? Does OmniSharp-roslyn automatically reference the new code? In the vim client we write the generated code to the new file buffer but don't automatically save it. But we currently have to do that manually to trigger a /bufferupdate before OmniSharp-roslyn will reference the generated code correctly. We can automate this process but I don't want to do it if here if it's supposed to happen server-side.

Confuset · 2019-03-21T08:58:01Z

who should actually add that new file to the Project file?

nickspoons · 2019-03-21T09:44:21Z

@Confuset do you mean the .csproj file? I believe that that is a different issue. AFAIK OmniSharp-roslyn never adds files to .csproj files. The old OmniSharp-server (which OmniSharp-roslyn replaced) used to do it which was cool.

In more recent .csproj structures the individual .cs files don't need to be listed and I suspect OmniSharp-roslyn will never implement adding files to .csproj for this reason.

Or did I misunderstand you?

Confuset · 2019-03-22T13:57:49Z

@nickspoons That was exactly what i was asking about. so this is separate issue and unrelated to omnisharp-roslyn. i will think about it and Maybe create an issue for that, because only .net core Project files do not Need to include the files in its .csprof files.

nickspoons · 2019-03-23T11:13:42Z

@Confuset the new csproj style is not just for .NET Core - it can also be used for .NET Framework projects - see the OmniSharp-roslyn projects.

There is also an existing issue to add this functionality, OmniSharp/omnisharp-roslyn#645, but it doesn't seem to have had much attention. You could bump it and see if the current devs have any more information?

Confuset · 2019-03-26T08:25:08Z

mh. but since omnisharp-vim is the one who is actually creating that file, as far as i understand, how would the interaction between vim and roslyn Workout? So would for example omnisharp-roslyn have a function for adding files and omnisharp-vim call that? Otherwise roslyn would Need to know that there is a new file that it has to pick it up…

nickspoons · 2019-03-26T20:41:44Z

Actually OmniSharp-roslyn creates the new file, then OmniSharp-vim (or vscode or whichever editor you use) populates it with the code action response.

OmniSharp-vim can't update the .csproj - OmniSharp-vim doesn't even know where the .csproj is located, and certainly doesn't know how to format it.

nickspoons · 2019-03-26T22:53:06Z

@Confuset have a look at the new Wiki article: Working with older .csproj Projects

I don't think this should be part of OmniSharp-vim but perhaps including a guide for how to do it yourself is good enough?

nickspoons · 2019-04-02T23:34:24Z

@Confuset I'm going to close this issue as it doesn't contain anything to be done or tracked in this repo.

The new analyzer code has been merged into OmniSharp-roslyn too. Get the latest pre-release version with:

:OmniSharpInstall 'v1.32.13'

Confuset · 2019-04-03T07:56:41Z

I totally agree.
Thanks for your help on this :-)

Confuset mentioned this issue Mar 13, 2019

False positive errors after linting #450

Closed

savpek mentioned this issue Mar 19, 2019

Support roslyn analyzers and code fixes dotnet/vscode-csharp#43

Closed

nickspoons closed this as completed Apr 2, 2019

Support Roslyn Analyzers and Refactorings #451

Support Roslyn Analyzers and Refactorings #451

Comments

Confuset commented Mar 13, 2019

nickspoons commented Mar 13, 2019

savpek commented Mar 13, 2019

Confuset commented Mar 13, 2019

nickspoons commented Mar 13, 2019

Confuset commented Mar 13, 2019

nickspoons commented Mar 13, 2019

savpek commented Mar 13, 2019

nickspoons commented Mar 13, 2019

savpek commented Mar 13, 2019

Confuset commented Mar 13, 2019

Confuset commented Mar 13, 2019 • edited Loading

savpek commented Mar 13, 2019

Confuset commented Mar 13, 2019

Confuset commented Mar 13, 2019

nickspoons commented Mar 13, 2019

Confuset commented Mar 14, 2019

Confuset commented Mar 14, 2019

savpek commented Mar 14, 2019 • edited Loading

Confuset commented Mar 15, 2019 • edited Loading

Confuset commented Mar 15, 2019

axvr commented Mar 16, 2019

savpek commented Mar 17, 2019 • edited Loading

nickspoons commented Mar 17, 2019

Confuset commented Mar 20, 2019

Confuset commented Mar 20, 2019 • edited Loading

nickspoons commented Mar 20, 2019

nickspoons commented Mar 20, 2019 • edited Loading

nickspoons commented Mar 20, 2019

Confuset commented Mar 21, 2019

nickspoons commented Mar 21, 2019

Confuset commented Mar 22, 2019

nickspoons commented Mar 23, 2019

Confuset commented Mar 26, 2019

nickspoons commented Mar 26, 2019

nickspoons commented Mar 26, 2019

nickspoons commented Apr 2, 2019

Confuset commented Apr 3, 2019

Confuset commented Mar 13, 2019 •

edited

Loading

savpek commented Mar 14, 2019 •

edited

Loading

Confuset commented Mar 15, 2019 •

edited

Loading

savpek commented Mar 17, 2019 •

edited

Loading

Confuset commented Mar 20, 2019 •

edited

Loading

nickspoons commented Mar 20, 2019 •

edited

Loading