search

OmniSharp / omnisharp-roslyn

OmniSharp server (HTTP, STDIO) based on Roslyn workspaces
MIT License
1.77k stars 421 forks source link

Pull methodName Up refactoring no longer available since release 1.38.2 #2381

Open JeremyCaron opened 2 years ago

JeremyCaron commented 2 years ago

Issue Description

The "Pull methodName Up" refactoring is no longer available on methods. This is one of the refactorings I use the most and have had to revert to 1.24.1 to use it.

Steps to Reproduce

Create an empty class that implements an empty interface. Add a method to the class and Ctrl-. to get the refactorings list. It should show "Pull methodName Up" but does not.

Expected Behavior

"Pull methodName Up" refactoring option should be available in the list.

Actual Behavior

Logs

OmniSharp log

Starting OmniSharp server at 4/11/2022, 9:14:31 PM Target: c:\code\allocate.co\allocate-web-api\allocate-web-api.sln OmniSharp server started with .NET 6.0.100 . Path: c:\Users\Jeremy\.vscode\extensions\ms-dotnettools.csharp-1.24.2-win32-x64\.omnisharp\1.38.2-net6.0\OmniSharp.dll PID: 6864 [info]: OmniSharp.Stdio.Host Starting OmniSharp on Windows 10.0.19043.0 (x64) [info]: OmniSharp.Services.DotNetCliService Checking the 'DOTNET_ROOT' environment variable to find a .NET SDK [info]: OmniSharp.Services.DotNetCliService Using the 'dotnet' on the PATH. [info]: OmniSharp.Services.DotNetCliService DotNetPath set to dotnet [info]: OmniSharp.MSBuild.Discovery.MSBuildLocator Located 1 MSBuild instance(s) 1: .NET Core SDK 6.0.100 17.0.0 - "C:\Program Files\dotnet\sdk\6.0.100\" [info]: OmniSharp.MSBuild.Discovery.MSBuildLocator Registered MSBuild instance: .NET Core SDK 6.0.100 17.0.0 - "C:\Program Files\dotnet\sdk\6.0.100\" [info]: OmniSharp.WorkspaceInitializer Invoking Workspace Options Provider: OmniSharp.Roslyn.CSharp.Services.CSharpFormattingWorkspaceOptionsProvider, Order: 0 [info]: OmniSharp.MSBuild.ProjectSystem ... loading lots of projects - snipped... Received response for /quickinfo but could not find request.

C# log

Installing C# dependencies... Platform: win32, x86_64 Downloading package 'OmniSharp for Windows (.NET 6 / x64)' (39624 KB).................... Done! Validating download... Integrity Check succeeded. Installing package 'OmniSharp for Windows (.NET 6 / x64)' Finished

Environment information

VSCode version: 1.66.1 C# Extension: 1.24.2

