Skip to content

Commit

Permalink
Fix Code Actions not applied when LSP editor is enabled (#71077)
Browse files Browse the repository at this point in the history 
* wip

* need to figure out why preview is trying to show

* remove commented out code

* fix tests

* Update SearchGraphQueryTests_NavigateTo.vb
  • Loading branch information
@akhera99
akhera99 authored Dec 13, 2023
1 parent a5c8991 commit 9367797
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 42 deletions.
2 changes: 1 addition & 1 deletion src/EditorFeatures/TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ protected static LSP.TextEdit GenerateTextEdit(string newText, int startLine, in
}
};

private protected static CodeActionResolveData CreateCodeActionResolveData(string uniqueIdentifier, LSP.Location location, string[]? codeActionPath = null, IEnumerable<string>? customTags = null)
private protected static CodeActionResolveData CreateCodeActionResolveData(string uniqueIdentifier, LSP.Location location, string[] codeActionPath, IEnumerable<string>? customTags = null)
=> new(uniqueIdentifier, customTags.ToImmutableArrayOrEmpty(), location.Range, CreateTextDocumentIdentifier(location.Uri), fixAllFlavors: null, nestedCodeActions: null, codeActionPath: codeActionPath);

private protected Task<TestLspServer> CreateTestLspServerAsync(string markup, bool mutatingLspWorkspace, LSP.ClientCapabilities clientCapabilities, bool callInitialized = true)
Expand Down
55 changes: 26 additions & 29 deletions src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionHelpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,15 @@ internal static class CodeActionHelpers
{
if (!IsCodeActionNotSupportedByLSP(suggestedAction))
{
using var _1 = ArrayBuilder<string>.GetInstance(out var codeActionPathList);
codeActions.Add(GenerateVSCodeAction(
request, documentText,
suggestedAction: suggestedAction,
codeActionKind: GetCodeActionKindFromSuggestedActionCategoryName(set.CategoryName!),
setPriority: set.Priority,
applicableRange: set.ApplicableToSpan.HasValue ? ProtocolConversions.TextSpanToRange(set.ApplicableToSpan.Value, documentText) : null,
currentSetNumber: currentSetNumber,
codeActionPathList: codeActionPathList,
currentHighestSetNumber: ref currentHighestSetNumber));
}
}
Expand Down Expand Up @@ -111,25 +113,25 @@ private static LSP.CodeAction[] GenerateCodeActions(
var diagnosticsForFix = GetApplicableDiagnostics(request.Context, suggestedAction);

using var _ = ArrayBuilder<LSP.CodeAction>.GetInstance(out var builder);
using var _1 = ArrayBuilder<string>.GetInstance(out var codeActionPath);
var nestedCodeActions = CollectNestedActions(request, codeActionKind, diagnosticsForFix, suggestedAction, codeActionPath, isTopLevelCodeAction: true);
using var _1 = ArrayBuilder<string>.GetInstance(out var codeActionPathList);
var nestedCodeActions = CollectNestedActions(request, codeActionKind, diagnosticsForFix, suggestedAction, codeActionPathList, isTopLevelCodeAction: true);

Command? nestedCodeActionCommand = null;
var title = codeAction.Title;

var codeActionPathList = codeActionPath.ToArray();
var codeActionPath = codeActionPathList.ToArray();
if (nestedCodeActions.Any())
{
nestedCodeActionCommand = new LSP.Command
{
CommandIdentifier = CodeActionsHandler.RunNestedCodeActionCommandName,
Title = title,
Arguments = [new CodeActionResolveData(title, codeAction.CustomTags, request.Range, request.TextDocument, null, nestedCodeActions: nestedCodeActions, codeActionPathList)]
Arguments = [new CodeActionResolveData(title, codeAction.CustomTags, request.Range, request.TextDocument, codeActionPath, fixAllFlavors: null, nestedCodeActions: nestedCodeActions)]
};
}

AddLSPCodeActions(builder, codeAction, request, codeActionKind, diagnosticsForFix, nestedCodeActionCommand,
nestedCodeActions, codeActionPathList, suggestedAction);
nestedCodeActions, codeActionPath, suggestedAction);

