From fe24b8a74459b6780c7c4342f394073c10538438 Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Sun, 5 Jul 2020 23:28:01 -0700 Subject: [PATCH] Code review feedback and move resource file --- .../Core.Wpf/EditorFeaturesWpfResources.resx | 3 - .../Suggestions/SuggestedActionsSource.cs | 4 +- .../xlf/EditorFeaturesWpfResources.cs.xlf | 5 - .../xlf/EditorFeaturesWpfResources.de.xlf | 5 - .../xlf/EditorFeaturesWpfResources.es.xlf | 5 - .../xlf/EditorFeaturesWpfResources.fr.xlf | 5 - .../xlf/EditorFeaturesWpfResources.it.xlf | 5 - .../xlf/EditorFeaturesWpfResources.ja.xlf | 5 - .../xlf/EditorFeaturesWpfResources.ko.xlf | 5 - .../xlf/EditorFeaturesWpfResources.pl.xlf | 5 - .../xlf/EditorFeaturesWpfResources.pt-BR.xlf | 5 - .../xlf/EditorFeaturesWpfResources.ru.xlf | 5 - .../xlf/EditorFeaturesWpfResources.tr.xlf | 5 - .../EditorFeaturesWpfResources.zh-Hans.xlf | 5 - .../EditorFeaturesWpfResources.zh-Hant.xlf | 5 - .../AbstractLanguageServerProtocolTests.cs | 9 +- .../Handler/CodeActions/CodeActionHelpers.cs | 118 ++++++++++++++++++ .../CodeActions/CodeActionResolveData.cs | 25 ++-- .../CodeActions/CodeActionResolveHandler.cs | 109 +++++----------- .../Handler/CodeActions/CodeActionsHandler.cs | 70 +++-------- .../CodeActions/RunCodeActionsHandler.cs | 32 ++--- .../Handler/Initialize/InitializeHandler.cs | 2 +- .../CodeActions/CodeActionResolveTests.cs | 20 +-- .../CodeActions/CodeActionsTests.cs | 14 +-- 24 files changed, 222 insertions(+), 249 deletions(-) create mode 100644 src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionHelpers.cs diff --git a/src/EditorFeatures/Core.Wpf/EditorFeaturesWpfResources.resx b/src/EditorFeatures/Core.Wpf/EditorFeaturesWpfResources.resx index abc628fc3e0..d3a76af4a44 100644 --- a/src/EditorFeatures/Core.Wpf/EditorFeaturesWpfResources.resx +++ b/src/EditorFeatures/Core.Wpf/EditorFeaturesWpfResources.resx @@ -153,9 +153,6 @@ Regex - Other Escape - - Suppress or Configure issues - Gathering Suggestions - '{0}' diff --git a/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs b/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs index 017cefa1757..628c53aec74 100644 --- a/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs +++ b/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs @@ -684,7 +684,7 @@ static CodeFixGroupKey GetGroupKey(CodeFix fix) // to avoid clutter in the light bulb menu. var wrappingSuggestedAction = new SuggestedActionWithNestedActions( ThreadingContext, _owner, workspace, _subjectBuffer, this, - codeAction: new NoChangeAction(EditorFeaturesWpfResources.Suppress_or_Configure_issues), + codeAction: new NoChangeAction(CodeFixesResources.Suppress_or_Configure_issues), nestedActionSets: suppressionSets.ToImmutable()); // Combine the spans and the category of each of the nested suggested actions @@ -693,7 +693,7 @@ static CodeFixGroupKey GetGroupKey(CodeFix fix) var wrappingSet = new SuggestedActionSet( category, actions: SpecializedCollections.SingletonEnumerable(wrappingSuggestedAction), - title: EditorFeaturesWpfResources.Suppress_or_Configure_issues, + title: CodeFixesResources.Suppress_or_Configure_issues, priority: SuggestedActionSetPriority.None, applicableToSpan: span); sets = sets.Add(wrappingSet); diff --git a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.cs.xlf b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.cs.xlf index bb9e67c0272..35b739582c5 100644 --- a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.cs.xlf +++ b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.cs.xlf @@ -72,11 +72,6 @@ Regulární výrazy – ostatní řídicí znaky - - Suppress or Configure issues - Potlačit nebo konfigurovat problémy - - \ No newline at end of file diff --git a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.de.xlf b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.de.xlf index 3f7c87d97cb..9841f3e5e08 100644 --- a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.de.xlf +++ b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.de.xlf @@ -72,11 +72,6 @@ RegEx - Anderes Escapezeichen - - Suppress or Configure issues - Issues unterdrücken oder konfigurieren - - \ No newline at end of file diff --git a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.es.xlf b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.es.xlf index 471a76b0c3e..415f9b2dd06 100644 --- a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.es.xlf +++ b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.es.xlf @@ -72,11 +72,6 @@ Regex - otro Escape - - Suppress or Configure issues - Suprimir o configurar incidencias - - \ No newline at end of file diff --git a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.fr.xlf b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.fr.xlf index d4e0173209e..761e751e092 100644 --- a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.fr.xlf +++ b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.fr.xlf @@ -72,11 +72,6 @@ Regex - autre échappement - - Suppress or Configure issues - Supprimer ou configurer les problèmes - - \ No newline at end of file diff --git a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.it.xlf b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.it.xlf index 1e2002a8d32..52146e8d09f 100644 --- a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.it.xlf +++ b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.it.xlf @@ -72,11 +72,6 @@ Regex - Altro carattere di escape - - Suppress or Configure issues - Elimina o configura problemi - - \ No newline at end of file diff --git a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.ja.xlf b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.ja.xlf index a8df419a35d..73c5ba03214 100644 --- a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.ja.xlf +++ b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.ja.xlf @@ -72,11 +72,6 @@ 正規表現 - 他のエスケープ - - Suppress or Configure issues - 問題の抑制または構成 - - \ No newline at end of file diff --git a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.ko.xlf b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.ko.xlf index 42704a0816a..9d55332e836 100644 --- a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.ko.xlf +++ b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.ko.xlf @@ -72,11 +72,6 @@ Regex - 기타 이스케이프 - - Suppress or Configure issues - 문제 표시 안 함 또는 구성 - - \ No newline at end of file diff --git a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.pl.xlf b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.pl.xlf index b6f3b62060d..220d14cea4e 100644 --- a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.pl.xlf +++ b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.pl.xlf @@ -72,11 +72,6 @@ Wyrażenie regularne — inna sekwencja ucieczki - - Suppress or Configure issues - Problemy z pomijaniem lub konfigurowaniem - - \ No newline at end of file diff --git a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.pt-BR.xlf b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.pt-BR.xlf index fddad3a55c4..01a2829c417 100644 --- a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.pt-BR.xlf +++ b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.pt-BR.xlf @@ -72,11 +72,6 @@ Regex – Outro Escape - - Suppress or Configure issues - Suprimir ou Configurar os problemas - - \ No newline at end of file diff --git a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.ru.xlf b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.ru.xlf index 4ba9de46390..399bd885551 100644 --- a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.ru.xlf +++ b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.ru.xlf @@ -72,11 +72,6 @@ Регулярные выражения — другая escape-последовательность - - Suppress or Configure issues - Подавление проблем или настройка уровня их серьезности - - \ No newline at end of file diff --git a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.tr.xlf b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.tr.xlf index 029aa377b2a..be138de9785 100644 --- a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.tr.xlf +++ b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.tr.xlf @@ -72,11 +72,6 @@ Regex - diğer kaçış - - Suppress or Configure issues - Sorunları Gizle veya Yapılandır - - \ No newline at end of file diff --git a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.zh-Hans.xlf b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.zh-Hans.xlf index d20299b06e7..b7f3abf2024 100644 --- a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.zh-Hans.xlf +++ b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.zh-Hans.xlf @@ -72,11 +72,6 @@ 正则表达式 - 其他转义 - - Suppress or Configure issues - 禁止或配置方面的问题 - - \ No newline at end of file diff --git a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.zh-Hant.xlf b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.zh-Hant.xlf index a11b02105d5..921573a3c0e 100644 --- a/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.zh-Hant.xlf +++ b/src/EditorFeatures/Core.Wpf/xlf/EditorFeaturesWpfResources.zh-Hant.xlf @@ -72,11 +72,6 @@ Regex - 其他逸出 - - Suppress or Configure issues - 隱藏或設定問題 - - \ No newline at end of file diff --git a/src/EditorFeatures/TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.cs b/src/EditorFeatures/TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.cs index 5cf615eeb9b..f1671270ecd 100644 --- a/src/EditorFeatures/TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.cs +++ b/src/EditorFeatures/TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.cs @@ -154,7 +154,7 @@ protected static LSP.SymbolInformation CreateSymbolInformation(LSP.SymbolKind ki protected static LSP.TextDocumentIdentifier CreateTextDocumentIdentifier(Uri uri, ProjectId? projectContext = null) { - var documentIdentifier = new LSP.VSTextDocumentIdentifier() { Uri = uri }; + var documentIdentifier = new LSP.VSTextDocumentIdentifier { Uri = uri }; if (projectContext != null) { @@ -209,12 +209,7 @@ protected static LSP.VSCompletionItem CreateCompletionItem(string text, LSP.Comp }; private protected static CodeActionResolveData CreateCodeActionResolveData(string codeActionTitle, LSP.Location location) - => new CodeActionResolveData() - { - DistinctTitle = codeActionTitle, - Range = location.Range, - TextDocument = CreateTextDocumentIdentifier(location.Uri) - }; + => new CodeActionResolveData(codeActionTitle, location.Range, CreateTextDocumentIdentifier(location.Uri)); /// /// Creates a solution with a document. diff --git a/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionHelpers.cs b/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionHelpers.cs new file mode 100644 index 00000000000..762c6cc0c66 --- /dev/null +++ b/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionHelpers.cs @@ -0,0 +1,118 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +#nullable enable + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CodeRefactorings; +using LSP = Microsoft.VisualStudio.LanguageServer.Protocol; + +namespace Microsoft.CodeAnalysis.LanguageServer.Handler.CodeActions +{ + internal static class CodeActionHelpers + { + public static async Task> GetCodeActionsAsync( + Document? document, + ICodeFixService codeFixService, + ICodeRefactoringService codeRefactoringService, + LSP.Range selection, + CancellationToken cancellationToken) + { + if (document == null) + { + return ImmutableArray.Empty; + } + + var (codeFixCollections, codeRefactorings) = await GetCodeFixesAndRefactoringsAsync( + document, codeFixService, + codeRefactoringService, selection, + cancellationToken).ConfigureAwait(false); + + var codeActions = codeFixCollections.SelectMany(c => c.Fixes.Select(f => f.Action)).Concat( + codeRefactorings.SelectMany(r => r.CodeActions.Select(ca => ca.action))); + + return codeActions; + } + + public static async Task<(ImmutableArray, ImmutableArray)> GetCodeFixesAndRefactoringsAsync( + Document document, + ICodeFixService codeFixService, + ICodeRefactoringService codeRefactoringService, + LSP.Range selection, + CancellationToken cancellationToken) + { + var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); + var textSpan = ProtocolConversions.RangeToTextSpan(selection, text); + + var codeFixCollections = Array.Empty().ToImmutableArray(); + var codeRefactorings = Array.Empty().ToImmutableArray(); + + var codeFixCollectionsTask = Task.Run( + async () => codeFixCollections = await codeFixService.GetFixesAsync( + document, textSpan, includeSuppressionFixes: true, cancellationToken).ConfigureAwait(false)); + var codeRefactoringsTask = Task.Run( + async () => codeRefactorings = await codeRefactoringService.GetRefactoringsAsync( + document, textSpan, cancellationToken).ConfigureAwait(false)); + + await Task.WhenAll(codeFixCollectionsTask, codeRefactoringsTask).ConfigureAwait(false); + return (codeFixCollections, codeRefactorings); + } + + public static CodeAction? GetCodeActionToResolve(string distinctTitle, ImmutableArray codeActions) + { + // Searching for the matching code action. We compare against the unique identifier + // (e.g. "Suppress or Configure issues.Configure IDExxxx.Warning") instead of the + // code action's title (e.g. "Warning") since there's a chance that multiple code + // actions may have the same title (e.g. there could be multiple code actions with + // the title "Warning" that appear in the code action menu if there are multiple + // diagnostics on the same line). + foreach (var c in codeActions) + { + var action = CheckForMatchingAction(c, distinctTitle, currentTitle: ""); + if (action != null) + { + return action; + } + } + + return null; + } + + private static CodeAction? CheckForMatchingAction(CodeAction codeAction, string goalTitle, string currentTitle) + { + // Adding a delimiter for nested code actions, e.g. 'Suppress or Configure issues.Suppress IDEXXXX.in Source' + if (currentTitle != "") + { + currentTitle += '.'; + } + + // If the unique identifier of the current code action matches the unique identifier of the code action + // we're looking for, return the code action. If not, check to see if one of the current code action's + // nested actions may be a match. + var updatedTitle = currentTitle + codeAction.Title; + if (updatedTitle == goalTitle) + { + return codeAction; + } + + foreach (var nestedAction in codeAction.NestedCodeActions) + { + var match = CheckForMatchingAction(nestedAction, goalTitle, updatedTitle); + if (match != null) + { + return match; + } + } + + return null; + } + } +} diff --git a/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionResolveData.cs b/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionResolveData.cs index 5e8d19f5846..00298af453e 100644 --- a/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionResolveData.cs +++ b/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionResolveData.cs @@ -7,22 +7,31 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Handler.CodeActions { /// - /// Provides data needed to resolve code actions. + /// This class provides the intermediate data passed between CodeActionsHandler, CodeActionResolveHandler, + /// and RunCodeActionsHandler. The class provides enough information for each handler to identify the code + /// action that it is dealing with. The information is passed along via the Data property in LSP.VSCodeAction. /// internal class CodeActionResolveData { /// - /// The unique title of a code action. No two code actions should - /// have the same unique title. + /// The unique identifier of a code action. No two code actions should have the same unique identifier. /// /// - /// The distinct tiel is currently set as: - /// name of top level code action + name of nested code action + nested nested code action etc. + /// The unique identifier is currently set as: + /// name of top level code action + '.' + name of nested code action + '.' + name of nested nested code action + etc. + /// e.g. 'Suppress or Configure issues.Suppress IDEXXXX.in Source' /// - public string DistinctTitle { get; set; } + public string UniqueIdentifier { get; } - public LSP.Range Range { get; set; } + public LSP.Range Range { get; } - public LSP.TextDocumentIdentifier TextDocument { get; set; } + public LSP.TextDocumentIdentifier TextDocument { get; } + + public CodeActionResolveData(string uniqueIdentifier, LSP.Range range, LSP.TextDocumentIdentifier textDocument) + { + UniqueIdentifier = uniqueIdentifier; + Range = range; + TextDocument = textDocument; + } } } diff --git a/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionResolveHandler.cs b/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionResolveHandler.cs index 46286ad407d..20f4aabee2e 100644 --- a/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionResolveHandler.cs +++ b/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionResolveHandler.cs @@ -6,6 +6,7 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.Composition; using System.Linq; using System.Threading; @@ -14,19 +15,18 @@ using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CodeRefactorings; using Microsoft.CodeAnalysis.Host.Mef; -using Microsoft.CodeAnalysis.LanguageServer.CustomProtocol; using Microsoft.CodeAnalysis.LanguageServer.Handler.CodeActions; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.VisualStudio.LanguageServer.Protocol; -using Newtonsoft.Json; using Newtonsoft.Json.Linq; -using CodeAction = Microsoft.CodeAnalysis.CodeActions.CodeAction; using LSP = Microsoft.VisualStudio.LanguageServer.Protocol; namespace Microsoft.CodeAnalysis.LanguageServer.Handler { /// - /// Resolves a code action by filling out its Edit or Command property. + /// Resolves a code action by filling out its Edit and/or Command property. + /// The handler is triggered only when a user hovers over a code action, which + /// saves us from having to compute unnecessary edits. /// [ExportLspMethod(MSLSPMethods.TextDocumentCodeActionResolveName), Shared] internal class CodeActionResolveHandler : AbstractRequestHandler @@ -54,19 +54,15 @@ internal class CodeActionResolveHandler : AbstractRequestHandler(); var document = SolutionProvider.GetDocument(data.TextDocument, clientName); - var codeActions = await CodeActionsHandler.GetCodeActionsAsync( + var codeActions = await CodeActionHelpers.GetCodeActionsAsync( document, _codeFixService, _codeRefactoringService, data.Range, cancellationToken).ConfigureAwait(false); - if (codeActions == null || !codeActions.Any()) - { - return codeAction; - } - - var codeActionToResolve = GetCodeActionToResolve(data.DistinctTitle, codeActions); + var codeActionToResolve = CodeActionHelpers.GetCodeActionToResolve( + data.UniqueIdentifier, codeActions.ToImmutableArray()); // We didn't find a matching action, so just return the action without an edit or command. if (codeActionToResolve == null) @@ -90,11 +86,12 @@ internal class CodeActionResolveHandler : AbstractRequestHandler operation is ApplyChangesOperation); + // Add workspace edits + var applyChangesOperations = operations.OfType(); if (applyChangesOperations.Any()) { var workspaceEdits = await ComputeWorkspaceEdits(applyChangesOperations, document!, cancellationToken).ConfigureAwait(false); - if (workspaceEdits.Any()) + if (workspaceEdits != null) { codeAction.Edit = new LSP.WorkspaceEdit { DocumentChanges = workspaceEdits }; } @@ -109,19 +106,24 @@ internal class CodeActionResolveHandler : AbstractRequestHandler !(operation is ApplyChangesOperation)); if (commandOperations || runAsCommand) { - codeAction.Command = SetCommand(codeAction, data); + codeAction.Command = new LSP.Command + { + CommandIdentifier = CodeActionsHandler.RunCodeActionCommandName, + Title = codeAction.Title, + Arguments = new object[] { data } + }; } return codeAction; // Local functions - static async Task ComputeWorkspaceEdits( - IEnumerable applyChangesOperations, + static async Task ComputeWorkspaceEdits( + IEnumerable applyChangesOperations, Document document, CancellationToken cancellationToken) { using var _ = ArrayBuilder.GetInstance(out var textDocumentEdits); - foreach (ApplyChangesOperation applyChangesOperation in applyChangesOperations) + foreach (var applyChangesOperation in applyChangesOperations) { var solution = document.Project.Solution; var changes = applyChangesOperation.ChangedSolution.GetChanges(solution); @@ -134,7 +136,7 @@ internal class CodeActionResolveHandler : AbstractRequestHandler pc.GetAddedDocuments().Concat(pc.GetAddedAdditionalDocuments().Concat(pc.GetAddedAnalyzerConfigDocuments()))); if (addedDocuments.Any()) { - return Array.Empty(); + return null; } var changedDocuments = projectChanges.SelectMany(pc => pc.GetChangedDocuments()); @@ -149,11 +151,11 @@ internal class CodeActionResolveHandler : AbstractRequestHandler(); + return null; } // Changed documents - await GetTextDocumentEdits( + await AddTextDocumentEdits( textDocumentEdits, applyChangesOperation, solution, changedDocuments, applyChangesOperation.ChangedSolution.GetDocument, solution.GetDocument, cancellationToken).ConfigureAwait(false); @@ -161,23 +163,23 @@ internal class CodeActionResolveHandler : AbstractRequestHandler( + static async Task AddTextDocumentEdits( ArrayBuilder textDocumentEdits, ApplyChangesOperation applyChangesOperation, Solution solution, @@ -191,67 +193,18 @@ internal class CodeActionResolveHandler : AbstractRequestHandler ProtocolConversions.TextChangeToTextEdit(tc, oldText)).ToArray(); - var documentIdentifier = new VersionedTextDocumentIdentifier() { Uri = newDoc.GetURI() }; - textDocumentEdits.Add(new TextDocumentEdit() { TextDocument = documentIdentifier, Edits = edits.ToArray() }); + var documentIdentifier = new VersionedTextDocumentIdentifier { Uri = newDoc.GetURI() }; + textDocumentEdits.Add(new TextDocumentEdit { TextDocument = documentIdentifier, Edits = edits.ToArray() }); } } } - - static LSP.Command SetCommand(VSCodeAction codeAction, CodeActionResolveData data) => new LSP.Command - { - CommandIdentifier = CodeActionsHandler.RunCodeActionCommandName, - Title = codeAction.Title, - Arguments = new object[] { data } - }; - } - - internal static CodeAction? GetCodeActionToResolve(string distinctTitle, IEnumerable codeActions) - { - // Searching for the matching code action. We compare against the distinct title (e.g. "Suppress IDExxxxNone") - // instead of the regular title (e.g. "None") since there's a chance that multiple code actions may have - // the same regular title. - CodeAction? codeActionToResolve = null; - foreach (var c in codeActions) - { - var action = CheckForMatchingAction(c, distinctTitle, currentTitle: ""); - if (action != null) - { - codeActionToResolve = action; - break; - } - } - - return codeActionToResolve; - } - - private static CodeAction? CheckForMatchingAction(CodeAction codeAction, string goalTitle, string currentTitle) - { - if (currentTitle + codeAction.Title == goalTitle) - { - return codeAction; - } - - foreach (var nestedAction in codeAction.NestedCodeActions) - { - var match = CheckForMatchingAction(nestedAction, goalTitle, currentTitle + codeAction.Title); - if (match != null) - { - return match; - } - } - - return null; } } } diff --git a/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionsHandler.cs b/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionsHandler.cs index 6a11433730d..7dc4f402684 100644 --- a/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionsHandler.cs +++ b/src/Features/LanguageServer/Protocol/Handler/CodeActions/CodeActionsHandler.cs @@ -14,7 +14,6 @@ using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CodeRefactorings; -using Microsoft.CodeAnalysis.Editor; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.LanguageServer.Handler.CodeActions; using Microsoft.CodeAnalysis.PooledObjects; @@ -26,7 +25,9 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Handler { /// - /// Handles the get code actions command. + /// Handles the initial request for code actions. Leaves the Edit and Command properties + /// of the returned VSCodeActions blank, as these properties should be populated by the + /// CodeActionsResolveHandler only when the user requests them. /// [ExportLspMethod(LSP.Methods.TextDocumentCodeActionName), Shared] internal class CodeActionsHandler : AbstractRequestHandler @@ -60,19 +61,14 @@ internal class CodeActionsHandler : AbstractRequestHandler(); } - var (codeFixCollections, codeRefactorings) = await GetCodeFixesAndRefactoringsAsync( - document, _codeFixService, _codeRefactoringService, - request.Range, cancellationToken).ConfigureAwait(false); + var (codeFixCollections, codeRefactorings) = await CodeActionHelpers.GetCodeFixesAndRefactoringsAsync( + document, _codeFixService, _codeRefactoringService, + request.Range, cancellationToken).ConfigureAwait(false); var codeFixes = codeFixCollections.SelectMany(c => c.Fixes); - - var suppressionActions = codeFixes.Where( - a => a.Action is AbstractConfigurationActionWithNestedActions && - (a.Action as AbstractConfigurationActionWithNestedActions)?.IsBulkConfigurationAction == false); - using var _ = ArrayBuilder.GetInstance(out var results); - // We go through code fixes and code refactorings separately so that we can properly set the CodeActionKind. + // Go through code fixes and code refactorings separately so that we can properly set the CodeActionKind. foreach (var codeFix in codeFixes) { // Filter out code actions with options since they'll show dialogs and we can't remote the UI and the options. @@ -122,6 +118,14 @@ internal class CodeActionsHandler : AbstractRequestHandler.GetInstance(out var nestedActions); + + // Adding a delimiter for nested code actions, e.g. 'Suppress or Configure issues.Suppress IDEXXXX.in Source' + if (parentTitle != "") + { + parentTitle += '.'; + } + + // Nested code actions' unique identifiers consist of: parent code action unique identifier + '.' + title of code action foreach (var action in codeAction.NestedCodeActions) { nestedActions.Add(GenerateVSCodeAction(request, action, codeActionKind, codeAction.Title)); @@ -133,51 +137,9 @@ internal class CodeActionsHandler : AbstractRequestHandler> GetCodeActionsAsync( - Document? document, - ICodeFixService codeFixService, - ICodeRefactoringService codeRefactoringService, - LSP.Range selection, - CancellationToken cancellationToken) - { - if (document == null) - { - return ImmutableArray.Empty; - } - - var (codeFixCollections, codeRefactorings) = await GetCodeFixesAndRefactoringsAsync( - document, codeFixService, - codeRefactoringService, selection, - cancellationToken).ConfigureAwait(false); - - var codeActions = codeFixCollections.SelectMany(c => c.Fixes.Select(f => f.Action)).Concat( - codeRefactorings.SelectMany(r => r.CodeActions.Select(ca => ca.action))); - - return codeActions; - } - - internal static async Task<(ImmutableArray, ImmutableArray)> GetCodeFixesAndRefactoringsAsync( - Document document, - ICodeFixService codeFixService, - ICodeRefactoringService codeRefactoringService, - LSP.Range selection, - CancellationToken cancellationToken) - { - var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); - var textSpan = ProtocolConversions.RangeToTextSpan(selection, text); - var codeFixCollections = await codeFixService.GetFixesAsync(document, textSpan, true, cancellationToken).ConfigureAwait(false); - var codeRefactorings = await codeRefactoringService.GetRefactoringsAsync(document, textSpan, cancellationToken).ConfigureAwait(false); - return (codeFixCollections, codeRefactorings); - } } } diff --git a/src/Features/LanguageServer/Protocol/Handler/CodeActions/RunCodeActionsHandler.cs b/src/Features/LanguageServer/Protocol/Handler/CodeActions/RunCodeActionsHandler.cs index 1960b9fa254..9288c9e2e53 100644 --- a/src/Features/LanguageServer/Protocol/Handler/CodeActions/RunCodeActionsHandler.cs +++ b/src/Features/LanguageServer/Protocol/Handler/CodeActions/RunCodeActionsHandler.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Immutable; using System.ComponentModel.Composition; using System.Linq; using System.Threading; @@ -12,7 +13,6 @@ using Microsoft.CodeAnalysis.Editor.Shared.Utilities; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.LanguageServer; -using Microsoft.CodeAnalysis.LanguageServer.CustomProtocol; using Microsoft.CodeAnalysis.LanguageServer.Handler; using Microsoft.CodeAnalysis.LanguageServer.Handler.CodeActions; using Microsoft.CodeAnalysis.LanguageServer.Handler.Commands; @@ -22,7 +22,11 @@ namespace Microsoft.VisualStudio.LanguageServices.LiveShare { /// - /// Runs code actions as a command on the server. + /// Runs a code action as a command on the server. + /// This is done when a code action cannot be applied as a WorkspaceEdit on the LSP client. + /// For example, all non-ApplyChangesOperations must be applied as a command. + /// TO-DO: Currently, any ApplyChangesOperation that adds or modifies an outside document must also be + /// applied as a command due to an LSP bug (see https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1147293/). /// Commands must be applied from the UI thread in VS. /// [ExportExecuteWorkspaceCommand(CodeActionsHandler.RunCodeActionCommandName)] @@ -54,28 +58,28 @@ internal class RunCodeActionsHandler : IExecuteWorkspaceCommandHandler { var runRequest = ((JToken)request.Arguments.Single()).ToObject(); var document = _solutionProvider.GetDocument(runRequest.TextDocument); - var codeActions = await CodeActionsHandler.GetCodeActionsAsync(document, _codeFixService, _codeRefactoringService, + var codeActions = await CodeActionHelpers.GetCodeActionsAsync(document, _codeFixService, _codeRefactoringService, runRequest.Range, cancellationToken).ConfigureAwait(false); if (codeActions == null) { return false; } - var actionToRun = CodeActionResolveHandler.GetCodeActionToResolve(runRequest.DistinctTitle, codeActions); - if (actionToRun != null) + var actionToRun = CodeActionHelpers.GetCodeActionToResolve(runRequest.UniqueIdentifier, codeActions.ToImmutableArray()); + if (actionToRun == null) { - foreach (var operation in await actionToRun.GetOperationsAsync(cancellationToken).ConfigureAwait(false)) - { - // TODO - This UI thread dependency should be removed. - // https://github.com/dotnet/roslyn/projects/45#card-20619668 - await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); - operation.Apply(document.Project.Solution.Workspace, cancellationToken); - } + return false; + } - return true; + foreach (var operation in await actionToRun.GetOperationsAsync(cancellationToken).ConfigureAwait(false)) + { + // TODO - This UI thread dependency should be removed. + // https://github.com/dotnet/roslyn/projects/45#card-20619668 + await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); + operation.Apply(document.Project.Solution.Workspace, cancellationToken); } - return false; + return true; } } } diff --git a/src/Features/LanguageServer/Protocol/Handler/Initialize/InitializeHandler.cs b/src/Features/LanguageServer/Protocol/Handler/Initialize/InitializeHandler.cs index 0b45dc54a39..6cbc4bba972 100644 --- a/src/Features/LanguageServer/Protocol/Handler/Initialize/InitializeHandler.cs +++ b/src/Features/LanguageServer/Protocol/Handler/Initialize/InitializeHandler.cs @@ -57,7 +57,7 @@ public Task HandleRequestAsync(LSP.InitializeParams reques DocumentHighlightProvider = true, ReferencesProvider = true, ProjectContextProvider = true, - ExecuteCommandProvider = new ExecuteCommandOptions(), + ExecuteCommandProvider = new LSP.ExecuteCommandOptions(), TextDocumentSync = new LSP.TextDocumentSyncOptions { Change = LSP.TextDocumentSyncKind.None diff --git a/src/Features/LanguageServer/ProtocolUnitTests/CodeActions/CodeActionResolveTests.cs b/src/Features/LanguageServer/ProtocolUnitTests/CodeActions/CodeActionResolveTests.cs index 3a7d66a24f4..146de676dbe 100644 --- a/src/Features/LanguageServer/ProtocolUnitTests/CodeActions/CodeActionResolveTests.cs +++ b/src/Features/LanguageServer/ProtocolUnitTests/CodeActions/CodeActionResolveTests.cs @@ -50,21 +50,21 @@ void M() children: Array.Empty(), data: CreateCodeActionResolveData(CSharpAnalyzersResources.Use_implicit_type, locations["caret"].Single()), diagnostics: null, - edit: new LSP.WorkspaceEdit() + edit: new LSP.WorkspaceEdit { DocumentChanges = new TextDocumentEdit[] { - new TextDocumentEdit() + new TextDocumentEdit { - TextDocument = new VersionedTextDocumentIdentifier() + TextDocument = new VersionedTextDocumentIdentifier { Uri = locations["caret"].Single().Uri }, Edits = new TextEdit[] { - new TextEdit() + new TextEdit { NewText = expectedMarkup, - Range = new LSP.Range() { Start = new Position(0, 0), End = new Position(6, 1) } + Range = new LSP.Range { Start = new Position(0, 0), End = new Position(6, 1) } } } } @@ -116,21 +116,21 @@ void M() FeaturesResources.Introduce_constant + string.Format(FeaturesResources.Introduce_constant_for_0, "1"), locations["caret"].Single()), diagnostics: null, - edit: new LSP.WorkspaceEdit() + edit: new LSP.WorkspaceEdit { DocumentChanges = new TextDocumentEdit[] { - new TextDocumentEdit() + new TextDocumentEdit { - TextDocument = new VersionedTextDocumentIdentifier() + TextDocument = new VersionedTextDocumentIdentifier { Uri = locations["caret"].Single().Uri }, Edits = new TextEdit[] { - new TextEdit() + new TextEdit { NewText = expectedMarkup, - Range = new LSP.Range() { Start = new Position(0, 0), End = new Position(6, 1) } + Range = new LSP.Range { Start = new Position(0, 0), End = new Position(6, 1) } } } } diff --git a/src/Features/LanguageServer/ProtocolUnitTests/CodeActions/CodeActionsTests.cs b/src/Features/LanguageServer/ProtocolUnitTests/CodeActions/CodeActionsTests.cs index 9b1a6f5311a..016eb162987 100644 --- a/src/Features/LanguageServer/ProtocolUnitTests/CodeActions/CodeActionsTests.cs +++ b/src/Features/LanguageServer/ProtocolUnitTests/CodeActions/CodeActionsTests.cs @@ -39,9 +39,9 @@ void M() children: Array.Empty(), data: new CodeActionResolveData { - DistinctTitle = CSharpAnalyzersResources.Use_implicit_type, + UniqueIdentifier = CSharpAnalyzersResources.Use_implicit_type, Range = caretLocation.Range, - TextDocument = new TextDocumentIdentifier() { Uri = caretLocation.Uri } + TextDocument = new TextDocumentIdentifier { Uri = caretLocation.Uri } }, diagnostics: null); @@ -71,15 +71,15 @@ void M() children: Array.Empty(), data: new CodeActionResolveData { - DistinctTitle = FeaturesResources.Introduce_constant + string.Format(FeaturesResources.Introduce_constant_for_0, "1"), + UniqueIdentifier = FeaturesResources.Introduce_constant + string.Format(FeaturesResources.Introduce_constant_for_0, "1"), Range = caretLocation.Range, - TextDocument = new TextDocumentIdentifier() { Uri = caretLocation.Uri } + TextDocument = new TextDocumentIdentifier { Uri = caretLocation.Uri } }, diagnostics: null); var results = await RunGetCodeActionsAsync(workspace.CurrentSolution, caretLocation); var introduceConstant = results[0].Children.FirstOrDefault( - r => ((CodeActionResolveData)r.Data).DistinctTitle == FeaturesResources.Introduce_constant + r => ((CodeActionResolveData)r.Data).UniqueIdentifier == FeaturesResources.Introduce_constant + string.Format(FeaturesResources.Introduce_constant_for_0, "1")); AssertJsonEquals(expected, introduceConstant); @@ -97,11 +97,11 @@ void M() } internal static LSP.CodeActionParams CreateCodeActionParams(LSP.Location caret) - => new LSP.CodeActionParams() + => new LSP.CodeActionParams { TextDocument = CreateTextDocumentIdentifier(caret.Uri), Range = caret.Range, - Context = new LSP.CodeActionContext() + Context = new LSP.CodeActionContext { // TODO - Code actions should respect context. } -- GitLab