[analysis_server] Update CodeActionsProducers to support either kind of CodeAction

No functional changes, just some additional refactors extracted from an upcoming CL to make it easier to review. This updates the `CodeActionProducer`s (and related code in `CodeActionComputer`) so their signatures use `CodeAction` instead of `CodeActionLiteral`, which means this code will be able to support `Command` code actions in future.

Mostly it's trivial changes, but the code that de-dupes and merges actions needed a little more updating to handle the different kinds.

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

Author: DanTup

pkg/analysis_server/lib/src/lsp/extensions/code_action.dart

@@ -0,0 +1,19 @@
+// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
+// 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.
+
+import 'package:analysis_server/lsp_protocol/protocol.dart';
+
+extension CodeActionExtensions on CodeAction {
+  /// The [Command] for this [CodeAction], whether it's a [CodeActionLiteral]
+  /// or a [Command].
+  Command? get command {
+    return map((literal) => literal.command, (command) => command);
+  }
+
+  /// The title for this [CodeAction], whether it's a [CodeActionLiteral]
+  /// or a [Command].
+  String get title {
+    return map((literal) => literal.title, (command) => command.title);
+  }
+}

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

@@ -20,12 +20,12 @@ import 'package:analyzer/src/dart/analysis/results.dart' as engine;
 import 'package:analyzer/src/util/performance/operation_performance.dart';
 import 'package:meta/meta.dart';
 
-typedef CodeActionWithPriority = ({CodeActionLiteral action, int priority});
+typedef CodeActionWithPriority = ({CodeAction action, int priority});
 
 typedef CodeActionWithPriorityAndIndex =
-    ({CodeActionLiteral action, int priority, int index});
+    ({CodeAction action, int priority, int index});
 
