[analysis_server] Add progress reporting to "Fix all in Workspace" command

We already have support for sending progress from command handlers, but each handler has to send them explicitly (so we don't trigger progress notifications for trivial commands). This adds that to the "Fix all in Workspace" command.

Fixes Dart-Code/Dart-Code#5773

Change-Id: I70d42357323ff88441d64098f96db5ee02e61dba
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/460044
Commit-Queue: Keerti Parthasarathy <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Keerti Parthasarathy <[email protected]>

Author: DanTup

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

@@ -58,29 +58,35 @@ abstract class AbstractFixAllInWorkspaceCommandHandler
     var workspace = DartChangeWorkspace(await server.currentSessions);
     var processor = BulkFixProcessor(server.instrumentationService, workspace);
 
-    var result = await processor.fixErrors(
-      server.contextManager.analysisContexts,
-    );
-    var errorMessage = result.errorMessage;
-    if (errorMessage != null) {
-      return error(ErrorCodes.RequestFailed, errorMessage);
-    }
+    progress.begin('Computing fixes…');
+    try {
+      var result = await processor.fixErrors(
+        server.contextManager.analysisContexts,
+      );
 
-    var changeBuilder = result.builder!;
-    var change = changeBuilder.sourceChange;
-    if (change.edits.isEmpty) {
-      return success(null);
+      var errorMessage = result.errorMessage;
+      if (errorMessage != null) {
+        return error(ErrorCodes.RequestFailed, errorMessage);
+      }
+
+      var changeBuilder = result.builder!;
+      var change = changeBuilder.sourceChange;
+      if (change.edits.isEmpty) {
+        return success(null);
+      }
+
+      var edit = createWorkspaceEdit(
+        server,
+        clientCapabilities,
+        change,
+        annotateChanges: requireConfirmation
+            ? ChangeAnnotations.requireConfirmation
+            : ChangeAnnotations.include,
+      );
+      return sendWorkspaceEditToClient(edit);
+    } finally {
+      progress.end();
     }
-
-    var edit = createWorkspaceEdit(
-      server,
-      clientCapabilities,
-      change,
-      annotateChanges: requireConfirmation
-          ? ChangeAnnotations.requireConfirmation
-          : ChangeAnnotations.include,
-    );
-    return sendWorkspaceEditToClient(edit);
   }
 }
 

pkg/analysis_server/test/lsp/commands/fix_all_in_workspace_test.dart

@@ -10,6 +10,7 @@ import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
 import '../../tool/lsp_spec/matchers.dart';
+import '../request_helpers_mixin.dart';
 import '../server_abstract.dart';
 
 void main() {
@@ -20,7 +21,8 @@ void main() {
 }
 
 abstract class AbstractFixAllInWorkspaceTest
-    extends AbstractLspAnalysisServerTest {
+    extends AbstractLspAnalysisServerTest
+    with LspProgressNotificationsMixin {
   String get commandId;
   String get commandName;
   bool get expectRequiresConfirmation;
@@ -229,6 +231,28 @@ C b() {
 ''');
   }
 
+  Future<void> test_progressNotifications_supported() async {
+    // Advertise that we (the client) support progress.
+    setWorkDoneProgressSupport();
+
+    // Expect (during the test) a progress token to be created, started, ended.
+    expect(progressUpdates, emitsInOrder(['CREATE', 'BEGIN', 'END']));
+
+    await initialize();
+    await executeCommand(Command(command: commandId, title: 'UNUSED'));
+  }
+
+  Future<void> test_progressNotifications_unsupported() async {
+    // Advertise that we do not support progress.
+    setWorkDoneProgressSupport(false);
+
+    // Expect no progress updates at all.
+    expect(progressUpdates, neverEmits(anything));
+
+    await initialize();
+    await executeCommand(Command(command: commandId, title: 'UNUSED'));
+  }
+
   Future<void> test_serverAdvertisesCommand() async {
     await initialize();
     expect(

pkg/analysis_server/test/lsp/server_abstract.dart

@@ -771,9 +771,9 @@ mixin ClientCapabilitiesHelperMixin {
     setTextDocumentDynamicRegistration('synchronization');
   }
 
-  void setWorkDoneProgressSupport() {
+  void setWorkDoneProgressSupport([bool supported = true]) {
     windowCapabilities = extendWindowCapabilities(windowCapabilities, {
-      'workDoneProgress': true,
+      'workDoneProgress': supported,
     });
   }
 
@@ -826,7 +826,7 @@ mixin LspAnalysisServerTestMixin
   /// initialize.
   ServerCapabilities? _serverCapabilities;
 
-  final validProgressTokens = <ProgressToken>{};
+  final _validProgressTokens = <ProgressToken>{};
 
   /// Default initialization options to be used if [initialize] is not provided
   /// options explicitly.
@@ -1574,15 +1574,15 @@ mixin LspAnalysisServerTestMixin
       request.params as Map<String, Object?>,
     );
     if (params.token != clientProvidedTestWorkDoneToken &&
-        !validProgressTokens.contains(params.token)) {
+        !_validProgressTokens.contains(params.token)) {
       throw Exception(
         'Server sent a progress notification for a token '
         'that has not been created: ${params.token}',
       );
     }
 
     if (WorkDoneProgressEnd.canParse(params.value, nullLspJsonReporter)) {
-      validProgressTokens.remove(params.token);
+      _validProgressTokens.remove(params.token);
     }
 
     if (params.token == analyzingProgressToken) {
@@ -1605,10 +1605,10 @@ mixin LspAnalysisServerTestMixin
     var params = WorkDoneProgressCreateParams.fromJson(
       request.params as Map<String, Object?>,
     );
-    if (validProgressTokens.contains(params.token)) {
+    if (_validProgressTokens.contains(params.token)) {
       throw Exception('Server tried to create already-active progress token');
     }
-    validProgressTokens.add(params.token);
+    _validProgressTokens.add(params.token);
   }
 
   /// Checks whether a notification is likely an error from the server (for