Dotnet Information .NET SDK (reflecting any global.json): Version: 6.0.100 Commit: 9e8b04bbff Runtime Environment: OS Name: Windows OS Version: 10.0.19043 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\6.0.100\ Host (useful for support): Version: 6.0.0 Commit: 4822e3c3aa .NET SDKs installed: 5.0.301 [C:\Program Files\dotnet\sdk] 6.0.100 [C:\Program Files\dotnet\sdk] .NET runtimes installed: Microsoft.AspNetCore.App 5.0.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 5.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 5.0.7 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] To install additional .NET runtimes or SDKs: https://aka.ms/dotnet-download
Visual Studio Code Extensions |Extension|Author|Version| |---|---|---| |bar|olback|0.2.0| |Bookmarks|alefragnani|13.2.4| |codestream|CodeStream|12.15.0| |copilot|GitHub|1.12.5517| |csharp|ms-dotnettools|1.24.2| |csharpextensions|jchannon|1.3.1| |csharpsortusings|jongrant|0.0.3| |debugger-for-chrome|msjsdiag|4.13.0| |docomment|k--kato|0.1.30| |dotnet-core-commands|matijarmk|1.0.6| |dotnet-core-essentials|KishoreIthadi|0.0.8| |dotnet-test-explorer|formulahendry|0.7.7| |gitlab-workflow|GitLab|3.42.0| |gitlens|eamodio|12.0.5| |jupyter|ms-toolsai|2022.3.1000901801| |markdown-pdf|yzane|1.4.4| |markdown-preview-enhanced|shd101wyy|0.6.2| |msbuild-project-tools|tintoy|0.4.3| |namespace|adrianwilczynski|1.1.2| |npm-intellisense|christian-kohler|1.4.1| |open-html-in-browser|peakchen90|2.1.9| |powershell|ms-vscode|2021.12.0| |python|ms-python|2022.4.1| |remote-containers|ms-vscode-remote|0.231.5| |remote-wsl|ms-vscode-remote|0.66.0| |sort-lines|Tyriar|1.9.1| |theme-cobalt2|wesbos|2.2.5| |todo-tree|Gruntfuggly|0.0.215| |vetur|octref|0.35.0| |vscode-coverage-gutters|ryanluker|2.9.1| |vscode-docker|ms-azuretools|1.21.0| |vscode-duplicate|mrmlnc|1.2.1| |vscode-eslint|dbaeumer|2.2.2| |vscode-markdownlint|DavidAnson|0.47.0| |vscode-npm-script|eg2|0.3.24| |vscode-nuget-package-manager|jmrog|1.1.6| |vscode-pylance|ms-python|2022.4.0|;
filipw commented 2 years ago

I will need to check this, however it was never intended to work in the first place. We even have code and tests to explicitly disable it that has been in place for 3 years now.

https://github.com/OmniSharp/omnisharp-roslyn/blob/696cf2159f89a6c9e7bbc296e6eb95e76ac916ac/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs#L105 https://github.com/OmniSharp/omnisharp-roslyn/blob/ff21ac475cdf8afb15579f2b5fb346945db2a7e9/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs#L194-L210

It might have worked "by accident" and got removed with the recent changes to how we interact with Roslyn.

filipw commented 2 years ago

possibly related to https://github.com/OmniSharp/omnisharp-roslyn/pull/2343 or https://github.com/OmniSharp/omnisharp-roslyn/pull/2363

JeremyCaron commented 2 years ago

I don't know who wrote the comment that this provides no additional value, but they could not be more wrong. If my class already implements an interface then extracting or generating a new one does nothing for me, I need to update the existing one.

tony-dwire-paradigm commented 1 year ago

I also use to use this all the time and thought it was just a bug in my setup that it disappeared. I would like to echo @JeremyCaron 's point that it makes no sense to generate a whole new interface for a type that already has one. I realize if there are multiple levels of interfaces or multiple interfaces on the type it is harder to pick, but the vast common case (at least in my 12 years of C#) is a class with a single interface that we want to pull a method up to.

JoeRobich commented 1 year ago

Testing with C# 1.24.1 and 1.24.0, I was unable to get this refactoring be offered with the following test file and the cursor at various places on the public void Run() line.

namespace TestConsole
{
    public interface IPerson
    {
    }

    public class Person : IPerson
    {
        public void Run()
        {
        }
    }
}

After looking at the Roslyn code for pull member up, I am unsure how this would have ever worked. The RefactoringProvider and CodeAction require a workspace service that has no default implementation and has not had an implementation in O#. To support this feature, we would need to expose the internal IPullMemberUpOptionsService from Roslyn through the ExternalAccess layer and provide an implementation on our end.

The open questions are what to return since we have no UI. Would a member go to a base class over an interface when both are specified? If there are multiple interfaces implemented by the class, would it go to the first specified? etc...

JeremyCaron commented 1 year ago

Thanks, I'll try rolling back too. I might have had the version numbers wrong though? filipw changed the title to say 1.38.2 on Apr 12, 2022

JoeRobich commented 1 year ago

filipw changed the title to say 1.38.2

1.38.2 is the O# version used by C# ext 1.24.2 where you saw the issue.