Minor LSP refactors/tidy up

Change-Id: I7402d9493c58b1e020655a3b6332edbb5425fc22
Reviewed-on: https://dart-review.googlesource.com/c/89360
Commit-Queue: Danny Tuppeny <dantup@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Auto-Submit: Danny Tuppeny <dantup@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart
index dc83f4f..41d3014 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart
@@ -13,8 +13,6 @@
 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/source_edits.dart';
-import 'package:analysis_server/src/protocol_server.dart' show SourceChange;
 import 'package:analysis_server/src/services/correction/assist.dart';
 import 'package:analysis_server/src/services/correction/assist_internal.dart';
 import 'package:analysis_server/src/services/correction/change_workspace.dart';
@@ -90,7 +88,7 @@
       assist.change.message,
       CodeActionKind.Refactor,
       const [],
-      _createWorkspaceEdit(assist.change),
+      createWorkspaceEdit(server, assist.change),
       null,
     ));
   }
@@ -105,25 +103,11 @@
       fix.change.message,
       CodeActionKind.QuickFix,
       [diagnostic],
-      _createWorkspaceEdit(fix.change),
+      createWorkspaceEdit(server, fix.change),
       null,
     ));
   }
 
-  /// Note: This code will fetch the version of each document being modified so
-  /// it's important to call this immediately after computing edits to ensure
-  /// the document is not modified before the version number is read.
-  WorkspaceEdit _createWorkspaceEdit(SourceChange change) {
-    return toWorkspaceEdit(
-        server.clientCapabilities?.workspace,
-        change.edits
-            .map((e) => new FileEditInformation(
-                server.getVersionedDocumentIdentifier(e.file),
-                server.getLineInfo(e.file),
-                e.edits))
-            .toList());
-  }
-
   Future<List<Either2<Command, CodeAction>>> _getAssistActions(
     HashSet<CodeActionKind> clientSupportedCodeActionKinds,
     bool clientSupportsLiteralCodeActions,
diff --git a/pkg/analysis_server/lib/src/lsp/mapping.dart b/pkg/analysis_server/lib/src/lsp/mapping.dart
index ecf3bbd..d69d15d 100644
--- a/pkg/analysis_server/lib/src/lsp/mapping.dart
+++ b/pkg/analysis_server/lib/src/lsp/mapping.dart
@@ -1,9 +1,11 @@
 import 'dart:collection';
 
 import 'package:analysis_server/lsp_protocol/protocol_generated.dart' as lsp;
-import 'package:analysis_server/lsp_protocol/protocol_generated.dart';
+import 'package:analysis_server/lsp_protocol/protocol_generated.dart'
+    show ResponseError;
 import 'package:analysis_server/lsp_protocol/protocol_special.dart' as lsp;
-import 'package:analysis_server/lsp_protocol/protocol_special.dart';
+import 'package:analysis_server/lsp_protocol/protocol_special.dart'
+    show ErrorOr, Either2, Either4;
 import 'package:analysis_server/src/lsp/constants.dart' as lsp;
 import 'package:analysis_server/src/lsp/dartdoc.dart';
 import 'package:analysis_server/src/lsp/lsp_analysis_server.dart' as lsp;
@@ -30,6 +32,21 @@
           _asMarkup(preferredFormats, content));
 }
 
+/// Note: This code will fetch the version of each document being modified so
+/// it's important to call this immediately after computing edits to ensure
+/// the document is not modified before the version number is read.
+lsp.WorkspaceEdit createWorkspaceEdit(
+    lsp.LspAnalysisServer server, server.SourceChange change) {
+  return toWorkspaceEdit(
+      server.clientCapabilities?.workspace,
+      change.edits
+          .map((e) => new FileEditInformation(
+              server.getVersionedDocumentIdentifier(e.file),
+              server.getLineInfo(e.file),
+              e.edits))
+          .toList());
+}
+
 lsp.CompletionItemKind elementKindToCompletionItemKind(
   HashSet<lsp.CompletionItemKind> clientSupportedCompletionKinds,
   server.ElementKind kind,
@@ -212,7 +229,7 @@
     return null;
   }
 
-  return new Location(
+  return new lsp.Location(
     Uri.file(targetFilePath).toString(),
     toRange(lineInfo, target.offset, target.length),
   );
@@ -261,7 +278,7 @@
     return null;
   }
 
