Issue 61676. Fix sorting by relevance in SuggestionCollector.

Bug: #61676
Change-Id: I713a664f4399bee9baa52a1f69b02483acc9f344
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/453220
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>

Author: scheglov

pkg/analysis_server/lib/src/services/completion/dart/completion_manager.dart

@@ -175,7 +175,9 @@ class DartCompletionManager {
     }
     // Compute relevance, sort and truncate list.
     isTruncated = collector.suggestions.length > maxSuggestions;
-    collector.finalize(RelevanceComputer(request, listener));
+    collector.finalize(
+      RelevanceComputer(request, listener, targetPrefix: request.targetPrefix),
+    );
     return collector;
   }
 

pkg/analysis_server/lib/src/services/completion/dart/relevance_computer.dart

@@ -40,11 +40,17 @@ class RelevanceComputer {
   /// computed. In the latter case, [_hasContainingMemberName] will be `false`.
   String? _cachedContainingMemberName;
 
+  /// The already typed string at the completion location, maybe empty.
+  /// See [DartCompletionRequest.targetPrefix].
+  /// It is used to inflate the relevance of perfect matches to `1.0`.
+  final String targetPrefix;
+  late final String targetPrefixLower = targetPrefix.toLowerCase();
+
   /// A textual representation of the location at which completion was
   /// requested.
   String? completionLocation;
 
-  RelevanceComputer(this.request, this.listener)
+  RelevanceComputer(this.request, this.listener, {required this.targetPrefix})
     : featureComputer = request.featureComputer;
 
   /// Return the name of the member containing the completion location, or
@@ -101,6 +107,16 @@ class RelevanceComputer {
 
   /// Compute the relevance for the given [CandidateSuggestion].
   int computeRelevance(CandidateSuggestion suggestion) {
+    // See https://github.com/dart-lang/sdk/issues/61679
+    // We have two checks to distinguish prefix `str` or `STR` when
+    // candidates are also `str` or `STR`. We want to prefer the same case.
+    if (_isExactPrefixMatch(suggestion)) {
+      return maximumRelevance;
+    }
+    if (_isExactPrefixMatchToLower(suggestion)) {
+      return maximumRelevance - 1;
+    }
+
     var neverType = request.libraryElement.typeProvider.neverType;
     switch (suggestion) {
       case TypedSuggestionCompletionMixin():
@@ -746,4 +762,14 @@ class RelevanceComputer {
       nullabilitySuffix: NullabilitySuffix.none,
     );
   }
+
+  bool _isExactPrefixMatch(CandidateSuggestion suggestion) {
+    return targetPrefixLower.isNotEmpty &&
+        suggestion.completion == targetPrefix;
+  }
+
+  bool _isExactPrefixMatchToLower(CandidateSuggestion suggestion) {
+    return targetPrefixLower.isNotEmpty &&
+        suggestion.completion.toLowerCase() == targetPrefixLower;
+  }
 }

pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart

@@ -106,7 +106,11 @@ class SuggestionBuilder {
   /// Initialize a newly created suggestion builder to build suggestions for the
   /// given [request].
   SuggestionBuilder(this.request, {this.listener, required this.useFilter})
-    : relevanceComputer = RelevanceComputer(request, listener) {
+    : relevanceComputer = RelevanceComputer(
+        request,
+        listener,
+        targetPrefix: request.targetPrefix,
+      ) {
     targetPrefixLower = request.targetPrefix.toLowerCase();
   }
 

pkg/analysis_server/lib/src/services/completion/dart/suggestion_collector.dart

@@ -99,7 +99,7 @@ class SuggestionCollector {
     // score.
     suggestions.sort((first, second) {
       if (first.matcherScore == second.matcherScore) {
-        return first.relevanceScore.compareTo(second.relevanceScore);
+        return second.relevanceScore.compareTo(first.relevanceScore);
       }
       return second.matcherScore.compareTo(first.matcherScore);
     });

pkg/analysis_server/test/domain_completion_test.dart

@@ -1257,6 +1257,49 @@ suggestions
 ''');
   }
 
+  Future<void> test_numResults_includesLocalVariable() async {
+    await _configureWithWorkspaceRoot();
+
+    // Create many imported declarations to exceed the maxResults cap.
+    newFile(
+      '$testPackageLibPath/a.dart',
+      [
+        for (var i = 1; i <= 50; i++) 'class C$i {}',
+        for (var i = 1; i <= 50; i++) 'void f$i() {}',
+        for (var i = 1; i <= 50; i++) 'var v$i = $i;',
+      ].join('\n'),
+    );
+
+    // No prefix, no filtering.
+    // Local variables 'foo0*' must be included.
+    var response = await _getTestCodeSuggestions('''
+import 'a.dart';
+void f() {
+  var foo01 = 0;
+  var foo02 = 0;
+  ^
+}
+''', maxResults: 10);
+
+    // Only include local variables to keep the result stable.
+    printerConfiguration.filter = (suggestion) {
+      return suggestion.completion.startsWith('foo0');
+    };
+
+    // Note the variables order: the closer, the higher.
+    assertResponseText(response, r'''
+suggestions
+  foo02
+    kind: localVariable
+    isNotImported: null
+    libraryUri: null
+  foo01
+    kind: localVariable
+    isNotImported: null
+    libraryUri: null
+''');
+  }
+
   Future<void> test_numResults_topLevelVariables() async {
     await _configureWithWorkspaceRoot();
 
@@ -2025,16 +2068,16 @@ void f() {
 replacement
   left: 4
 suggestions
-  foo01
+  foo02
     kind: topLevelVariable
     isNotImported: null
     libraryUri: null
-    relevance: 510
-  foo02
+    relevance: 558
+  foo01
     kind: topLevelVariable
     isNotImported: null
     libraryUri: null
-    relevance: 558
+    relevance: 510
 ''');
   }
 

pkg/analysis_server/test/lsp/completion_dart_test.dart

@@ -1378,7 +1378,7 @@ void g() {
     await verifyCompletions(
       mainFileUri,
       content,
-      expectCompletions: ['(a, b) {}', '(a, b) =>'],
+      expectCompletions: ['(a, b) =>', '(a, b) {}'],
       applyEditsFor: '(a, b) =>',
       expectedContent: expectedContent,
     );
@@ -1406,7 +1406,7 @@ void g() {
       content,
       // Display text does not contain 'required' because it makes the
       // completion much longer, we just include it in the completion text.
-      expectCompletions: ['({a, b}) {}', '({a, b}) =>'],
+      expectCompletions: ['({a, b}) =>', '({a, b}) {}'],
       applyEditsFor: '({a, b}) =>',
       expectedContent: expectedContent,
     );

pkg/analysis_server/test/services/completion/dart/completion_test.dart

@@ -2558,12 +2558,12 @@ void f() {
 replacement
   left: 4
 suggestions
+  true
+    kind: keyword
   falsetrue
     kind: topLevelVariable
   truefalse
     kind: topLevelVariable
-  true
-    kind: keyword
 ''');
   }
 
@@ -4726,12 +4726,12 @@ class D {f(){} g(){f^(f);}}
 replacement
   left: 1
 suggestions
+  f
+    kind: methodInvocation
   if
     kind: keyword
   final
     kind: keyword
-  f
-    kind: methodInvocation
   for
     kind: keyword
   false
@@ -5176,6 +5176,8 @@ class Bar<T extends Foo> {const Bar(T k);T^ m(T a, T b){}final T f = null;}
 replacement
   left: 1
 suggestions
+  T
+    kind: typeParameter
   static
     kind: keyword
   const
@@ -5184,8 +5186,6 @@ suggestions
     kind: keyword
   factory
     kind: keyword
-  T
-    kind: typeParameter
   covariant
     kind: keyword
   get
@@ -8495,10 +8495,10 @@ void f(p) {
 replacement
   left: 3
 suggestions
-  str
-    kind: localVariable
   STR
     kind: topLevelVariable
+  str
+    kind: localVariable
 ''');
   }
 
@@ -8517,12 +8517,12 @@ void f(p) {
 replacement
   left: 3
 suggestions
+  STR
+    kind: topLevelVariable
   str
     kind: localVariable
   String
     kind: class
-  STR
-    kind: topLevelVariable
   String.fromCharCode
     kind: constructorInvocation
   String.fromCharCodes