[analysis_server] Support adding multiple imports for @OVERRIDES completions

Related to Dart-Code/Dart-Code#4116.

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

Author: DanTup

pkg/analysis_server/lib/lsp_protocol/protocol_custom_generated.dart

@@ -211,9 +211,8 @@ class CompletionItemResolutionInfo implements ToJsonable {
   );
 
   static CompletionItemResolutionInfo fromJson(Map<String, Object?> json) {
-    if (DartNotImportedCompletionResolutionInfo.canParse(
-        json, nullLspJsonReporter)) {
-      return DartNotImportedCompletionResolutionInfo.fromJson(json);
+    if (DartCompletionResolutionInfo.canParse(json, nullLspJsonReporter)) {
+      return DartCompletionResolutionInfo.fromJson(json);
     }
     if (PubPackageCompletionItemResolutionInfo.canParse(
         json, nullLspJsonReporter)) {
@@ -247,79 +246,30 @@ class CompletionItemResolutionInfo implements ToJsonable {
   String toString() => jsonEncoder.convert(toJson());
 }
 
-class DartDiagnosticServer implements ToJsonable {
-  static const jsonHandler = LspJsonHandler(
-    DartDiagnosticServer.canParse,
-    DartDiagnosticServer.fromJson,
-  );
-
-  DartDiagnosticServer({
-    required this.port,
-  });
-  static DartDiagnosticServer fromJson(Map<String, Object?> json) {
-    final portJson = json['port'];
-    final port = portJson as int;
-    return DartDiagnosticServer(
-      port: port,
-    );
-  }
-
-  final int port;
-
-  @override
-  Map<String, Object?> toJson() {
-    var result = <String, Object?>{};
-    result['port'] = port;
-    return result;
-  }
-
-  static bool canParse(Object? obj, LspJsonReporter reporter) {
-    if (obj is Map<String, Object?>) {
-      return _canParseInt(obj, reporter, 'port',
-          allowsUndefined: false, allowsNull: false);
-    } else {
-      reporter.reportError('must be of type DartDiagnosticServer');
-      return false;
-    }
-  }
-
-  @override
-  bool operator ==(Object other) {
-    return other is DartDiagnosticServer &&
-        other.runtimeType == DartDiagnosticServer &&
-        port == other.port;
-  }
-
-  @override
-  int get hashCode => port.hashCode;
-
-  @override
-  String toString() => jsonEncoder.convert(toJson());
-}
-
-class DartNotImportedCompletionResolutionInfo
+class DartCompletionResolutionInfo
     implements CompletionItemResolutionInfo, ToJsonable {
   static const jsonHandler = LspJsonHandler(
-    DartNotImportedCompletionResolutionInfo.canParse,
-    DartNotImportedCompletionResolutionInfo.fromJson,
+    DartCompletionResolutionInfo.canParse,
+    DartCompletionResolutionInfo.fromJson,
   );
 
-  DartNotImportedCompletionResolutionInfo({
+  DartCompletionResolutionInfo({
     required this.file,
-    required this.libraryUri,
+    required this.importUris,
     this.ref,
   });
-  static DartNotImportedCompletionResolutionInfo fromJson(
-      Map<String, Object?> json) {
+  static DartCompletionResolutionInfo fromJson(Map<String, Object?> json) {
     final fileJson = json['file'];
     final file = fileJson as String;
-    final libraryUriJson = json['libraryUri'];
-    final libraryUri = libraryUriJson as String;
+    final importUrisJson = json['importUris'];
+    final importUris = (importUrisJson as List<Object?>)
+        .map((item) => item as String)
+        .toList();
     final refJson = json['ref'];
     final ref = refJson as String?;
-    return DartNotImportedCompletionResolutionInfo(
+    return DartCompletionResolutionInfo(
       file: file,
-      libraryUri: libraryUri,
+      importUris: importUris,
       ref: ref,
     );
   }
@@ -329,8 +279,8 @@ class DartNotImportedCompletionResolutionInfo
   /// This is used to compute where to add the import.
   final String file;
 
-  /// The URI to be imported if this completion is selected.
-  final String libraryUri;
+  /// The URIs to be imported if this completion is selected.
+  final List<String> importUris;
 
   /// The ElementLocation of the item being completed.
   ///
@@ -341,7 +291,7 @@ class DartNotImportedCompletionResolutionInfo
   Map<String, Object?> toJson() {
     var result = <String, Object?>{};
     result['file'] = file;
-    result['libraryUri'] = libraryUri;
+    result['importUris'] = importUris;
     if (ref != null) {
       result['ref'] = ref;
     }
@@ -354,39 +304,89 @@ class DartNotImportedCompletionResolutionInfo
           allowsUndefined: false, allowsNull: false)) {
         return false;
       }
-      if (!_canParseString(obj, reporter, 'libraryUri',
+      if (!_canParseListString(obj, reporter, 'importUris',
           allowsUndefined: false, allowsNull: false)) {
         return false;
       }
       return _canParseString(obj, reporter, 'ref',
           allowsUndefined: true, allowsNull: false);
     } else {
-      reporter.reportError(
-          'must be of type DartNotImportedCompletionResolutionInfo');
+      reporter.reportError('must be of type DartCompletionResolutionInfo');
       return false;
     }
   }
 
   @override
   bool operator ==(Object other) {
-    return other is DartNotImportedCompletionResolutionInfo &&
-        other.runtimeType == DartNotImportedCompletionResolutionInfo &&
+    return other is DartCompletionResolutionInfo &&
+        other.runtimeType == DartCompletionResolutionInfo &&
         file == other.file &&
-        libraryUri == other.libraryUri &&
+        listEqual(
+            importUris, other.importUris, (String a, String b) => a == b) &&
         ref == other.ref;
   }
 
   @override
   int get hashCode => Object.hash(
         file,
-        libraryUri,
+        lspHashCode(importUris),
         ref,
       );
 
   @override
   String toString() => jsonEncoder.convert(toJson());
 }
 
+class DartDiagnosticServer implements ToJsonable {
+  static const jsonHandler = LspJsonHandler(
+    DartDiagnosticServer.canParse,
+    DartDiagnosticServer.fromJson,
+  );
+
+  DartDiagnosticServer({
+    required this.port,
+  });
+  static DartDiagnosticServer fromJson(Map<String, Object?> json) {
+    final portJson = json['port'];
+    final port = portJson as int;
+    return DartDiagnosticServer(
+      port: port,
+    );
+  }
+
+  final int port;
+
+  @override
+  Map<String, Object?> toJson() {
+    var result = <String, Object?>{};
+    result['port'] = port;
+    return result;
+  }
+
+  static bool canParse(Object? obj, LspJsonReporter reporter) {
+    if (obj is Map<String, Object?>) {
+      return _canParseInt(obj, reporter, 'port',
+          allowsUndefined: false, allowsNull: false);
+    } else {
+      reporter.reportError('must be of type DartDiagnosticServer');
+      return false;
+    }
+  }
+
+  @override
+  bool operator ==(Object other) {
+    return other is DartDiagnosticServer &&
+        other.runtimeType == DartDiagnosticServer &&
+        port == other.port;
+  }
+
+  @override
+  int get hashCode => port.hashCode;
+
+  @override
+  String toString() => jsonEncoder.convert(toJson());
+}
+
 class Element implements ToJsonable {
   static const jsonHandler = LspJsonHandler(
     Element.canParse,
@@ -2396,6 +2396,32 @@ bool _canParseListOutline(
   return true;
 }
 
+bool _canParseListString(
+    Map<String, Object?> map, LspJsonReporter reporter, String fieldName,
+    {required bool allowsUndefined, required bool allowsNull}) {
+  reporter.push(fieldName);
+  try {
+    if (!allowsUndefined && !map.containsKey(fieldName)) {
+      reporter.reportError('must not be undefined');
+      return false;
+    }
+    final value = map[fieldName];
+    final nullCheck = allowsNull || allowsUndefined;
+    if (!nullCheck && value == null) {
+      reporter.reportError('must not be null');
+      return false;
+    }
+    if ((!nullCheck || value != null) &&
+        (value is! List<Object?> || value.any((item) => item is! String))) {
+      reporter.reportError('must be of type List<String>');
+      return false;
+    }
+  } finally {
+    reporter.pop();
+  }
+  return true;
+}
+
 bool _canParseLiteral(
     Map<String, Object?> map, LspJsonReporter reporter, String fieldName,
     {required bool allowsUndefined,

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

@@ -451,23 +451,21 @@ class CompletionHandler extends MessageHandler<CompletionParams, CompletionList>
         final insertionRange =
             toRange(unit.lineInfo, itemReplacementOffset, itemInsertLength);
 
-        // For not-imported items, we need to include the file+uri to be able
-        // to compute the import-inserting edits in the `completionItem/resolve`
-        // call later.
+        // For items that need imports, we'll round-trip some additional info
+        // to allow their additional edits (and documentation) to be handled
+        // lazily to reduce the payload.
         CompletionItemResolutionInfo? resolutionInfo;
-        final libraryUri = item.libraryUri;
-
-        if (useNotImportedCompletions &&
-            libraryUri != null &&
-            (item.isNotImported ?? false)) {
-          final dartElement =
-              item is DartCompletionSuggestion ? item.dartElement : null;
-
-          resolutionInfo = DartNotImportedCompletionResolutionInfo(
-            file: unit.path,
-            libraryUri: libraryUri,
-            ref: dartElement?.location?.encoding,
-          );
+        if (item is DartCompletionSuggestion) {
+          final dartElement = item.dartElement;
+          final importUris = item.requiredImports;
+
+          if (importUris.isNotEmpty) {
+            resolutionInfo = DartCompletionResolutionInfo(
+              file: unit.path,
+              importUris: importUris.map((uri) => uri.toString()).toList(),
+              ref: dartElement?.location?.encoding,
+            );
+          }
         }
 
         return toCompletionItem(

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

@@ -38,18 +38,18 @@ class CompletionResolveHandler
   ) async {
     final resolutionInfo = params.data;
 
-    if (resolutionInfo is DartNotImportedCompletionResolutionInfo) {
-      return resolveDartNotImportedCompletion(params, resolutionInfo, token);
+    if (resolutionInfo is DartCompletionResolutionInfo) {
+      return resolveDartCompletion(params, resolutionInfo, token);
     } else if (resolutionInfo is PubPackageCompletionItemResolutionInfo) {
       return resolvePubPackageCompletion(params, resolutionInfo, token);
     } else {
       return success(params);
     }
   }
 
-  Future<ErrorOr<CompletionItem>> resolveDartNotImportedCompletion(
+  Future<ErrorOr<CompletionItem>> resolveDartCompletion(
     CompletionItem item,
-    DartNotImportedCompletionResolutionInfo data,
+    DartCompletionResolutionInfo data,
     CancellationToken token,
   ) async {
     final clientCapabilities = server.clientCapabilities;
@@ -60,7 +60,7 @@ class CompletionResolveHandler
     }
 
     final file = data.file;
-    final libraryUri = Uri.parse(data.libraryUri);
+    final importUris = data.importUris.map(Uri.parse).toList();
     final elementLocationReference = data.ref;
     final elementLocation = elementLocationReference != null
         ? ElementLocationImpl.con2(elementLocationReference)
@@ -91,7 +91,9 @@ class CompletionResolveHandler
 
         final builder = ChangeBuilder(session: session);
         await builder.addDartFileEdit(file, (builder) {
-          builder.importLibraryElement(libraryUri);
+          for (final uri in importUris) {
+            builder.importLibraryElement(uri);
+          }
         });
 
         if (token.isCancellationRequested) {
@@ -136,26 +138,35 @@ class CompletionResolveHandler
               : null;
         }
 
-        // If the only URI we have is a file:// URI, display it as relative to
-        // the file we're importing into, rather than the full URI.
-        final pathContext = server.resourceProvider.pathContext;
-        final autoImportDisplayUri = libraryUri.isScheme('file')
-            // Compute the relative path and then put into a URI so the display
-            // always uses forward slashes (as a URI) regardless of platform.
-            ? Uri.file(pathContext.relative(
-                libraryUri.toFilePath(),
-                from: pathContext.dirname(file),
-              ))
-            : libraryUri;
+        String? detail = item.detail;
+        if (changes.edits.isNotEmpty && importUris.isNotEmpty) {
+          if (importUris.length == 1) {
+            // If the only URI we have is a file:// URI, display it as relative to
+            // the file we're importing into, rather than the full URI.
+            final pathContext = server.resourceProvider.pathContext;
+            final libraryUri = importUris.first;
+            final autoImportDisplayUri = libraryUri.isScheme('file')
+                // Compute the relative path and then put into a URI so the display
+                // always uses forward slashes (as a URI) regardless of platform.
+                ? Uri.file(pathContext.relative(
+                    libraryUri.toFilePath(),
+                    from: pathContext.dirname(file),
+                  ))
+                : libraryUri;
+
+            detail =
+                "Auto import from '$autoImportDisplayUri'\n\n${item.detail ?? ''}"
+                    .trim();
+          } else {
+            detail = "Auto import required URIs\n\n${item.detail ?? ''}".trim();
+          }
+        }
 
         return success(CompletionItem(
           label: item.label,
           kind: item.kind,
           tags: item.tags,
-          detail: changes.edits.isNotEmpty
-              ? "Auto import from '$autoImportDisplayUri'\n\n${item.detail ?? ''}"
-                  .trim()
-              : item.detail,
+          detail: detail,
           documentation: documentation,
           deprecated: item.deprecated,
           preselect: item.preselect,