Reduce JSON for LSP completions by moving edits to resolve
See https://github.com/dart-lang/sdk/issues/37163.
Change-Id: I336245590560f7b30315a1dbda59ea56e5e9e043
Bug: https://github.com/dart-lang/sdk/issues/37163
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/104862
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Danny Tuppeny <dantup@google.com>
diff --git a/pkg/analysis_server/lib/lsp_protocol/protocol_custom_generated.dart b/pkg/analysis_server/lib/lsp_protocol/protocol_custom_generated.dart
index 8f4be64..deb8901 100644
--- a/pkg/analysis_server/lib/lsp_protocol/protocol_custom_generated.dart
+++ b/pkg/analysis_server/lib/lsp_protocol/protocol_custom_generated.dart
@@ -132,43 +132,56 @@
CompletionItemResolutionInfo.canParse,
CompletionItemResolutionInfo.fromJson);
- CompletionItemResolutionInfo(
- this.file, this.offset, this.libraryId, this.autoImportDisplayUri) {
+ CompletionItemResolutionInfo(this.file, this.offset, this.libId,
+ this.displayUri, this.rOffset, this.rLength) {
if (file == null) {
throw 'file is required but was not provided';
}
if (offset == null) {
throw 'offset is required but was not provided';
}
- if (libraryId == null) {
- throw 'libraryId is required but was not provided';
+ if (libId == null) {
+ throw 'libId is required but was not provided';
}
- if (autoImportDisplayUri == null) {
- throw 'autoImportDisplayUri is required but was not provided';
+ if (displayUri == null) {
+ throw 'displayUri is required but was not provided';
+ }
+ if (rOffset == null) {
+ throw 'rOffset is required but was not provided';
+ }
+ if (rLength == null) {
+ throw 'rLength is required but was not provided';
}
}
static CompletionItemResolutionInfo fromJson(Map<String, dynamic> json) {
final file = json['file'];
final offset = json['offset'];
- final libraryId = json['libraryId'];
- final autoImportDisplayUri = json['autoImportDisplayUri'];
+ final libId = json['libId'];
+ final displayUri = json['displayUri'];
+ final rOffset = json['rOffset'];
+ final rLength = json['rLength'];
return new CompletionItemResolutionInfo(
- file, offset, libraryId, autoImportDisplayUri);
+ file, offset, libId, displayUri, rOffset, rLength);
}
- final String autoImportDisplayUri;
+ final String displayUri;
final String file;
- final num libraryId;
+ final num libId;
final num offset;
+ final num rLength;
+ final num rOffset;
Map<String, dynamic> toJson() {
Map<String, dynamic> __result = {};
__result['file'] = file ?? (throw 'file is required but was not set');
__result['offset'] = offset ?? (throw 'offset is required but was not set');
- __result['libraryId'] =
- libraryId ?? (throw 'libraryId is required but was not set');
- __result['autoImportDisplayUri'] = autoImportDisplayUri ??
- (throw 'autoImportDisplayUri is required but was not set');
+ __result['libId'] = libId ?? (throw 'libId is required but was not set');
+ __result['displayUri'] =
+ displayUri ?? (throw 'displayUri is required but was not set');
+ __result['rOffset'] =
+ rOffset ?? (throw 'rOffset is required but was not set');
+ __result['rLength'] =
+ rLength ?? (throw 'rLength is required but was not set');
return __result;
}
@@ -178,10 +191,14 @@
obj['file'] is String &&
obj.containsKey('offset') &&
obj['offset'] is num &&
- obj.containsKey('libraryId') &&
- obj['libraryId'] is num &&
- obj.containsKey('autoImportDisplayUri') &&
- obj['autoImportDisplayUri'] is String;
+ obj.containsKey('libId') &&
+ obj['libId'] is num &&
+ obj.containsKey('displayUri') &&
+ obj['displayUri'] is String &&
+ obj.containsKey('rOffset') &&
+ obj['rOffset'] is num &&
+ obj.containsKey('rLength') &&
+ obj['rLength'] is num;
}
@override
@@ -189,8 +206,10 @@
if (other is CompletionItemResolutionInfo) {
return file == other.file &&
offset == other.offset &&
- libraryId == other.libraryId &&
- autoImportDisplayUri == other.autoImportDisplayUri &&
+ libId == other.libId &&
+ displayUri == other.displayUri &&
+ rOffset == other.rOffset &&
+ rLength == other.rLength &&
true;
}
return false;
@@ -201,8 +220,10 @@
int hash = 0;
hash = JenkinsSmiHash.combine(hash, file.hashCode);
hash = JenkinsSmiHash.combine(hash, offset.hashCode);
- hash = JenkinsSmiHash.combine(hash, libraryId.hashCode);
- hash = JenkinsSmiHash.combine(hash, autoImportDisplayUri.hashCode);
+ hash = JenkinsSmiHash.combine(hash, libId.hashCode);
+ hash = JenkinsSmiHash.combine(hash, displayUri.hashCode);
+ hash = JenkinsSmiHash.combine(hash, rOffset.hashCode);
+ hash = JenkinsSmiHash.combine(hash, rLength.hashCode);
return JenkinsSmiHash.finish(hash);
}
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion_resolve.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion_resolve.dart
index c519374..aa54dae 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion_resolve.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion_resolve.dart
@@ -51,12 +51,12 @@
// extracting (with support for the different types of responses between
// the servers). Where is an appropriate place to put it?
- var library = server.declarationsTracker.getLibrary(data.libraryId);
+ var library = server.declarationsTracker.getLibrary(data.libId);
if (library == null) {
return error(
ErrorCodes.InvalidParams,
- 'Library ID is not valid: ${data.libraryId}',
- data.libraryId.toString(),
+ 'Library ID is not valid: ${data.libId}',
+ data.libId.toString(),
);
}
@@ -157,8 +157,8 @@
return success(CompletionItem(
newLabel,
item.kind,
- data.autoImportDisplayUri != null
- ? "Auto import from '${data.autoImportDisplayUri}'\n\n${item.detail ?? ''}"
+ data.displayUri != null
+ ? "Auto import from '${data.displayUri}'\n\n${item.detail ?? ''}"
.trim()
: item.detail,
documentation,
@@ -168,7 +168,12 @@
item.filterText,
newLabel,
item.insertTextFormat,
- item.textEdit,
+ new TextEdit(
+ // TODO(dantup): If `clientSupportsSnippets == true` then we should map
+ // `selection` in to a snippet (see how Dart Code does this).
+ toRange(lineInfo, item.data.rOffset, item.data.rLength),
+ newLabel,
+ ),
thisFilesChanges
.expand((change) =>
change.edits.map((edit) => toTextEdit(lineInfo, edit)))
diff --git a/pkg/analysis_server/lib/src/lsp/mapping.dart b/pkg/analysis_server/lib/src/lsp/mapping.dart
index 28326d5..11bae89 100644
--- a/pkg/analysis_server/lib/src/lsp/mapping.dart
+++ b/pkg/analysis_server/lib/src/lsp/mapping.dart
@@ -183,22 +183,18 @@
null, // filterText uses label if not set
null, // insertText is deprecated, but also uses label if not set
null, // insertTextFormat (we always use plain text so can ommit this)
- new lsp.TextEdit(
- // TODO(dantup): If `clientSupportsSnippets == true` then we should map
- // `selection` in to a snippet (see how Dart Code does this).
- toRange(lineInfo, replacementOffset, replacementLength),
- label,
- ),
+ null, // textEdit - added on during resolve
null, // additionalTextEdits, used for adding imports, etc.
null, // commitCharacters
null, // command
// data, used for completionItem/resolve.
new lsp.CompletionItemResolutionInfo(
- file,
- offset,
- includedSuggestionSet.id,
- includedSuggestionSet.displayUri ?? library.uri?.toString(),
- ),
+ file,
+ offset,
+ includedSuggestionSet.id,
+ includedSuggestionSet.displayUri ?? library.uri?.toString(),
+ replacementOffset,
+ replacementLength),
);
}
diff --git a/pkg/analysis_server/test/lsp/completion_test.dart b/pkg/analysis_server/test/lsp/completion_test.dart
index c2721c0..4e7b84b 100644
--- a/pkg/analysis_server/test/lsp/completion_test.dart
+++ b/pkg/analysis_server/test/lsp/completion_test.dart
@@ -252,8 +252,9 @@
final completion = res.singleWhere((c) => c.label == 'InOtherFile');
expect(completion, isNotNull);
- // Expect no docs, since these are added during resolve.
+ // Expect no docs or text edit, since these are added during resolve.
expect(completion.documentation, isNull);
+ expect(completion.textEdit, isNull);
// Resolve the completion item (via server) to get its edits. This is the
// LSP's equiv of getSuggestionDetails() and is invoked by LSP clients to
@@ -272,6 +273,9 @@
isTrue,
);
+ // Ensure the edit was added on.
+ expect(resolved.textEdit, isNotNull);
+
// There should be no command for this item because it doesn't need imports
// in other files. Same-file completions are in additionalEdits.
expect(resolved.command, isNull);
diff --git a/pkg/analysis_server/tool/lsp_spec/generate_all.dart b/pkg/analysis_server/tool/lsp_spec/generate_all.dart
index 70d21840..08a111d 100644
--- a/pkg/analysis_server/tool/lsp_spec/generate_all.dart
+++ b/pkg/analysis_server/tool/lsp_spec/generate_all.dart
@@ -109,8 +109,10 @@
[
field('file', type: 'string'),
field('offset', type: 'number'),
- field('libraryId', type: 'number'),
- field('autoImportDisplayUri', type: 'string')
+ field('libId', type: 'number'),
+ field('displayUri', type: 'string'),
+ field('rOffset', type: 'number'),
+ field('rLength', type: 'number')
],
),
];