return builder.ToArray();
}
Expand All @@ -139,30 +141,30 @@ private static LSP.CodeAction[] GenerateCodeActions(
LSP.CodeActionKind codeActionKind,
LSP.Diagnostic[]? diagnosticsForFix,
IUnifiedSuggestedAction suggestedAction,
ArrayBuilder<string> codeActionPath,
ArrayBuilder<string> codeActionPathList,
bool isTopLevelCodeAction = false)
{
var codeAction = suggestedAction.OriginalCodeAction;
using var _1 = ArrayBuilder<LSP.CodeAction>.GetInstance(out var nestedCodeActions);

codeActionPath.Add(codeAction.Title);
codeActionPathList.Add(codeAction.Title);
if (suggestedAction is UnifiedSuggestedActionWithNestedActions unifiedSuggestedActions)
{
foreach (var actionSet in unifiedSuggestedActions.NestedActionSets)
{
foreach (var action in actionSet.Actions)
{
nestedCodeActions.AddRange(CollectNestedActions(request, codeActionKind, diagnosticsForFix, action, codeActionPath));
nestedCodeActions.AddRange(CollectNestedActions(request, codeActionKind, diagnosticsForFix, action, codeActionPathList));
}
}
}
else
{
if (!isTopLevelCodeAction)
{
var codeActionPathList = codeActionPath.ToArray();
var codeActionPath = codeActionPathList.ToArray();
AddLSPCodeActions(nestedCodeActions, codeAction, request, codeActionKind, diagnosticsForFix,
nestedCodeActionCommand: null, nestedCodeActions: null, codeActionPathList, suggestedAction);
nestedCodeActionCommand: null, nestedCodeActions: null, codeActionPath, suggestedAction);
}
}

Expand All @@ -177,7 +179,7 @@ private static void AddLSPCodeActions(
LSP.Diagnostic[]? diagnosticsForFix,
Command? nestedCodeActionCommand,
ImmutableArray<LSP.CodeAction>? nestedCodeActions,
string[] codeActionPathList,
string[] codeActionPath,
IUnifiedSuggestedAction suggestedAction)
{
var title = codeAction.Title;
Expand All @@ -190,7 +192,7 @@ private static void AddLSPCodeActions(
Kind = codeActionKind,
Diagnostics = diagnosticsForFix,
Command = nestedCodeActionCommand,
Data = new CodeActionResolveData(title, codeAction.CustomTags, request.Range, request.TextDocument, fixAllFlavors: null, nestedCodeActions, codeActionPathList)
Data = new CodeActionResolveData(title, codeAction.CustomTags, request.Range, request.TextDocument, codeActionPath, fixAllFlavors: null, nestedCodeActions)
});