-/// A base for classes that produce [CodeActionLiteral]s for the LSP handler.
+/// A base for classes that produce [CodeAction]s for the LSP handler.
 abstract class AbstractCodeActionsProducer
     with RequestHandlerMixin<LspAnalysisServer> {
   final File file;
@@ -69,7 +69,7 @@ abstract class AbstractCodeActionsProducer
   /// important to call this immediately after computing edits to ensure the
   /// document is not modified before the version number is read.
   @protected
-  CodeActionLiteral createAssistAction(
+  CodeActionLiteral createAssistCodeActionLiteral(
     protocol.SourceChange change,
     CodeActionKind kind,
     String? loggedAssistId,
@@ -118,7 +118,7 @@ abstract class AbstractCodeActionsProducer
   /// important to call this immediately after computing edits to ensure the
   /// document is not modified before the version number is read.
   @protected
-  CodeActionLiteral createFixAction(
+  CodeActionLiteral createFixCodeActionLiteral(
     protocol.SourceChange change,
     CodeActionKind kind,
     String? loggedFixId,

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

@@ -42,6 +42,11 @@ class AnalysisOptionsCodeActionsProducer extends AbstractCodeActionsProducer {
   Future<List<CodeActionWithPriority>> getFixActions(
     OperationPerformance? performance,
   ) async {
+    // These fixes are only provided as literal CodeActions.
+    if (!supportsLiterals) {
+      return [];
+    }
+
     var session = await server.getAnalysisSession(path);
     if (session == null) {
       return [];
@@ -102,13 +107,15 @@ class AnalysisOptionsCodeActionsProducer extends AbstractCodeActionsProducer {
           if (!shouldIncludeKind(kind)) {
             return null;
           }
-          var action = createFixAction(
-            fix.change,
-            kind,
-            fix.change.id,
-            diagnostic,
-            path,
-            lineInfo,
+          var action = CodeAction.t1(
+            createFixCodeActionLiteral(
+              fix.change,
+              kind,
+              fix.change.id,
+              diagnostic,
+              path,
+              lineInfo,
+            ),
           );
           return (action: action, priority: fix.kind.priority);
         }).nonNulls,

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

@@ -9,6 +9,7 @@ library;
 import 'package:analysis_server/lsp_protocol/protocol.dart';
 import 'package:analysis_server/src/lsp/client_capabilities.dart';
 import 'package:analysis_server/src/lsp/error_or.dart';
+import 'package:analysis_server/src/lsp/extensions/code_action.dart';
 import 'package:analysis_server/src/lsp/handlers/code_actions/abstract_code_actions_producer.dart';
 import 'package:analysis_server/src/lsp/handlers/code_actions/analysis_options.dart';
 import 'package:analysis_server/src/lsp/handlers/code_actions/dart.dart';
@@ -344,17 +345,21 @@ class _CodeActionSorter {
         }).toList();
     dedupedActionsWithIndex.sort(_compareCodeActions);
 
-    return dedupedActionsWithIndex
-        .map((action) => CodeAction.t1(action.action))
-        .toList();
+    return dedupedActionsWithIndex.map((action) => action.action).toList();
   }
 
-  /// Creates a comparer for [CodeActionLiteral]s that compares the column distance from
-  /// [pos].
-  int Function(CodeActionLiteral a, CodeActionLiteral b)
-  _codeActionColumnDistanceComparer(Position pos) {
-    Position posOf(CodeActionLiteral action) {
-      var diagnostics = action.diagnostics;
+  /// Creates a comparer for [CodeAction]s that compares the column
+  /// distance from [pos].
+  ///
+  /// If a [CodeAction] has no diagnostics, considers the action at [pos].
+  int Function(CodeAction a, CodeAction b) _codeActionColumnDistanceComparer(
+    Position pos,
+  ) {
+    Position posOf(CodeAction action) {
+      var diagnostics = action.map(
+        (literal) => literal.diagnostics,
+        (command) => null,
+      );
       return diagnostics != null && diagnostics.isNotEmpty
           ? diagnostics.first.range.start
           : pos;
@@ -423,35 +428,53 @@ class _CodeActionSorter {
       // Otherwise, find the action nearest to the caret.
       var comparer = _codeActionColumnDistanceComparer(position);
       actions.sort((a, b) => comparer(a.action, b.action));
-      var first = actions.first.action;
+      var first = actions.first;
+      var firstAction = first.action;
       var priority = actions.first.priority;
 
-      // Get any actions with the same fix (edit/command) for merging diagnostics.
+      // If this action is not a literal (it is a command), just return as-is
+      // because we can't merge any diagnostics into it.
+      var firstLiteral = firstAction.map(
+        (literal) => literal,
+        (command) => null,
+      );
+      if (firstLiteral == null) {
+        return first;
+      }
+
+      // Get any literal actions with the same fix (edit/command) for merging
+      // diagnostics.
       var others = actions
           .skip(1)
-          .where(
-            (other) =>
-                // Compare either edits or commands based on which the selected action has.
-                first.edit != null
-                    ? first.edit == other.action.edit
-                    : first.command != null
-                    ? first.command == other.action.command
-                    : false,
-          );
+          .map(
+            (action) =>
+                action.action.map((literal) => literal, (command) => null),
+          )
+          .nonNulls
+          .where((other) {
+            // Compare either edits or commands based on which the selected action has.
+            return firstLiteral.edit != null
+                ? firstLiteral.edit == other.edit
+                : firstLiteral.command != null
+                ? firstLiteral.command == other.command
+                : false;
+          });
 
       // Build a new CodeAction that merges the diagnostics from each same
       // code action onto a single one.
       return (
-        action: CodeActionLiteral(
-          title: first.title,
-          kind: first.kind,
-          // Merge diagnostics from all of the matching CodeActions.
-          diagnostics: [
-            ...?first.diagnostics,
-            for (var other in others) ...?other.action.diagnostics,
-          ],
-          edit: first.edit,
-          command: first.command,
+        action: CodeAction.t1(
+          CodeActionLiteral(
+            title: firstAction.title,
+            kind: firstLiteral.kind,
+            // Merge diagnostics from all of the matching CodeActions.
+            diagnostics: [
+              ...?firstLiteral.diagnostics,
+              for (var other in others) ...?other.diagnostics,
+            ],
+            edit: firstLiteral.edit,
+            command: firstAction.command,
+          ),
         ),
         priority: priority,
       );

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

@@ -72,7 +72,7 @@ class DartCodeActionsProducer extends AbstractCodeActionsProducer {
       (() => Commands.serverSupportedCommands.contains(command))(),
       'serverSupportedCommands did not contain $command',
     );
-    return _commandOrCodeActionLiteral(
+    return _maybeWrapCommandInCodeActionLiteral(
       actionKind,
       Command(
         title: title,
@@ -102,7 +102,7 @@ class DartCodeActionsProducer extends AbstractCodeActionsProducer {
       'serverSupportedCommands did not contain $command',
     );
 
-    return _commandOrCodeActionLiteral(
+    return _maybeWrapCommandInCodeActionLiteral(
       actionKind,
       Command(
         title: name,
@@ -165,21 +165,21 @@ class DartCodeActionsProducer extends AbstractCodeActionsProducer {
 
       return assists
           .map((assist) {
-            var kind = toCodeActionKind(
-              assist.change.id,
-              CodeActionKind.Refactor,
-            );
+            var change = assist.change;
+            var kind = toCodeActionKind(change.id, CodeActionKind.Refactor);
             // 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 = createAssistAction(
-              assist.change,
-              kind,
-              assist.change.id,
-              unitResult.path,
-              unitResult.lineInfo,
+            var action = CodeAction.t1(
+              createAssistCodeActionLiteral(
+                assist.change,
+                kind,
+                assist.change.id,
+                unitResult.path,
+                unitResult.lineInfo,
+              ),
             );
             return (action: action, priority: assist.kind.priority);
           })
@@ -272,13 +272,15 @@ class DartCodeActionsProducer extends AbstractCodeActionsProducer {
               if (!shouldIncludeKind(kind)) {
                 return null;
               }
-              var action = createFixAction(
-                fix.change,
-                kind,
-                fix.change.id,
-                diagnostic,
-                path,
-                lineInfo,
+              var action = CodeAction.t1(
+                createFixCodeActionLiteral(
+                  fix.change,
+                  kind,
+                  fix.change.id,
+                  diagnostic,
+                  path,
+                  lineInfo,
+                ),
               );
               return (action: action, priority: fix.kind.priority);
             }).nonNulls,
@@ -531,9 +533,13 @@ class DartCodeActionsProducer extends AbstractCodeActionsProducer {
     ];
   }
 
-  /// Wraps a command in a CodeAction if the client supports it so that a
-  /// CodeActionKind can be supplied.
-  CodeAction _commandOrCodeActionLiteral(CodeActionKind kind, Command command) {
+  /// Returns a [CodeAction] that is [command] wrapped in a [CodeActionLiteral]
+  /// (to allow a [CodeActionKind] to be supplied) if the client supports it,
+  /// otherwise the bare command.
+  CodeAction _maybeWrapCommandInCodeActionLiteral(
+    CodeActionKind kind,
+    Command command,
+  ) {
     return supportsLiterals
         ? CodeAction.t1(
           CodeActionLiteral(title: command.title, kind: kind, command: command),

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

@@ -90,12 +90,14 @@ class PluginCodeActionsProducer extends AbstractCodeActionsProducer {
     }
 
     return (
-      action: createAssistAction(
-        assist.change,
-        kind,
-        'assist from plugin',
-        path,
-        lineInfo,
+      action: CodeAction.t1(
+        createAssistCodeActionLiteral(
+          assist.change,
+          kind,
+          'assist from plugin',
+          path,
+          lineInfo,
+        ),
       ),
       priority: assist.priority,
     );
@@ -119,13 +121,15 @@ class PluginCodeActionsProducer extends AbstractCodeActionsProducer {
         return null;
       }
       return (
-        action: createFixAction(
-          fix.change,
-          kind,
-          'fix from plugin',
-          diagnostic,
-          path,
-          lineInfo,
+        action: CodeAction.t1(
+          createFixCodeActionLiteral(
+            fix.change,
+            kind,
+            'fix from plugin',
+            diagnostic,
+            path,
+            lineInfo,
+          ),
         ),
         priority: fix.priority,
       );

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

@@ -39,6 +39,11 @@ class PubspecCodeActionsProducer extends AbstractCodeActionsProducer {
   Future<List<CodeActionWithPriority>> getFixActions(
     OperationPerformance? performance,
   ) async {
+    // These fixes are only provided as literal CodeActions.
+    if (!supportsLiterals) {
+      return [];
+    }
+
     var session = await server.getAnalysisSession(path);
     if (session == null) {
       return [];
@@ -88,13 +93,15 @@ class PubspecCodeActionsProducer extends AbstractCodeActionsProducer {
           if (!shouldIncludeKind(kind)) {
             return null;
           }
-          var action = createFixAction(
-            fix.change,
-            kind,
-            fix.change.id,
-            diagnostic,
-            path,
-            lineInfo,
+          var action = CodeAction.t1(
+            createFixCodeActionLiteral(
+              fix.change,
+              kind,
+              fix.change.id,
+              diagnostic,
+              path,
+              lineInfo,
+            ),
           );
           return (action: action, priority: fix.kind.priority);
         }).nonNulls,

pkg/analysis_server/test/lsp/code_actions_abstract.dart

@@ -4,6 +4,7 @@
 
 import 'package:analysis_server/lsp_protocol/protocol.dart';
 import 'package:analysis_server/src/lsp/constants.dart';
+import 'package:analysis_server/src/lsp/extensions/code_action.dart';
 import 'package:analysis_server/src/services/correction/assist_internal.dart';
 import 'package:analysis_server/src/services/correction/fix_internal.dart';
 import 'package:analyzer/src/test_utilities/test_code_format.dart';

pkg/analysis_server/test/lsp/code_actions_assists_test.dart

@@ -6,13 +6,13 @@ import 'dart:convert';
 
 import 'package:analysis_server/lsp_protocol/protocol.dart';
 import 'package:analysis_server/src/analysis_server.dart';
+import 'package:analysis_server/src/lsp/extensions/code_action.dart';
 import 'package:analyzer/src/test_utilities/test_code_format.dart';
 import 'package:analyzer_plugin/protocol/protocol_common.dart' as plugin;
 import 'package:analyzer_plugin/protocol/protocol_generated.dart' as plugin;
 import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
-import '../utils/lsp_protocol_extensions.dart';
 import '../utils/test_code_extensions.dart';
 import 'code_actions_abstract.dart';