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.

Sign up for GitHub

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

Jump to bottom

Record whether a CodeAction is a fix or not #2430

Merged
merged 2 commits into from
Aug 11, 2022
Merged

Record whether a CodeAction is a fix or not #2430

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3cad230
Record whether a CodeAction is a fix or not
333fred Aug 10, 2022
bd462d5
Switch to using strings for code action kinds, instead of a bool. Mov…
333fred Aug 11, 2022
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
4 changes: 3 additions & 1 deletion src/OmniSharp.Abstractions/Models/v2/CodeActions/CodeAction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ namespace OmniSharp.Models.V2.CodeActions
{
public class OmniSharpCodeAction
{
public OmniSharpCodeAction(string identifier, string name)
public OmniSharpCodeAction(string identifier, string name, bool isCodeFix)
{
Identifier = identifier;
Name = name;
IsCodeFix = isCodeFix;
}

public string Identifier { get; }
public string Name { get; }
public bool IsCodeFix { get; }
}
}
4 changes: 3 additions & 1 deletion src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/AvailableCodeAction.cs
View file
Open in desktop
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 bool IsCodeFix { get; }
333fred marked this conversation as resolved.
Show resolved Hide resolved

public AvailableCodeAction(CodeAction codeAction, CodeAction parentCodeAction = null)
public AvailableCodeAction(CodeAction codeAction, bool isCodeFix, CodeAction parentCodeAction = null)
{
this.CodeAction = codeAction ?? throw new ArgumentNullException(nameof(codeAction));
this.ParentCodeAction = parentCodeAction;
this.IsCodeFix = isCodeFix;
}

public string GetIdentifier()
Expand Down
22 changes: 11 additions & 11 deletions src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs
View file
Open in desktop
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, bool IsCodeFix)>();

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, bool IsCodeFix)> 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, bool IsCodeFix)> 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, IsCodeFix: true)),
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, bool IsCodeFix)> codeActions)
{
var codeActionOptions = CodeActionOptionsFactory.Create(Options);
var availableRefactorings = OrderedCodeRefactoringProviders.Value;
Expand All @@ -194,7 +194,7 @@ private async Task CollectRefactoringActions(Document document, TextSpan span, L
var context = OmniSharpCodeFixContextFactory.CreateCodeRefactoringContext(
document,
span,
(a, _) => codeActions.Add(a),
(a, _) => codeActions.Add((a, IsCodeFix: false)),
codeActionOptions,
CancellationToken.None);

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

private IEnumerable<AvailableCodeAction> ConvertToAvailableCodeAction(IEnumerable<CodeAction> actions)
private IEnumerable<AvailableCodeAction> ConvertToAvailableCodeAction(IEnumerable<(CodeAction CodeAction, bool IsCodeFix)> 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.IsCodeFix, action.CodeAction));
}

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

Expand Down
2 changes: 1 addition & 1 deletion src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/GetCodeActionsService.cs
View file
Open in desktop
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.IsCodeFix);
}
}
}
37 changes: 19 additions & 18 deletions tests/OmniSharp.Cake.Tests/CodeActionsV2Facts.cs
View file
Open in desktop
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;", IsCodeFix: true), 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", IsCodeFix: false), 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, bool IsCodeFix)>
{
"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;", IsCodeFix: true),
("System.Text.RegularExpressions.Regex", IsCodeFix: true),
("Extract method", IsCodeFix: false),
("Extract local function", IsCodeFix: false),
("Introduce local for 'Regex.Match(\"foo\", \"bar\")'", IsCodeFix: false),
("Introduce parameter for 'Regex.Match(\"foo\", \"bar\")' -> and update call sites directly", IsCodeFix: false),
("Introduce parameter for 'Regex.Match(\"foo\", \"bar\")' -> into extracted method to invoke at call sites", IsCodeFix: false),
("Introduce parameter for 'Regex.Match(\"foo\", \"bar\")' -> into new overload", IsCodeFix: false),
("Introduce parameter for all occurrences of 'Regex.Match(\"foo\", \"bar\")' -> and update call sites directly", IsCodeFix: false),
("Introduce parameter for all occurrences of 'Regex.Match(\"foo\", \"bar\")' -> into extracted method to invoke at call sites", IsCodeFix: false),
("Introduce parameter for all occurrences of 'Regex.Match(\"foo\", \"bar\")' -> into new overload", IsCodeFix: false)
};
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, bool IsCodeFix)>> FindRefactoringNamesAsync(string code)
{
var codeActions = await FindRefactoringsAsync(code);

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

private async Task<IEnumerable<OmniSharpCodeAction>> FindRefactoringsAsync(string code)
Expand Down
78 changes: 39 additions & 39 deletions tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsV2Facts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
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 Roslyn.Test.Utilities;
using TestUtility;
using Xunit;
using Xunit.Abstractions;
Expand Down Expand Up @@ -34,7 +34,7 @@ public void Whatever()
}";

