Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Codeactions with options/ui. #1406

Merged
merged 58 commits into from
Jul 10, 2019
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
eea9dfc
Initial version of actions with options.
savpek Feb 20, 2019
cbf1877
Fixes.
savpek Feb 20, 2019
70d3dc9
Fixes for extract interface.
savpek Feb 21, 2019
9d95a01
First test.
savpek Feb 21, 2019
84224b5
More tests.
savpek Feb 21, 2019
5437959
Tweaks and comments.
savpek Feb 21, 2019
761f657
Original global.json
savpek Feb 21, 2019
274e3c7
Merge branch 'master' into feature/codeactions-with-options
savpek Feb 23, 2019
ca9ecb3
Blacklisting.
savpek Feb 23, 2019
a7c21ff
Merge branch 'feature/codeactions-with-options' of https://github.com…
savpek Feb 23, 2019
8a298e5
Added test to keep track is official support added.
savpek Feb 24, 2019
c773575
Tests for blacklisting.
savpek Feb 24, 2019
f17aeeb
small comment tweak.
savpek Feb 24, 2019
9dd4cef
Merge branch 'master' into feature/codeactions-with-options
savpek Feb 27, 2019
2a199b1
Merge branch 'master' into feature/codeactions-with-options
savpek Mar 2, 2019
fca2bb4
Merge branch 'master' into feature/codeactions-with-options
savpek Mar 13, 2019
17eb6d9
Merge branch 'master' into feature/codeactions-with-options
savpek Mar 16, 2019
ac93122
Merge branch 'master' into feature/codeactions-with-options
savpek Mar 17, 2019
4659787
Moved to dispatch proxy.
savpek Mar 20, 2019
596cbe6
Merge from upstream/master
savpek Mar 20, 2019
5690dc1
Fixes for extract interface test.
savpek Mar 20, 2019
401e66e
Removed debug line which was forgotten.
savpek Mar 20, 2019
4a65088
Merge branch 'master' into feature/codeactions-with-options
savpek Mar 21, 2019
9efd76d
Merge branch 'master' into feature/codeactions-with-options
savpek Mar 22, 2019
f1a6061
Few review fixes.
savpek Mar 22, 2019
a06b47b
Merge branch 'feature/codeactions-with-options' of https://github.com…
savpek Mar 22, 2019
f136bf1
Refactored action facts to base class.
savpek Mar 22, 2019
a04812a
Merge branch 'master' into feature/codeactions-with-options
savpek Apr 1, 2019
0332ef6
Removed multiple reflection calls.
savpek Apr 3, 2019
1f47071
Merge branch 'feature/codeactions-with-options' of https://github.com…
savpek Apr 3, 2019
e60dc68
Merge branch 'master' into feature/codeactions-with-options
savpek Apr 3, 2019
1aeab24
Merge branch 'master' into feature/codeactions-with-options
savpek Apr 8, 2019
963c708
Attempt to fix test with net 472 hash code genenerator function.
savpek Apr 8, 2019
515f691
Additional fix.
savpek Apr 8, 2019
c6a3683
Trigger build.
savpek Apr 14, 2019
57543fc
Testfix to support multiple hashcode implementations per frameworks.
savpek Apr 14, 2019
2afb64a
Merge branch 'master' into feature/codeactions-with-options
savpek Apr 14, 2019
9949e56
Merge branch 'master' into feature/codeactions-with-options
savpek Apr 15, 2019
1138ac8
Merge branch 'master' into feature/codeactions-with-options
savpek Apr 18, 2019
79e0390
Merge branch 'master' into feature/codeactions-with-options
savpek Apr 19, 2019
7099e22
Merge branch 'master' into feature/codeactions-with-options
savpek May 1, 2019
26aaac9
Merge branch 'master' into feature/codeactions-with-options
savpek May 8, 2019
9b9ac63
Merge branch 'master' into feature/codeactions-with-options
savpek May 20, 2019
0471c2e
Merge branch 'master' into feature/codeactions-with-options
savpek May 21, 2019
dab7a8c
Merge branch 'master' into feature/codeactions-with-options
savpek May 23, 2019
03c31f6
Merge branch 'master' into feature/codeactions-with-options
savpek May 26, 2019
a463572
Merge branch 'master' into feature/codeactions-with-options
savpek May 31, 2019
bbc31ad
Merge branch 'master' into feature/codeactions-with-options
filipw Jun 11, 2019
2c13068
Merge branch 'master' into feature/codeactions-with-options
savpek Jun 20, 2019
90c7d93
Merge.
savpek Jun 23, 2019
fbf7d36
Testfix.
savpek Jun 23, 2019
49c01c1
Merge branch 'master' into feature/codeactions-with-options
savpek Jun 25, 2019
281ad00
Merge branch 'master' into feature/codeactions-with-options
savpek Jun 27, 2019
22e1789
Merge branch 'master' into feature/codeactions-with-options
david-driscoll Jun 27, 2019
672a012
Merge branch 'master' into feature/codeactions-with-options
savpek Jun 28, 2019
c8085c0
Merge branch 'master' into feature/codeactions-with-options
filipw Jul 5, 2019
67277c1
Merge branch 'master' into feature/codeactions-with-options
savpek Jul 7, 2019
01a5aca
Merge branch 'master' into feature/codeactions-with-options
savpek Jul 10, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions build/Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@

