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

Record whether a CodeAction is a fix or not #2430

Merged
merged 2 commits into from
Aug 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 9 additions & 1 deletion src/OmniSharp.Abstractions/Models/v2/CodeActions/CodeAction.cs
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
using System.Diagnostics;

namespace OmniSharp.Models.V2.CodeActions
{
public class OmniSharpCodeAction
{
public OmniSharpCodeAction(string identifier, string name)
public OmniSharpCodeAction(string identifier, string name, string codeActionKind)
{
Debug.Assert(codeActionKind is CodeActions.CodeActionKind.QuickFix
or CodeActions.CodeActionKind.Refactor
or CodeActions.CodeActionKind.RefactorExtract
or CodeActions.CodeActionKind.RefactorInline);
Identifier = identifier;
Name = name;
CodeActionKind = codeActionKind;
}

public string Identifier { get; }
public string Name { get; }
public string CodeActionKind { get; }
}
}
10 changes: 10 additions & 0 deletions src/OmniSharp.Abstractions/Models/v2/CodeActions/CodeActionKind.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
namespace OmniSharp.Models.V2.CodeActions
{
public static class CodeActionKind
{
public const string QuickFix = nameof(QuickFix);
public const string Refactor = nameof(Refactor);
public const string RefactorInline = nameof(RefactorInline);
public const string RefactorExtract = nameof(RefactorExtract);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Threading.Tasks;
using MediatR;
using Microsoft.CodeAnalysis;
using Newtonsoft.Json.Linq;
using OmniSharp.Extensions.JsonRpc;
using OmniSharp.Extensions.LanguageServer.Protocol;
using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities;
Expand All @@ -14,8 +13,11 @@
using OmniSharp.Models.V2.CodeActions;
using OmniSharp.Extensions.LanguageServer.Protocol.Document;
using OmniSharp.Extensions.LanguageServer.Protocol.Workspace;
using OmniSharp.Models;
using Diagnostic = OmniSharp.Extensions.LanguageServer.Protocol.Models.Diagnostic;
using CodeActionKind = OmniSharp.Extensions.LanguageServer.Protocol.Models.CodeActionKind;
using OmniSharpCodeActionKind = OmniSharp.Models.V2.CodeActions.CodeActionKind;
using System;
using Range = OmniSharp.Extensions.LanguageServer.Protocol.Models.Range;

namespace OmniSharp.LanguageServerProtocol.Handlers
{
Expand Down Expand Up @@ -80,18 +82,12 @@ public override async Task<CommandOrCodeActionContainer> Handle(CodeActionParams

foreach (var ca in omnisharpResponse.CodeActions)
{
CodeActionKind kind;
if (ca.Identifier.StartsWith("using ")) { kind = CodeActionKind.QuickFix; }
else if (ca.Identifier.StartsWith("Inline ")) { kind = CodeActionKind.RefactorInline; }
else if (ca.Identifier.StartsWith("Extract ")) { kind = CodeActionKind.RefactorExtract; }
else if (ca.Identifier.StartsWith("Change ")) { kind = CodeActionKind.QuickFix; }
else { kind = CodeActionKind.Refactor; }

codeActions.Add(
new CodeAction
{
Title = ca.Name,
Kind = kind,
Kind = OmniSharpCodeActionHandler.FromOmniSharpCodeActionKind(ca.CodeActionKind),
Diagnostics = new Container<Diagnostic>(),
Edit = new WorkspaceEdit(),
Command = Command.Create("omnisharp/executeCodeAction")
Expand Down Expand Up @@ -168,6 +164,16 @@ ExecuteCommandRegistrationOptions IRegistration<ExecuteCommandRegistrationOption
return _executeCommandRegistrationOptions;
}

private static CodeActionKind FromOmniSharpCodeActionKind(string omnisharpCodeAction)
=> omnisharpCodeAction switch
{
OmniSharpCodeActionKind.QuickFix => CodeActionKind.QuickFix,
OmniSharpCodeActionKind.Refactor => CodeActionKind.Refactor,
OmniSharpCodeActionKind.RefactorInline => CodeActionKind.RefactorInline,
OmniSharpCodeActionKind.RefactorExtract => CodeActionKind.RefactorExtract,
_ => throw new InvalidOperationException($"Unexpected code action kind {omnisharpCodeAction}")
};

protected override CodeActionRegistrationOptions CreateRegistrationOptions(CodeActionCapability capability, ClientCapabilities clientCapabilities)
{
return new CodeActionRegistrationOptions()
Expand All @@ -176,7 +182,8 @@ protected override CodeActionRegistrationOptions CreateRegistrationOptions(CodeA
CodeActionKinds = new Container<CodeActionKind>(
CodeActionKind.SourceOrganizeImports,
CodeActionKind.Refactor,
CodeActionKind.RefactorExtract),
CodeActionKind.RefactorExtract,
CodeActionKind.RefactorInline),
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ public class AvailableCodeAction
{
public CodeAction CodeAction { get; }
public CodeAction ParentCodeAction { get; }
public string CodeActionKind { get; }

public AvailableCodeAction(CodeAction codeAction, CodeAction parentCodeAction = null)
public AvailableCodeAction(CodeAction codeAction, string codeActionKind, CodeAction parentCodeAction = null)
{
this.CodeAction = codeAction ?? throw new ArgumentNullException(nameof(codeAction));
this.ParentCodeAction = parentCodeAction;
this.CodeActionKind = codeActionKind;
}

public string GetIdentifier()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,15 @@ protected async Task<IEnumerable<AvailableCodeAction>> GetAvailableCodeActions(I
return Array.Empty<AvailableCodeAction>();
}

var codeActions = new List<CodeAction>();
var codeActions = new List<(CodeAction CodeAction, string CodeActionKind)>();

var sourceText = await document.GetTextAsync();
var span = GetTextSpan(request, sourceText);

await CollectCodeFixesActions(document, span, codeActions);
await CollectRefactoringActions(document, span, codeActions);

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

var availableActions = ConvertToAvailableCodeAction(distinctActions);

Expand Down Expand Up @@ -117,7 +117,7 @@ private TextSpan GetTextSpan(ICodeActionRequest request, SourceText sourceText)
return new TextSpan(position, length: 0);
}

private async Task CollectCodeFixesActions(Document document, TextSpan span, List<CodeAction> codeActions)
private async Task CollectCodeFixesActions(Document document, TextSpan span, List<(CodeAction CodeAction, string CodeActionKind)> codeActions)
{
var diagnosticsWithProjects = await _diagnostics.GetDiagnostics(ImmutableArray.Create(document.FilePath));

Expand All @@ -135,7 +135,7 @@ private async Task CollectCodeFixesActions(Document document, TextSpan span, Lis
}
}

private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerable<Diagnostic> diagnostics, List<CodeAction> codeActions)
private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerable<Diagnostic> diagnostics, List<(CodeAction CodeAction, string CodeActionKind)> codeActions)
{
var codeActionOptions = CodeActionOptionsFactory.Create(Options);

Expand All @@ -149,7 +149,7 @@ private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerabl
document,
span,
fixableDiagnostics,
(a, _) => codeActions.Add(a),
(a, _) => codeActions.Add((a, CodeActionKind.QuickFix)),
codeActionOptions,
CancellationToken.None);

Expand Down Expand Up @@ -182,7 +182,7 @@ private bool HasFix(CodeFixProvider codeFixProvider, string diagnosticId)
|| (customDiagVsFixMap.ContainsKey(diagnosticId) && codeFixProvider.FixableDiagnosticIds.Any(id => id == customDiagVsFixMap[diagnosticId]));
}

private async Task CollectRefactoringActions(Document document, TextSpan span, List<CodeAction> codeActions)
private async Task CollectRefactoringActions(Document document, TextSpan span, List<(CodeAction CodeAction, string CodeActionKind)> codeActions)
{
var codeActionOptions = CodeActionOptionsFactory.Create(Options);
var availableRefactorings = OrderedCodeRefactoringProviders.Value;
Expand All @@ -194,7 +194,23 @@ private async Task CollectRefactoringActions(Document document, TextSpan span, L
var context = OmniSharpCodeFixContextFactory.CreateCodeRefactoringContext(
document,
span,
(a, _) => codeActions.Add(a),
(a, _) =>
{
string kind;
if (a.Title.StartsWith("Inline "))
{
kind = CodeActionKind.RefactorInline;
}
else if (a.Title.StartsWith("Extract "))
{
kind = CodeActionKind.RefactorExtract;
}
else
{
kind = CodeActionKind.Refactor;
}
codeActions.Add((a, kind));
},
codeActionOptions,
CancellationToken.None);

Expand All @@ -207,17 +223,17 @@ private async Task CollectRefactoringActions(Document document, TextSpan span, L
}
}

private IEnumerable<AvailableCodeAction> ConvertToAvailableCodeAction(IEnumerable<CodeAction> actions)
private IEnumerable<AvailableCodeAction> ConvertToAvailableCodeAction(IEnumerable<(CodeAction CodeAction, string CodeActionKind)> actions)
{
return actions.SelectMany(action =>
{
var nestedActions = action.GetNestedCodeActions();
var nestedActions = action.CodeAction.GetNestedCodeActions();
if (!nestedActions.IsDefaultOrEmpty)
{
return nestedActions.Select(nestedAction => new AvailableCodeAction(nestedAction, action));
return nestedActions.Select(nestedAction => new AvailableCodeAction(nestedAction, action.CodeActionKind, action.CodeAction));
}

return new[] { new AvailableCodeAction(action) };
return new[] { new AvailableCodeAction(action.CodeAction, action.CodeActionKind) };
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public override async Task<GetCodeActionsResponse> Handle(GetCodeActionsRequest

private static OmniSharpCodeAction ConvertToOmniSharpCodeAction(AvailableCodeAction availableAction)
{
return new OmniSharpCodeAction(availableAction.GetIdentifier(), availableAction.GetTitle());
return new OmniSharpCodeAction(availableAction.GetIdentifier(), availableAction.GetTitle(), availableAction.CodeActionKind);
}
}
}
37 changes: 19 additions & 18 deletions tests/OmniSharp.Cake.Tests/CodeActionsV2Facts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using OmniSharp.Models.UpdateBuffer;
using OmniSharp.Models.V2;
using OmniSharp.Models.V2.CodeActions;
using Roslyn.Test.Utilities;
using TestUtility;
using Xunit;
using Xunit.Abstractions;
Expand All @@ -30,7 +31,7 @@ public async Task Can_get_code_actions_from_roslyn()
const string code = "var regex = new Reg[||]ex();";

var refactorings = await FindRefactoringNamesAsync(code);
Assert.Contains("using System.Text.RegularExpressions;", refactorings);
Assert.Contains(("using System.Text.RegularExpressions;", CodeActionKind.QuickFix), refactorings);
}

[Fact]
Expand All @@ -46,7 +47,7 @@ public void Whatever()
}";

var refactorings = await FindRefactoringNamesAsync(code);
Assert.Contains("Extract method", refactorings);
Assert.Contains(("Extract method", CodeActionKind.RefactorExtract), refactorings);
}

[Fact]
Expand All @@ -62,21 +63,21 @@ public void Whatever()
}";

var refactorings = await FindRefactoringNamesAsync(code);
var expected = new List<string>
var expected = new List<(string, string CodeActionKind)>
{
"using System.Text.RegularExpressions;",
"System.Text.RegularExpressions.Regex",
"Extract method",
"Extract local function",
"Introduce local for 'Regex.Match(\"foo\", \"bar\")'",
"Introduce parameter for 'Regex.Match(\"foo\", \"bar\")' -> and update call sites directly",
"Introduce parameter for 'Regex.Match(\"foo\", \"bar\")' -> into extracted method to invoke at call sites",
"Introduce parameter for 'Regex.Match(\"foo\", \"bar\")' -> into new overload",
"Introduce parameter for all occurrences of 'Regex.Match(\"foo\", \"bar\")' -> and update call sites directly",
"Introduce parameter for all occurrences of 'Regex.Match(\"foo\", \"bar\")' -> into extracted method to invoke at call sites",
"Introduce parameter for all occurrences of 'Regex.Match(\"foo\", \"bar\")' -> into new overload"
("using System.Text.RegularExpressions;", CodeActionKind.QuickFix),
("System.Text.RegularExpressions.Regex", CodeActionKind.QuickFix),
("Extract method", CodeActionKind.RefactorExtract),
("Extract local function", CodeActionKind.RefactorExtract),
("Introduce local for 'Regex.Match(\"foo\", \"bar\")'", CodeActionKind.Refactor),
("Introduce parameter for 'Regex.Match(\"foo\", \"bar\")' -> and update call sites directly", CodeActionKind.Refactor),
("Introduce parameter for 'Regex.Match(\"foo\", \"bar\")' -> into extracted method to invoke at call sites", CodeActionKind.Refactor),
("Introduce parameter for 'Regex.Match(\"foo\", \"bar\")' -> into new overload", CodeActionKind.Refactor),
("Introduce parameter for all occurrences of 'Regex.Match(\"foo\", \"bar\")' -> and update call sites directly", CodeActionKind.Refactor),
("Introduce parameter for all occurrences of 'Regex.Match(\"foo\", \"bar\")' -> into extracted method to invoke at call sites", CodeActionKind.Refactor),
("Introduce parameter for all occurrences of 'Regex.Match(\"foo\", \"bar\")' -> into new overload", CodeActionKind.Refactor)
};
Assert.Equal(expected, refactorings);
AssertEx.Equal(expected, refactorings);
}

[Fact]
Expand Down Expand Up @@ -122,7 +123,7 @@ public void Whatever()
}";

var refactorings = await FindRefactoringNamesAsync(code);
Assert.Empty(refactorings.Where(x => x.StartsWith("Rename file to")));
Assert.Empty(refactorings.Where(x => x.Name.StartsWith("Rename file to")));
}

private async Task<RunCodeActionResponse> RunRefactoringAsync(string code, string refactoringName)
Expand All @@ -134,11 +135,11 @@ private async Task<RunCodeActionResponse> RunRefactoringAsync(string code, strin
return await RunRefactoringsAsync(code, identifier);
}

private async Task<IEnumerable<string>> FindRefactoringNamesAsync(string code)
private async Task<IEnumerable<(string Name, string CodeActionKind)>> FindRefactoringNamesAsync(string code)
{
var codeActions = await FindRefactoringsAsync(code);

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

private async Task<IEnumerable<OmniSharpCodeAction>> FindRefactoringsAsync(string code)
Expand Down
Loading