-  return new Location(
+  return new lsp.Location(
     Uri.file(result.location.file).toString(),
     toRange(lineInfo, location.offset, location.length),
   );
@@ -513,41 +530,41 @@
 }
 
 lsp.TextDocumentEdit toTextDocumentEdit(FileEditInformation edit) {
-  return new TextDocumentEdit(
+  return new lsp.TextDocumentEdit(
     edit.doc,
     edit.edits.map((e) => toTextEdit(edit.lineInfo, e)).toList(),
   );
 }
 
 lsp.TextEdit toTextEdit(server.LineInfo lineInfo, server.SourceEdit edit) {
-  return new TextEdit(
+  return new lsp.TextEdit(
     toRange(lineInfo, edit.offset, edit.length),
     edit.replacement,
   );
 }
 
 lsp.WorkspaceEdit toWorkspaceEdit(
-  WorkspaceClientCapabilities capabilities,
+  lsp.WorkspaceClientCapabilities capabilities,
   List<FileEditInformation> edits,
 ) {
   final clientSupportsTextDocumentEdits =
       capabilities?.workspaceEdit?.documentChanges == true;
   if (clientSupportsTextDocumentEdits) {
-    return new WorkspaceEdit(
+    return new lsp.WorkspaceEdit(
         null,
         Either2<
-            List<TextDocumentEdit>,
+            List<lsp.TextDocumentEdit>,
             List<
-                Either4<TextDocumentEdit, CreateFile, RenameFile,
-                    DeleteFile>>>.t1(
+                Either4<lsp.TextDocumentEdit, lsp.CreateFile, lsp.RenameFile,
+                    lsp.DeleteFile>>>.t1(
           edits.map(toTextDocumentEdit).toList(),
         ));
   } else {
-    return new WorkspaceEdit(toWorkspaceEditChanges(edits), null);
+    return new lsp.WorkspaceEdit(toWorkspaceEditChanges(edits), null);
   }
 }
 
-Map<String, List<TextEdit>> toWorkspaceEditChanges(
+Map<String, List<lsp.TextEdit>> toWorkspaceEditChanges(
     List<FileEditInformation> edits) {
   createEdit(FileEditInformation file) {
     final edits =
@@ -555,7 +572,7 @@
     return new MapEntry(file.doc.uri, edits);
   }
 
-  return Map<String, List<TextEdit>>.fromEntries(edits.map(createEdit));
+  return Map<String, List<lsp.TextEdit>>.fromEntries(edits.map(createEdit));
 }
 
 lsp.MarkupContent _asMarkup(
diff --git a/pkg/analysis_server/test/lsp/code_actions_fixes_test.dart b/pkg/analysis_server/test/lsp/code_actions_fixes_test.dart
index 2f824c3..4d692a1 100644
--- a/pkg/analysis_server/test/lsp/code_actions_fixes_test.dart
+++ b/pkg/analysis_server/test/lsp/code_actions_fixes_test.dart
@@ -50,7 +50,7 @@
 
     // Ensure applying the changes will give us the expected content.
     final contents = {
-      mainFilePath: content,
+      mainFilePath: withoutMarkers(content),
     };
     applyDocumentChanges(contents, fixAction.edit.documentChanges);
     expect(contents[mainFilePath], equals(expectedContent));
@@ -88,7 +88,7 @@
 
     // Ensure applying the changes will give us the expected content.
     final contents = {
-      mainFilePath: content,
+      mainFilePath: withoutMarkers(content),
     };
     applyChanges(contents, fixAction.edit.changes);
     expect(contents[mainFilePath], equals(expectedContent));
diff --git a/pkg/analysis_server/test/lsp/code_actions_source_test.dart b/pkg/analysis_server/test/lsp/code_actions_source_test.dart
index e064771..3ea0d20 100644
--- a/pkg/analysis_server/test/lsp/code_actions_source_test.dart
+++ b/pkg/analysis_server/test/lsp/code_actions_source_test.dart
@@ -72,7 +72,7 @@
 
     // Ensure applying the changes will give us the expected content.
     final contents = {
-      mainFilePath: content,
+      mainFilePath: withoutMarkers(content),
     };
     applyDocumentChanges(contents, editParams.edit.documentChanges);
     expect(contents[mainFilePath], equals(expectedContent));
@@ -129,7 +129,7 @@
 
     // Ensure applying the changes will give us the expected content.
     final contents = {
-      mainFilePath: content,
+      mainFilePath: withoutMarkers(content),
     };
     applyChanges(contents, editParams.edit.changes);
     expect(contents[mainFilePath], equals(expectedContent));
@@ -270,7 +270,7 @@
 
     // Ensure applying the changes will give us the expected content.
     final contents = {
-      mainFilePath: content,
+      mainFilePath: withoutMarkers(content),
     };
     applyDocumentChanges(contents, editParams.edit.documentChanges);
     expect(contents[mainFilePath], equals(expectedContent));
@@ -320,7 +320,7 @@
 
     // Ensure applying the changes will give us the expected content.
     final contents = {
-      mainFilePath: content,
+      mainFilePath: withoutMarkers(content),
     };
     applyChanges(contents, editParams.edit.changes);
     expect(contents[mainFilePath], equals(expectedContent));
diff --git a/pkg/analysis_server/test/lsp/server_abstract.dart b/pkg/analysis_server/test/lsp/server_abstract.dart
index d403a6c..b8f21a0 100644
--- a/pkg/analysis_server/test/lsp/server_abstract.dart
+++ b/pkg/analysis_server/test/lsp/server_abstract.dart
@@ -112,11 +112,29 @@
     // number of inserts followed by a single remove or replace edit. If multiple
     // inserts have the same position, the order in the array defines the order in
     // which the inserted strings appear in the resulting text.
-    if (changes.length > 1) {
-      // TODO(dantup): Implement multi-edit edits.
-      throw 'Test helper applyTextEdits does not support applying multiple edits';
-    } else if (changes.length == 1) {
-      newContent = applyTextEdit(newContent, changes.single);
+
+    /// Ensures changes are simple enough to apply easily without any complicated
+    /// logic.
+    void validateChangesCanBeApplied() {
+      bool intersectsWithOrComesAfter(Position pos, Position other) =>
+          pos.line > other.line ||
+          (pos.line == other.line || pos.character >= other.character);
+
+      Position earliestPositionChanged;
+      for (final change in changes) {
+        if (earliestPositionChanged != null &&
+            intersectsWithOrComesAfter(
+                change.range.end, earliestPositionChanged)) {
+          throw 'Test helper applyTextEdits does not support applying multiple edits '
+              'where the edits are not in reverse order.';
+        }
+        earliestPositionChanged = change.range.start;
+      }
+    }
+
+    validateChangesCanBeApplied();
+    for (final change in changes) {
+      newContent = applyTextEdit(newContent, change);
     }
 
     return newContent;
@@ -213,7 +231,7 @@
     }
   }
 
-  Future<List<TextEdit>> formatDocument(String fileUri) async {
+  Future<List<TextEdit>> formatDocument(String fileUri) {
     final request = makeRequest(
       Method.textDocument_formatting,
       new DocumentFormattingParams(
@@ -225,7 +243,7 @@
   }
 
   Future<List<TextEdit>> formatOnType(
-      String fileUri, Position pos, String character) async {
+      String fileUri, Position pos, String character) {
     final request = makeRequest(
       Method.textDocument_onTypeFormatting,
       new DocumentOnTypeFormattingParams(
@@ -242,7 +260,7 @@
     String fileUri, {
     Range range,
     List<CodeActionKind> kinds,
-  }) async {
+  }) {
     final request = makeRequest(
       Method.textDocument_codeAction,
       new CodeActionParams(
@@ -257,7 +275,7 @@
   }
 
   Future<List<CompletionItem>> getCompletion(Uri uri, Position pos,
-      {CompletionContext context}) async {
+      {CompletionContext context}) {
     final request = makeRequest(
       Method.textDocument_completion,
       new CompletionParams(
@@ -269,7 +287,7 @@
     return expectSuccessfulResponseTo<List<CompletionItem>>(request);
   }
 
-  Future<List<Location>> getDefinition(Uri uri, Position pos) async {
+  Future<List<Location>> getDefinition(Uri uri, Position pos) {
     final request = makeRequest(
       Method.textDocument_definition,
       new TextDocumentPositionParams(
@@ -280,8 +298,7 @@
     return expectSuccessfulResponseTo<List<Location>>(request);
   }
 
-  Future<List<DocumentHighlight>> getDocumentHighlights(
-      Uri uri, Position pos) async {
+  Future<List<DocumentHighlight>> getDocumentHighlights(Uri uri, Position pos) {
     final request = makeRequest(
       Method.textDocument_documentHighlight,
       new TextDocumentPositionParams(
@@ -293,7 +310,7 @@
   }
 
   Future<Either2<List<DocumentSymbol>, List<SymbolInformation>>>
-      getDocumentSymbols(String fileUri) async {
+      getDocumentSymbols(String fileUri) {
     final request = makeRequest(
       Method.textDocument_documentSymbol,
       new DocumentSymbolParams(
@@ -303,7 +320,7 @@
     return expectSuccessfulResponseTo(request);
   }
 
-  Future<Hover> getHover(Uri uri, Position pos) async {
+  Future<Hover> getHover(Uri uri, Position pos) {
     final request = makeRequest(
       Method.textDocument_hover,
       new TextDocumentPositionParams(
@@ -316,7 +333,7 @@
     Uri uri,
     Position pos, {
     includeDeclarations = false,
-  }) async {
+  }) {
     final request = makeRequest(
       Method.textDocument_references,
       new ReferenceParams(
@@ -328,7 +345,7 @@
     return expectSuccessfulResponseTo<List<Location>>(request);
   }
 
-  Future<SignatureHelp> getSignatureHelp(Uri uri, Position pos) async {
+  Future<SignatureHelp> getSignatureHelp(Uri uri, Position pos) {
     final request = makeRequest(
       Method.textDocument_signatureHelp,
       new TextDocumentPositionParams(
@@ -503,8 +520,8 @@
     return rangesFromMarkersImpl(content).toList();
   }
 
-  Future replaceFile(int newVersion, Uri uri, String content) async {
-    await changeFile(
+  Future replaceFile(int newVersion, Uri uri, String content) {
+    return changeFile(
       newVersion,
       uri,
       [new TextDocumentContentChangeEvent(null, null, content)],
@@ -513,7 +530,7 @@
 
   /// Sends [responseParams] to the server as a successful response to
   /// a server-initiated [request].
-  void respondTo<T>(RequestMessage request, T responseParams) async {
+  void respondTo<T>(RequestMessage request, T responseParams) {
     channel.sendResponseToServer(
         new ResponseMessage(request.id, responseParams, null, jsonRpcVersion));
   }