var refactorings = await FindRefactoringNamesAsync(code, roslynAnalyzersEnabled);
Assert.Contains("using System;", refactorings);
Assert.Contains(("using System;", IsCodeFix: true), refactorings);
}

[Theory]
Expand Down Expand Up @@ -105,7 +105,7 @@ public void Whatever()
}";

var refactorings = await FindRefactoringNamesAsync(code, roslynAnalyzersEnabled);
Assert.Contains("Extract method", refactorings);
Assert.Contains(("Extract method", IsCodeFix: false), refactorings);
}

[Theory]
Expand All @@ -124,51 +124,51 @@ public void Whatever()

var refactorings = await FindRefactoringNamesAsync(code, roslynAnalyzersEnabled);

List<string> expected = roslynAnalyzersEnabled ? new List<string>
var expected = roslynAnalyzersEnabled ? new List<(string Name, bool IsCodeFix)>
{
"Fix formatting",
"using System;",
("Fix formatting", IsCodeFix: true),
("using System;", IsCodeFix: true),
#if NETCOREAPP
"using Internal;",
"Fully qualify 'Console' -> Internal.Console",
"Fully qualify 'Console' -> System.Console",
("using Internal;", IsCodeFix: true),
("Fully qualify 'Console' -> Internal.Console", IsCodeFix: true),
("Fully qualify 'Console' -> System.Console", IsCodeFix: true),
#else
"System.Console",
("System.Console", IsCodeFix: true),
#endif
"Generate variable 'Console' -> Generate property 'Console'",
"Generate variable 'Console' -> Generate field 'Console'",
"Generate variable 'Console' -> Generate read-only field 'Console'",
"Generate variable 'Console' -> Generate local 'Console'",
"Generate variable 'Console' -> Generate parameter 'Console'",
"Generate type 'Console' -> Generate class 'Console' in new file",
"Generate type 'Console' -> Generate class 'Console'",
"Generate type 'Console' -> Generate nested class 'Console'",
"Extract local function",
"Extract method",
"Introduce local for 'Console.Write(\"should be using System;\")'"
} : new List<string>
("Generate variable 'Console' -> Generate property 'Console'", IsCodeFix: true),
("Generate variable 'Console' -> Generate field 'Console'", IsCodeFix: true),
("Generate variable 'Console' -> Generate read-only field 'Console'", IsCodeFix: true),
("Generate variable 'Console' -> Generate local 'Console'", IsCodeFix: true),
("Generate variable 'Console' -> Generate parameter 'Console'", IsCodeFix: true),
("Generate type 'Console' -> Generate class 'Console' in new file", IsCodeFix: true),
("Generate type 'Console' -> Generate class 'Console'", IsCodeFix: true),
("Generate type 'Console' -> Generate nested class 'Console'", IsCodeFix: true),
("Extract local function", IsCodeFix: false),
("Extract method", IsCodeFix: false),
("Introduce local for 'Console.Write(\"should be using System;\")'", IsCodeFix: false)
} : new List<(string Name, bool IsCodeFix)>
{
"using System;",
("using System;", IsCodeFix: true),
#if NETCOREAPP
"using Internal;",
"Fully qualify 'Console' -> Internal.Console",
"Fully qualify 'Console' -> System.Console",
("using Internal;", IsCodeFix: true),
("Fully qualify 'Console' -> Internal.Console", IsCodeFix: true),
("Fully qualify 'Console' -> System.Console", IsCodeFix: true),
#else
"System.Console",
("System.Console", IsCodeFix: true),
#endif
"Generate variable 'Console' -> Generate property 'Console'",
"Generate variable 'Console' -> Generate field 'Console'",
"Generate variable 'Console' -> Generate read-only field 'Console'",
"Generate variable 'Console' -> Generate local 'Console'",
"Generate variable 'Console' -> Generate parameter 'Console'",
"Generate type 'Console' -> Generate class 'Console' in new file",
"Generate type 'Console' -> Generate class 'Console'",
"Generate type 'Console' -> Generate nested class 'Console'",
"Extract local function",
"Extract method",
"Introduce local for 'Console.Write(\"should be using System;\")'"
("Generate variable 'Console' -> Generate property 'Console'", IsCodeFix: true),
("Generate variable 'Console' -> Generate field 'Console'", IsCodeFix: true),
("Generate variable 'Console' -> Generate read-only field 'Console'", IsCodeFix: true),
("Generate variable 'Console' -> Generate local 'Console'", IsCodeFix: true),
("Generate variable 'Console' -> Generate parameter 'Console'", IsCodeFix: true),
("Generate type 'Console' -> Generate class 'Console' in new file", IsCodeFix: true),
("Generate type 'Console' -> Generate class 'Console'", IsCodeFix: true),
("Generate type 'Console' -> Generate nested class 'Console'", IsCodeFix: true),
("Extract local function", IsCodeFix: false),
("Extract method", IsCodeFix: false),
("Introduce local for 'Console.Write(\"should be using System;\")'", IsCodeFix: false)
};
Assert.Equal(expected.OrderBy(x => x), refactorings.OrderBy(x => x));
AssertEx.Equal(expected.OrderBy(x => x.Name), refactorings.OrderBy(x => x.Name));
}

