From 7d6216940f5d300bf782cdaab6156f3b4b8af1c4 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Thu, 5 May 2022 15:29:25 +0000 Subject: [PATCH] [analysis_server] Always include exact matches in completion when truncating Change-Id: I21b56392961217482dbcf5c67b9594709b27bcc0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/243721 Commit-Queue: Brian Wilkerson Reviewed-by: Brian Wilkerson --- .../src/lsp/handlers/handler_completion.dart | 72 +++++++++++++++---- .../test/lsp/completion_dart_test.dart | 52 ++++++++++++-- 2 files changed, 107 insertions(+), 17 deletions(-) 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 5c1996dbb1b..c4b32d4e0ac 100644 --- a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart +++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart @@ -32,6 +32,7 @@ import 'package:analyzer/src/util/performance/operation_performance.dart'; import 'package:analyzer_plugin/protocol/protocol_common.dart'; import 'package:analyzer_plugin/protocol/protocol_generated.dart' as plugin; import 'package:analyzer_plugin/src/utilities/completion/completion_target.dart'; +import 'package:collection/collection.dart'; class CompletionHandler extends MessageHandler with LspPluginRequestHandlerMixin { @@ -157,8 +158,7 @@ class CompletionHandler extends MessageHandler } } - serverResultsFuture ??= - Future.value(success(_CompletionResults(isIncomplete: false))); + serverResultsFuture ??= Future.value(success(_CompletionResults.empty())); final pluginResultsFuture = _getPluginResults( clientCapabilities, lineInfo.result, path.result, offset); @@ -174,12 +174,15 @@ class CompletionHandler extends MessageHandler .toList(); final unrankedItems = serverResults.result.unrankedItems; - // Truncate ranked items to allow room for all unranked items. + // Truncate ranked items allowing 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 truncatedRankedItems = untruncatedRankedItems.length <= maxRankedItems + ? untruncatedRankedItems + : _truncateResults( + untruncatedRankedItems, + serverResults.result.targetPrefix, + maxRankedItems, + ); final truncatedItems = truncatedRankedItems.followedBy(unrankedItems).toList(); @@ -308,11 +311,12 @@ class CompletionHandler extends MessageHandler completionPreference: CompletionPreference.replace, ); final target = completionRequest.target; - final fuzzy = _FuzzyFilterHelper(completionRequest.targetPrefix); + final targetPrefix = completionRequest.targetPrefix; + final fuzzy = _FuzzyFilterHelper(targetPrefix); if (triggerCharacter != null) { if (!_triggerCharacterValid(offset, triggerCharacter, target)) { - return success(_CompletionResults(isIncomplete: false)); + return success(_CompletionResults.empty()); } } @@ -556,12 +560,13 @@ class CompletionHandler extends MessageHandler return success(_CompletionResults( isIncomplete: false, + targetPrefix: targetPrefix, rankedItems: rankedResults, unrankedItems: unrankedResults)); } on AbortCompletion { - return success(_CompletionResults(isIncomplete: false)); + return success(_CompletionResults.empty()); } on InconsistentAnalysisException { - return success(_CompletionResults(isIncomplete: false)); + return success(_CompletionResults.empty()); } } @@ -614,8 +619,9 @@ class CompletionHandler extends MessageHandler ), ) .toList(); - return success(_CompletionResults( - isIncomplete: false, unrankedItems: completionItems)); + return success( + _CompletionResults.unranked(completionItems, isIncomplete: false), + ); } /// Returns true if [node] is part of an invocation and already has an argument @@ -708,6 +714,31 @@ class CompletionHandler extends MessageHandler return true; // Any other trigger character can be handled always. } + /// Truncates [items] to [maxItems] but additionally includes any items that + /// exactly match [prefix]. + Iterable _truncateResults( + List items, + String prefix, + int maxItems, + ) { + // Take the top `maxRankedItem` plus any exact matches. + final prefixLower = prefix.toLowerCase(); + bool isExactMatch(CompletionItem item) => + (item.filterText ?? item.label).toLowerCase() == prefixLower; + + // Sort the items by relevance using sortText. + items.sort(sortTextComparer); + + // Skip the text comparisons if we don't have a prefix (plugin results, or + // just no prefix when completion was invoked). + final shouldInclude = prefixLower.isEmpty + ? (int index, CompletionItem item) => index < maxItems + : (int index, CompletionItem item) => + index < maxItems || isExactMatch(item); + + return items.whereIndexed(shouldInclude); + } + /// Compares [CompletionItem]s by the `sortText` field, which is derived from /// relevance. /// @@ -750,13 +781,28 @@ class _CompletionResults { /// Items that cannot be ranked, and should avoid being truncated. final List unrankedItems; + /// Any prefixed used to filter the results. + final String targetPrefix; + final bool isIncomplete; _CompletionResults({ this.rankedItems = const [], this.unrankedItems = const [], + required this.targetPrefix, required this.isIncomplete, }); + + _CompletionResults.empty() : this(targetPrefix: '', isIncomplete: false); + + _CompletionResults.unranked( + List unrankedItems, { + required bool isIncomplete, + }) : this( + unrankedItems: unrankedItems, + targetPrefix: '', + isIncomplete: isIncomplete, + ); } /// Helper to simplify fuzzy filtering. diff --git a/pkg/analysis_server/test/lsp/completion_dart_test.dart b/pkg/analysis_server/test/lsp/completion_dart_test.dart index e7b5d8734ce..8c28b615f2f 100644 --- a/pkg/analysis_server/test/lsp/completion_dart_test.dart +++ b/pkg/analysis_server/test/lsp/completion_dart_test.dart @@ -1117,6 +1117,51 @@ void f() { expect(res.items.map((item) => item.label).contains('aaa'), isTrue); } + /// Exact matches should always be included when completion lists are + /// truncated, even if they ranked poorly. + Future test_maxCompletionItems_doesNotExcludeExactMatches() async { + final content = ''' +import 'a.dart'; +void f() { + var a = Item^ +} + '''; + + // Create classes `Item1` to `Item20` along with a field named `item`. + // The classes will rank higher in the position above and push + // the field out without an exception to include exact matches. + newFile( + join(projectFolderPath, 'lib', 'a.dart'), + [ + 'String item = "";', + for (var i = 1; i <= 20; i++) 'class Item$i {}', + ].join('\n'), + ); + + final initialAnalysis = waitForAnalysisComplete(); + await provideConfig( + () => initialize( + workspaceCapabilities: withApplyEditSupport( + withConfigurationSupport(emptyWorkspaceClientCapabilities))), + {'maxCompletionItems': 10}, + ); + await openFile(mainFileUri, withoutMarkers(content)); + await initialAnalysis; + final res = + await getCompletionList(mainFileUri, positionFromMarker(content)); + + // We expect 11 items, because the exact match was not in the top 10 and + // was included additionally. + expect(res.items, hasLength(11)); + expect(res.isIncomplete, isTrue); + + // Ensure the 'Item' field is included. + expect( + res.items.map((item) => item.label), + contains('item'), + ); + } + /// 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. @@ -1128,11 +1173,11 @@ void f() { } '''; - // Create a class with fields for1 to for20 in the other file. + // Create fields for1 to for20 in the other file. newFile( join(projectFolderPath, 'lib', 'a.dart'), [ - for (var i = 1; i <= 20; i++) ' String for$i = ' ';', + for (var i = 1; i <= 20; i++) 'String for$i = ' ';', ].join('\n'), ); @@ -1154,8 +1199,7 @@ void f() { expect(res.items, hasLength(10)); expect(res.isIncomplete, isTrue); - // Also ensure the 'for' snippet is included. - + // Ensure the 'for' snippet is included. expect( res.items .where((item) => item.kind == CompletionItemKind.Snippet)