<PackageReference Update="System.Reactive" Version="4.1.2" />

<PackageReference Update="System.Reflection.DispatchProxy" Version="4.5.1" />

<PackageReference Update="xunit.runner.visualstudio" Version="$(XunitPackageVersion)" />
<PackageReference Update="xunit" Version="$(XunitPackageVersion)" />
</ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/OmniSharp.Host/WorkspaceInitializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public static void Initialize(IServiceProvider serviceProvider, CompositionHost
var projectEventForwarder = compositionHost.GetExport<ProjectEventForwarder>();
projectEventForwarder.Initialize();
var projectSystems = compositionHost.GetExports<IProjectSystem>();

foreach (var projectSystem in projectSystems)
{
try
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,19 @@ protected async Task<IEnumerable<AvailableCodeAction>> GetAvailableCodeActions(I

var distinctActions = codeActions.GroupBy(x => x.Title).Select(x => x.First());

// 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(distinctActions)
.Where(a => !a.CodeAction.GetType().GetTypeInfo().IsSubclassOf(typeof(CodeActionWithOptions)));
var availableActions = ConvertToAvailableCodeAction(distinctActions);

return availableActions;
return FilterBlacklistedCodeActions(availableActions);
}

private static IEnumerable<AvailableCodeAction> FilterBlacklistedCodeActions(IEnumerable<AvailableCodeAction> 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.)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we reduce this to one reflection invocation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Ravi was looking at only calling x.CodeAction.GetType().Name once instead of three times.

Copy link
Contributor Author

@savpek savpek Jun 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

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.)
);
}

private TextSpan GetTextSpan(ICodeActionRequest request, SourceText sourceText)
Expand Down
2 changes: 2 additions & 0 deletions src/OmniSharp.Roslyn/HostServicesAggregator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ public HostServicesAggregator(
}
}

builder.Add(typeof(PickMemberWorkspaceService).Assembly);

_assemblies = builder.ToImmutableArray();
}

Expand Down
2 changes: 1 addition & 1 deletion src/OmniSharp.Roslyn/OmniSharp.Roslyn.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@
<PackageReference Include="Microsoft.Extensions.Caching.Memory" />
<PackageReference Include="Microsoft.CodeAnalysis.Common" />
<PackageReference Include="System.ComponentModel.Composition" />
<PackageReference Include="System.Reflection.DispatchProxy" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using System;
using System.Composition;
using System.Reflection;
using Microsoft.CodeAnalysis.Host.Mef;

