[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 <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

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<CompletionParams, CompletionList>
     with LspPluginRequestHandlerMixin {
@@ -157,8 +158,7 @@ class CompletionHandler extends MessageHandler<CompletionParams, CompletionList>
       }
     }
 
-    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<CompletionParams, CompletionList>
         .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<CompletionParams, CompletionList>
       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<CompletionParams, CompletionList>
 
       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<CompletionParams, CompletionList>
           ),
         )
         .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<CompletionParams, CompletionList>
     return true; // Any other trigger character can be handled always.
   }
 
+  /// Truncates [items] to [maxItems] but additionally includes any items that
+  /// exactly match [prefix].
+  Iterable<CompletionItem> _truncateResults(
+    List<CompletionItem> 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<CompletionItem> 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<CompletionItem> unrankedItems, {
+    required bool isIncomplete,
+  }) : this(
+          unrankedItems: unrankedItems,
+          targetPrefix: '',
+          isIncomplete: isIncomplete,
+        );
 }
 
 /// Helper to simplify fuzzy filtering.

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<void> 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)