[analysis_server] Support assist CodeActions via Commands when the client does not support codeActionLiterals

This changes assist CodeActions to return Commands if the client does not support codeActionLiterals (code actions with inline edits). The implementation of "applyCodeAction" will just recompute the code action from the textDocument/range/codeActionKind and then send it to the editor via workspace/applyEdit.

On it's own, this only means that LSP clients that do not support codeActionLiterals will now get assists, but a future change will make the textDocument/codeAction request available over DTD allowing them to be invoked by clients other than the editor itself.

This is currently only assists and not fixes/refactors.

Change-Id: I8204a51303b048e3928d5cbadf5aca76e81c82f3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/427224
Reviewed-by: Samuel Rawlins <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Brian Wilkerson <[email protected]>

Author: DanTup

pkg/analysis_server/lib/src/lsp/constants.dart

@@ -104,6 +104,7 @@ abstract final class Commands {
   /// commands should be sent to the server for execution (as opposed to being
   /// executed in the local plugin).
   static final serverSupportedCommands = [
+    applyCodeAction,
     sortMembers,
     organizeImports,
     fixAll,
@@ -116,6 +117,7 @@ abstract final class Commands {
     // Add commands for each of the new refactorings.
     ...RefactoringProcessor.generators.keys,
   ];
+  static const applyCodeAction = 'dart.edit.codeAction.apply';
   static const sortMembers = 'dart.edit.sortMembers';
   static const organizeImports = 'dart.edit.organizeImports';
   static const fixAll = 'dart.edit.fixAll';

pkg/analysis_server/lib/src/lsp/handlers/code_actions/abstract_code_actions_producer.dart

@@ -2,13 +2,18 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+/// @docImport 'package:analysis_server/src/lsp/handlers/commands/apply_code_action.dart';
+library;
+
 import 'dart:async';
 
 import 'package:analysis_server/lsp_protocol/protocol.dart';
 import 'package:analysis_server/src/lsp/client_capabilities.dart';
 import 'package:analysis_server/src/lsp/constants.dart';
 import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
 import 'package:analysis_server/src/lsp/mapping.dart';
+import 'package:analysis_server/src/protocol_server.dart'
+    hide AnalysisOptions, Position;
 import 'package:analysis_server/src/protocol_server.dart' as protocol;
 import 'package:analysis_server/src/request_handler_mixin.dart';
 import 'package:analyzer/dart/analysis/analysis_options.dart';
@@ -33,35 +38,83 @@ abstract class AbstractCodeActionsProducer
   final int offset;
   final int length;
   final bool Function(CodeActionKind?) shouldIncludeKind;
-  final LspClientCapabilities capabilities;
+
+  /// The capabilities of the caller making the request for [CodeAction]s.
+  final LspClientCapabilities callerCapabilities;
+
+  /// The capabilities of the editor (which may or may not be the same as
+  /// [callerCapabilities] depending on whether the request came from the editor
+  /// or another client - such as over DTD).
+  final LspClientCapabilities editorCapabilities;
 
   final AnalysisOptions analysisOptions;
 
   @override
   final LspAnalysisServer server;
 
+  /// Whether [CodeAction]s can be [Command]s or not.
+  ///
+  /// This is usually true (because there is no capability for this), however
+  /// it will be disabled by [ApplyCodeActionCommandHandler] so that we can't
+  /// recursively return command-based actions.
+  final bool allowCommands;
+
+  /// Whether [CodeAction]s can be [CodeActionLiteral]s or not.
+  ///
+  /// This is usually based on the callers capabilities, however
+  /// for [ApplyCodeActionCommandHandler] it will be based on the editor's
+  /// capabilities since it will compute the edits and send them to the editor
+  /// directly.
+  final bool allowCodeActionLiterals;
+
   AbstractCodeActionsProducer(
     this.server,
     this.file,
     this.lineInfo, {
     required this.offset,
     required this.length,
     required this.shouldIncludeKind,
-    required this.capabilities,
+    required this.callerCapabilities,
+    required this.editorCapabilities,
+    required this.allowCommands,
+    required this.allowCodeActionLiterals,
     required this.analysisOptions,
   });
 
-  String get name;
+  Set<DiagnosticTag> get callerSupportedDiagnosticTags =>
+      callerCapabilities.diagnosticTags;
 
-  String get path => file.path;
+  bool get callerSupportsCodeDescription =>
+      callerCapabilities.diagnosticCodeDescription;
 
-  Set<DiagnosticTag> get supportedDiagnosticTags => capabilities.diagnosticTags;
+  bool get editorSupportsApplyEdit => editorCapabilities.applyEdit;
 
-  bool get supportsApplyEdit => capabilities.applyEdit;
+  String get name;
 
-  bool get supportsCodeDescription => capabilities.diagnosticCodeDescription;
+  String get path => file.path;
 
-  bool get supportsLiterals => capabilities.literalCodeActions;
+  /// Creates a command to apply a CodeAction later.
+  Command createApplyCodeActionCommand(
+    String title,
+    CodeActionKind kind,
+    String? loggedId,
+    OptionalVersionedTextDocumentIdentifier textDocument,
+    Range range,
+  ) {
+    return Command(
+      title: title,
+      command: Commands.applyCodeAction,
+      // The arguments here must match those in `ApplyCodeActionCommandHandler`.
+      arguments: [
+        {
+          'textDocument': textDocument,
+          'range': range,
+          'kind': kind,
+          'loggedAction': loggedId,
+        },
+      ],
+    );
+  }
 
   /// Creates a CodeAction to apply this assist.
   ///
@@ -87,7 +140,7 @@ abstract class AbstractCodeActionsProducer
       command: createLogActionCommand(loggedAssistId),
       edit: createWorkspaceEdit(
         server,
-        capabilities,
+        editorCapabilities,
         change,
         allowSnippets: true,
         filePath: path,
@@ -96,6 +149,45 @@ abstract class AbstractCodeActionsProducer
     );
   }
 
+  /// Creates a [CodeAction] that is:
+  ///
+  /// - a [CodeActionLiteral] if [allowCodeActionLiterals], or
+  /// - a [Command] for [Commands.applyCodeAction] if [allowCommands], or
+  /// - null
+  CodeAction? createCodeActionLiteralOrApplyCommand(
+    String path,
+    OptionalVersionedTextDocumentIdentifier textDocument,
+    Range range,
+    LineInfo lineInfo,
+    SourceChange change,
+    CodeActionKind kind,
+    String? loggedAssistId,
+  ) {
+    if (allowCodeActionLiterals) {
+      return CodeAction.t1(
+        createAssistCodeActionLiteral(
+          change,
+          kind,
+          loggedAssistId,
+          path,
+          lineInfo,
+        ),
+      );
+    } else if (allowCommands) {
+      return CodeAction.t2(
+        createApplyCodeActionCommand(
+          change.message,
+          kind,
+          loggedAssistId,
+          textDocument,
+          range,
+        ),
+      );
+    } else {
+      return null;
+    }
+  }
+
   /// Create an LSP [Diagnostic] for [diagnostic].
   @protected
   Diagnostic createDiagnostic(
@@ -107,8 +199,8 @@ abstract class AbstractCodeActionsProducer
       server.uriConverter,
       (_) => lineInfo,
       protocol.newAnalysisError_fromEngine(result, diagnostic),
-      supportedTags: supportedDiagnosticTags,
-      clientSupportsCodeDescription: supportsCodeDescription,
+      supportedTags: callerSupportedDiagnosticTags,
+      clientSupportsCodeDescription: callerSupportsCodeDescription,
     );
   }
 