namespace OmniSharp
{
[MetadataAttribute]
[AttributeUsage(AttributeTargets.Class)]
public class ExportWorkspaceServiceFactoryWithAssemblyQualifiedNameAttribute : ExportAttribute
{
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))
{
var type = Assembly.Load(typeAssembly).GetType(typeName)
?? throw new InvalidOperationException($"Could not resolve '{typeName} from '{typeAssembly}'");

this.ServiceType = type.AssemblyQualifiedName;
this.Layer = layer ?? throw new ArgumentNullException(nameof(layer));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;

namespace OmniSharp
{
public class ExtractInterfaceWorkspaceService : DispatchProxy
{
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");

var toSameFileEnumValue = Enum.Parse(enumType, "SameFile");

var interfaceName = args[3] ?? throw new InvalidOperationException($"{nameof(ExtractInterfaceWorkspaceService)} default interface name was null.");

var resultObject = Activator.CreateInstance(resultTypeInternal, new object[] {
false, // isCancelled
((List<ISymbol>)args[2]).ToImmutableArray(), // InterfaceMembers selected -> select all.
interfaceName,
$"{interfaceName}.cs",
toSameFileEnumValue
});

return typeof(Task).GetMethod("FromResult").MakeGenericMethod(resultTypeInternal).Invoke(null, new[] { resultObject });
}
}
}
Original file line number Diff line number Diff line change
@@ -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.ExtractInterface.IExtractInterfaceOptionsService")]
public class ExtractInterfaceWorkspaceServiceFactory : IWorkspaceServiceFactory
{
public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
{
// Generates proxy class to get around issue that IExtractInterfaceOptionsService is internal at this point.
var internalType = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.ExtractInterface.IExtractInterfaceOptionsService");
return (IWorkspaceService)typeof(DispatchProxy).GetMethod(nameof(DispatchProxy.Create)).MakeGenericMethod(internalType, typeof(ExtractInterfaceWorkspaceService)).Invoke(null, null);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using System;
using System.Reflection;

namespace OmniSharp
{
public class PickMemberWorkspaceService : DispatchProxy
{
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");
return Activator.CreateInstance(resultTypeInternal, new object[] { args[1], args[2] });
}
}
}
Original file line number Diff line number Diff line change
@@ -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.PickMembers.IPickMembersService")]
public class PickMemberWorkspaceServiceFactory : IWorkspaceServiceFactory
{
public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
{
// Generates proxy class to get around issue that IPickMembersService is internal at this point.
var internalType = Assembly.Load("Microsoft.CodeAnalysis.Features").GetType("Microsoft.CodeAnalysis.PickMembers.IPickMembersService");
return (IWorkspaceService)typeof(DispatchProxy).GetMethod(nameof(DispatchProxy.Create)).MakeGenericMethod(internalType, typeof(PickMemberWorkspaceService)).Invoke(null, null);
}
}
}
105 changes: 5 additions & 100 deletions tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsV2Facts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand All @@ -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<RunCodeActionService>(OmniSharpEndpoints.V2.RunCodeAction);
var document = host.Workspace.CurrentSolution.Projects.First().Documents.First();
Expand Down Expand Up @@ -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<RunCodeActionService>(OmniSharpEndpoints.V2.RunCodeAction);
var document = host.Workspace.CurrentSolution.Projects.First().Documents.First();
Expand All @@ -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<RunCodeActionResponse> 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<IEnumerable<string>> FindRefactoringNamesAsync(string code, bool roslynAnalyzersEnabled = false)
{
var codeActions = await FindRefactoringsAsync(code, configurationData: TestHelpers.GetConfigurationDataWithAnalyzerConfig(roslynAnalyzersEnabled));

return codeActions.Select(a => a.Name);
}

private async Task<IEnumerable<OmniSharpCodeAction>> FindRefactoringsAsync(string code, IDictionary<string, string> configurationData = null)
{
var testFile = new TestFile(BufferPath, code);

using (var host = CreateOmniSharpHost(new[] { testFile }, configurationData))
{
var requestHandler = host.GetRequestHandler<GetCodeActionsService>(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<RunCodeActionResponse> RunRefactoringsAsync(string code, string identifier, bool wantsChanges = false)
{
var testFile = new TestFile(BufferPath, code);

using (var host = CreateOmniSharpHost(testFile))
{
var requestHandler = host.GetRequestHandler<RunCodeActionService>(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 }
};
}
}
}
Loading