[Analyzer] Ask user whether to proceed with renames that produce errors

Fixes Dart-Code/Dart-Code#2745.

Change-Id: Ia2770f18ebca410bce93bd2f3042e294083dc435
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/167621
Commit-Queue: Danny Tuppeny <danny@tuppeny.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

Author: DanTup

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

@@ -52,6 +52,8 @@ const dartTypeFormattingCharacters = ['}', ';'];
 /// A [ProgressToken] used for reporting progress when the server is analyzing.
 final analyzingProgressToken = Either2<num, String>.t2('ANALYZING');
 
+final emptyWorkspaceEdit = WorkspaceEdit();
+
 /// Constants for command IDs that are exchanged between LSP client/server.
 abstract class Commands {
   /// A list of all commands IDs that can be sent to the client to inform which
@@ -127,3 +129,9 @@ abstract class ServerErrorCodes {
   ///   if it crashes 5 times in the last 180 seconds."
   static const ClientServerInconsistentState = ErrorCodes(-32099);
 }
+
+/// Strings used in user prompts (window/showMessageRequest).
+abstract class UserPromptActions {
+  static const String cancel = 'Cancel';
+  static const String renameAnyway = 'Rename Anyway';
+}

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

@@ -126,7 +126,7 @@ class RenameHandler extends MessageHandler<RenameParams, WorkspaceEdit> {
       if (token.isCancellationRequested) {
         return cancelled();
       }
-      if (initStatus.hasError) {
+      if (initStatus.hasFatalError) {
         return error(
             ServerErrorCodes.RenameNotValid, initStatus.problem.message, null);
       }
@@ -144,9 +144,30 @@ class RenameHandler extends MessageHandler<RenameParams, WorkspaceEdit> {
       if (token.isCancellationRequested) {
         return cancelled();
       }
-      if (finalStatus.hasError) {
+      if (finalStatus.hasFatalError) {
         return error(
             ServerErrorCodes.RenameNotValid, finalStatus.problem.message, null);
+      } else if (finalStatus.hasError || finalStatus.hasWarning) {
+        // Ask the user whether to proceed with the rename.
+        final userChoice = await server.showUserPrompt(
+          MessageType.Warning,
+          finalStatus.message,
+          [
+            MessageActionItem(title: UserPromptActions.renameAnyway),
+            MessageActionItem(title: UserPromptActions.cancel),
+          ],
+        );
+
+        if (token.isCancellationRequested) {
+          return cancelled();
+        }
+
+        if (userChoice.title != UserPromptActions.renameAnyway) {
+          // Return an empty workspace edit response so we do not perform any
+          // rename, but also so we do not cause the client to show the user an
+          // error after they clicked cancel.
+          return success(emptyWorkspaceEdit);
+        }
       }
 
       // Compute the actual change.

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

@@ -599,6 +599,17 @@ class LspAnalysisServer extends AbstractAnalysisServer {
     ));
   }
 
+  /// Shows the user a prompt with some actions to select using ShowMessageRequest.
+  Future<MessageActionItem> showUserPrompt(
+      MessageType type, String message, List<MessageActionItem> actions) async {
+    final response = await sendRequest(
+      Method.window_showMessageRequest,
+      ShowMessageRequestParams(type: type, message: message, actions: actions),
+    );
+
+    return MessageActionItem.fromJson(response.result);
+  }
+
   Future<void> shutdown() {
     // Defer closing the channel so that the shutdown response can be sent and
     // logged.

pkg/analysis_server/test/lsp/rename_test.dart

@@ -4,9 +4,11 @@
 
 import 'package:analysis_server/lsp_protocol/protocol_generated.dart';
 import 'package:analysis_server/src/lsp/constants.dart';
+import 'package:meta/meta.dart';
 import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
+import '../tool/lsp_spec/matchers.dart';
 import 'server_abstract.dart';
 
 void main() {
@@ -130,6 +132,84 @@ class RenameTest extends AbstractLspAnalysisServerTest {
         content, 'MyNewClass', expectedContent);
   }
 
+  Future<void> test_rename_duplicateName_applyAfterDocumentChanges() async {
+    // Perform a refactor that results in a prompt to the user, but then modify
+    // the document before accepting/rejecting to make the rename invalid.
+    const content = '''
+    class MyOtherClass {}
+    class MyClass {}
+    final a = n^ew MyClass();
+    ''';
+    final result = await _test_rename_prompt(
+      content,
+      'MyOtherClass',
+      expectedMessage:
+          'Library already declares class with name \'MyOtherClass\'.',
+      action: UserPromptActions.renameAnyway,
+      beforeResponding: () => replaceFile(999, mainFileUri, 'Updated content'),
+    );
+    expect(result.result, isNull);
+    expect(result.error, isNotNull);
+    expect(result.error, isResponseError(ErrorCodes.ContentModified));
+  }
+
+  Future<void> test_rename_duplicateName_applyAnyway() async {
+    const content = '''
+    class MyOtherClass {}
+    class MyClass {}
+    final a = n^ew MyClass();
+    ''';
+    const expectedContent = '''
+    class MyOtherClass {}
+    class MyOtherClass {}
+    final a = new MyOtherClass();
+    ''';
+    final response = await _test_rename_prompt(
+      content,
+      'MyOtherClass',
+      expectedMessage:
+          'Library already declares class with name \'MyOtherClass\'.',
+      action: UserPromptActions.renameAnyway,
+    );
+
+    if (response.error != null) {
+      throw response.error;
+    }
+
+    final result = WorkspaceEdit.fromJson(response.result);
+
+    // Ensure applying the changes will give us the expected content.
+    final contents = {
+      mainFilePath: withoutMarkers(content),
+    };
+    applyDocumentChanges(
+      contents,
+      result.documentChanges,
+    );
+    expect(contents[mainFilePath], equals(expectedContent));
+  }
+
+  Future<void> test_rename_duplicateName_reject() async {
+    const content = '''
+    class MyOtherClass {}
+    class MyClass {}
+    final a = n^ew MyClass();
+    ''';
+    final response = await _test_rename_prompt(
+      content,
+      'MyOtherClass',
+      expectedMessage:
+          'Library already declares class with name \'MyOtherClass\'.',
+      action: UserPromptActions.cancel,
+    );
+    // Expect a successful empty response if cancelled.
+    expect(response.error, isNull);
+    expect(
+      WorkspaceEdit.fromJson(response.result),
+      equals(emptyWorkspaceEdit),
+    );
+  }
+
   Future<void> test_rename_importPrefix() {
     const content = '''
     import 'dart:async' as myPr^efix;
@@ -237,17 +317,6 @@ class RenameTest extends AbstractLspAnalysisServerTest {
     expect(error.message, contains('name must not contain'));
   }
 
-  Future<void> test_rename_rejectedForDuplicateName() async {
-    const content = '''
-    class MyOtherClass {}
-    class MyClass {}
-    final a = n^ew MyClass();
-    ''';
-    final error = await _test_rename_failure(content, 'MyOtherClass');
-    expect(error.code, equals(ServerErrorCodes.RenameNotValid));
-    expect(error.message, contains('already declares class with name'));
-  }
-
   Future<void> test_rename_rejectedForSameName() async {
     const content = '''
     class My^Class {}
@@ -413,6 +482,55 @@ class RenameTest extends AbstractLspAnalysisServerTest {
     return result.error;
   }
 
+  /// Tests a rename that is expected to cause an error, which will trigger
+  /// a ShowMessageRequest from the server to the client to allow the refactor
+  /// to be continued or rejected.
+  Future<ResponseMessage> _test_rename_prompt(
+    String content,
+    String newName, {
+    @required String expectedMessage,
+    Future<void> Function() beforeResponding,
+    @required String action,
+    int openFileVersion = 222,
+    int renameRequestFileVersion = 222,
+  }) async {
+    await initialize(
+      workspaceCapabilities:
+          withDocumentChangesSupport(emptyWorkspaceClientCapabilities),
+    );
+    await openFile(mainFileUri, withoutMarkers(content),
+        version: openFileVersion);
+
+    // Expect the server to call us back with a ShowMessageRequest prompt about
+    // the errors for us to accept/reject.
+    return handleExpectedRequest(
+      Method.window_showMessageRequest,
+      ShowMessageRequestParams.fromJson,
+      () => renameRaw(
+        mainFileUri,
+        renameRequestFileVersion,
+        positionFromMarker(content),
+        newName,
+      ),
+      handler: (ShowMessageRequestParams params) async {
+        // Ensure the warning prompt is as expected.
+        expect(params.type, equals(MessageType.Warning));
+        expect(params.message, equals(expectedMessage));
+        expect(params.actions, hasLength(2));
+        expect(params.actions[0],
+            equals(MessageActionItem(title: UserPromptActions.renameAnyway)));
+        expect(params.actions[1],
+            equals(MessageActionItem(title: UserPromptActions.cancel)));
+
+        // Allow the test to run some code before we send the response.
+        await beforeResponding?.call();
+
+        // Respond to the request with the required action.
+        return MessageActionItem(title: action);
+      },
+    );
+  }
+
   Future<void> _test_rename_withDocumentChanges(
     String content,
     String newName,