@@ -137,7 +229,7 @@ abstract class AbstractCodeActionsProducer
       command: createLogActionCommand(loggedFixId),
       edit: createWorkspaceEdit(
         server,
-        capabilities,
+        editorCapabilities,
         change,
         allowSnippets: true,
         filePath: path,

pkg/analysis_server/lib/src/lsp/handlers/code_actions/analysis_options.dart

@@ -26,7 +26,10 @@ class AnalysisOptionsCodeActionsProducer extends AbstractCodeActionsProducer {
     required super.offset,
     required super.length,
     required super.shouldIncludeKind,
-    required super.capabilities,
+    required super.editorCapabilities,
+    required super.callerCapabilities,
+    required super.allowCodeActionLiterals,
+    required super.allowCommands,
     required super.analysisOptions,
   });
 
@@ -43,7 +46,8 @@ class AnalysisOptionsCodeActionsProducer extends AbstractCodeActionsProducer {
     OperationPerformance? performance,
   ) async {
     // These fixes are only provided as literal CodeActions.
-    if (!supportsLiterals) {
+    if (!allowCodeActionLiterals) {
+      // TODO(dantup): Support this (via createCodeActionLiteralOrApplyCommand)
       return [];
     }
 

pkg/analysis_server/lib/src/lsp/handlers/code_actions/code_action_computer.dart

@@ -2,6 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+/// @docImport 'package:analysis_server/src/lsp/handlers/commands/apply_code_action.dart';
 /// @docImport 'package:language_server_protocol/protocol_special.dart';
 /// @docImport 'package:analysis_server/src/lsp/handlers/handler_code_actions.dart';
 library;
@@ -28,8 +29,14 @@ import 'package:collection/collection.dart' show groupBy;
 import 'package:path/src/context.dart';
 
 /// A helper class for computing [CodeAction]s that is used by both the
-/// [CodeActionHandler] and an upcoming command handler.
-class CodeActionComputer with HandlerHelperMixin<LspAnalysisServer> {
+/// [CodeActionHandler] and [ApplyCodeActionCommandHandler].
+class CodeActionComputer
+    with
+        HandlerHelperMixin<
+          // TODO(dantup): Make this (and the code action producers) work with
+          //  either server.
+          LspAnalysisServer
+        > {
   /// The text document to compute code actions for.
   final TextDocumentIdentifier textDocument;
 
@@ -55,15 +62,20 @@ class CodeActionComputer with HandlerHelperMixin<LspAnalysisServer> {
   /// Whether [CodeAction]s can be [CodeActionLiteral]s or not.
   ///
   /// This is usually based on the callers capabilities, however
-  /// for [ApplyCodeActionCommandHandler] it will be based on the editors
+  /// for [ApplyCodeActionCommandHandler] it will be based on the editor's
   /// capabilities since it will compute the edits and send them to the editor
   /// directly.
   final bool allowCodeActionLiterals;
 
   final OperationPerformanceImpl performance;
 
-  /// The capabilities of the client.
-  final LspClientCapabilities capabilities;
+  /// The capabilities of the caller making the request for [CodeAction]s.
+  final LspClientCapabilities callerCapabilities;
+
+  /// The capabilities of the editor (which may or may not be the same as
+  /// [callerCapabilities] depending on whether the request came from the editor
+  /// or another client - such as over DTD).
+  final LspClientCapabilities editorCapabilities;
 
   @override
   final LspAnalysisServer server;
@@ -79,7 +91,8 @@ class CodeActionComputer with HandlerHelperMixin<LspAnalysisServer> {
     this.server,
     this.textDocument,
     this.range, {
-    required this.capabilities,
+    required this.editorCapabilities,
+    required this.callerCapabilities,
     required this.only,
     required this.supportedKinds,
     required this.triggerKind,
@@ -226,7 +239,10 @@ class CodeActionComputer with HandlerHelperMixin<LspAnalysisServer> {
           libraryResult,
           unit,
           shouldIncludeKind: shouldIncludeKind,
-          capabilities: capabilities,
+          editorCapabilities: editorCapabilities,
+          callerCapabilities: callerCapabilities,
+          allowCodeActionLiterals: allowCodeActionLiterals,
+          allowCommands: allowCommands,
           triggerKind: triggerKind,
           analysisOptions: analysisOptions,
           willBeDeduplicated: true,
@@ -240,7 +256,10 @@ class CodeActionComputer with HandlerHelperMixin<LspAnalysisServer> {
           offset: offset,
           length: length,
           shouldIncludeKind: shouldIncludeKind,
-          capabilities: capabilities,
+          editorCapabilities: editorCapabilities,
+          callerCapabilities: callerCapabilities,
+          allowCodeActionLiterals: allowCodeActionLiterals,
+          allowCommands: allowCommands,
           analysisOptions: analysisOptions,
         ),
       if (isAnalysisOptions)
@@ -252,7 +271,10 @@ class CodeActionComputer with HandlerHelperMixin<LspAnalysisServer> {
           offset: offset,
           length: length,
           shouldIncludeKind: shouldIncludeKind,
-          capabilities: capabilities,
+          editorCapabilities: editorCapabilities,
+          callerCapabilities: callerCapabilities,
+          allowCodeActionLiterals: allowCodeActionLiterals,
+          allowCommands: allowCommands,
           analysisOptions: analysisOptions,
         ),
       PluginCodeActionsProducer(
@@ -263,7 +285,10 @@ class CodeActionComputer with HandlerHelperMixin<LspAnalysisServer> {
         offset: offset,
         length: length,
         shouldIncludeKind: shouldIncludeKind,
-        capabilities: capabilities,
+        editorCapabilities: editorCapabilities,
+        callerCapabilities: callerCapabilities,
+        allowCodeActionLiterals: allowCodeActionLiterals,
+        allowCommands: allowCommands,
         analysisOptions: analysisOptions,
       ),
     ];