From eea9dfc8925d75de103ae41d61efab3fd4cb5633 Mon Sep 17 00:00:00 2001 From: Savpek Date: Wed, 20 Feb 2019 17:47:51 +0200 Subject: [PATCH 01/22] Initial version of actions with options. --- global.json | 1 - .../Refactoring/V2/BaseCodeActionService.cs | 6 +--- .../HostServicesAggregator.cs | 3 ++ src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj | 1 + ...ctoryWithAssemblyQualifiedNameAttribute.cs | 26 ++++++++++++++++ .../ExtractInterfaceWorkspaceService.cs | 31 +++++++++++++++++++ ...ExtractInterfaceWorkspaceServiceFactory.cs | 23 ++++++++++++++ .../PickMemberWorkspaceService.cs | 19 ++++++++++++ .../PickMemberWorkspaceServiceFactory.cs | 20 ++++++++++++ 9 files changed, 124 insertions(+), 6 deletions(-) create mode 100644 src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs create mode 100644 src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs create mode 100644 src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs create mode 100644 src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs create mode 100644 src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs diff --git a/global.json b/global.json index 4628116978..b7b530d442 100644 --- a/global.json +++ b/global.json @@ -1,5 +1,4 @@ { "sdk": { - "version": "2.1.301" } } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index c8598a6192..ec13d546c8 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -77,11 +77,7 @@ protected async Task> GetAvailableCodeActions(I await CollectCodeFixesActions(document, span, codeActions); await CollectRefactoringActions(document, span, codeActions); - // Be sure to filter out any code actions that inherit from CodeActionWithOptions. - // This isn't a great solution and might need changing later, but every Roslyn code action - // derived from this type tries to display a dialog. For now, this is a reasonable solution. - var availableActions = ConvertToAvailableCodeAction(codeActions) - .Where(a => !a.CodeAction.GetType().GetTypeInfo().IsSubclassOf(typeof(CodeActionWithOptions))); + var availableActions = ConvertToAvailableCodeAction(codeActions); return availableActions; } diff --git a/src/OmniSharp.Roslyn/HostServicesAggregator.cs b/src/OmniSharp.Roslyn/HostServicesAggregator.cs index edbd6ef451..c0a2f39792 100644 --- a/src/OmniSharp.Roslyn/HostServicesAggregator.cs +++ b/src/OmniSharp.Roslyn/HostServicesAggregator.cs @@ -6,6 +6,7 @@ using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.Extensions.Logging; +using OmniSharp.Roslyn.WorkspaceServices; using OmniSharp.Services; namespace OmniSharp @@ -52,6 +53,8 @@ public HostServicesAggregator( } } + builder.Add(typeof(PickMemberWorkspaceServiceFactory).Assembly); + _assemblies = builder.ToImmutableArray(); } diff --git a/src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj b/src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj index a111489d6e..023d8d1f71 100644 --- a/src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj +++ b/src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj @@ -15,6 +15,7 @@ + diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs new file mode 100644 index 0000000000..fb95345b5d --- /dev/null +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs @@ -0,0 +1,26 @@ +using System; +using System.ComponentModel.Composition; +using System.Reflection; +using Microsoft.CodeAnalysis.Host.Mef; + +namespace OmniSharp.Roslyn.WorkspaceServices +{ + [MetadataAttribute] + [AttributeUsage(AttributeTargets.Class)] + public class ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute : ExportAttribute + { + public string ServiceType { get; } + public string Layer { get; } + + public ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute(string typeAssembly, string typeName, string layer = ServiceLayer.Host) + : base(typeof(IWorkspaceServiceFactory)) + { + var type = Assembly.Load(typeAssembly).GetType(typeName) + ?? throw new InvalidOperationException($"Could not resolve '{typeName} from '{typeAssembly}'"); + + Console.WriteLine($"Resolved to type: {type.AssemblyQualifiedName}"); + this.ServiceType = type.AssemblyQualifiedName; + this.Layer = layer ?? throw new ArgumentNullException(nameof(layer)); + } + } +} diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs new file mode 100644 index 0000000000..2a97b35a73 --- /dev/null +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs @@ -0,0 +1,31 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Reflection; +using System.Threading.Tasks; +using Castle.DynamicProxy; +using Microsoft.CodeAnalysis; + +namespace OmniSharp.Roslyn.WorkspaceServices +{ + public class ExtractInterfaceWorkspaceService : IInterceptor + { + public void Intercept(IInvocation invocation) + { + // IPickMember and PickMemberResults are internal types -> workaround with proxy. + // This service simply passes all members through as selected and doesn't try show UI. + // When roslyn exposes this interface and members -> get rid of this workaround. + var resultTypeInternal = Assembly.Load("Microsoft.CodeAnalysis.ExtractInterface").GetType("Microsoft.CodeAnalysis.PickMembers.PickMembersResult"); + + var resultObject = Activator.CreateInstance(resultTypeInternal, new object[] { + false, // isCancelled + ((List)invocation.Arguments[2]).ToImmutableArray(), // InterfaceMembers selected -> take all. + invocation.Arguments[3], // Interface name, use default. + $"{invocation.Arguments[3]}.cs", // FileName + 0 // to same file (enum) + }); + + invocation.ReturnValue = Task.Run(() => resultObject); + } + } +} \ No newline at end of file diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs new file mode 100644 index 0000000000..02722d0530 --- /dev/null +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs @@ -0,0 +1,23 @@ +using System.Composition; +using System.Reflection; +using Castle.DynamicProxy; +using Microsoft.CodeAnalysis.Host; +using Microsoft.CodeAnalysis.Host.Mef; + +namespace OmniSharp.Roslyn.WorkspaceServices +{ + [Shared] + [ExportWorkspaceServiceFactoryWithAssemblyQualifiedName("Microsoft.CodeAnalysis.Features", "Microsoft.CodeAnalysis.ExtractInterface.IExtractInterfaceOptionsService")] + public class ExtractInterfaceWorkspaceServiceFactory : IWorkspaceServiceFactory + { + public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) + { + // IExtractInterfaceOptions service and result types are internal -> workaround with proxy. + // This service simply passes all members through as selected and doesn't try show UI. + // When roslyn exposes this interface and members -> get rid of this workaround. + ProxyGenerator generator = new ProxyGenerator(); + var internalType = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.ExtractInterface.IExtractInterfaceOptionsService"); + return (IWorkspaceService)generator.CreateInterfaceProxyWithoutTarget(internalType, new[] { typeof(IWorkspaceService)}, new ExtractInterfaceWorkspaceService()); + } + } +} \ No newline at end of file diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs new file mode 100644 index 0000000000..3c1f0e3e3f --- /dev/null +++ b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs @@ -0,0 +1,19 @@ +using System; +using System.Reflection; +using Castle.DynamicProxy; + +namespace OmniSharp.Roslyn.WorkspaceServices +{ + public class PickMemberWorkspaceService : IInterceptor + { + public void Intercept(IInvocation invocation) + { + // IPickMember and PickMemberResults are internal types -> workaround with proxy. + // This service simply passes all members through as selected and doesn't try show UI. + // When roslyn exposes this interface and members -> get rid of this workaround. + var resultTypeInternal = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.PickMembers.PickMembersResult"); + var resultInstance = Activator.CreateInstance(resultTypeInternal, new object[] { invocation.Arguments[1], invocation.Arguments[2] }); + invocation.ReturnValue = resultInstance; + } + } +} \ No newline at end of file diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs new file mode 100644 index 0000000000..62152a0ad2 --- /dev/null +++ b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs @@ -0,0 +1,20 @@ +using System.Composition; +using System.Reflection; +using Castle.DynamicProxy; +using Microsoft.CodeAnalysis.Host; +using Microsoft.CodeAnalysis.Host.Mef; + +namespace OmniSharp.Roslyn.WorkspaceServices +{ + [Shared] + [ExportWorkspaceServiceFactoryWithAssemblyQualifiedName("Microsoft.CodeAnalysis.Features", "Microsoft.CodeAnalysis.PickMembers.IPickMembersService")] + public class PickMemberWorkspaceServiceFactory : IWorkspaceServiceFactory + { + public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) + { + ProxyGenerator generator = new ProxyGenerator(); + var internalType = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.PickMembers.IPickMembersService"); + return (IWorkspaceService)generator.CreateInterfaceProxyWithoutTarget(internalType, new[] { typeof(IWorkspaceService)}, new PickMemberWorkspaceService()); + } + } +} \ No newline at end of file From cbf1877b9219ffcf362f83c9574e0b0cf12337f4 Mon Sep 17 00:00:00 2001 From: Savpek Date: Wed, 20 Feb 2019 18:00:59 +0200 Subject: [PATCH 02/22] Fixes. --- src/OmniSharp.Roslyn/HostServicesAggregator.cs | 3 +-- ...rkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs | 4 ++-- .../WorkspaceServices/ExtractInterfaceWorkspaceService.cs | 2 +- .../ExtractInterfaceWorkspaceServiceFactory.cs | 2 +- .../WorkspaceServices/PickMemberWorkspaceService.cs | 2 +- .../WorkspaceServices/PickMemberWorkspaceServiceFactory.cs | 2 +- 6 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/OmniSharp.Roslyn/HostServicesAggregator.cs b/src/OmniSharp.Roslyn/HostServicesAggregator.cs index c0a2f39792..12a551b649 100644 --- a/src/OmniSharp.Roslyn/HostServicesAggregator.cs +++ b/src/OmniSharp.Roslyn/HostServicesAggregator.cs @@ -6,7 +6,6 @@ using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.Extensions.Logging; -using OmniSharp.Roslyn.WorkspaceServices; using OmniSharp.Services; namespace OmniSharp @@ -53,7 +52,7 @@ public HostServicesAggregator( } } - builder.Add(typeof(PickMemberWorkspaceServiceFactory).Assembly); + builder.Add(typeof(PickMemberWorkspaceService).Assembly); _assemblies = builder.ToImmutableArray(); } diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs index fb95345b5d..4eaa64c037 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs @@ -1,9 +1,9 @@ using System; -using System.ComponentModel.Composition; +using System.Composition; using System.Reflection; using Microsoft.CodeAnalysis.Host.Mef; -namespace OmniSharp.Roslyn.WorkspaceServices +namespace OmniSharp { [MetadataAttribute] [AttributeUsage(AttributeTargets.Class)] diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs index 2a97b35a73..c9b9b50629 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs @@ -6,7 +6,7 @@ using Castle.DynamicProxy; using Microsoft.CodeAnalysis; -namespace OmniSharp.Roslyn.WorkspaceServices +namespace OmniSharp { public class ExtractInterfaceWorkspaceService : IInterceptor { diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs index 02722d0530..2a84ec0f19 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs @@ -4,7 +4,7 @@ using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; -namespace OmniSharp.Roslyn.WorkspaceServices +namespace OmniSharp { [Shared] [ExportWorkspaceServiceFactoryWithAssemblyQualifiedName("Microsoft.CodeAnalysis.Features", "Microsoft.CodeAnalysis.ExtractInterface.IExtractInterfaceOptionsService")] diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs index 3c1f0e3e3f..b54362a9a1 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs @@ -2,7 +2,7 @@ using System.Reflection; using Castle.DynamicProxy; -namespace OmniSharp.Roslyn.WorkspaceServices +namespace OmniSharp { public class PickMemberWorkspaceService : IInterceptor { diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs index 62152a0ad2..b900d6de42 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs @@ -4,7 +4,7 @@ using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; -namespace OmniSharp.Roslyn.WorkspaceServices +namespace OmniSharp { [Shared] [ExportWorkspaceServiceFactoryWithAssemblyQualifiedName("Microsoft.CodeAnalysis.Features", "Microsoft.CodeAnalysis.PickMembers.IPickMembersService")] From 70d3dc98f64cc93b5e1c853905219c49bf7f5b62 Mon Sep 17 00:00:00 2001 From: Savpek Date: Thu, 21 Feb 2019 19:31:00 +0200 Subject: [PATCH 03/22] Fixes for extract interface. --- .../ExtractInterfaceWorkspaceService.cs | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs index c9b9b50629..b88c47060d 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Linq; using System.Reflection; using System.Threading.Tasks; using Castle.DynamicProxy; @@ -15,17 +16,33 @@ public void Intercept(IInvocation invocation) // IPickMember and PickMemberResults are internal types -> workaround with proxy. // This service simply passes all members through as selected and doesn't try show UI. // When roslyn exposes this interface and members -> get rid of this workaround. - var resultTypeInternal = Assembly.Load("Microsoft.CodeAnalysis.ExtractInterface").GetType("Microsoft.CodeAnalysis.PickMembers.PickMembersResult"); + var resultTypeInternal = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.ExtractInterface.ExtractInterfaceOptionsResult"); + var enumType = resultTypeInternal.GetNestedTypes().Single(x => x.Name == "ExtractLocation"); + + var toSameFileEnumValue = Enum.Parse(enumType, "SameFile"); + + var interfaceName = invocation.Arguments[3] ?? throw new InvalidOperationException($"{nameof(ExtractInterfaceWorkspaceService)} default interface name was null."); var resultObject = Activator.CreateInstance(resultTypeInternal, new object[] { false, // isCancelled - ((List)invocation.Arguments[2]).ToImmutableArray(), // InterfaceMembers selected -> take all. - invocation.Arguments[3], // Interface name, use default. - $"{invocation.Arguments[3]}.cs", // FileName - 0 // to same file (enum) + ((List)invocation.Arguments[2]).ToImmutableArray(), // InterfaceMembers selected -> select all. + interfaceName, + $"{interfaceName}.cs", + toSameFileEnumValue }); - invocation.ReturnValue = Task.Run(() => resultObject); + //Type taskGenericType = MakeGenerictTaskTypeFromResult(resultTypeInternal); + var fromResultMethod = typeof(Task).GetMethod("FromResult").MakeGenericMethod(resultTypeInternal).Invoke(null, new[] { resultObject }); + invocation.ReturnValue = fromResultMethod; + //invocation.ReturnValue = taskGenericType.GetMethod("FromResult").Invoke(null, new[] { resultObject }); + } + + private static Type MakeGenerictTaskTypeFromResult(Type resultTypeInternal) + { + var taskType = typeof(Task<>); + Type[] typeArgs = { resultTypeInternal }; + var taskGenericType = taskType.MakeGenericType(typeArgs); + return taskGenericType; } } -} \ No newline at end of file +} From 9d95a01289573476fe83b5d44a843da2aa67cc7d Mon Sep 17 00:00:00 2001 From: Savpek Date: Thu, 21 Feb 2019 19:46:25 +0200 Subject: [PATCH 04/22] First test. --- .../ExtractInterfaceWorkspaceService.cs | 2 +- ...ExtractInterfaceWorkspaceServiceFactory.cs | 2 +- .../PickMemberWorkspaceService.cs | 2 +- .../CodeActionsWithOptionsFacts.cs | 140 ++++++++++++++++++ 4 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs index b88c47060d..b77e2117f4 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs @@ -15,7 +15,7 @@ public void Intercept(IInvocation invocation) { // IPickMember and PickMemberResults are internal types -> workaround with proxy. // This service simply passes all members through as selected and doesn't try show UI. - // When roslyn exposes this interface and members -> get rid of this workaround. + // When roslyn exposes this interface and members -> remove this workaround. var resultTypeInternal = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.ExtractInterface.ExtractInterfaceOptionsResult"); var enumType = resultTypeInternal.GetNestedTypes().Single(x => x.Name == "ExtractLocation"); diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs index 2a84ec0f19..a404581da9 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs @@ -14,7 +14,7 @@ public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) { // IExtractInterfaceOptions service and result types are internal -> workaround with proxy. // This service simply passes all members through as selected and doesn't try show UI. - // When roslyn exposes this interface and members -> get rid of this workaround. + // When roslyn exposes this interface and members -> remove this workaround. ProxyGenerator generator = new ProxyGenerator(); var internalType = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.ExtractInterface.IExtractInterfaceOptionsService"); return (IWorkspaceService)generator.CreateInterfaceProxyWithoutTarget(internalType, new[] { typeof(IWorkspaceService)}, new ExtractInterfaceWorkspaceService()); diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs index b54362a9a1..a1a38ff518 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs @@ -10,7 +10,7 @@ public void Intercept(IInvocation invocation) { // IPickMember and PickMemberResults are internal types -> workaround with proxy. // This service simply passes all members through as selected and doesn't try show UI. - // When roslyn exposes this interface and members -> get rid of this workaround. + // When roslyn exposes this interface and members -> remove this workaround. var resultTypeInternal = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.PickMembers.PickMembersResult"); var resultInstance = Activator.CreateInstance(resultTypeInternal, new object[] { invocation.Arguments[1], invocation.Arguments[2] }); invocation.ReturnValue = resultInstance; diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs new file mode 100644 index 0000000000..5857acaf97 --- /dev/null +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs @@ -0,0 +1,140 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using OmniSharp.Models; +using OmniSharp.Models.V2; +using OmniSharp.Models.V2.CodeActions; +using OmniSharp.Roslyn.CSharp.Services.Refactoring.V2; +using TestUtility; +using Xunit; +using Xunit.Abstractions; + +namespace OmniSharp.Roslyn.CSharp.Tests +{ + public class CodeActionsWithOptionsFacts : AbstractTestFixture + { + private readonly string BufferPath = $"{Path.DirectorySeparatorChar}somepath{Path.DirectorySeparatorChar}buffer.cs"; + + public CodeActionsWithOptionsFacts(ITestOutputHelper output) + : base(output) + { + } + + [Fact] + public async Task Can_generate_constructor() + { + const string code = + @"public class Class1[||] + { + public string PropertyHere { get; set; } + }"; + const string expected = + @"public class Class1[||] + { + public Class1(string propertyHere) + { + PropertyHere = propertyHere; + } + + public string PropertyHere { get; set; } + } + "; + var response = await RunRefactoringAsync(code, "Generate constructor..."); + AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); + } + + private static void AssertIgnoringIndent(string expected, string actual) + { + Assert.Equal(TrimLines(expected), TrimLines(actual), false, true, true); + } + + private static string TrimLines(string source) + { + return string.Join("\n", source.Split('\n').Select(s => s.Trim())); + } + + private async Task RunRefactoringAsync(string code, string refactoringName, bool wantsChanges = false) + { + var refactorings = await FindRefactoringsAsync(code); + Assert.Contains(refactoringName, refactorings.Select(a => a.Name)); + + var identifier = refactorings.First(action => action.Name.Equals(refactoringName)).Identifier; + return await RunRefactoringsAsync(code, identifier, wantsChanges); + } + + private async Task> FindRefactoringNamesAsync(string code) + { + var codeActions = await FindRefactoringsAsync(code); + + return codeActions.Select(a => a.Name); + } + + private async Task> FindRefactoringsAsync(string code, IDictionary configurationData = null) + { + var testFile = new TestFile(BufferPath, code); + + using (var host = CreateOmniSharpHost(new[] { testFile }, configurationData)) + { + var requestHandler = host.GetRequestHandler(OmniSharpEndpoints.V2.GetCodeActions); + + var span = testFile.Content.GetSpans().Single(); + var range = testFile.Content.GetRangeFromSpan(span); + + var request = new GetCodeActionsRequest + { + Line = range.Start.Line, + Column = range.Start.Offset, + FileName = BufferPath, + Buffer = testFile.Content.Code, + Selection = GetSelection(range), + }; + + var response = await requestHandler.Handle(request); + + return response.CodeActions; + } + } + + private async Task RunRefactoringsAsync(string code, string identifier, bool wantsChanges = false) + { + var testFile = new TestFile(BufferPath, code); + + using (var host = CreateOmniSharpHost(testFile)) + { + var requestHandler = host.GetRequestHandler(OmniSharpEndpoints.V2.RunCodeAction); + + var span = testFile.Content.GetSpans().Single(); + var range = testFile.Content.GetRangeFromSpan(span); + + var request = new RunCodeActionRequest + { + Line = range.Start.Line, + Column = range.Start.Offset, + Selection = GetSelection(range), + FileName = BufferPath, + Buffer = testFile.Content.Code, + Identifier = identifier, + WantsTextChanges = wantsChanges, + WantsAllCodeActionOperations = true + }; + + return await requestHandler.Handle(request); + } + } + + private static Range GetSelection(TextRange range) + { + if (range.IsEmpty) + { + return null; + } + + return new Range + { + Start = new Point { Line = range.Start.Line, Column = range.Start.Offset }, + End = new Point { Line = range.End.Line, Column = range.End.Offset } + }; + } + } +} From 84224b5b2c84eaec143a31b09a09502ae2fbc701 Mon Sep 17 00:00:00 2001 From: Savpek Date: Thu, 21 Feb 2019 21:04:17 +0200 Subject: [PATCH 05/22] More tests. --- .../CodeActionsWithOptionsFacts.cs | 119 +++++++++++++++++- 1 file changed, 116 insertions(+), 3 deletions(-) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs index 5857acaf97..dc956eb8e6 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs @@ -22,7 +22,7 @@ public CodeActionsWithOptionsFacts(ITestOutputHelper output) } [Fact] - public async Task Can_generate_constructor() + public async Task Can_generate_constructor_with_default_arguments() { const string code = @"public class Class1[||] @@ -30,7 +30,7 @@ public async Task Can_generate_constructor() public string PropertyHere { get; set; } }"; const string expected = - @"public class Class1[||] + @"public class Class1 { public Class1(string propertyHere) { @@ -44,6 +44,119 @@ public Class1(string propertyHere) AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); } + [Fact] + public async Task Can_generate_overrides() + { + const string code = + @"public class Class1[||] + { + }"; + const string expected = + @"public class Class1 + { + public override bool Equals(object obj) + { + return base.Equals(obj); + } + + public override int GetHashCode() + { + return base.GetHashCode(); + } + + public override string ToString() + { + return base.ToString(); + } + } + "; + var response = await RunRefactoringAsync(code, "Generate overrides..."); + AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); + } + + [Fact] + public async Task Can_generate_equals_for_object() + { + const string code = + @"public class Class1[||] + { + public string PropertyHere { get; set; } + }"; + const string expected = + @"public class Class1 + { + public string PropertyHere { get; set; } + + public override bool Equals(object obj) + { + return obj is Class1 @class && + PropertyHere == @class.PropertyHere; + } + } + "; + var response = await RunRefactoringAsync(code, "Generate Equals(object)..."); + AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); + } + + [Fact] + public async Task Can_generate_equals_and_hashcode_for_object() + { + const string code = + @"public class Class1[||] + { + public string PropertyHere { get; set; } + }"; + + // Be aware that if used with newer .NET framework than omnisharp uses (4.6). + // this will result more modern result with HashCode.Combine(PropertyHere); + const string expected = + @" + using System.Collections.Generic; + public class Class1 + { + public string PropertyHere { get; set; } + public override bool Equals(object obj) + { + return obj is Class1 @class && + PropertyHere == @class.PropertyHere; + } + public override int GetHashCode() + { + return 1887327142 + EqualityComparer.Default.GetHashCode(PropertyHere); + } + } + "; + var response = await RunRefactoringAsync(code, "Generate Equals and GetHashCode..."); + + AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); + } + + [Fact(Skip = "This feature isn't available before roslyn analyzers are available, extract interface is action to one of analysis.")] + public async Task Can_extract_interface() + { + const string code = + @"public class Class1[||] + { + public string PropertyHere { get; set; } + }"; + + const string expected = + @" + public interface IClass1 + { + string PropertyHere { get; set; } + } + + public class Class1 + { + public string PropertyHere { get; set; } + } + "; + var response = await RunRefactoringAsync(code, "Extract interface..."); + + AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); + } + private static void AssertIgnoringIndent(string expected, string actual) { Assert.Equal(TrimLines(expected), TrimLines(actual), false, true, true); @@ -51,7 +164,7 @@ private static void AssertIgnoringIndent(string expected, string actual) private static string TrimLines(string source) { - return string.Join("\n", source.Split('\n').Select(s => s.Trim())); + return string.Join("\n", source.Split('\n').Select(s => s.Trim())).Replace("\n","").Replace("\r",""); } private async Task RunRefactoringAsync(string code, string refactoringName, bool wantsChanges = false) From 543795913ef153d662f7588291b650766efc2d21 Mon Sep 17 00:00:00 2001 From: Savpek Date: Thu, 21 Feb 2019 21:12:25 +0200 Subject: [PATCH 06/22] Tweaks and comments. --- ...kspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs | 2 ++ .../WorkspaceServices/ExtractInterfaceWorkspaceService.cs | 5 ++--- .../ExtractInterfaceWorkspaceServiceFactory.cs | 4 +--- .../WorkspaceServices/PickMemberWorkspaceServiceFactory.cs | 1 + 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs index 4eaa64c037..b3141a4580 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs @@ -12,6 +12,8 @@ public class ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute : E public string ServiceType { get; } public string Layer { get; } + // Theres builtin public attribute for this, but since we target internal types + // this is needed to build service. MEF doesn't care is it internal or not. public ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute(string typeAssembly, string typeName, string layer = ServiceLayer.Host) : base(typeof(IWorkspaceServiceFactory)) { diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs index b77e2117f4..ad65c02187 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs @@ -13,9 +13,10 @@ public class ExtractInterfaceWorkspaceService : IInterceptor { public void Intercept(IInvocation invocation) { - // IPickMember and PickMemberResults are internal types -> workaround with proxy. + // IExtractInterfaceOptionsService and extract interface results are internal types -> workaround with proxy. // This service simply passes all members through as selected and doesn't try show UI. // When roslyn exposes this interface and members -> remove this workaround. + var resultTypeInternal = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.ExtractInterface.ExtractInterfaceOptionsResult"); var enumType = resultTypeInternal.GetNestedTypes().Single(x => x.Name == "ExtractLocation"); @@ -31,10 +32,8 @@ public void Intercept(IInvocation invocation) toSameFileEnumValue }); - //Type taskGenericType = MakeGenerictTaskTypeFromResult(resultTypeInternal); var fromResultMethod = typeof(Task).GetMethod("FromResult").MakeGenericMethod(resultTypeInternal).Invoke(null, new[] { resultObject }); invocation.ReturnValue = fromResultMethod; - //invocation.ReturnValue = taskGenericType.GetMethod("FromResult").Invoke(null, new[] { resultObject }); } private static Type MakeGenerictTaskTypeFromResult(Type resultTypeInternal) diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs index a404581da9..1202b93dfc 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs @@ -12,9 +12,7 @@ public class ExtractInterfaceWorkspaceServiceFactory : IWorkspaceServiceFactory { public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) { - // IExtractInterfaceOptions service and result types are internal -> workaround with proxy. - // This service simply passes all members through as selected and doesn't try show UI. - // When roslyn exposes this interface and members -> remove this workaround. + // Generates proxy class to get around issue that IExtractInterfaceOptionsService is internal at this point. ProxyGenerator generator = new ProxyGenerator(); var internalType = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.ExtractInterface.IExtractInterfaceOptionsService"); return (IWorkspaceService)generator.CreateInterfaceProxyWithoutTarget(internalType, new[] { typeof(IWorkspaceService)}, new ExtractInterfaceWorkspaceService()); diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs index b900d6de42..e932af9479 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs @@ -12,6 +12,7 @@ public class PickMemberWorkspaceServiceFactory : IWorkspaceServiceFactory { public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) { + // Generates proxy class to get around issue that IPickMembersService is internal at this point. ProxyGenerator generator = new ProxyGenerator(); var internalType = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.PickMembers.IPickMembersService"); return (IWorkspaceService)generator.CreateInterfaceProxyWithoutTarget(internalType, new[] { typeof(IWorkspaceService)}, new PickMemberWorkspaceService()); From 761f65756be3f1ca229f25b9f24b0b911bc0b277 Mon Sep 17 00:00:00 2001 From: Savpek Date: Thu, 21 Feb 2019 21:28:00 +0200 Subject: [PATCH 07/22] Original global.json --- global.json | 1 + 1 file changed, 1 insertion(+) diff --git a/global.json b/global.json index b7b530d442..4628116978 100644 --- a/global.json +++ b/global.json @@ -1,4 +1,5 @@ { "sdk": { + "version": "2.1.301" } } From ca9ecb3f541009d1207f9c9237af07891888e722 Mon Sep 17 00:00:00 2001 From: Savpek Date: Sat, 23 Feb 2019 20:43:32 +0200 Subject: [PATCH 08/22] Blacklisting. --- .../Services/Refactoring/V2/BaseCodeActionService.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index ec13d546c8..6a8238d565 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -79,7 +79,16 @@ protected async Task> GetAvailableCodeActions(I var availableActions = ConvertToAvailableCodeAction(codeActions); - return availableActions; + return FilterBlacklistedCodeActions(availableActions); + } + + private static IEnumerable FilterBlacklistedCodeActions(IEnumerable codeActions) + { + // Most of actions with UI works fine with defaults, however there's few exceptions: + return codeActions.Where(x => + x.CodeAction.Title != "Generate new type..." && // Blacklisted because doesn't give additional value over non UI generate type (when defaults used.) + x.CodeAction.Title != "Change signature..." // Blacklisted because cannot be used without proper UI. + ); } private TextSpan GetTextSpan(ICodeActionRequest request, SourceText sourceText) From 8a298e56f3b5f72af2e82ec993e8a8fa6e3962b4 Mon Sep 17 00:00:00 2001 From: Savpek Date: Sun, 24 Feb 2019 11:58:00 +0200 Subject: [PATCH 09/22] Added test to keep track is official support added. --- src/OmniSharp.Host/WorkspaceInitializer.cs | 2 +- .../CodeActionsWithOptionsFacts.cs | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/OmniSharp.Host/WorkspaceInitializer.cs b/src/OmniSharp.Host/WorkspaceInitializer.cs index d76d804b24..a3de9496fd 100644 --- a/src/OmniSharp.Host/WorkspaceInitializer.cs +++ b/src/OmniSharp.Host/WorkspaceInitializer.cs @@ -26,7 +26,7 @@ public static void Initialize(IServiceProvider serviceProvider, CompositionHost var projectEventForwarder = compositionHost.GetExport(); projectEventForwarder.Initialize(); var projectSystems = compositionHost.GetExports(); - + foreach (var projectSystem in projectSystems) { try diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs index dc956eb8e6..27e05836f7 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs @@ -1,6 +1,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Reflection; using System.Threading.Tasks; using OmniSharp.Models; using OmniSharp.Models.V2; @@ -157,6 +158,24 @@ public class Class1 AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); } + [Fact] + public void Check_better_alternative_available_for_codeaction_with_options() + { + // This keeps record is Microsoft.CodeAnalysis.PickMembers.IPickMembersService public or not + // and should fail on case where interface is exposed. Which likely means that this is officially + // supported scenario by roslyn. + // + // In case it's exposed by roslyn team these services can be simplified. + // Steps are likely following: + // - Remove Castle.Core (proxies not needed) + // - Replace ExportWorkspaceServiceFactoryWithAssemblyQualifiedName with Microsoft.CodeAnalysis.Host.Mef.ExportWorkspaceServiceAttribute + // - Fix proxy classes to implement IPickMembersService / IExtractInterfaceOptionsService ... instead of proxy and reflection. + // - Remove all factories using ExportWorkspaceServiceFactoryWithAssemblyQualifiedName and factory itself. + // Following issue may have additional information: https://github.com/dotnet/roslyn/issues/33277 + var pickMemberServiceType = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.PickMembers.IPickMembersService"); + Assert.False(pickMemberServiceType.IsPublic); + } + private static void AssertIgnoringIndent(string expected, string actual) { Assert.Equal(TrimLines(expected), TrimLines(actual), false, true, true); From c773575a2001d715c6bd79c35daabdccae4a558b Mon Sep 17 00:00:00 2001 From: Savpek Date: Sun, 24 Feb 2019 13:41:55 +0200 Subject: [PATCH 10/22] Tests for blacklisting. --- .../Refactoring/V2/BaseCodeActionService.cs | 3 +- .../CodeActionsWithOptionsFacts.cs | 50 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index 6a8238d565..e15da93f94 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -87,7 +87,8 @@ private static IEnumerable FilterBlacklistedCodeActions(IEn // Most of actions with UI works fine with defaults, however there's few exceptions: return codeActions.Where(x => x.CodeAction.Title != "Generate new type..." && // Blacklisted because doesn't give additional value over non UI generate type (when defaults used.) - x.CodeAction.Title != "Change signature..." // Blacklisted because cannot be used without proper UI. + x.CodeAction.Title != "Change signature..." && // Blacklisted because cannot be used without proper UI. + x.CodeAction.Title != "Pull members up to base type..." ); } diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs index 27e05836f7..dcb4a04193 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs @@ -132,6 +132,56 @@ public override int GetHashCode() AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); } + [Fact] + // How to implement this witout proper UI? + public async Task Blacklists_Change_signature() + { + const string code = + @"public class Class1 + { + public void Foo(string a[||], string b) + { + } + }"; + + var response = await FindRefactoringNamesAsync(code); + + Assert.DoesNotContain("Change signature...", response); + } + + [Fact] + // Theres generate type without options that gives ~same result with default ui. No point to bring this. + public async Task Blacklists_generate_type_with_UI() + { + const string code = + @"public class Class1: NonExistentBaseType[||] + { + }"; + + var response = await FindRefactoringNamesAsync(code); + + Assert.DoesNotContain("Generate type 'NonExistentBaseType' -> Generate new type...", response); + } + + [Fact] + // Theres generate type without options that gives ~same result with default ui. No point to bring this. + public async Task Blacklists_pull_members_up_with_UI() + { + const string code = + @" + public class Class1: BaseClass + { + public string Foo[||] { get; set; } + } + + public class BaseClass {} +"; + + var response = await FindRefactoringNamesAsync(code); + + Assert.DoesNotContain("Pull 'Foo' up -> Pull members up to base type...", response); + } + [Fact(Skip = "This feature isn't available before roslyn analyzers are available, extract interface is action to one of analysis.")] public async Task Can_extract_interface() { From f17aeeb0a51efafc70536d9936b4a365d3a90309 Mon Sep 17 00:00:00 2001 From: Savpek Date: Sun, 24 Feb 2019 13:45:08 +0200 Subject: [PATCH 11/22] small comment tweak. --- .../Services/Refactoring/V2/BaseCodeActionService.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index e15da93f94..13bcc31144 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -86,9 +86,9 @@ private static IEnumerable FilterBlacklistedCodeActions(IEn { // Most of actions with UI works fine with defaults, however there's few exceptions: return codeActions.Where(x => - x.CodeAction.Title != "Generate new type..." && // Blacklisted because doesn't give additional value over non UI generate type (when defaults used.) - x.CodeAction.Title != "Change signature..." && // Blacklisted because cannot be used without proper UI. - x.CodeAction.Title != "Pull members up to base type..." + x.CodeAction.Title != "Generate new type..." && // Blacklisted because doesn't give additional value over non UI generate type (when defaults used.) + x.CodeAction.Title != "Change signature..." && // Blacklisted because cannot be used without proper UI. + x.CodeAction.Title != "Pull members up to base type..." // Blacklisted because doesn't give additional value over non UI generate type (when defaults used.) ); } From 4659787c81b721befc8fe44fc8022aaf5b66dfc2 Mon Sep 17 00:00:00 2001 From: Savpek Date: Wed, 20 Mar 2019 19:12:42 +0200 Subject: [PATCH 12/22] Moved to dispatch proxy. --- src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj | 2 +- .../ExtractInterfaceWorkspaceService.cs | 19 +++++-------------- ...ExtractInterfaceWorkspaceServiceFactory.cs | 4 +--- .../PickMemberWorkspaceService.cs | 8 +++----- .../PickMemberWorkspaceServiceFactory.cs | 4 +--- .../CodeActionsWithOptionsFacts.cs | 2 +- 6 files changed, 12 insertions(+), 27 deletions(-) diff --git a/src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj b/src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj index 1a23769ea7..c3b5cb8791 100644 --- a/src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj +++ b/src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj @@ -16,7 +16,7 @@ - + diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs index ad65c02187..a6f3ded726 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs @@ -4,14 +4,13 @@ using System.Linq; using System.Reflection; using System.Threading.Tasks; -using Castle.DynamicProxy; using Microsoft.CodeAnalysis; namespace OmniSharp { - public class ExtractInterfaceWorkspaceService : IInterceptor + public class ExtractInterfaceWorkspaceService : DispatchProxy { - public void Intercept(IInvocation invocation) + protected override object Invoke(MethodInfo targetMethod, object[] args) { // IExtractInterfaceOptionsService and extract interface results are internal types -> workaround with proxy. // This service simply passes all members through as selected and doesn't try show UI. @@ -22,26 +21,18 @@ public void Intercept(IInvocation invocation) var toSameFileEnumValue = Enum.Parse(enumType, "SameFile"); - var interfaceName = invocation.Arguments[3] ?? throw new InvalidOperationException($"{nameof(ExtractInterfaceWorkspaceService)} default interface name was null."); + var interfaceName = args[3] ?? throw new InvalidOperationException($"{nameof(ExtractInterfaceWorkspaceService)} default interface name was null."); var resultObject = Activator.CreateInstance(resultTypeInternal, new object[] { false, // isCancelled - ((List)invocation.Arguments[2]).ToImmutableArray(), // InterfaceMembers selected -> select all. + ((List)args[2]).ToImmutableArray(), // InterfaceMembers selected -> select all. interfaceName, $"{interfaceName}.cs", toSameFileEnumValue }); var fromResultMethod = typeof(Task).GetMethod("FromResult").MakeGenericMethod(resultTypeInternal).Invoke(null, new[] { resultObject }); - invocation.ReturnValue = fromResultMethod; - } - - private static Type MakeGenerictTaskTypeFromResult(Type resultTypeInternal) - { - var taskType = typeof(Task<>); - Type[] typeArgs = { resultTypeInternal }; - var taskGenericType = taskType.MakeGenericType(typeArgs); - return taskGenericType; + return resultObject; } } } diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs index 1202b93dfc..3f6b63679a 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceServiceFactory.cs @@ -1,6 +1,5 @@ using System.Composition; using System.Reflection; -using Castle.DynamicProxy; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; @@ -13,9 +12,8 @@ public class ExtractInterfaceWorkspaceServiceFactory : IWorkspaceServiceFactory public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) { // Generates proxy class to get around issue that IExtractInterfaceOptionsService is internal at this point. - ProxyGenerator generator = new ProxyGenerator(); var internalType = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.ExtractInterface.IExtractInterfaceOptionsService"); - return (IWorkspaceService)generator.CreateInterfaceProxyWithoutTarget(internalType, new[] { typeof(IWorkspaceService)}, new ExtractInterfaceWorkspaceService()); + 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 a1a38ff518..e3ce331500 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceService.cs @@ -1,19 +1,17 @@ using System; using System.Reflection; -using Castle.DynamicProxy; namespace OmniSharp { - public class PickMemberWorkspaceService : IInterceptor + public class PickMemberWorkspaceService : DispatchProxy { - public void Intercept(IInvocation invocation) + protected override object Invoke(MethodInfo targetMethod, object[] args) { // IPickMember and PickMemberResults are internal types -> workaround with proxy. // This service simply passes all members through as selected and doesn't try show UI. // When roslyn exposes this interface and members -> remove this workaround. var resultTypeInternal = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.PickMembers.PickMembersResult"); - var resultInstance = Activator.CreateInstance(resultTypeInternal, new object[] { invocation.Arguments[1], invocation.Arguments[2] }); - invocation.ReturnValue = resultInstance; + return Activator.CreateInstance(resultTypeInternal, new object[] { args[1], args[2] }); } } } \ No newline at end of file diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs index e932af9479..ae2ed63200 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/PickMemberWorkspaceServiceFactory.cs @@ -1,6 +1,5 @@ using System.Composition; using System.Reflection; -using Castle.DynamicProxy; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; @@ -13,9 +12,8 @@ public class PickMemberWorkspaceServiceFactory : IWorkspaceServiceFactory public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) { // Generates proxy class to get around issue that IPickMembersService is internal at this point. - ProxyGenerator generator = new ProxyGenerator(); var internalType = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.PickMembers.IPickMembersService"); - return (IWorkspaceService)generator.CreateInterfaceProxyWithoutTarget(internalType, new[] { typeof(IWorkspaceService)}, new PickMemberWorkspaceService()); + return (IWorkspaceService)typeof(DispatchProxy).GetMethod(nameof(DispatchProxy.Create)).MakeGenericMethod(internalType, typeof(PickMemberWorkspaceService)).Invoke(null, null); } } } \ No newline at end of file diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs index dcb4a04193..c0eb98e79b 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs @@ -182,7 +182,7 @@ public class BaseClass {} Assert.DoesNotContain("Pull 'Foo' up -> Pull members up to base type...", response); } - [Fact(Skip = "This feature isn't available before roslyn analyzers are available, extract interface is action to one of analysis.")] + [Fact] public async Task Can_extract_interface() { const string code = From 5690dc12669f3eeb2952b528755f773f0df5f38b Mon Sep 17 00:00:00 2001 From: Savpek Date: Wed, 20 Mar 2019 19:34:09 +0200 Subject: [PATCH 13/22] Fixes for extract interface test. --- .../ExtractInterfaceWorkspaceService.cs | 4 +--- .../CodeActionsWithOptionsFacts.cs | 11 ++++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs index a6f3ded726..6dba8d05d9 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExtractInterfaceWorkspaceService.cs @@ -15,7 +15,6 @@ protected override object Invoke(MethodInfo targetMethod, object[] args) // IExtractInterfaceOptionsService and extract interface results are internal types -> workaround with proxy. // This service simply passes all members through as selected and doesn't try show UI. // When roslyn exposes this interface and members -> remove this workaround. - var resultTypeInternal = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.ExtractInterface.ExtractInterfaceOptionsResult"); var enumType = resultTypeInternal.GetNestedTypes().Single(x => x.Name == "ExtractLocation"); @@ -31,8 +30,7 @@ protected override object Invoke(MethodInfo targetMethod, object[] args) toSameFileEnumValue }); - var fromResultMethod = typeof(Task).GetMethod("FromResult").MakeGenericMethod(resultTypeInternal).Invoke(null, new[] { resultObject }); - return resultObject; + return typeof(Task).GetMethod("FromResult").MakeGenericMethod(resultTypeInternal).Invoke(null, new[] { resultObject }); } } } diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs index c0eb98e79b..57cdc2023e 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs @@ -198,13 +198,14 @@ public interface IClass1 string PropertyHere { get; set; } } - public class Class1 + public class Class1 : IClass1 { public string PropertyHere { get; set; } } "; - var response = await RunRefactoringAsync(code, "Extract interface..."); + var response = await RunRefactoringAsync(code, "Extract Interface..."); + var foo = ((ModifiedFileResponse)response.Changes.First()).Buffer; AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); } @@ -217,7 +218,7 @@ public void Check_better_alternative_available_for_codeaction_with_options() // // In case it's exposed by roslyn team these services can be simplified. // Steps are likely following: - // - Remove Castle.Core (proxies not needed) + // - Remove proxies // - Replace ExportWorkspaceServiceFactoryWithAssemblyQualifiedName with Microsoft.CodeAnalysis.Host.Mef.ExportWorkspaceServiceAttribute // - Fix proxy classes to implement IPickMembersService / IExtractInterfaceOptionsService ... instead of proxy and reflection. // - Remove all factories using ExportWorkspaceServiceFactoryWithAssemblyQualifiedName and factory itself. @@ -233,12 +234,12 @@ private static void AssertIgnoringIndent(string expected, string actual) private static string TrimLines(string source) { - return string.Join("\n", source.Split('\n').Select(s => s.Trim())).Replace("\n","").Replace("\r",""); + return string.Join("", source.Split('\n').Select(s => s.Trim())); } private async Task RunRefactoringAsync(string code, string refactoringName, bool wantsChanges = false) { - var refactorings = await FindRefactoringsAsync(code); + var refactorings = await FindRefactoringsAsync(code, configurationData: TestHelpers.GetConfigurationDataWithAnalyzerConfig(true)); Assert.Contains(refactoringName, refactorings.Select(a => a.Name)); var identifier = refactorings.First(action => action.Name.Equals(refactoringName)).Identifier; From 401e66e8a732d2d0ee3be5e514d8f8af0c1a0809 Mon Sep 17 00:00:00 2001 From: Savpek Date: Wed, 20 Mar 2019 19:49:56 +0200 Subject: [PATCH 14/22] Removed debug line which was forgotten. --- ...tWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs b/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs index b3141a4580..b07491141f 100644 --- a/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs +++ b/src/OmniSharp.Roslyn/WorkspaceServices/ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute.cs @@ -20,7 +20,6 @@ public ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute(string ty var type = Assembly.Load(typeAssembly).GetType(typeName) ?? throw new InvalidOperationException($"Could not resolve '{typeName} from '{typeAssembly}'"); - Console.WriteLine($"Resolved to type: {type.AssemblyQualifiedName}"); this.ServiceType = type.AssemblyQualifiedName; this.Layer = layer ?? throw new ArgumentNullException(nameof(layer)); } From f1a6061311e559738d3beab7aa5b8155983cd986 Mon Sep 17 00:00:00 2001 From: Savpek Date: Fri, 22 Mar 2019 18:11:20 +0200 Subject: [PATCH 15/22] Few review fixes. --- build/Packages.props | 2 ++ .../Services/Refactoring/V2/BaseCodeActionService.cs | 6 +++--- src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj | 3 +-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/build/Packages.props b/build/Packages.props index 8df4b99402..a91f4537a5 100644 --- a/build/Packages.props +++ b/build/Packages.props @@ -72,6 +72,8 @@ + + diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index a888d27c59..5c48e1547a 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -99,9 +99,9 @@ private static IEnumerable FilterBlacklistedCodeActions(IEn { // Most of actions with UI works fine with defaults, however there's few exceptions: return codeActions.Where(x => - x.CodeAction.Title != "Generate new type..." && // Blacklisted because doesn't give additional value over non UI generate type (when defaults used.) - x.CodeAction.Title != "Change signature..." && // Blacklisted because cannot be used without proper UI. - x.CodeAction.Title != "Pull members up to base type..." // Blacklisted because doesn't give additional value over non UI generate type (when defaults used.) + x.CodeAction.GetType().Name != "GenerateTypeCodeActionWithOption" && // Blacklisted because doesn't give additional value over non UI generate type (when defaults used.) + x.CodeAction.GetType().Name != "ChangeSignatureCodeAction" && // Blacklisted because cannot be used without proper UI. + x.CodeAction.GetType().Name != "PullMemberUpWithDialogCodeAction" // Blacklisted because doesn't give additional value over non UI generate type (when defaults used.) ); } diff --git a/src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj b/src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj index c3b5cb8791..bb1913324d 100644 --- a/src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj +++ b/src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj @@ -16,7 +16,6 @@ - + - From f136bf1e2b6f4583892cc5e00b020d69e8e64943 Mon Sep 17 00:00:00 2001 From: Savpek Date: Fri, 22 Mar 2019 22:08:54 +0200 Subject: [PATCH 16/22] Refactored action facts to base class. --- .../CodeActionsV2Facts.cs | 105 +------------- .../CodeActionsWithOptionsFacts.cs | 98 +------------ .../AbstractCodeActionsTestFixture.cs | 136 ++++++++++++++++++ 3 files changed, 142 insertions(+), 197 deletions(-) create mode 100644 tests/TestUtility/AbstractCodeActionsTestFixture.cs diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsV2Facts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsV2Facts.cs index ccaf723162..f63b81f05e 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsV2Facts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsV2Facts.cs @@ -12,10 +12,8 @@ namespace OmniSharp.Roslyn.CSharp.Tests { - public class CodeActionsV2Facts : AbstractTestFixture + public class CodeActionsV2Facts : AbstractCodeActionsTestFixture { - private readonly string BufferPath = $"{Path.DirectorySeparatorChar}somepath{Path.DirectorySeparatorChar}buffer.cs"; - public CodeActionsV2Facts(ITestOutputHelper output) : base(output) { @@ -88,7 +86,7 @@ public class c {public c() {Guid.NewGuid();}}"; public class c {public c() {Guid.NewGuid();}}"; - var response = await RunRefactoringAsync(code, "Remove Unnecessary Usings", roslynAnalyzersEnabled: roslynAnalyzersEnabled); + var response = await RunRefactoringAsync(code, "Remove Unnecessary Usings", isAnalyzersEnabled: roslynAnalyzersEnabled); AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); } @@ -182,7 +180,7 @@ private static void NewMethod() Console.Write(""should be using System;""); } }"; - var response = await RunRefactoringAsync(code, "Extract Method", roslynAnalyzersEnabled: roslynAnalyzersEnabled); + var response = await RunRefactoringAsync(code, "Extract Method", isAnalyzersEnabled: roslynAnalyzersEnabled); AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); } @@ -192,7 +190,7 @@ private static void NewMethod() public async Task Can_generate_type_and_return_name_of_new_file(bool roslynAnalyzersEnabled) { using (var testProject = await TestAssets.Instance.GetTestProjectAsync("ProjectWithMissingType")) - using (var host = CreateOmniSharpHost(testProject.Directory, configurationData: TestHelpers.GetConfigurationDataWithAnalyzerConfig(roslynAnalyzersEnabled))) + using (var host = OmniSharpTestHost.Create(testProject.Directory, testOutput: TestOutput, configurationData: TestHelpers.GetConfigurationDataWithAnalyzerConfig(roslynAnalyzersEnabled))) { var requestHandler = host.GetRequestHandler(OmniSharpEndpoints.V2.RunCodeAction); var document = host.Workspace.CurrentSolution.Projects.First().Documents.First(); @@ -233,7 +231,7 @@ internal class Z public async Task Can_send_rename_and_fileOpen_responses_when_codeAction_renames_file(bool roslynAnalyzersEnabled) { using (var testProject = await TestAssets.Instance.GetTestProjectAsync("ProjectWithMismatchedFileName")) - using (var host = CreateOmniSharpHost(testProject.Directory, configurationData: TestHelpers.GetConfigurationDataWithAnalyzerConfig(roslynAnalyzersEnabled))) + using (var host = OmniSharpTestHost.Create(testProject.Directory, testOutput: TestOutput, configurationData: TestHelpers.GetConfigurationDataWithAnalyzerConfig(roslynAnalyzersEnabled))) { var requestHandler = host.GetRequestHandler(OmniSharpEndpoints.V2.RunCodeAction); var document = host.Workspace.CurrentSolution.Projects.First().Documents.First(); @@ -259,98 +257,5 @@ public async Task Can_send_rename_and_fileOpen_responses_when_codeAction_renames Assert.Equal(FileModificationType.Opened, changes[1].ModificationType); } } - - private static void AssertIgnoringIndent(string expected, string actual) - { - Assert.Equal(TrimLines(expected), TrimLines(actual), false, true, true); - } - - private static string TrimLines(string source) - { - return string.Join("\n", source.Split('\n').Select(s => s.Trim())); - } - - private async Task RunRefactoringAsync(string code, string refactoringName, bool wantsChanges = false, bool roslynAnalyzersEnabled = false) - { - var refactorings = await FindRefactoringsAsync(code, configurationData: TestHelpers.GetConfigurationDataWithAnalyzerConfig(roslynAnalyzersEnabled)); - Assert.Contains(refactoringName, refactorings.Select(a => a.Name)); - - var identifier = refactorings.First(action => action.Name.Equals(refactoringName)).Identifier; - return await RunRefactoringsAsync(code, identifier, wantsChanges); - } - - private async Task> FindRefactoringNamesAsync(string code, bool roslynAnalyzersEnabled = false) - { - var codeActions = await FindRefactoringsAsync(code, configurationData: TestHelpers.GetConfigurationDataWithAnalyzerConfig(roslynAnalyzersEnabled)); - - return codeActions.Select(a => a.Name); - } - - private async Task> FindRefactoringsAsync(string code, IDictionary configurationData = null) - { - var testFile = new TestFile(BufferPath, code); - - using (var host = CreateOmniSharpHost(new[] { testFile }, configurationData)) - { - var requestHandler = host.GetRequestHandler(OmniSharpEndpoints.V2.GetCodeActions); - - var span = testFile.Content.GetSpans().Single(); - var range = testFile.Content.GetRangeFromSpan(span); - - var request = new GetCodeActionsRequest - { - Line = range.Start.Line, - Column = range.Start.Offset, - FileName = BufferPath, - Buffer = testFile.Content.Code, - Selection = GetSelection(range), - }; - - var response = await requestHandler.Handle(request); - - return response.CodeActions; - } - } - - private async Task RunRefactoringsAsync(string code, string identifier, bool wantsChanges = false) - { - var testFile = new TestFile(BufferPath, code); - - using (var host = CreateOmniSharpHost(testFile)) - { - var requestHandler = host.GetRequestHandler(OmniSharpEndpoints.V2.RunCodeAction); - - var span = testFile.Content.GetSpans().Single(); - var range = testFile.Content.GetRangeFromSpan(span); - - var request = new RunCodeActionRequest - { - Line = range.Start.Line, - Column = range.Start.Offset, - Selection = GetSelection(range), - FileName = BufferPath, - Buffer = testFile.Content.Code, - Identifier = identifier, - WantsTextChanges = wantsChanges, - WantsAllCodeActionOperations = true - }; - - return await requestHandler.Handle(request); - } - } - - private static Range GetSelection(TextRange range) - { - if (range.IsEmpty) - { - return null; - } - - return new Range - { - Start = new Point { Line = range.Start.Line, Column = range.Start.Offset }, - End = new Point { Line = range.End.Line, Column = range.End.Offset } - }; - } } } diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs index 57cdc2023e..3918e862c8 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs @@ -13,10 +13,8 @@ namespace OmniSharp.Roslyn.CSharp.Tests { - public class CodeActionsWithOptionsFacts : AbstractTestFixture + public class CodeActionsWithOptionsFacts : AbstractCodeActionsTestFixture { - private readonly string BufferPath = $"{Path.DirectorySeparatorChar}somepath{Path.DirectorySeparatorChar}buffer.cs"; - public CodeActionsWithOptionsFacts(ITestOutputHelper output) : base(output) { @@ -205,7 +203,6 @@ public class Class1 : IClass1 "; var response = await RunRefactoringAsync(code, "Extract Interface..."); - var foo = ((ModifiedFileResponse)response.Changes.First()).Buffer; AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); } @@ -226,98 +223,5 @@ public void Check_better_alternative_available_for_codeaction_with_options() var pickMemberServiceType = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.PickMembers.IPickMembersService"); Assert.False(pickMemberServiceType.IsPublic); } - - private static void AssertIgnoringIndent(string expected, string actual) - { - Assert.Equal(TrimLines(expected), TrimLines(actual), false, true, true); - } - - private static string TrimLines(string source) - { - return string.Join("", source.Split('\n').Select(s => s.Trim())); - } - - private async Task RunRefactoringAsync(string code, string refactoringName, bool wantsChanges = false) - { - var refactorings = await FindRefactoringsAsync(code, configurationData: TestHelpers.GetConfigurationDataWithAnalyzerConfig(true)); - Assert.Contains(refactoringName, refactorings.Select(a => a.Name)); - - var identifier = refactorings.First(action => action.Name.Equals(refactoringName)).Identifier; - return await RunRefactoringsAsync(code, identifier, wantsChanges); - } - - private async Task> FindRefactoringNamesAsync(string code) - { - var codeActions = await FindRefactoringsAsync(code); - - return codeActions.Select(a => a.Name); - } - - private async Task> FindRefactoringsAsync(string code, IDictionary configurationData = null) - { - var testFile = new TestFile(BufferPath, code); - - using (var host = CreateOmniSharpHost(new[] { testFile }, configurationData)) - { - var requestHandler = host.GetRequestHandler(OmniSharpEndpoints.V2.GetCodeActions); - - var span = testFile.Content.GetSpans().Single(); - var range = testFile.Content.GetRangeFromSpan(span); - - var request = new GetCodeActionsRequest - { - Line = range.Start.Line, - Column = range.Start.Offset, - FileName = BufferPath, - Buffer = testFile.Content.Code, - Selection = GetSelection(range), - }; - - var response = await requestHandler.Handle(request); - - return response.CodeActions; - } - } - - private async Task RunRefactoringsAsync(string code, string identifier, bool wantsChanges = false) - { - var testFile = new TestFile(BufferPath, code); - - using (var host = CreateOmniSharpHost(testFile)) - { - var requestHandler = host.GetRequestHandler(OmniSharpEndpoints.V2.RunCodeAction); - - var span = testFile.Content.GetSpans().Single(); - var range = testFile.Content.GetRangeFromSpan(span); - - var request = new RunCodeActionRequest - { - Line = range.Start.Line, - Column = range.Start.Offset, - Selection = GetSelection(range), - FileName = BufferPath, - Buffer = testFile.Content.Code, - Identifier = identifier, - WantsTextChanges = wantsChanges, - WantsAllCodeActionOperations = true - }; - - return await requestHandler.Handle(request); - } - } - - private static Range GetSelection(TextRange range) - { - if (range.IsEmpty) - { - return null; - } - - return new Range - { - Start = new Point { Line = range.Start.Line, Column = range.Start.Offset }, - End = new Point { Line = range.End.Line, Column = range.End.Offset } - }; - } } } diff --git a/tests/TestUtility/AbstractCodeActionsTestFixture.cs b/tests/TestUtility/AbstractCodeActionsTestFixture.cs new file mode 100644 index 0000000000..8ea3143c2a --- /dev/null +++ b/tests/TestUtility/AbstractCodeActionsTestFixture.cs @@ -0,0 +1,136 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using OmniSharp; +using OmniSharp.Models.V2; +using OmniSharp.Models.V2.CodeActions; +using OmniSharp.Roslyn.CSharp.Services.Refactoring.V2; +using TestUtility.Logging; +using Xunit; +using Xunit.Abstractions; + +namespace TestUtility +{ + public abstract class AbstractCodeActionsTestFixture + { + private readonly string _fileTypeExtension; + + protected ITestOutputHelper TestOutput { get; } + + private string BufferPath => $"{Path.DirectorySeparatorChar}somepath{Path.DirectorySeparatorChar}buffer{_fileTypeExtension}"; + + protected AbstractCodeActionsTestFixture(ITestOutputHelper output, string fileTypeExtension = ".cs") + { + TestOutput = output; + + _fileTypeExtension = fileTypeExtension; + } + + protected void AssertIgnoringIndent(string expected, string actual) + { + Assert.Equal(TrimLines(expected), TrimLines(actual), false, true, true); + } + + private static string TrimLines(string source) + { + return string.Join("", source.Split('\n').Select(s => s.Trim())); + } + + protected OmniSharpTestHost CreateOmniSharpHost(TestFile[] testFiles, IEnumerable> configurationData = null) + { + var host = OmniSharpTestHost.Create(path: null, testOutput: this.TestOutput, configurationData: configurationData); + + if (testFiles.Length > 0) + { + host.AddFilesToWorkspace(testFiles); + } + + return host; + } + + protected async Task RunRefactoringAsync(string code, string refactoringName, bool wantsChanges = false, bool isAnalyzersEnabled = true) + { + var refactorings = await FindRefactoringsAsync(code, configurationData: TestHelpers.GetConfigurationDataWithAnalyzerConfig(isAnalyzersEnabled)); + Assert.Contains(refactoringName, refactorings.Select(a => a.Name)); + + var identifier = refactorings.First(action => action.Name.Equals(refactoringName)).Identifier; + return await RunRefactoringsAsync(code, identifier, wantsChanges); + } + + protected async Task> FindRefactoringNamesAsync(string code, bool isAnalyzersEnabled = true) + { + var codeActions = await FindRefactoringsAsync(code, TestHelpers.GetConfigurationDataWithAnalyzerConfig(isAnalyzersEnabled)); + + return codeActions.Select(a => a.Name); + } + + protected async Task> FindRefactoringsAsync(string code, IDictionary configurationData = null) + { + var testFile = new TestFile(BufferPath, code); + + using (var host = CreateOmniSharpHost(new[] { testFile }, configurationData)) + { + var requestHandler = host.GetRequestHandler(OmniSharpEndpoints.V2.GetCodeActions); + + var span = testFile.Content.GetSpans().Single(); + var range = testFile.Content.GetRangeFromSpan(span); + + var request = new GetCodeActionsRequest + { + Line = range.Start.Line, + Column = range.Start.Offset, + FileName = BufferPath, + Buffer = testFile.Content.Code, + Selection = GetSelection(range), + }; + + var response = await requestHandler.Handle(request); + + return response.CodeActions; + } + } + + private async Task RunRefactoringsAsync(string code, string identifier, bool wantsChanges = false) + { + var testFile = new TestFile(BufferPath, code); + + using (var host = CreateOmniSharpHost(new [] { testFile })) + { + var requestHandler = host.GetRequestHandler(OmniSharpEndpoints.V2.RunCodeAction); + + var span = testFile.Content.GetSpans().Single(); + var range = testFile.Content.GetRangeFromSpan(span); + + var request = new RunCodeActionRequest + { + Line = range.Start.Line, + Column = range.Start.Offset, + Selection = GetSelection(range), + FileName = BufferPath, + Buffer = testFile.Content.Code, + Identifier = identifier, + WantsTextChanges = wantsChanges, + WantsAllCodeActionOperations = true + }; + + return await requestHandler.Handle(request); + } + } + + private static Range GetSelection(TextRange range) + { + if (range.IsEmpty) + { + return null; + } + + return new Range + { + Start = new Point { Line = range.Start.Line, Column = range.Start.Offset }, + End = new Point { Line = range.End.Line, Column = range.End.Offset } + }; + } + } +} From 0332ef64b319c0204655156bca80e77370c4f862 Mon Sep 17 00:00:00 2001 From: Savpek Date: Wed, 3 Apr 2019 19:31:41 +0300 Subject: [PATCH 17/22] Removed multiple reflection calls. --- .../Services/Refactoring/V2/BaseCodeActionService.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index 5c48e1547a..5f85ccd68b 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -98,11 +98,13 @@ protected async Task> GetAvailableCodeActions(I private static IEnumerable FilterBlacklistedCodeActions(IEnumerable codeActions) { // Most of actions with UI works fine with defaults, however there's few exceptions: - return codeActions.Where(x => - x.CodeAction.GetType().Name != "GenerateTypeCodeActionWithOption" && // Blacklisted because doesn't give additional value over non UI generate type (when defaults used.) - x.CodeAction.GetType().Name != "ChangeSignatureCodeAction" && // Blacklisted because cannot be used without proper UI. - x.CodeAction.GetType().Name != "PullMemberUpWithDialogCodeAction" // Blacklisted because doesn't give additional value over non UI generate type (when defaults used.) - ); + return codeActions.Where(x => { + var actionName = x.CodeAction.GetType().Name; + + return actionName != "GenerateTypeCodeActionWithOption" && // Blacklisted because doesn't give additional value over non UI generate type (when defaults used.) + actionName != "ChangeSignatureCodeAction" && // Blacklisted because cannot be used without proper UI. + actionName != "PullMemberUpWithDialogCodeAction"; // Blacklisted because doesn't give additional value over non UI generate type (when defaults used.) + }); } private TextSpan GetTextSpan(ICodeActionRequest request, SourceText sourceText) From 963c70810916cf518823556e71b13324136f236b Mon Sep 17 00:00:00 2001 From: Savpek Date: Mon, 8 Apr 2019 22:00:05 +0300 Subject: [PATCH 18/22] Attempt to fix test with net 472 hash code genenerator function. --- .../CodeActionsWithOptionsFacts.cs | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs index 3918e862c8..12dfb1bb65 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.IO; using System.Linq; using System.Reflection; @@ -106,27 +107,27 @@ public async Task Can_generate_equals_and_hashcode_for_object() public string PropertyHere { get; set; } }"; - // Be aware that if used with newer .NET framework than omnisharp uses (4.6). - // this will result more modern result with HashCode.Combine(PropertyHere); - const string expected = - @" + var hashCodeImplementation = Environment.Version.Major >= 4 && Environment.Version.Minor > 6 ? + "HashCode.Combine(PropertyHere)" : "1887327142 + EqualityComparer.Default.GetHashCode(PropertyHere)"; + + string expected = + $@" using System.Collections.Generic; public class Class1 - { - public string PropertyHere { get; set; } + {{ + public string PropertyHere {{ get; set; }} public override bool Equals(object obj) - { + {{ return obj is Class1 @class && PropertyHere == @class.PropertyHere; - } + }} public override int GetHashCode() - { - return 1887327142 + EqualityComparer.Default.GetHashCode(PropertyHere); - } - } + {{ + return {hashCodeImplementation}; + }} + }} "; var response = await RunRefactoringAsync(code, "Generate Equals and GetHashCode..."); - AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); } From 515f69193f9453ada76616d0663b0a44f1132ae2 Mon Sep 17 00:00:00 2001 From: Savpek Date: Mon, 8 Apr 2019 22:04:39 +0300 Subject: [PATCH 19/22] Additional fix. --- .../CodeActionsWithOptionsFacts.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs index 12dfb1bb65..ea7c7639c9 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs @@ -107,12 +107,16 @@ public async Task Can_generate_equals_and_hashcode_for_object() public string PropertyHere { get; set; } }"; - var hashCodeImplementation = Environment.Version.Major >= 4 && Environment.Version.Minor > 6 ? + var is472OrNewer = Environment.Version.Major >= 4 && Environment.Version.Minor > 6; + + var hashCodeImplementation = is472OrNewer ? "HashCode.Combine(PropertyHere)" : "1887327142 + EqualityComparer.Default.GetHashCode(PropertyHere)"; string expected = $@" + {(is472OrNewer ? "using System;" : "")} using System.Collections.Generic; + public class Class1 {{ public string PropertyHere {{ get; set; }} From c6a3683230468cd6a0eac6e4a5e0af808665be06 Mon Sep 17 00:00:00 2001 From: Savpek Date: Sun, 14 Apr 2019 14:19:49 +0300 Subject: [PATCH 20/22] Trigger build. From 57543fcb9b54b589dcaa96fd3ed23678b5a646fc Mon Sep 17 00:00:00 2001 From: Savpek Date: Sun, 14 Apr 2019 17:59:19 +0300 Subject: [PATCH 21/22] Testfix to support multiple hashcode implementations per frameworks. --- .../CodeActionsWithOptionsFacts.cs | 29 +++---------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs index ea7c7639c9..43741158b7 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs @@ -107,32 +107,11 @@ public async Task Can_generate_equals_and_hashcode_for_object() public string PropertyHere { get; set; } }"; - var is472OrNewer = Environment.Version.Major >= 4 && Environment.Version.Minor > 6; - - var hashCodeImplementation = is472OrNewer ? - "HashCode.Combine(PropertyHere)" : "1887327142 + EqualityComparer.Default.GetHashCode(PropertyHere)"; - - string expected = - $@" - {(is472OrNewer ? "using System;" : "")} - using System.Collections.Generic; - - public class Class1 - {{ - public string PropertyHere {{ get; set; }} - public override bool Equals(object obj) - {{ - return obj is Class1 @class && - PropertyHere == @class.PropertyHere; - }} - public override int GetHashCode() - {{ - return {hashCodeImplementation}; - }} - }} - "; var response = await RunRefactoringAsync(code, "Generate Equals and GetHashCode..."); - AssertIgnoringIndent(expected, ((ModifiedFileResponse)response.Changes.First()).Buffer); + + // This doesn't check exact class form because different framework version implement hashcode differently. + Assert.Contains("public override int GetHashCode()", ((ModifiedFileResponse)response.Changes.First()).Buffer); + Assert.Contains("public override bool Equals(object obj)", ((ModifiedFileResponse)response.Changes.First()).Buffer); } [Fact] From fbf7d36997a670a7831a6d705026053d5451c7df Mon Sep 17 00:00:00 2001 From: Savpek Date: Sun, 23 Jun 2019 22:04:03 +0300 Subject: [PATCH 22/22] Testfix. --- .../OmniSharp.Roslyn.CSharp.Tests/CustomRoslynAnalyzerFacts.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CustomRoslynAnalyzerFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CustomRoslynAnalyzerFacts.cs index a27b7b4e3c..9a7deb8a4d 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CustomRoslynAnalyzerFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CustomRoslynAnalyzerFacts.cs @@ -248,7 +248,7 @@ public async Task WhenDiagnosticsRulesAreUpdated_ThenReAnalyzerFilesInProject() host.Workspace.WorkspaceChanged += (_, e) => { if (e.Kind == WorkspaceChangeKind.ProjectChanged) { workspaceUpdatedCheck.Set(); } }; host.Workspace.UpdateDiagnosticOptionsForProject(projectId, testRulesUpdated.ToImmutableDictionary()); - Assert.True(workspaceUpdatedCheck.WaitOne(timeout: TimeSpan.FromSeconds(3))); + Assert.True(workspaceUpdatedCheck.WaitOne(timeout: TimeSpan.FromSeconds(15))); var result = await host.RequestCodeCheckAsync("testFile_4.cs"); Assert.DoesNotContain(result.QuickFixes, f => f.Text.Contains(testAnalyzerRef.Id.ToString()));