[analysis_server] Don't truncate unranked items from LSP code completion
Change-Id: I12ce64185c5008b48e40bac82a000c687220dc7d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/243327
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/lsp/client_configuration.dart b/pkg/analysis_server/lib/src/lsp/client_configuration.dart
index a5ef8f7..d31fd8a 100644
--- a/pkg/analysis_server/lib/src/lsp/client_configuration.dart
+++ b/pkg/analysis_server/lib/src/lsp/client_configuration.dart
@@ -195,10 +195,13 @@
int? get lineLength =>
_settings['lineLength'] as int? ?? _fallback?.lineLength;
- /// Maximum number of CompletionItems per completion request.
+ /// Requested maximum number of CompletionItems per completion request.
///
- /// If more than this are available, the list is truncated and isIncomplete
- /// is set to true.
+ /// If more than this are available, ranked items in the list will be
+ /// truncated and `isIncomplete` is set to `true`.
+ ///
+ /// Unranked items are never truncated so it's still possible that more than
+ /// this number of items will be returned.
int get maxCompletionItems =>
_settings['maxCompletionItems'] as int? ??
_fallback?.maxCompletionItems ??
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart
index 8f93e61..97c569e 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart
@@ -99,7 +99,7 @@
}
final offset = offsetResult.result;
- Future<ErrorOr<CompletionList>>? serverResultsFuture;
+ Future<ErrorOr<_CompletionResults>>? serverResultsFuture;
final pathContext = server.resourceProvider.pathContext;
final fileExtension = pathContext.extension(path.result);
@@ -158,28 +158,31 @@
}
serverResultsFuture ??=
- Future.value(success(CompletionList(isIncomplete: false, items: [])));
+ Future.value(success(_CompletionResults(isIncomplete: false)));
final pluginResultsFuture = _getPluginResults(
clientCapabilities, lineInfo.result, path.result, offset);
- // Await both server + plugin results together to allow async/IO to
- // overlap.
- final serverAndPluginResults =
- await Future.wait([serverResultsFuture, pluginResultsFuture]);
- final serverResults = serverAndPluginResults[0];
- final pluginResults = serverAndPluginResults[1];
+ final serverResults = await serverResultsFuture;
+ final pluginResults = await pluginResultsFuture;
- if (serverResults.isError) return serverResults;
- if (pluginResults.isError) return pluginResults;
+ if (serverResults.isError) return failure(serverResults);
+ if (pluginResults.isError) return failure(pluginResults);
- final untruncatedItems = serverResults.result.items
+ final untruncatedRankedItems = serverResults.result.rankedItems
.followedBy(pluginResults.result.items)
.toList();
+ final unrankedItems = serverResults.result.unrankedItems;
- final truncatedItems = untruncatedItems.length > maxResults
- ? (untruncatedItems..sort(sortTextComparer)).sublist(0, maxResults)
- : untruncatedItems;
+ // Truncate ranked items to allow room for all unranked items.
+ final maxRankedItems = math.max(maxResults - unrankedItems.length, 0);
+ final truncatedRankedItems = untruncatedRankedItems.length > maxRankedItems
+ ? (untruncatedRankedItems..sort(sortTextComparer))
+ .sublist(0, maxRankedItems)
+ : untruncatedRankedItems;
+
+ final truncatedItems =
+ truncatedRankedItems.followedBy(unrankedItems).toList();
// If we're tracing performance (only Dart), record the number of results
// after truncation.
@@ -190,7 +193,7 @@
// marked as such.
isIncomplete: serverResults.result.isIncomplete ||
pluginResults.result.isIncomplete ||
- truncatedItems.length != untruncatedItems.length,
+ truncatedRankedItems.length != untruncatedRankedItems.length,
items: truncatedItems,
));
}
@@ -288,7 +291,7 @@
));
}
- Future<ErrorOr<CompletionList>> _getServerDartItems(
+ Future<ErrorOr<_CompletionResults>> _getServerDartItems(
LspClientCapabilities capabilities,
ResolvedUnitResult unit,
CompletionPerformance completionPerformance,
@@ -310,7 +313,7 @@
if (triggerCharacter != null) {
if (!_triggerCharacterValid(offset, triggerCharacter, target)) {
- return success(CompletionList(isIncomplete: false, items: []));
+ return success(_CompletionResults(isIncomplete: false));
}
}
@@ -364,7 +367,7 @@
? false
: server.clientConfiguration.global.completeFunctionCalls;
- final results = performance.run('mapSuggestions', (performance) {
+ final rankedResults = performance.run('mapSuggestions', (performance) {
return serverSuggestions.map(
(item) {
var itemReplacementOffset =
@@ -510,7 +513,7 @@
.clientConfiguration.global.previewCommitCharacters,
completeFunctionCalls: completeFunctionCalls,
));
- results.addAll(setResults);
+ rankedResults.addAll(setResults);
}
});
}
@@ -522,44 +525,60 @@
// the root, so skip snippets entirely if not.
final isEditableFile =
unit.session.analysisContext.contextRoot.isAnalyzed(unit.path);
+ List<CompletionItem> unrankedResults;
if (capabilities.completionSnippets &&
snippetsEnabled &&
isEditableFile) {
- await performance.runAsync('addSnippets', (performance) async {
- results.addAll(await _getDartSnippetItems(
+ unrankedResults =
+ await performance.runAsync('getSnippets', (performance) async {
+ // `await` required for `performance.runAsync` to count time.
+ return await _getDartSnippetItems(
clientCapabilities: capabilities,
unit: unit,
offset: offset,
lineInfo: unit.lineInfo,
- ));
+ );
});
+ } else {
+ unrankedResults = [];
}
// Perform fuzzy matching based on the identifier in front of the caret to
// reduce the size of the payload.
- final matchingResults = performance.run('fuzzyFilter', (performance) {
- final fuzzyPattern = completionRequest.targetPrefix;
- final fuzzyMatcher =
- FuzzyMatcher(fuzzyPattern, matchStyle: MatchStyle.TEXT);
+ final fuzzyPattern = completionRequest.targetPrefix;
+ final fuzzyMatcher =
+ FuzzyMatcher(fuzzyPattern, matchStyle: MatchStyle.TEXT);
- return results
+ final matchingRankedResults =
+ performance.run('fuzzyFilterRanked', (performance) {
+ return rankedResults
.where((e) => fuzzyMatcher.score(e.filterText ?? e.label) > 0)
.toList();
});
- // Transmitted count will be set after combining with plugins.
- completionPerformance.computedSuggestionCount = matchingResults.length;
+ final matchingUnrankedResults =
+ performance.run('fuzzyFilterRanked', (performance) {
+ return unrankedResults
+ .where((e) => fuzzyMatcher.score(e.filterText ?? e.label) > 0)
+ .toList();
+ });
- return success(
- CompletionList(isIncomplete: false, items: matchingResults));
+ // transmittedCount will be set after combining with plugins + truncation.
+ completionPerformance.computedSuggestionCount =
+ matchingRankedResults.length + matchingUnrankedResults.length;
+
+ return success(_CompletionResults(
+ isIncomplete: false,
+ rankedItems: matchingRankedResults,
+ unrankedItems: matchingUnrankedResults));
} on AbortCompletion {
- return success(CompletionList(isIncomplete: false, items: []));
+ return success(_CompletionResults(isIncomplete: false));
} on InconsistentAnalysisException {
- return success(CompletionList(isIncomplete: false, items: []));
+ return success(_CompletionResults(isIncomplete: false));
}
}
- Future<ErrorOr<CompletionList>> _getServerYamlItems(
+ Future<ErrorOr<_CompletionResults>> _getServerYamlItems(
YamlCompletionGenerator generator,
LspClientCapabilities capabilities,
String path,
@@ -600,7 +619,8 @@
),
)
.toList();
- return success(CompletionList(isIncomplete: false, items: completionItems));
+ return success(_CompletionResults(
+ isIncomplete: false, unrankedItems: completionItems));
}
/// Returns true if [node] is part of an invocation and already has an argument
@@ -726,3 +746,20 @@
return item1Text.compareTo(item2Text);
}
}
+
+/// A set of completion items split into ranked and unranked items.
+class _CompletionResults {
+ /// Items that can be ranked using their relevance/sortText.
+ final List<CompletionItem> rankedItems;
+
+ /// Items that cannot be ranked, and should avoid being truncated.
+ final List<CompletionItem> unrankedItems;
+
+ final bool isIncomplete;
+
+ _CompletionResults({
+ this.rankedItems = const [],
+ this.unrankedItems = const [],
+ required this.isIncomplete,
+ });
+}
diff --git a/pkg/analysis_server/test/lsp/completion_dart_test.dart b/pkg/analysis_server/test/lsp/completion_dart_test.dart
index 3a55f69..9b2f453 100644
--- a/pkg/analysis_server/test/lsp/completion_dart_test.dart
+++ b/pkg/analysis_server/test/lsp/completion_dart_test.dart
@@ -1117,6 +1117,54 @@
expect(res.items.map((item) => item.label).contains('aaa'), isTrue);
}
+ /// Snippet completions should be kept when maxCompletionItems truncates
+ /// because they are not ranked like other completions and might be
+ /// truncated when they are exactly what the user wants.
+ Future<void> test_maxCompletionItems_doesNotExcludeSnippets() async {
+ final content = '''
+import 'a.dart';
+void f() {
+ fo^
+}
+ ''';
+
+ // Create a class with fields for1 to for20 in the other file.
+ newFile(
+ join(projectFolderPath, 'lib', 'a.dart'),
+ [
+ for (var i = 1; i <= 20; i++) ' String for$i = ' ';',
+ ].join('\n'),
+ );
+
+ final initialAnalysis = waitForAnalysisComplete();
+ await provideConfig(
+ () => initialize(
+ textDocumentCapabilities: withCompletionItemSnippetSupport(
+ emptyTextDocumentClientCapabilities),
+ workspaceCapabilities: withApplyEditSupport(
+ withConfigurationSupport(emptyWorkspaceClientCapabilities))),
+ {'maxCompletionItems': 10},
+ );
+ await openFile(mainFileUri, withoutMarkers(content));
+ await initialAnalysis;
+ final res =
+ await getCompletionList(mainFileUri, positionFromMarker(content));
+
+ // Should be capped at 10 and marked as incomplete.
+ expect(res.items, hasLength(10));
+ expect(res.isIncomplete, isTrue);
+
+ // Also ensure the 'for' snippet is included.
+
+ expect(
+ res.items
+ .where((item) => item.kind == CompletionItemKind.Snippet)
+ .map((item) => item.label)
+ .contains('for'),
+ isTrue,
+ );
+ }
+
Future<void> test_namedArg_flutterChildren() async {
final content = '''
import 'package:flutter/widgets.dart';