if (suggestedAction is UnifiedCodeFixSuggestedAction unifiedCodeFixSuggestedAction && unifiedCodeFixSuggestedAction.FixAllFlavors is not null)
Expand All @@ -201,7 +203,7 @@ private static void AddLSPCodeActions(
{
CommandIdentifier = CodeActionsHandler.RunFixAllCodeActionCommandName,
Title = fixAllTitle,
Arguments = [new CodeActionResolveData(fixAllTitle, codeAction.CustomTags, request.Range, request.TextDocument, fixAllFlavors.ToArray(), nestedCodeActions: null, codeActionPathList)]
Arguments = [new CodeActionResolveData(fixAllTitle, codeAction.CustomTags, request.Range, request.TextDocument, codeActionPath, fixAllFlavors.ToArray(), nestedCodeActions: null)]
};

builder.Add(new LSP.CodeAction
Expand All @@ -210,7 +212,7 @@ private static void AddLSPCodeActions(
Command = command,
Kind = codeActionKind,
Diagnostics = diagnosticsForFix,
Data = new CodeActionResolveData(fixAllTitle, codeAction.CustomTags, request.Range, request.TextDocument, fixAllFlavors.ToArray(), nestedCodeActions: null, codeActionPathList)
Data = new CodeActionResolveData(fixAllTitle, codeAction.CustomTags, request.Range, request.TextDocument, codeActionPath, fixAllFlavors.ToArray(), nestedCodeActions: null)
});
}
}
Expand All @@ -222,22 +224,14 @@ private static VSInternalCodeAction GenerateVSCodeAction(
CodeActionPriority setPriority,
LSP.Range? applicableRange,
int currentSetNumber,
ref int currentHighestSetNumber,
string currentTitle = "")
ArrayBuilder<string> codeActionPathList,
ref int currentHighestSetNumber)
{
if (!string.IsNullOrEmpty(currentTitle))
{
// Adding a delimiter for nested code actions, e.g. 'Suppress or Configure issues|Suppress IDEXXXX|in Source'
currentTitle += '|';
}

var codeAction = suggestedAction.OriginalCodeAction;
currentTitle += codeAction.Title;

var diagnosticsForFix = GetApplicableDiagnostics(request.Context, suggestedAction);

// Nested code actions' unique identifiers consist of: parent code action unique identifier + '|' + title of code action
var nestedActions = GenerateNestedVSCodeActions(request, documentText, suggestedAction, codeActionKind, ref currentHighestSetNumber, currentTitle);
var nestedActions = GenerateNestedVSCodeActions(request, documentText, suggestedAction, codeActionKind, ref currentHighestSetNumber, codeActionPathList);
var codeActionPath = codeActionPathList.ToArray();

return new VSInternalCodeAction
{
Expand All @@ -248,7 +242,7 @@ private static VSInternalCodeAction GenerateVSCodeAction(
Priority = UnifiedSuggestedActionSetPriorityToPriorityLevel(setPriority),
Group = $"Roslyn{currentSetNumber}",
ApplicableRange = applicableRange,
Data = new CodeActionResolveData(currentTitle, codeAction.CustomTags, request.Range, request.TextDocument, fixAllFlavors: null, nestedCodeActions: null, codeActionPath: null)
Data = new CodeActionResolveData(codeAction.Title, codeAction.CustomTags, request.Range, request.TextDocument, fixAllFlavors: null, nestedCodeActions: null, codeActionPath: codeActionPath)
};

static VSInternalCodeAction[] GenerateNestedVSCodeActions(
Expand All @@ -257,8 +251,11 @@ static VSInternalCodeAction[] GenerateNestedVSCodeActions(
IUnifiedSuggestedAction suggestedAction,
CodeActionKind codeActionKind,
ref int currentHighestSetNumber,
string currentTitle)
ArrayBuilder<string> codeActionPath)
{
var codeAction = suggestedAction.OriginalCodeAction;
codeActionPath.Add(codeAction.Title);

if (suggestedAction is not UnifiedSuggestedActionWithNestedActions suggestedActionWithNestedActions)
{
return Array.Empty<VSInternalCodeAction>();
Expand All @@ -275,7 +272,7 @@ static VSInternalCodeAction[] GenerateNestedVSCodeActions(
request, documentText, nestedSuggestedAction, codeActionKind, nestedActionSet.Priority,
applicableRange: nestedActionSet.ApplicableToSpan.HasValue
? ProtocolConversions.TextSpanToRange(nestedActionSet.ApplicableToSpan.Value, documentText) : null,
nestedSetNumber, ref currentHighestSetNumber, currentTitle));
nestedSetNumber, codeActionPath, ref currentHighestSetNumber));
}
}

Expand Down
11 changes: 5 additions & 6 deletions src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionResolveData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,31 +32,30 @@ internal class CodeActionResolveData

public LSP.TextDocumentIdentifier TextDocument { get; }

public string[] CodeActionPath { get; }

[JsonProperty(NullValueHandling = NullValueHandling.Ignore)]
public string[]? FixAllFlavors { get; }

[JsonProperty(NullValueHandling = NullValueHandling.Ignore)]
public ImmutableArray<CodeAction>? NestedCodeActions { get; }

[JsonProperty(NullValueHandling = NullValueHandling.Ignore)]
public string[]? CodeActionPath { get; }