[Theory]
Expand Down
6 changes: 3 additions & 3 deletions tests/OmniSharp.Roslyn.CSharp.Tests/CodeActionsWithOptionsFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ public void Foo(string a[||], string b)

var response = await FindRefactoringNamesAsync(code);

Assert.DoesNotContain("Change signature...", response);
Assert.DoesNotContain(("Change signature...", IsCodeFix: false), response);
}

[Fact]
Expand All @@ -188,7 +188,7 @@ public async Task Blacklists_generate_type_with_UI()

var response = await FindRefactoringNamesAsync(code);

Assert.DoesNotContain("Generate type 'NonExistentBaseType' -> Generate new type...", response);
Assert.DoesNotContain(("Generate type 'NonExistentBaseType' -> Generate new type...", IsCodeFix: false), response);
}

[Fact]
Expand All @@ -206,7 +206,7 @@ public class BaseClass {}";

var response = await FindRefactoringNamesAsync(code);

Assert.DoesNotContain("Pull 'Foo' up -> Pull members up to base type...", response);
Assert.DoesNotContain(("Pull 'Foo' up -> Pull members up to base type...", IsCodeFix: false), response);
}

[Fact]
Expand Down
4 changes: 2 additions & 2 deletions tests/TestUtility/AbstractCodeActionsTestFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ protected async Task<RunCodeActionResponse> RunRefactoringAsync(string code, str
return await RunRefactoringsAsync(code, identifier, wantsChanges);
}

protected async Task<IEnumerable<string>> FindRefactoringNamesAsync(string code, bool isAnalyzersEnabled = true, bool analyzeOpenDocumentsOnly = false)
protected async Task<IEnumerable<(string Name, bool IsCodeFix)>> FindRefactoringNamesAsync(string code, bool isAnalyzersEnabled = true, bool analyzeOpenDocumentsOnly = false)
{
var codeActions = await FindRefactoringsAsync(code, TestHelpers.GetConfigurationDataWithAnalyzerConfig(isAnalyzersEnabled, analyzeOpenDocumentsOnly: analyzeOpenDocumentsOnly));

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

protected async Task<IEnumerable<OmniSharpCodeAction>> FindRefactoringsAsync(string code, IConfiguration configurationData = null)
Expand Down