[analysis_server] Make LSP executeCommand() a shared handler to support the legacy server

DanTup · Commit Queue · commit be90b4d7b09a · 2025-04-30T16:44:32.000-07:00
As a pre-cursor to supporting `executeCommand()` over DTD, this converts the handler from an LspHandler to a SharedHandler so it can work with either a base LSP or Legacy server. Not all commands themselves work in both modes, but the executeCommand() handler itself and some basic commands (like `logAction`, which I used for testing) do. Change-Id: I90de88a1870649fb121cc9fa45732b4d03a45504 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425263 Commit-Queue: Elliott Brooks <elliottbrooks@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Reviewed-by: Elliott Brooks <elliottbrooks@google.com>
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/commands/abstract_refactor.dart b/pkg/analysis_server/lib/src/lsp/handlers/commands/abstract_refactor.dart
@@ -10,6 +10,7 @@ import 'package:analysis_server/src/lsp/constants.dart';
 import 'package:analysis_server/src/lsp/error_or.dart';
 import 'package:analysis_server/src/lsp/handlers/commands/simple_edit_handler.dart';
 import 'package:analysis_server/src/lsp/handlers/handlers.dart';
+import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
 import 'package:analysis_server/src/lsp/progress.dart';
 import 'package:analysis_server/src/protocol_server.dart';
 import 'package:analysis_server/src/services/refactoring/legacy/refactoring.dart';
@@ -22,7 +23,8 @@ final _manager = LspRefactorManager._();
 
 /// A base class for refactoring commands that need to create Refactorings from
 /// client-supplied arguments.
