Skip to content

Commit

Permalink
Reduce LSP completion JSON by removing optional fields set to defaults
Browse files Browse the repository at this point in the history
This addresses some of the things mentioned in #37163 but doesn't entirely solve the issue (for example it doesn't touch docs yet).

Change-Id: Ib0a094695905120ac5e222dae52165b9a5f9a825
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/104860
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
  • Loading branch information
DanTup authored and commit-bot@chromium.org committed Jun 5, 2019
1 parent 6031ca4 commit 91b3281
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 16 deletions.
32 changes: 18 additions & 14 deletions pkg/analysis_server/lib/src/lsp/mapping.dart
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,16 @@ lsp.CompletionItem declarationToCompletionItem(
declaration.relevanceTags
.forEach((t) => itemRelevance += (tagBoosts[t] ?? 0));

// Because we potentially send thousands of these items, we should minimise
// the generated JSON as much as possible - for example using nulls in place
// of empty lists/false where possible.
return new lsp.CompletionItem(
label,
completionKind,
getDeclarationCompletionDetail(declaration, completionKind, useDeprecated),
asStringOrMarkupContent(formats, cleanDartdoc(declaration.docComplete)),
useDeprecated ? declaration.isDeprecated : null,
false, // preselect
useDeprecated && declaration.isDeprecated ? true : null,
null, // preselect
// Relevance is a number, highest being best. LSP does text sort so subtract
// from a large number so that a text sort will result in the correct order.
// 555 -> 999455
Expand All @@ -180,16 +183,15 @@ lsp.CompletionItem declarationToCompletionItem(
(1000000 - itemRelevance).toString(),
null, // filterText uses label if not set
null, // insertText is deprecated, but also uses label if not set
// We don't have completions that use snippets, so we always return PlainText.
lsp.InsertTextFormat.PlainText,
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,
),
[], // additionalTextEdits, used for adding imports, etc.
[], // commitCharacters
null, // additionalTextEdits, used for adding imports, etc.
null, // commitCharacters
null, // command
// data, used for completionItem/resolve.
new lsp.CompletionItemResolutionInfo(
Expand Down Expand Up @@ -375,7 +377,7 @@ String getCompletionDetail(
} else if (hasParameterType) {
return '$prefix${suggestion.parameterType}';
} else {
return prefix;
return prefix.isNotEmpty ? prefix : null;
}
}

Expand Down Expand Up @@ -414,7 +416,7 @@ String getDeclarationCompletionDetail(
} else if (hasReturnType) {
return '$prefix${declaration.returnType}';
} else {
return prefix;
return prefix.isNotEmpty ? prefix : null;
}
}

Expand Down Expand Up @@ -555,13 +557,16 @@ lsp.CompletionItem toCompletionItem(
: suggestionKindToCompletionItemKind(
supportedCompletionItemKinds, suggestion.kind, label);

// Because we potentially send thousands of these items, we should minimise
// the generated JSON as much as possible - for example using nulls in place
// of empty lists/false where possible.
return new lsp.CompletionItem(
label,
completionKind,
getCompletionDetail(suggestion, completionKind, useDeprecated),
asStringOrMarkupContent(formats, cleanDartdoc(suggestion.docComplete)),
useDeprecated ? suggestion.isDeprecated : null,
false, // preselect
useDeprecated && suggestion.isDeprecated ? true : null,
null, // preselect
// Relevance is a number, highest being best. LSP does text sort so subtract
// from a large number so that a text sort will result in the correct order.
// 555 -> 999455
Expand All @@ -570,16 +575,15 @@ lsp.CompletionItem toCompletionItem(
(1000000 - suggestion.relevance).toString(),
null, // filterText uses label if not set
null, // insertText is deprecated, but also uses label if not set
// We don't have completions that use snippets, so we always return PlainText.
lsp.InsertTextFormat.PlainText,
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),
suggestion.completion,
),
[], // additionalTextEdits, used for adding imports, etc.
[], // commitCharacters
null, // additionalTextEdits, used for adding imports, etc.
null, // commitCharacters
null, // command
null, // data, useful for if using lazy resolve, this comes back to us
);
Expand Down
6 changes: 4 additions & 2 deletions pkg/analysis_server/test/lsp/completion_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ class CompletionTest extends AbstractLspAnalysisServerTest {
final res = await getCompletion(mainFileUri, positionFromMarker(content));
expect(res.any((c) => c.label == 'abcdefghij'), isTrue);
final item = res.singleWhere((c) => c.label == 'abcdefghij');
expect(item.insertTextFormat, equals(InsertTextFormat.PlainText));
expect(item.insertTextFormat,
anyOf(equals(InsertTextFormat.PlainText), isNull));
// ignore: deprecated_member_use_from_same_package
expect(item.insertText, anyOf(equals('abcdefghij'), isNull));
final updated = applyTextEdits(withoutMarkers(content), [item.textEdit]);
Expand Down Expand Up @@ -445,7 +446,8 @@ main() {
final res = await getCompletion(mainFileUri, positionFromMarker(content));
expect(res.any((c) => c.label == 'abcdefghij'), isTrue);
final item = res.singleWhere((c) => c.label == 'abcdefghij');
expect(item.insertTextFormat, equals(InsertTextFormat.PlainText));
expect(item.insertTextFormat,
anyOf(equals(InsertTextFormat.PlainText), isNull));
// ignore: deprecated_member_use_from_same_package
expect(item.insertText, anyOf(equals('abcdefghij'), isNull));
final updated = applyTextEdits(withoutMarkers(content), [item.textEdit]);
Expand Down

0 comments on commit 91b3281

Please sign in to comment.