public CodeActionResolveData(
string uniqueIdentifier,
ImmutableArray<string> customTags,
LSP.Range range,
LSP.TextDocumentIdentifier textDocument,
string[] codeActionPath,
string[]? fixAllFlavors,
ImmutableArray<CodeAction>? nestedCodeActions,
string[]? codeActionPath)
ImmutableArray<CodeAction>? nestedCodeActions)
{
UniqueIdentifier = uniqueIdentifier;
CustomTags = customTags;
Range = range;
TextDocument = textDocument;
CodeActionPath = codeActionPath;
FixAllFlavors = fixAllFlavors;
NestedCodeActions = nestedCodeActions;
CodeActionPath = codeActionPath;
}
}
}
13 changes: 8 additions & 5 deletions src/Features/LanguageServer/ProtocolUnitTests/CodeActions/CodeActionsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ void M()
""";
await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace, initializationOptions: new InitializationOptions() { ClientCapabilities = new VSInternalClientCapabilities { SupportsVisualStudioExtensions = true } });

var titlePath = new[] { CSharpAnalyzersResources.Use_implicit_type };
var caretLocation = testLspServer.GetLocations("caret").Single();
var expected = CreateCodeAction(
title: CSharpAnalyzersResources.Use_implicit_type,
Expand All @@ -45,6 +46,7 @@ void M()
data: CreateCodeActionResolveData(
CSharpAnalyzersResources.Use_implicit_type,
caretLocation,
codeActionPath: titlePath,
customTags: new[] { PredefinedCodeRefactoringProviderNames.UseImplicitType }),
priority: VSInternalPriorityLevel.Low,
groupName: "Roslyn2",
Expand Down Expand Up @@ -73,24 +75,25 @@ void M()
await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace, CapabilitiesWithVSExtensions);

var caretLocation = testLspServer.GetLocations("caret").Single();
var titlePath = new[] { FeaturesResources.Introduce_constant, string.Format(FeaturesResources.Introduce_constant_for_0, "1") };
var expected = CreateCodeAction(
title: string.Format(FeaturesResources.Introduce_constant_for_0, "1"),
kind: CodeActionKind.Refactor,
children: Array.Empty<VSInternalCodeAction>(),
data: CreateCodeActionResolveData(
FeaturesResources.Introduce_constant + '|' + string.Format(FeaturesResources.Introduce_constant_for_0, "1"),
caretLocation),
string.Format(FeaturesResources.Introduce_constant_for_0, "1"),
caretLocation,
codeActionPath: titlePath),
priority: VSInternalPriorityLevel.Normal,
groupName: "Roslyn3",
applicableRange: new LSP.Range { Start = new Position { Line = 4, Character = 12 }, End = new Position { Line = 4, Character = 12 } },
diagnostics: null);

var results = await RunGetCodeActionsAsync(testLspServer, CreateCodeActionParams(caretLocation));

var topLevelAction = Assert.Single(results.Where(action => action.Title == FeaturesResources.Introduce_constant));
var expectedChildActionTitle = FeaturesResources.Introduce_constant + '|' + string.Format(FeaturesResources.Introduce_constant_for_0, "1");
var topLevelAction = Assert.Single(results.Where(action => action.Title == titlePath[0]));
var introduceConstant = topLevelAction.Children.FirstOrDefault(
r => ((JObject)r.Data!).ToObject<CodeActionResolveData>()!.UniqueIdentifier == expectedChildActionTitle);
r => ((JObject)r.Data!).ToObject<CodeActionResolveData>()!.UniqueIdentifier == titlePath[1]);

AssertJsonEquals(expected, introduceConstant);
}
Expand Down
3 changes: 2 additions & 1 deletion src/Features/LanguageServer/ProtocolUnitTests/CodeActions/RunCodeActionsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ class B
Uri = caretLocation.Uri
};

var commandArgument = new CodeActionResolveData(string.Format(FeaturesResources.Move_type_to_0, "B.cs"), customTags: ImmutableArray<string>.Empty, caretLocation.Range, documentId, fixAllFlavors: null, nestedCodeActions: null, codeActionPath: null);
var titlePath = new[] { string.Format(FeaturesResources.Move_type_to_0, "B.cs") };
var commandArgument = new CodeActionResolveData(string.Format(FeaturesResources.Move_type_to_0, "B.cs"), customTags: ImmutableArray<string>.Empty, caretLocation.Range, documentId, fixAllFlavors: null, nestedCodeActions: null, codeActionPath: titlePath);

var results = await ExecuteRunCodeActionCommandAsync(testLspServer, commandArgument);

Expand Down

0 comments on commit 9367797

Please sign in to comment.