-abstract class AbstractRefactorCommandHandler extends SimpleEditCommandHandler
+abstract class AbstractRefactorCommandHandler
+    extends SimpleEditCommandHandler<LspAnalysisServer>
     with PositionalArgCommandHandler {
   AbstractRefactorCommandHandler(super.server);
 
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/commands/fix_all.dart b/pkg/analysis_server/lib/src/lsp/handlers/commands/fix_all.dart
@@ -16,7 +16,7 @@ import 'package:analysis_server/src/lsp/temporary_overlay_operation.dart';
 import 'package:analysis_server/src/services/correction/bulk_fix_processor.dart';
 import 'package:analysis_server/src/utilities/source_change_merger.dart';
 
-class FixAllCommandHandler extends SimpleEditCommandHandler {
+class FixAllCommandHandler extends SimpleEditCommandHandler<LspAnalysisServer> {
   FixAllCommandHandler(super.server);
 
   @override
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/commands/fix_all_in_workspace.dart b/pkg/analysis_server/lib/src/lsp/handlers/commands/fix_all_in_workspace.dart
@@ -7,14 +7,15 @@ import 'package:analysis_server/src/lsp/constants.dart';
 import 'package:analysis_server/src/lsp/error_or.dart';
 import 'package:analysis_server/src/lsp/handlers/commands/simple_edit_handler.dart';
 import 'package:analysis_server/src/lsp/handlers/handlers.dart';
+import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
 import 'package:analysis_server/src/lsp/mapping.dart';
 import 'package:analysis_server/src/lsp/progress.dart';
 import 'package:analysis_server/src/lsp/source_edits.dart';
 import 'package:analysis_server/src/services/correction/bulk_fix_processor.dart';
 import 'package:analysis_server_plugin/src/correction/dart_change_workspace.dart';
 
 abstract class AbstractFixAllInWorkspaceCommandHandler
-    extends SimpleEditCommandHandler {
+    extends SimpleEditCommandHandler<LspAnalysisServer> {
   AbstractFixAllInWorkspaceCommandHandler(super.server);
 
   /// Whether to require confirmation from the user to apply these changes.
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/commands/log_action.dart b/pkg/analysis_server/lib/src/lsp/handlers/commands/log_action.dart
@@ -3,12 +3,13 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analysis_server/lsp_protocol/protocol.dart';
+import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/lsp/error_or.dart';
 import 'package:analysis_server/src/lsp/handlers/handlers.dart';
 import 'package:analysis_server/src/lsp/progress.dart';
 
 class LogActionCommandHandler
-    extends CommandHandler<ExecuteCommandParams, Object> {
+    extends CommandHandler<ExecuteCommandParams, Object, AnalysisServer> {
   LogActionCommandHandler(super.server);
 
   @override
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/commands/refactor_command_handler.dart b/pkg/analysis_server/lib/src/lsp/handlers/commands/refactor_command_handler.dart
@@ -8,6 +8,7 @@ import 'package:analysis_server/src/lsp/constants.dart';
 import 'package:analysis_server/src/lsp/error_or.dart';
 import 'package:analysis_server/src/lsp/handlers/commands/simple_edit_handler.dart';
 import 'package:analysis_server/src/lsp/handlers/handlers.dart';
+import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
 import 'package:analysis_server/src/lsp/mapping.dart';
 import 'package:analysis_server/src/lsp/progress.dart';
 import 'package:analysis_server/src/lsp/source_edits.dart';
@@ -18,7 +19,8 @@ import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
 
 /// A command handler for any of the commands used to implement refactorings.
-class RefactorCommandHandler extends SimpleEditCommandHandler {
+class RefactorCommandHandler
+    extends SimpleEditCommandHandler<LspAnalysisServer> {
   @override
   final String commandName;
 
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/commands/simple_edit_handler.dart b/pkg/analysis_server/lib/src/lsp/handlers/commands/simple_edit_handler.dart
@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analysis_server/lsp_protocol/protocol.dart';
+import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/lsp/constants.dart';
 import 'package:analysis_server/src/lsp/error_or.dart';
 import 'package:analysis_server/src/lsp/handlers/handlers.dart';
@@ -14,8 +15,8 @@ import 'package:analyzer/src/dart/scanner/scanner.dart' as engine;
 import 'package:analyzer/src/generated/parser.dart' as engine;
 import 'package:analyzer_plugin/protocol/protocol_common.dart';
 
-abstract class SimpleEditCommandHandler
-    extends CommandHandler<ExecuteCommandParams, Object> {
+abstract class SimpleEditCommandHandler<S extends AnalysisServer>
+    extends CommandHandler<ExecuteCommandParams, Object, S> {
   SimpleEditCommandHandler(super.server);
 
   String get commandName;
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_execute_command.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_execute_command.dart
@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analysis_server/lsp_protocol/protocol.dart';
+import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/lsp/constants.dart';
 import 'package:analysis_server/src/lsp/error_or.dart';
 import 'package:analysis_server/src/lsp/handlers/commands/fix_all.dart';
@@ -15,32 +16,41 @@ import 'package:analysis_server/src/lsp/handlers/commands/send_workspace_edit.da
 import 'package:analysis_server/src/lsp/handlers/commands/sort_members.dart';
 import 'package:analysis_server/src/lsp/handlers/commands/validate_refactor.dart';
 import 'package:analysis_server/src/lsp/handlers/handlers.dart';
+import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
 import 'package:analysis_server/src/lsp/progress.dart';
 import 'package:analysis_server/src/lsp/registration/feature_registration.dart';
 import 'package:analysis_server/src/services/refactoring/framework/refactoring_processor.dart';
 
 /// Handles workspace/executeCommand messages by delegating to a specific
 /// handler based on the command.
 class ExecuteCommandHandler
-    extends LspMessageHandler<ExecuteCommandParams, Object?> {
-  final Map<String, CommandHandler<ExecuteCommandParams, Object>>
+    extends SharedMessageHandler<ExecuteCommandParams, Object?> {
+  final Map<
+    String,
+    CommandHandler<ExecuteCommandParams, Object, AnalysisServer>
+  >
   commandHandlers;
 
   ExecuteCommandHandler(super.server)
     : commandHandlers = {
+        // Commands that can run for any underlying server type.
         Commands.sortMembers: SortMembersCommandHandler(server),
         Commands.organizeImports: OrganizeImportsCommandHandler(server),
-        Commands.fixAll: FixAllCommandHandler(server),
-        Commands.fixAllInWorkspace: FixAllInWorkspaceCommandHandler(server),
-        Commands.previewFixAllInWorkspace:
-            PreviewFixAllInWorkspaceCommandHandler(server),
-        Commands.performRefactor: PerformRefactorCommandHandler(server),
-        Commands.validateRefactor: ValidateRefactorCommandHandler(server),
         Commands.sendWorkspaceEdit: SendWorkspaceEditCommandHandler(server),
         Commands.logAction: LogActionCommandHandler(server),
-        // Add commands for each of the refactorings.
-        for (var entry in RefactoringProcessor.generators.entries)
-          entry.key: RefactorCommandHandler(server, entry.key, entry.value),
+
+        // Commands that currently require an underlying LSP server.
+        if (server is LspAnalysisServer) ...{
+          Commands.fixAll: FixAllCommandHandler(server),
+          Commands.fixAllInWorkspace: FixAllInWorkspaceCommandHandler(server),
+          Commands.previewFixAllInWorkspace:
+              PreviewFixAllInWorkspaceCommandHandler(server),
+          Commands.performRefactor: PerformRefactorCommandHandler(server),
+          Commands.validateRefactor: ValidateRefactorCommandHandler(server),
+          // Add commands for each of the refactorings.
+          for (var entry in RefactoringProcessor.generators.entries)
+            entry.key: RefactorCommandHandler(server, entry.key, entry.value),
+        },
       } {
     server.executeCommandHandler = this;
   }
@@ -52,6 +62,12 @@ class ExecuteCommandHandler
   LspJsonHandler<ExecuteCommandParams> get jsonHandler =>
       ExecuteCommandParams.jsonHandler;
 
+  @override
+  // TODO(dantup): This will need to be relaxed to support calling over DTD,
+  //  but at that point we must also add this flag to Commands so that we can
+  //  control which commands require trusted callers.
+  bool get requiresTrustedCaller => true;
+
   @override
   Future<ErrorOr<Object?>> handle(
     ExecuteCommandParams params,
@@ -70,14 +86,17 @@ class ExecuteCommandHandler
       server.analyticsManager.executedCommand(params.command);
     }
     var workDoneToken = params.workDoneToken;
-    var progress =
-        workDoneToken != null
-            ? ProgressReporter.clientProvided(server, workDoneToken)
-            // Use editor client capabilities, as that's who gets progress
-            // notifications, not the caller.
-            : server.editorClientCapabilities?.workDoneProgress ?? false
-            ? ProgressReporter.serverCreated(server)
-            : ProgressReporter.noop;
+    ProgressReporter progress = ProgressReporter.noop;
+    if (server case LspAnalysisServer server) {
+      progress =
+          workDoneToken != null
+              ? ProgressReporter.clientProvided(server, workDoneToken)
+              // Use editor client capabilities, as that's who gets progress
+              // notifications, not the caller.
+              : server.editorClientCapabilities?.workDoneProgress ?? false
+              ? ProgressReporter.serverCreated(server)
+              : ProgressReporter.noop;
+    }
 
     // To make passing arguments easier in commands, instead of a
     // `List<Object?>` we now use `Map<String, Object?>`.
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_states.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_states.dart
@@ -92,7 +92,6 @@ class InitializedLspStateMessageHandler extends InitializedStateMessageHandler {
         DocumentLinkHandler.new,
         ReferencesHandler.new,
         CodeActionHandler.new,
-        ExecuteCommandHandler.new,
         ChangeWorkspaceFoldersHandler.new,
         PrepareRenameHandler.new,
         RenameHandler.new,
@@ -133,6 +132,7 @@ class InitializedStateMessageHandler extends ServerStateMessageHandler {
         DocumentSymbolHandler.new,
         EditableArgumentsHandler.new,
         EditArgumentHandler.new,
+        ExecuteCommandHandler.new,
         ExperimentalEchoHandler.new,
         FormatOnTypeHandler.new,
         FormatRangeHandler.new,
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handlers.dart b/pkg/analysis_server/lib/src/lsp/handlers/handlers.dart
@@ -36,9 +36,10 @@ Iterable<T> convert<T, E>(Iterable<E> items, T? Function(E) converter) {
   return items.map(converter).where((item) => item != null).cast<T>();
 }
 
-abstract class CommandHandler<P, R> with Handler<R>, HandlerHelperMixin {
+abstract class CommandHandler<P, R, S extends AnalysisServer>
+    with Handler<R>, HandlerHelperMixin {
   @override
-  final LspAnalysisServer server;
+  final S server;
 
   CommandHandler(this.server);
 
diff --git a/pkg/analysis_server/test/lsp/request_helpers_mixin.dart b/pkg/analysis_server/test/lsp/request_helpers_mixin.dart
@@ -209,6 +209,25 @@ mixin LspRequestHelpersMixin {
   Range entireRange(String code) =>
       Range(start: startOfDocPos, end: positionFromOffset(code.length, code));
 
+  Future<T> executeCommand<T>(
+    Command command, {
+    T Function(Map<String, Object?>)? decoder,
+    ProgressToken? workDoneToken,
+  }) {
+    var request = makeRequest(
+      Method.workspace_executeCommand,
+      ExecuteCommandParams(
+        command: command.command,
+        arguments: command.arguments,
+        workDoneToken: workDoneToken,
+      ),
+    );
+    return expectSuccessfulResponseTo<T, Map<String, Object?>>(
+      request,
+      decoder ?? (result) => result as T,
+    );
+  }
+
   void expect(Object? actual, Matcher matcher, {String? reason}) =>
       test.expect(actual, matcher, reason: reason);
 
diff --git a/pkg/analysis_server/test/lsp/server_abstract.dart b/pkg/analysis_server/test/lsp/server_abstract.dart
@@ -1048,6 +1048,7 @@ mixin LspAnalysisServerTestMixin on LspRequestHelpersMixin, LspEditHelpersMixin
     return executeCommand(command);
   }
 
+  @override
   Future<T> executeCommand<T>(
     Command command, {
     T Function(Map<String, Object?>)? decoder,
@@ -1061,17 +1062,10 @@ mixin LspAnalysisServerTestMixin on LspRequestHelpersMixin, LspEditHelpersMixin
         'Is it missing from serverSupportedCommands?',
       );
     }
-    var request = makeRequest(
-      Method.workspace_executeCommand,
-      ExecuteCommandParams(
-        command: command.command,
-        arguments: command.arguments,
-        workDoneToken: workDoneToken,
-      ),
-    );
-    return expectSuccessfulResponseTo<T, Map<String, Object?>>(
-      request,
-      decoder ?? (result) => result as T,
+    return super.executeCommand(
+      command,
+      decoder: decoder,
+      workDoneToken: workDoneToken,
     );
   }
 
diff --git a/pkg/analysis_server/test/lsp_over_legacy/execute_command_test.dart b/pkg/analysis_server/test/lsp_over_legacy/execute_command_test.dart
@@ -0,0 +1,42 @@
+// 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';
+import 'package:analysis_server/src/lsp/constants.dart';
+import 'package:test/test.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import 'abstract_lsp_over_legacy.dart';
+
+void main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(ExecuteCommandTest);
+  });
+}
+
+@reflectiveTest
+class ExecuteCommandTest extends LspOverLegacyTest {
+  Future<void> test_logAction() async {
+    var testActionName = 'test.action';
+
+    await waitForTasksFinished();
+    await executeCommand(
+      Command(
+        title: 'Log Action',
+        command: Commands.logAction,
+        arguments: [
+          {'action': testActionName},
+        ],
+      ),
+    );
+
+    expect(
+      server.analyticsManager
+          .getRequestData(Method.workspace_executeCommand.toString())
+          .additionalEnumCounts['command']!
+          .keys,
+      contains(testActionName),
+    );
+  }
+}
diff --git a/pkg/analysis_server/test/lsp_over_legacy/test_all.dart b/pkg/analysis_server/test/lsp_over_legacy/test_all.dart
@@ -10,6 +10,7 @@ import 'document_highlights_test.dart' as document_highlights;
 import 'document_symbols_test.dart' as document_symbols;
 import 'edit_argument_test.dart' as edit_argument;
 import 'editable_arguments_test.dart' as editable_arguments;
+import 'execute_command_test.dart' as execute_command;
 import 'format_test.dart' as format;
 import 'hover_test.dart' as hover;
 import 'implementation_test.dart' as implementation;
@@ -28,6 +29,7 @@ void main() {
     document_symbols.main();
     edit_argument.main();
     editable_arguments.main();
+    execute_command.main();
     format.main();
     hover.main();
     implementation.main();
