[analysis_server] Use a static identifier for plugin-supplied fixes/assists

Change-Id: Ifc6310aabe60a8c5671203b66b11fe98e0c5cbbe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/416800
Reviewed-by: Phil Quitslund <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

Author: DanTup

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

@@ -70,14 +70,15 @@ abstract class AbstractCodeActionsProducer
   @protected
   CodeAction createAssistAction(
     protocol.SourceChange change,
+    String? loggedAssistId,
     String path,
     LineInfo lineInfo,
   ) {
     return CodeAction(
       title: change.message,
       kind: toCodeActionKind(change.id, CodeActionKind.Refactor),
       diagnostics: const [],
-      command: createLogActionCommand(change.id),
+      command: createLogActionCommand(loggedAssistId),
       edit: createWorkspaceEdit(
         server,
         capabilities,
@@ -112,6 +113,7 @@ abstract class AbstractCodeActionsProducer
   @protected
   CodeAction createFixAction(
     protocol.SourceChange change,
+    String? loggedFixId,
     Diagnostic diagnostic,
     String path,
     LineInfo lineInfo,
@@ -120,7 +122,7 @@ abstract class AbstractCodeActionsProducer
       title: change.message,
       kind: toCodeActionKind(change.id, CodeActionKind.QuickFix),
       diagnostics: [diagnostic],
-      command: createLogActionCommand(change.id),
+      command: createLogActionCommand(loggedFixId),
       edit: createWorkspaceEdit(
         server,
         capabilities,

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

@@ -95,7 +95,13 @@ class AnalysisOptionsCodeActionsProducer extends AbstractCodeActionsProducer {
       var diagnostic = createDiagnostic(lineInfo, result, error);
       codeActions.addAll(
         fixes.map((fix) {
-          var action = createFixAction(fix.change, diagnostic, path, lineInfo);
+          var action = createFixAction(
+            fix.change,
+            fix.change.id,
+            diagnostic,
+            path,
+            lineInfo,
+          );
           return (action: action, priority: fix.kind.priority);
         }),
       );

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

@@ -164,6 +164,7 @@ class DartCodeActionsProducer extends AbstractCodeActionsProducer {
       return assists.map((assist) {
         var action = createAssistAction(
           assist.change,
+          assist.change.id,
           unitResult.path,
           unitResult.lineInfo,
         );
@@ -242,6 +243,7 @@ class DartCodeActionsProducer extends AbstractCodeActionsProducer {
             fixes.map((fix) {
               var action = createFixAction(
                 fix.change,
+                fix.change.id,
                 diagnostic,
                 path,
                 lineInfo,

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

@@ -78,7 +78,12 @@ class PluginCodeActionsProducer extends AbstractCodeActionsProducer {
 
   CodeActionWithPriority _convertAssist(plugin.PrioritizedSourceChange assist) {
     return (
-      action: createAssistAction(assist.change, path, lineInfo),
+      action: createAssistAction(
+        assist.change,
+        'assist from plugin',
+        path,
+        lineInfo,
+      ),
       priority: assist.priority,
     );
   }
@@ -95,7 +100,13 @@ class PluginCodeActionsProducer extends AbstractCodeActionsProducer {
     );
     return fixes.fixes.map(
       (fix) => (
-        action: createFixAction(fix.change, diagnostic, path, lineInfo),
+        action: createFixAction(
+          fix.change,
+          'fix from plugin',
+          diagnostic,
+          path,
+          lineInfo,
+        ),
         priority: fix.priority,
       ),
     );

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

@@ -81,7 +81,13 @@ class PubspecCodeActionsProducer extends AbstractCodeActionsProducer {
       var diagnostic = createDiagnostic(lineInfo, result, error);
       codeActions.addAll(
         fixes.map((fix) {
-          var action = createFixAction(fix.change, diagnostic, path, lineInfo);
+          var action = createFixAction(
+            fix.change,
+            fix.change.id,
+            diagnostic,
+            path,
+            lineInfo,
+          );
           return (action: action, priority: fix.kind.priority);
         }),
       );

pkg/analysis_server/test/lsp/code_actions_abstract.dart

@@ -20,6 +20,7 @@ abstract class AbstractCodeActionsTest extends AbstractLspAnalysisServerTest {
     String content, {
     CodeActionKind? kind,
     String? command,
+    List<Object>? commandArgs,
     String? title,
     CodeActionTriggerKind? triggerKind,
     String? filePath,
@@ -47,6 +48,7 @@ abstract class AbstractCodeActionsTest extends AbstractLspAnalysisServerTest {
       codeActions,
       kind: kind,
       command: command,
+      commandArgs: commandArgs,
       title: title,
     );
     if (action == null) {
@@ -100,18 +102,23 @@ abstract class AbstractCodeActionsTest extends AbstractLspAnalysisServerTest {
 
   /// Finds the single action matching [title], [kind] and [command].
   ///
+  /// If [command] and/or [commandArgs] are supplied, ensures the action has
+  /// a matching command/args.
+  ///
   /// Throws if zero or more than one actions match.
   CodeAction? findAction(
     List<Either2<Command, CodeAction>> actions, {
     String? title,
     CodeActionKind? kind,
     String? command,
+    List<Object>? commandArgs,
   }) {
     return findActions(
       actions,
       title: title,
       kind: kind,
       command: command,
+      commandArgs: commandArgs,
     ).singleOrNull;
   }
 
@@ -120,21 +127,38 @@ abstract class AbstractCodeActionsTest extends AbstractLspAnalysisServerTest {
     String? title,
     CodeActionKind? kind,
     String? command,
+    List<Object>? commandArgs,
   }) {
     return actions
         .map((action) => action.map((cmd) => null, (action) => action))
         .where((action) => title == null || action?.title == title)
         .where((action) => kind == null || action?.kind == kind)
+        // Some tests filter by only supplying a command, so if there is no
+        // title given, filter by the command. If a title was given, don't
+        // filter by the command and assert it below. This results in a better
+        // failure message if the action existed by title but without the correct
+        // command.
         .where(
-          (action) => command == null || action?.command?.command == command,
+          (action) =>
+              title != null ||
+              command == null ||
+              action?.command?.command == command,
         )
         .map((action) {
           // Always expect a command (either to execute, or for logging)
-          assert(action!.command != null);
-          // Expect an edit if we weren't looking for a command-action.
-          if (command == null) {
-            assert(action!.edit != null);
+          expect(action!.command, isNotNull);
+
+          if (command != null) {
+            expect(action.command!.command, command);
+          } else {
+            // Expect an edit if we weren't looking for a command-action.
+            expect(action.edit, isNotNull);
           }
+
+          if (commandArgs != null) {
+            expect(action.command!.arguments, equals(commandArgs));
+          }
+
           return action;
         })
         .nonNulls
@@ -183,6 +207,7 @@ abstract class AbstractCodeActionsTest extends AbstractLspAnalysisServerTest {
     String? filePath,
     CodeActionKind? kind,
     String? command,
+    List<Object>? commandArgs,
     String? title,
     ProgressToken? commandWorkDoneToken,
     bool openTargetFile = false,
@@ -202,13 +227,16 @@ $expected''';
       content,
       kind: kind,
       command: command,
+      commandArgs: commandArgs,
       title: title,
       openTargetFile: openTargetFile,
     );
 
     // Verify the edits either by executing the command we expected, or
     // the edits attached directly to the code action.
-    if (command != null) {
+    // Don't try to execute 'dart.logAction' because it will never produce
+    // edits.
+    if (command != null && command != 'dart.logAction') {
       return await verifyCommandEdits(
         action.command!,
         expected,

pkg/analysis_server/test/lsp/code_actions_assists_test.dart

@@ -216,6 +216,10 @@ bar
       expectedContent,
       kind: CodeActionKind('refactor.fooToBar'),
       title: "Change 'foo' to 'bar'",
+      command: 'dart.logAction',
+      commandArgs: [
+        {'action': 'assist from plugin'},
+      ],
     );
   }
 

pkg/analysis_server/test/lsp/code_actions_fixes_test.dart

@@ -97,6 +97,10 @@ bar
       expectedContent,
       kind: CodeActionKind('quickfix.fooToBar'),
       title: "Change 'foo' to 'bar'",
+      command: 'dart.logAction',
+      commandArgs: [
+        {'action': 'fix from plugin'},
+      ],
     );
   }