From 9b7f6cafe681a6c2cb5850ad78aef6f6a0849490 Mon Sep 17 00:00:00 2001 From: filipw Date: Thu, 1 Oct 2020 23:09:13 +0200 Subject: [PATCH 1/5] added support for Extract Class with some reasonable defaults --- ...ssOptionsServiceWorkspaceServiceFactory.cs | 19 ++++++ .../ExtractClassWorkspaceService.cs | 64 +++++++++++++++++++ ...ExtractInterfaceWorkspaceServiceFactory.cs | 2 +- .../PickMemberWorkspaceService.cs | 2 +- 4 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassOptionsServiceWorkspaceServiceFactory.cs create mode 100644 src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassWorkspaceService.cs diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassOptionsServiceWorkspaceServiceFactory.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassOptionsServiceWorkspaceServiceFactory.cs new file mode 100644 index 0000000000..f135c50464 --- /dev/null +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassOptionsServiceWorkspaceServiceFactory.cs @@ -0,0 +1,19 @@ +using System.Composition; +using System.Reflection; +using Microsoft.CodeAnalysis.Host; +using Microsoft.CodeAnalysis.Host.Mef; + +namespace OmniSharp +{ + [Shared] + [ExportWorkspaceServiceFactoryWithAssemblyQualifiedName("Microsoft.CodeAnalysis.Features", "Microsoft.CodeAnalysis.ExtractClass.IExtractClassOptionsService")] + public class ExtractClassOptionsServiceWorkspaceServiceFactory : IWorkspaceServiceFactory + { + public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) + { + // Generates proxy class to get around issue that IExtractClassOptionsService is internal at this point. + var internalType = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.ExtractClass.IExtractClassOptionsService"); + return (IWorkspaceService)typeof(DispatchProxy).GetMethod(nameof(DispatchProxy.Create)).MakeGenericMethod(internalType, typeof(ExtractClassWorkspaceService)).Invoke(null, null); + } + } +} diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassWorkspaceService.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassWorkspaceService.cs new file mode 100644 index 0000000000..ac6755058f --- /dev/null +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassWorkspaceService.cs @@ -0,0 +1,64 @@ +using System; +using System.Collections.Immutable; +using System.Linq; +using System.Reflection; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; + +namespace OmniSharp +{ + public class ExtractClassWorkspaceService : DispatchProxy + { + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + var featuresAssembly = Assembly.Load("Microsoft.CodeAnalysis.Features"); + var extractClassOptionsType = featuresAssembly.GetType("Microsoft.CodeAnalysis.ExtractClass.ExtractClassOptions"); + var extractClassMemberAnalysisResultType = featuresAssembly.GetType("Microsoft.CodeAnalysis.ExtractClass.ExtractClassMemberAnalysisResult"); + + var originalType = (INamedTypeSymbol)args[1]; + var selectedSymbol = (ISymbol)args[2]; + + var symbolsToUse = selectedSymbol == null ? originalType.GetMembers().Where(member => member switch + { + IMethodSymbol methodSymbol => methodSymbol.MethodKind == MethodKind.Ordinary, + IFieldSymbol fieldSymbol => !fieldSymbol.IsImplicitlyDeclared, + _ => member.Kind == SymbolKind.Property || member.Kind == SymbolKind.Event + }).ToArray() : new ISymbol[1] { selectedSymbol }; + + var extractClassMemberAnalysisResultImmutableArray = typeof(ImmutableArray).GetMethods() + .Where(x => x.Name == "CreateRange") + .Select(method => new + { + method, + parameters = method.GetParameters(), + genericArguments = method.GetGenericArguments() + }) + .Where(method => method.genericArguments.Length == 1 && method.parameters.Length == 1) + .Select(x => x.method) + .First() + .MakeGenericMethod(extractClassMemberAnalysisResultType).Invoke(null, new object[] + { + typeof(Enumerable).GetMethod("Cast") + .MakeGenericMethod(extractClassMemberAnalysisResultType).Invoke(null, new object[] + { + symbolsToUse.Select(symbol => Activator.CreateInstance(extractClassMemberAnalysisResultType, new object[] + { + symbol, + false // mark abstract + })) + }) + }); + + const string name = "NewBaseType"; + + var resultObject = Activator.CreateInstance(extractClassOptionsType, new object[] { + $"{name}.cs", + name, + true, // same file + extractClassMemberAnalysisResultImmutableArray + }); + + return typeof(Task).GetMethod("FromResult").MakeGenericMethod(extractClassOptionsType).Invoke(null, new[] { resultObject }); + } + } +} diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs index 3f6b63679a..d5954082b4 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs @@ -16,4 +16,4 @@ public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) return (IWorkspaceService)typeof(DispatchProxy).GetMethod(nameof(DispatchProxy.Create)).MakeGenericMethod(internalType, typeof(ExtractInterfaceWorkspaceService)).Invoke(null, null); } } -} \ No newline at end of file +} diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs index e3ce331500..a35eb50099 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs @@ -14,4 +14,4 @@ protected override object Invoke(MethodInfo targetMethod, object[] args) return Activator.CreateInstance(resultTypeInternal, new object[] { args[1], args[2] }); } } -} \ No newline at end of file +} From 8cd47bb894ada006cb73410e5698ff1f4baa8a59 Mon Sep 17 00:00:00 2001 From: filipw Date: Fri, 2 Oct 2020 14:44:25 +0200 Subject: [PATCH 2/5] added extract class tests --- .../CodeActionsWithOptionsFacts.cs | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs index 86b1de7a5a..c236fcbeac 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs @@ -15,6 +15,55 @@ public CodeActionsWithOptionsFacts(ITestOutputHelper output) { } + [Fact] + public async Task Can_extract_base_class() + { + const string code = + @"public class Class1[||] + { + public string Property { get; set; } + + public void Method() { } + }"; + const string expected = + @"public class NewBaseType + { + public string Property { get; set; } + + public void Method() { } + } + + public class Class1 : NewBaseType + { + }"; + var response = await RunRefactoringAsync(code, "Pull member(s) up to new base class..."); + AssertUtils.AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); + } + + [Fact] + public async Task Can_extract_base_class_from_specific_member() + { + const string code = + @"public class Class1 + { + public string Property[||] { get; set; } + + public void Method() { } + }"; + const string expected = + @"public class NewBaseType + { + public string Property { get; set; } + } + + public class Class1 : NewBaseType + { + public void Method() { } + }"; + var response = await RunRefactoringAsync(code, "Pull member(s) up to new base class..."); + AssertUtils.AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); + } + [Fact] public async Task Can_generate_constructor_with_default_arguments() { From 78ee64e1c81142de410cd2586da65f5b4f7412c8 Mon Sep 17 00:00:00 2001 From: filipw Date: Fri, 2 Oct 2020 15:49:04 +0200 Subject: [PATCH 3/5] added comments --- .../ExtractClassWorkspaceService.cs | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassWorkspaceService.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassWorkspaceService.cs index ac6755058f..9bc53d6a55 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassWorkspaceService.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassWorkspaceService.cs @@ -11,10 +11,21 @@ public class ExtractClassWorkspaceService : DispatchProxy { protected override object Invoke(MethodInfo targetMethod, object[] args) { + // the args correspond to the following interface method on IExtractClassOptionsService + // http://sourceroslyn.io/#Microsoft.CodeAnalysis.Features/ExtractClass/IExtractClassOptionsService.cs,30b2d7f792fbcc68 + // internal interface IExtractClassOptionsService : IWorkspaceService + // { + // Task GetExtractClassOptionsAsync(Document document, INamedTypeSymbol originalType, ISymbol? selectedMember); + // } + // if it changes, this implementation must be changed accordingly + var featuresAssembly = Assembly.Load("Microsoft.CodeAnalysis.Features"); var extractClassOptionsType = featuresAssembly.GetType("Microsoft.CodeAnalysis.ExtractClass.ExtractClassOptions"); var extractClassMemberAnalysisResultType = featuresAssembly.GetType("Microsoft.CodeAnalysis.ExtractClass.ExtractClassMemberAnalysisResult"); + // we need to create Enumerable.Cast() where ExtractClassMemberAnalysisResult is not accessible publicly + var genericCast = typeof(Enumerable).GetMethod("Cast").MakeGenericMethod(extractClassMemberAnalysisResultType); + var originalType = (INamedTypeSymbol)args[1]; var selectedSymbol = (ISymbol)args[2]; @@ -25,6 +36,7 @@ protected override object Invoke(MethodInfo targetMethod, object[] args) _ => member.Kind == SymbolKind.Property || member.Kind == SymbolKind.Event }).ToArray() : new ISymbol[1] { selectedSymbol }; + // we need to create ImmutableArray.CreateRange() where ExtractClassMemberAnalysisResult is not accessible publicly var extractClassMemberAnalysisResultImmutableArray = typeof(ImmutableArray).GetMethods() .Where(x => x.Name == "CreateRange") .Select(method => new @@ -38,26 +50,36 @@ protected override object Invoke(MethodInfo targetMethod, object[] args) .First() .MakeGenericMethod(extractClassMemberAnalysisResultType).Invoke(null, new object[] { - typeof(Enumerable).GetMethod("Cast") - .MakeGenericMethod(extractClassMemberAnalysisResultType).Invoke(null, new object[] + // at this point we have IEnumerable and need to cas to IEnumerable + // which we can then pass to ImmutableArray.CreateRange() + genericCast.Invoke(null, new object[] { + // this constrcutor corresponds to + // http://sourceroslyn.io/#Microsoft.CodeAnalysis.Features/ExtractClass/ExtractClassOptions.cs,ced9042e0a010e24 + // public ExtractClassMemberAnalysisResult(ISymbol member,bool makeAbstract) + // if it changes, this implementation must be changed accordingly symbolsToUse.Select(symbol => Activator.CreateInstance(extractClassMemberAnalysisResultType, new object[] { symbol, - false // mark abstract + false })) }) }); const string name = "NewBaseType"; + // this constrcutor corresponds to + // http://sourceroslyn.io/#Microsoft.CodeAnalysis.Features/ExtractClass/ExtractClassOptions.cs,6f65491c71285819,references + // public ExtractClassOptions(string fileName, string typeName, bool sameFile, ImmutableArray memberAnalysisResults) + // if it changes, this implementation must be changed accordingly var resultObject = Activator.CreateInstance(extractClassOptionsType, new object[] { $"{name}.cs", name, - true, // same file + true, extractClassMemberAnalysisResultImmutableArray }); + // the return type is Task return typeof(Task).GetMethod("FromResult").MakeGenericMethod(extractClassOptionsType).Invoke(null, new[] { resultObject }); } } From a9da2fccd62e496d3120b5561f34c9367d2fc0bf Mon Sep 17 00:00:00 2001 From: filipw Date: Fri, 2 Oct 2020 15:51:14 +0200 Subject: [PATCH 4/5] added note about Roslyn update --- .../CodeActionsWithOptionsFacts.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs index c236fcbeac..3cab727129 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs @@ -36,6 +36,10 @@ public void Method() { } public class Class1 : NewBaseType { }"; + + // TODO: this refactoring was renamed to 'Extract base class...' in latest Roslyn + // it needs to be updated accordingly when we pull in new Roslyn build + // https://github.com/dotnet/roslyn/commit/2d98f81de3908f39cd582d3de0c51c738c558700 var response = await RunRefactoringAsync(code, "Pull member(s) up to new base class..."); AssertUtils.AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); } @@ -60,6 +64,10 @@ public class Class1 : NewBaseType { public void Method() { } }"; + + // TODO: this refactoring was renamed to 'Extract base class...' in latest Roslyn + // it needs to be updated accordingly when we pull in new Roslyn build + // https://github.com/dotnet/roslyn/commit/2d98f81de3908f39cd582d3de0c51c738c558700 var response = await RunRefactoringAsync(code, "Pull member(s) up to new base class..."); AssertUtils.AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); } From e79d1e42d3772bd04dce85d50ede1e2beccb2a04 Mon Sep 17 00:00:00 2001 From: Filip W Date: Fri, 2 Oct 2020 19:50:22 +0200 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Joey Robichaud --- .../WorkspaceServices/ExtractClassWorkspaceService.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassWorkspaceService.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassWorkspaceService.cs index 9bc53d6a55..78ef9c9899 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassWorkspaceService.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractClassWorkspaceService.cs @@ -50,11 +50,11 @@ protected override object Invoke(MethodInfo targetMethod, object[] args) .First() .MakeGenericMethod(extractClassMemberAnalysisResultType).Invoke(null, new object[] { - // at this point we have IEnumerable and need to cas to IEnumerable + // at this point we have IEnumerable and need to cast to IEnumerable // which we can then pass to ImmutableArray.CreateRange() genericCast.Invoke(null, new object[] { - // this constrcutor corresponds to + // this constructor corresponds to // http://sourceroslyn.io/#Microsoft.CodeAnalysis.Features/ExtractClass/ExtractClassOptions.cs,ced9042e0a010e24 // public ExtractClassMemberAnalysisResult(ISymbol member,bool makeAbstract) // if it changes, this implementation must be changed accordingly @@ -68,7 +68,7 @@ protected override object Invoke(MethodInfo targetMethod, object[] args) const string name = "NewBaseType"; - // this constrcutor corresponds to + // this constructor corresponds to // http://sourceroslyn.io/#Microsoft.CodeAnalysis.Features/ExtractClass/ExtractClassOptions.cs,6f65491c71285819,references // public ExtractClassOptions(string fileName, string typeName, bool sameFile, ImmutableArray memberAnalysisResults) // if it changes, this implementation must be changed accordingly