[analysis_server] Support "Fix" CodeActions via commands if client doesn't support CodeActionLiterals

This updates fixes to return either CodeActionLiterals or Commands based on the capabilities of the client (similar to was done previously for assists).

Change-Id: I688cb80aef415d329d65092edba247e5ec7a5a13
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428783
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/handlers/code_actions/abstract_code_actions_producer.dart

@@ -116,28 +116,25 @@ abstract class AbstractCodeActionsProducer
     );
   }
 
-  /// Creates a CodeAction to apply this assist.
+  /// Creates a CodeAction to apply this change.
   ///
   /// This code will fetch the version of each document being modified so it's
   /// important to call this immediately after computing edits to ensure the
   /// document is not modified before the version number is read.
   @protected
-  CodeActionLiteral createAssistCodeActionLiteral(
+  CodeActionLiteral createCodeActionLiteral(
     protocol.SourceChange change,
     CodeActionKind kind,
-    String? loggedAssistId,
+    String? loggedId,
     String path,
-    LineInfo lineInfo,
-  ) {
-    assert(
-      kind == CodeActionKind.Refactor ||
-          '$kind'.startsWith('${CodeActionKind.Refactor}.'),
-    );
+    LineInfo lineInfo, {
+    Diagnostic? diagnostic,
+  }) {
     return CodeActionLiteral(
       title: change.message,
       kind: kind,
-      diagnostics: const [],
-      command: createLogActionCommand(loggedAssistId),
+      diagnostics: diagnostic != null ? [diagnostic] : const [],
+      command: createLogActionCommand(loggedId),
       edit: createWorkspaceEdit(
         server,
         editorCapabilities,
@@ -161,24 +158,26 @@ abstract class AbstractCodeActionsProducer
     LineInfo lineInfo,
     SourceChange change,
     CodeActionKind kind,
-    String? loggedAssistId,
-  ) {
+    String? loggedId, {
+    Diagnostic? diagnostic,
+  }) {
     if (allowCodeActionLiterals) {
       return CodeAction.t1(
-        createAssistCodeActionLiteral(
+        createCodeActionLiteral(
           change,
           kind,
-          loggedAssistId,
+          loggedId,
           path,
           lineInfo,
+          diagnostic: diagnostic,
         ),
       );
     } else if (allowCommands) {
       return CodeAction.t2(
         createApplyCodeActionCommand(
           change.message,
           kind,
-          loggedAssistId,
+          loggedId,
           textDocument,
           range,
         ),
@@ -204,40 +203,6 @@ abstract class AbstractCodeActionsProducer
     );
   }
 
-  /// Creates a CodeAction to apply this fix.
-  ///
-  /// This code will fetch the version of each document being modified so it's
-  /// important to call this immediately after computing edits to ensure the
-  /// document is not modified before the version number is read.
-  @protected
-  CodeActionLiteral createFixCodeActionLiteral(
-    protocol.SourceChange change,
-    CodeActionKind kind,
-    String? loggedFixId,
-    Diagnostic diagnostic,
-    String path,
-    LineInfo lineInfo,
-  ) {
-    assert(
-      kind == CodeActionKind.QuickFix ||
-          '$kind'.startsWith('${CodeActionKind.QuickFix}.'),
-    );
-    return CodeActionLiteral(
-      title: change.message,
-      kind: kind,
-      diagnostics: [diagnostic],
-      command: createLogActionCommand(loggedFixId),
-      edit: createWorkspaceEdit(
-        server,
-        editorCapabilities,
-        change,
-        allowSnippets: true,
-        filePath: path,
-        lineInfo: lineInfo,
-      ),
-    );
-  }
-
   /// Creates a command to log that a CodeAction was selected.
   ///
   /// Code Actions that provide their edits inline (and not via a command) do

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

@@ -112,13 +112,13 @@ class AnalysisOptionsCodeActionsProducer extends AbstractCodeActionsProducer {
             return null;
           }
           var action = CodeAction.t1(
-            createFixCodeActionLiteral(
+            createCodeActionLiteral(
               fix.change,
               kind,
               fix.change.id,
-              diagnostic,
               path,
               lineInfo,
+              diagnostic: diagnostic,
             ),
           );
           return (action: action, priority: fix.kind.priority);

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

@@ -197,12 +197,6 @@ class DartCodeActionsProducer extends AbstractCodeActionsProducer {
   Future<List<CodeActionWithPriority>> getFixActions(
     OperationPerformance? performance,
   ) async {
-    // These fixes are only provided as literal CodeActions.
-    if (!allowCodeActionLiterals) {
-      // TODO(dantup): Support this (via createCodeActionLiteralOrApplyCommand)
-      return [];
-    }
-
     var lineInfo = unitResult.lineInfo;
     var codeActions = <CodeActionWithPriority>[];
 
@@ -264,25 +258,26 @@ class DartCodeActionsProducer extends AbstractCodeActionsProducer {
           );
           codeActions.addAll(
             fixes.map((fix) {
-              var kind = toCodeActionKind(
-                fix.change.id,
-                CodeActionKind.QuickFix,
-              );
+              var change = fix.change;
+              var kind = toCodeActionKind(change.id, CodeActionKind.QuickFix);
               // TODO(dantup): Find a way to filter these earlier, so we don't
               //  compute fixes we will filter out.
               if (!shouldIncludeKind(kind)) {
                 return null;
               }
-              var action = CodeAction.t1(
-                createFixCodeActionLiteral(
-                  fix.change,
-                  kind,
-                  fix.change.id,
-                  diagnostic,
-                  path,
-                  lineInfo,
-                ),
+              var action = createCodeActionLiteralOrApplyCommand(
+                unitResult.path,
+                docIdentifier,
+                range,
+                lineInfo,
+                change,
+                kind,
+                change.id,
+                diagnostic: diagnostic,
               );
+              if (action == null) {
+                return null;
+              }
               return (action: action, priority: fix.kind.priority);
             }).nonNulls,
           );

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

@@ -100,7 +100,7 @@ class PluginCodeActionsProducer extends AbstractCodeActionsProducer {
 
     return (
       action: CodeAction.t1(
-        createAssistCodeActionLiteral(
+        createCodeActionLiteral(
           assist.change,
           kind,
           'assist from plugin',
@@ -131,13 +131,13 @@ class PluginCodeActionsProducer extends AbstractCodeActionsProducer {
       }
       return (
         action: CodeAction.t1(
-          createFixCodeActionLiteral(
+          createCodeActionLiteral(
             fix.change,
             kind,
             'fix from plugin',
-            diagnostic,
             path,
             lineInfo,
+            diagnostic: diagnostic,
           ),
         ),
         priority: fix.priority,

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

@@ -98,13 +98,13 @@ class PubspecCodeActionsProducer extends AbstractCodeActionsProducer {
             return null;
           }
           var action = CodeAction.t1(
-            createFixCodeActionLiteral(
+            createCodeActionLiteral(
               fix.change,
               kind,
               fix.change.id,
-              diagnostic,
               path,
               lineInfo,
+              diagnostic: diagnostic,
             ),
           );
           return (action: action, priority: fix.kind.priority);

pkg/analysis_server/test/lsp/code_actions_mixin.dart

@@ -90,6 +90,32 @@ mixin CodeActionsTestMixin
     return action.asCodeActionLiteral;
   }
 
+  /// Initializes the server with some basic configuration and expects to find
+  /// a [Command] code action (not a literal, even with a command) with
+  /// [command]/[title].
+  Future<Command> expectCommandCodeAction(
+    String content, {
+    CodeActionKind? kind,
+    String? command,
+    List<Object>? commandArgs,
+    String? title,
+    CodeActionTriggerKind? triggerKind,
+    String? filePath,
+    bool openTargetFile = false,
+  }) async {
+    var action = await expectCodeAction(
+      TestCode.parse(content),
+      kind: kind,
+      command: command,
+      commandArgs: commandArgs,
+      title: title,
+      triggerKind: triggerKind,
+      filePath: filePath,
+      openTargetFile: openTargetFile,
+    );
+    return action.asCommand;
+  }
+
   /// Verifies a command execution was logged to analytics.
   ///
   /// Implementations are provided by the in-process test base classes. This
@@ -322,4 +348,43 @@ $expected''';
       workDoneToken: commandWorkDoneToken,
     );
   }
+
+  /// Initializes the server with some basic configuration and expects to find
+  /// a [Command] code action (and not a literal, even with a command) with
+  /// [title] that applies edits resulting in [expected].
+  Future<LspChangeVerifier> verifyCommandCodeActionEdits(
+    String content,
+    String expected, {
+    String? filePath,
+    String? command,
+    List<Object>? commandArgs,
+    String? title,
+    ProgressToken? commandWorkDoneToken,
+    bool openTargetFile = false,
+  }) async {
+    filePath ??= testFilePath;
+
+    // For convenience, if a test doesn't provide an full set of edits
+    // we assume only a single edit of the file that was being modified.
+    if (!expected.startsWith(LspChangeVerifier.editMarkerStart)) {
+      expected = '''
+${LspChangeVerifier.editMarkerStart} ${relativePath(filePath)}
+$expected''';
+    }
+
+    var commandAction = await expectCommandCodeAction(
+      filePath: filePath,
+      content,
+      command: command,
+      commandArgs: commandArgs,
+      title: title,
+      openTargetFile: openTargetFile,
+    );
+
+    return await verifyCodeActionEdits(
+      CodeAction.t2(commandAction),
+      expected,
+      workDoneToken: commandWorkDoneToken,
+    );
+  }
 }

pkg/analysis_server/test/shared/shared_code_actions_assists_tests.dart

@@ -15,7 +15,6 @@ import 'package:test/test.dart';
 import '../lsp/code_actions_mixin.dart';
 import '../lsp/request_helpers_mixin.dart';
 import '../lsp/server_abstract.dart';
-import '../utils/lsp_protocol_extensions.dart';
 import '../utils/test_code_extensions.dart';
 import 'shared_test_interface.dart';
 
@@ -90,70 +89,53 @@ Future? f;
     setSnippetTextEditSupport();
     setSupportedCodeActionKinds([CodeActionKind.Refactor]);
 
-    var code = TestCode.parse('''
+    const content = '''
 import 'package:flutter/widgets.dart';
 Widget build() {
   return Te^xt('');
 }
-''');
-
-    var action = await expectCodeAction(
-      code,
-      kind: CodeActionKind('refactor.flutter.wrap.center'),
-      title: 'Wrap with Center',
-    );
-
-    // Ensure we are a CodeAction literal.
-    expect(action.isCodeActionLiteral, true);
+''';
 
-    await verifyCodeActionEdits(action, r'''
+    const expectedContent = r'''
 >>>>>>>>>> lib/test.dart
 import 'package:flutter/widgets.dart';
 Widget build() {
   return Center($0child: Text(''));
 }
-''');
+''';
+
+    await verifyCodeActionLiteralEdits(
+      content,
+      expectedContent,
+      title: 'Wrap with Center',
+    );
   }
 
   Future<void> test_codeActionLiterals_unsupported() async {
     setSnippetTextEditSupport();
     setSupportedCodeActionKinds(null); // no codeActionLiteralSupport
 
-    var code = TestCode.parse('''
+    const content = '''
 import 'package:flutter/widgets.dart';
 Widget build() {
   return Te[!!]xt('');
 }
-''');
-
-    var action = await expectCodeAction(
-      openTargetFile: true, // Open document to verify we get a version back.
-      code,
-      title: 'Wrap with Center',
-      command: Commands.applyCodeAction,
-      commandArgs: [
-        {
-          'textDocument': {'uri': testFileUri.toString(), 'version': 1},
-          'range': code.range.range.toJson(),
-          'kind': 'refactor.flutter.wrap.center',
-          'loggedAction': 'dart.assist.flutter.wrap.center',
-        },
-      ],
-    );
-
-    // We don't support literals, so we expect the raw command instead.
-    expect(action.isCommand, true);
-    var command = action.asCommand;
+''';
 
-    // Verify that executing the command produces the correct edits (which will
-    // come back via `workspace/applyEdit`).
-    await verifyCommandEdits(command, r'''
+    const expectedContent = r'''
 >>>>>>>>>> lib/test.dart
 import 'package:flutter/widgets.dart';
 Widget build() {
   return Center($0child: Text(''));
 }
-''');
+''';
+
+    await verifyCommandCodeActionEdits(
+      content,
+      expectedContent,
+      command: Commands.applyCodeAction,
+      title: 'Wrap with Center',
+    );
 
     expectCommandLogged(Commands.applyCodeAction);
     expectCommandLogged('dart.assist.flutter.wrap.center');