[analysis_server] Allow the ApplyCodeAction command to be called via LSP-over-Legacy

This updates the `ApplyCodeActionCommandHandler` to be a "shared" command that can work for either kind of server (although it does not yet allow calling over DTD - the executeCommand handler still requires trusted callers).

It also moves all of the tests into a shared mixin so they will be run for both server kinds.

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

Author: DanTup

pkg/analysis_server/lib/src/analysis_server.dart

@@ -85,6 +85,7 @@ import 'package:analyzer_plugin/src/utilities/client_uri_converter.dart';
 import 'package:collection/collection.dart';
 import 'package:http/http.dart' as http;
 import 'package:meta/meta.dart';
+import 'package:path/path.dart' as path;
 import 'package:watcher/watcher.dart';
 
 /// The function for sending `openUri` request to the client.
@@ -455,6 +456,8 @@ abstract class AnalysisServer {
   /// Returns owners of files.
   OwnedFiles get ownedFiles => contextManager.ownedFiles;
 
+  path.Context get pathContext => resourceProvider.pathContext;
+
   /// Whether or not the client supports showMessageRequest to show the user
   /// a message and allow them to respond by clicking buttons.
   ///

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

@@ -8,9 +8,9 @@ library;
 import 'dart:async';
 
 import 'package:analysis_server/lsp_protocol/protocol.dart';
+import 'package:analysis_server/src/analysis_server.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;
@@ -32,7 +32,7 @@ typedef CodeActionWithPriorityAndIndex =
 
 /// A base for classes that produce [CodeAction]s for the LSP handler.
 abstract class AbstractCodeActionsProducer
-    with RequestHandlerMixin<LspAnalysisServer> {
+    with RequestHandlerMixin<AnalysisServer> {
   final File file;
   final LineInfo lineInfo;
   final int offset;
@@ -50,7 +50,7 @@ abstract class AbstractCodeActionsProducer
   final AnalysisOptions analysisOptions;
 
   @override
-  final LspAnalysisServer server;
+  final AnalysisServer server;
 
   /// Whether [CodeAction]s can be [Command]s or not.
   ///

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

@@ -8,6 +8,7 @@
 library;
 
 import 'package:analysis_server/lsp_protocol/protocol.dart';
+import 'package:analysis_server/src/analysis_server.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';
@@ -17,7 +18,6 @@ import 'package:analysis_server/src/lsp/handlers/code_actions/dart.dart';
 import 'package:analysis_server/src/lsp/handlers/code_actions/plugins.dart';
 import 'package:analysis_server/src/lsp/handlers/code_actions/pubspec.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:analyzer/dart/analysis/analysis_options.dart';
 import 'package:analyzer/dart/analysis/results.dart';
@@ -30,13 +30,7 @@ import 'package:path/src/context.dart';
 
 /// A helper class for computing [CodeAction]s that is used by both the
 /// [CodeActionHandler] and [ApplyCodeActionCommandHandler].
-class CodeActionComputer
-    with
-        HandlerHelperMixin<
-          // TODO(dantup): Make this (and the code action producers) work with
-          //  either server.
-          LspAnalysisServer
-        > {
+class CodeActionComputer with HandlerHelperMixin<AnalysisServer> {
   /// The text document to compute code actions for.
   final TextDocumentIdentifier textDocument;
 
@@ -78,7 +72,7 @@ class CodeActionComputer
   final LspClientCapabilities editorCapabilities;
 
   @override
-  final LspAnalysisServer server;
+  final AnalysisServer server;
 
   /// The kinds of [CodeAction]s that the caller supports.
   ///

pkg/analysis_server/lib/src/lsp/handlers/commands/apply_code_action.dart

@@ -3,12 +3,12 @@
 // 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/code_actions/code_action_computer.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:language_server_protocol/json_parsing.dart';
 
@@ -24,7 +24,7 @@ import 'package:language_server_protocol/json_parsing.dart';
 /// the server in the same session and therefore the arguments are not specified
 /// and can easily change - the server only needs to be consistent with itself.
 class ApplyCodeActionCommandHandler
-    extends SimpleEditCommandHandler<LspAnalysisServer> {
+    extends SimpleEditCommandHandler<AnalysisServer> {
   ApplyCodeActionCommandHandler(super.server);
 
   @override
@@ -117,11 +117,9 @@ class ApplyCodeActionCommandHandler
     var loggedAction = loggedActionParameter as String?;
 
     // Verify the document is still fresh.
-    if (textDocument.version case var version?) {
-      var filePath = server.uriConverter.fromClientUri(textDocument.uri);
-      if (server.getDocumentVersion(filePath) != version) {
-        return fileModifiedError;
-      }
+    var filePath = server.uriConverter.fromClientUri(textDocument.uri);
+    if (fileHasBeenModified(filePath, textDocument.version)) {
+      return fileModifiedError;
     }
 
     // Also record the action we're triggering.
@@ -149,6 +147,10 @@ class ApplyCodeActionCommandHandler
     );
     var actions = await computer.compute();
 
+    if (cancellationToken.isCancellationRequested) {
+      return cancelled(cancellationToken);
+    }
+
     return actions.mapResult((actions) async {
       return switch (actions) {
         null || [] => error(

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

@@ -39,11 +39,10 @@ class ExecuteCommandHandler
         Commands.organizeImports: OrganizeImportsCommandHandler(server),
         Commands.sendWorkspaceEdit: SendWorkspaceEditCommandHandler(server),
         Commands.logAction: LogActionCommandHandler(server),
+        Commands.applyCodeAction: ApplyCodeActionCommandHandler(server),
 
         // Commands that currently require an underlying LSP server.
         if (server is LspAnalysisServer) ...{
-          // TODO(dantup): Make code actions shared.
-          Commands.applyCodeAction: ApplyCodeActionCommandHandler(server),
           Commands.fixAll: FixAllCommandHandler(server),
           Commands.fixAllInWorkspace: FixAllInWorkspaceCommandHandler(server),
           Commands.previewFixAllInWorkspace:

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

@@ -113,7 +113,11 @@ mixin HandlerHelperMixin<S extends AnalysisServer> {
 
   bool fileHasBeenModified(String path, int? clientVersion) {
     var serverDocumentVersion = server.getDocumentVersion(path);
-    return clientVersion != null && clientVersion != serverDocumentVersion;
+    // Allow nulls on either side, because if a client doesn't support versions
+    // somewhere we can't just reject everything.
+    return clientVersion != null &&
+        serverDocumentVersion != null &&
+        clientVersion != serverDocumentVersion;
   }
 
   ErrorOr<T> fileNotAnalyzedError<T>(String path) => error<T>(

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

@@ -50,7 +50,6 @@ import 'package:analyzer_plugin/src/utilities/client_uri_converter.dart';
 import 'package:collection/collection.dart';
 import 'package:http/http.dart' as http;
 import 'package:meta/meta.dart';
-import 'package:path/path.dart' as path;
 
 /// Instances of the class [LspAnalysisServer] implement an LSP-based server
 /// that listens on a [LspServerCommunicationChannel] for LSP messages and
@@ -265,8 +264,6 @@ class LspAnalysisServer extends AnalysisServer {
     };
   }
 
-  path.Context get pathContext => resourceProvider.pathContext;
-
   @override
   set pluginManager(PluginManager value) {
     if (AnalysisServer.supportsPlugins) {

pkg/analysis_server/lib/src/services/refactoring/framework/refactoring_context.dart

@@ -2,8 +2,8 @@
 // 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/src/analysis_server.dart';
 import 'package:analysis_server/src/lsp/client_capabilities.dart';
-import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
 import 'package:analysis_server/src/services/search/search_engine.dart';
 import 'package:analysis_server_plugin/edit/correction_utils.dart';
 import 'package:analysis_server_plugin/src/correction/change_workspace.dart';
@@ -97,7 +97,7 @@ class AbstractRefactoringContext {
 
 /// The context in which a refactoring was requested.
 class RefactoringContext extends AbstractRefactoringContext {
-  final LspAnalysisServer server;
+  final AnalysisServer server;
 
   /// Initialize a newly created refactoring context.
   RefactoringContext({

pkg/analysis_server/test/lsp/code_actions_mixin.dart

@@ -82,17 +82,6 @@ mixin CodeActionsTestMixin on AbstractLspAnalysisServerTest {
     return action.asCodeActionLiteral;
   }
 
-  /// Expects that command [commandName] was logged to the analytics manager.
-  void expectCommandLogged(String commandName) {
-    expect(
-      server.analyticsManager
-          .getRequestData(Method.workspace_executeCommand.toString())
-          .additionalEnumCounts['command']!
-          .keys,
-      contains(commandName),
-    );
-  }
-
   /// Initializes the server with some basic configuration and expects not to
   /// find a [CodeAction] with [kind]/[command]/[title].
   Future<void> expectNoAction(