Improve the computation of the completion prefix

There are a couple of tests where the new code fails to produce a prefix, but in those cases the prefixes are `""` and `{`, which are not useful for filtering (and wrong for replacing). The new code does, however, now find prefixes that the old code missed, which improves the filtering. The changes to the tests reflect the improved filtering. Change-Id: I3d48162ee56de1b94ecb3a88d11e8a2167fb8f2f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/372860 Reviewed-by: Keerti Parthasarathy <keertip@google.com> Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
2024-10-06 13:57:58 +00:00 · 2024-06-24 16:04:53 +00:00 · 2024-06-24 16:04:53 +00:00 · e159bfee64
parent 77feaada19
commit e159bfee64
13 changed files with 78 additions and 62 deletions
--- a/pkg/analysis_server/lib/src/services/completion/dart/completion_manager.dart
+++ b/pkg/analysis_server/lib/src/services/completion/dart/completion_manager.dart
@ -91,7 +91,6 @@ class DartCompletionManager {
    required DartCompletionRequest request,
    bool suggestOverrides = true,
    bool suggestUris = true,
-    required bool useFilter,
  }) async {
    request.checkAborted();

@ -101,7 +100,8 @@ class DartCompletionManager {
      if (selection == null) {
        throw AbortCompletion();
      }
-      var targetPrefix = request.targetPrefix;
+      var tokenData = TokenData.fromSelection(selection);
+      var targetPrefix = tokenData?.prefix ?? '';
      var matcher =
          targetPrefix.isEmpty ? NoPrefixMatcher() : FuzzyMatcher(targetPrefix);
      var state = CompletionState(request, selection, budget, matcher);
@ -164,8 +164,7 @@ class DartCompletionManager {
        performance: performance,
        request: request,
        suggestOverrides: enableOverrideContributor,
-        suggestUris: enableUriContributor,
-        useFilter: useFilter);
+        suggestUris: enableUriContributor);

    var builder =
        SuggestionBuilder(request, useFilter: useFilter, listener: listener);
@ -458,3 +457,78 @@ class NotImportedSuggestions {
  /// processed all available libraries, e.g. we ran out of budget.
  bool isIncomplete = false;
 }
+
+/// Information about the token containing the selection.
+class TokenData {
+  /// The token containing the offset.
+  ///
+  /// The token can be any token, including a comment token.
+  final Token token;
+
+  /// The prefix before the selection offset.
+  ///
+  /// This will be an empty string if the token isn't either an identifier or
+  /// keyword, or if the selection offset is at the beginning of the token.
+  final String prefix;
+
+  TokenData._(this.token, this.prefix);
+
+  /// Returns token data representing the token containing the offset of the
+  /// [selection], or `null` if the offset isn't within any token.
+  static TokenData? fromSelection(Selection selection) {
+    var coveringNode = selection.coveringNode;
+    var selectionOffset = selection.offset;
+    // Start at the last token in the covering node and walk backward in the
+    // token stream until we've found the left-most token whose offset is before
+    // the `selectionOffset`.
+    var currentToken = coveringNode.endToken;
+    while ((currentToken.isSynthetic ||
+            currentToken.offset > selectionOffset ||
+            (currentToken.offset == selectionOffset &&
+                !currentToken.isKeywordOrIdentifier)) &&
+        !currentToken.isEof) {
+      currentToken = currentToken.previous!;
+    }
+    if (currentToken.isEof) {
+      return null;
+    }
+    if (selectionOffset > currentToken.end) {
+      // The selection is between two tokens. Check to see whether it's inside a
+      // comment token.
+      Token? commentToken = currentToken.next!.precedingComments;
+      while (commentToken != null) {
+        if (selectionOffset >= commentToken.offset &&
+            selectionOffset <= commentToken.end) {
+          return TokenData._(commentToken, '');
+        }
+        commentToken = commentToken.next;
+      }
+      return null;
+    }
+    if (currentToken.isKeywordOrIdentifier) {
+      var offsetInToken = selectionOffset - currentToken.offset;
+      var prefix = currentToken.lexeme.substring(0, offsetInToken);
+      return TokenData._(currentToken, prefix);
+    } else if (currentToken.type == TokenType.STRING) {
+      // Compute a prefix inside string literals to support completion of URIs
+      // in directives.
+      var lexeme = currentToken.lexeme;
+      var startOfContent = 1;
+      if (lexeme.startsWith("r'''") || lexeme.startsWith('r"""')) {
+        startOfContent = 4;
+      } else if (lexeme.startsWith("r'") || lexeme.startsWith('r"')) {
+        startOfContent = 2;
+      } else if (lexeme.startsWith("'''") || lexeme.startsWith('"""')) {
+        startOfContent = 3;
+      }
+      var offsetInToken = selectionOffset - currentToken.offset;
+      if (offsetInToken < startOfContent) {
+        // The cursor is inside the opening quote sequence.
+        return TokenData._(currentToken, '');
+      }
+      var prefix = currentToken.lexeme.substring(startOfContent, offsetInToken);
+      return TokenData._(currentToken, prefix);
+    }
+    return TokenData._(currentToken, '');
+  }
+}
--- a/pkg/analysis_server/test/services/completion/dart/declaration/imported_reference_test.dart
+++ b/pkg/analysis_server/test/services/completion/dart/declaration/imported_reference_test.dart
@ -1821,12 +1821,6 @@ A0 S1;
 replacement
  left: 1
 suggestions
-  A0
-    kind: class
-  S2.B
-    kind: class
-  _B0
-    kind: class
  S2
    kind: library
 ''');
--- a/pkg/analysis_server/test/services/completion/dart/declaration/library_prefix_test.dart
+++ b/pkg/analysis_server/test/services/completion/dart/declaration/library_prefix_test.dart
@ -212,8 +212,6 @@ A Sew;
 replacement
  left: 1
 suggestions
-  S0.B
-    kind: class
  S0
    kind: library
 ''');
--- a/pkg/analysis_server/test/services/completion/dart/declaration/local_reference_test.dart
+++ b/pkg/analysis_server/test/services/completion/dart/declaration/local_reference_test.dart
@ -2139,12 +2139,6 @@ A0 S1;
 replacement
  left: 1
 suggestions
-  A0
-    kind: class
-  S2.B
-    kind: class
-  _B0
-    kind: class
  S2
    kind: library
 ''');
--- a/pkg/analysis_server/test/services/completion/dart/declaration/type_member_test.dart
+++ b/pkg/analysis_server/test/services/completion/dart/declaration/type_member_test.dart
@ -1447,12 +1447,6 @@ A0 S1;
 replacement
  left: 1
 suggestions
-  A0
-    kind: class
-  S2.B
-    kind: class
-  _B0
-    kind: class
  S2
    kind: library
 ''');
--- a/pkg/analysis_server/test/services/completion/dart/declaration/uri_test.dart
+++ b/pkg/analysis_server/test/services/completion/dart/declaration/uri_test.dart
@ -412,10 +412,6 @@ suggestions
    kind: import
  dart:typed_data
    kind: import
-  package:
-    kind: import
-  package:test/
-    kind: import
  package:test/test.dart
    kind: import
  dart:core
--- a/pkg/analysis_server/test/services/completion/dart/location/class_declaration_test.dart
+++ b/pkg/analysis_server/test/services/completion/dart/location/class_declaration_test.dart
@ -91,22 +91,12 @@ suggestions
    await computeSuggestions('''
 class A e^ implements foo { }
 ''');
-    // TODO(brianwilkerson): The keyword `with` should not be suggested when
-    //  using protocol 2 (so this these should require a conditional check).
-    //  The reason it is being suggested is as follows: The `e` is ignored by
-    //  the parser so it doesn't show up in the AST. As a result, the "entity"
-    //  is the implements clause, and the code doesn't find the unattached token
-    //  for `e`. As a result, there is no prefix, so the fuzzy matcher returns 1
-    //  (a perfect match). We need to improve the detection of a prefix in order
-    //  to fix this bug.
    assertResponse(r'''
 replacement
  left: 1
 suggestions
  extends
    kind: keyword
-  with
-    kind: keyword
 ''');
  }

@ -120,8 +110,6 @@ replacement
 suggestions
  extends
    kind: keyword
-  with
-    kind: keyword
 ''');
  }

--- a/pkg/analysis_server/test/services/completion/dart/location/function_declaration_test.dart
+++ b/pkg/analysis_server/test/services/completion/dart/location/function_declaration_test.dart
@ -90,8 +90,6 @@ suggestions
    kind: keyword
  async*
    kind: keyword
-  sync*
-    kind: keyword
 ''');
  }

--- a/pkg/analysis_server/test/services/completion/dart/location/function_expression_test.dart
+++ b/pkg/analysis_server/test/services/completion/dart/location/function_expression_test.dart
@ -93,8 +93,6 @@ suggestions
    kind: keyword
  async*
    kind: keyword
-  sync*
-    kind: keyword
 ''');
  }

@ -110,8 +108,6 @@ suggestions
    kind: keyword
  async*
    kind: keyword
-  sync*
-    kind: keyword
 ''');
  }

@ -127,8 +123,6 @@ suggestions
    kind: keyword
  async*
    kind: keyword
-  sync*
-    kind: keyword
 ''');
  }

--- a/pkg/analysis_server/test/services/completion/dart/location/import_directive_test.dart
+++ b/pkg/analysis_server/test/services/completion/dart/location/import_directive_test.dart
@ -245,14 +245,10 @@ import "foo" d^ hide foo;
 replacement
  left: 1
 suggestions
-  as
-    kind: keyword
  deferred as
    kind: keyword
  hide
    kind: keyword
-  show
-    kind: keyword
 ''');
  }

@ -323,14 +319,10 @@ import "foo" d^ show foo;
 replacement
  left: 1
 suggestions
-  as
-    kind: keyword
  deferred as
    kind: keyword
  hide
    kind: keyword
-  show
-    kind: keyword
 ''');
  }
 }
--- a/pkg/analysis_server/test/services/completion/dart/location/method_declaration_test.dart
+++ b/pkg/analysis_server/test/services/completion/dart/location/method_declaration_test.dart
@ -281,8 +281,6 @@ suggestions
    kind: keyword
  async*
    kind: keyword
-  sync*
-    kind: keyword
 ''');
  }

--- a/pkg/analysis_server/test/services/completion/dart/location/switch_statement_test.dart
+++ b/pkg/analysis_server/test/services/completion/dart/location/switch_statement_test.dart
@ -62,8 +62,6 @@ replacement
 suggestions
  case
    kind: keyword
-  default:
-    kind: keyword
 ''');
  }

--- a/pkg/analysis_server/test/services/completion/dart/location/type_test_test.dart
+++ b/pkg/analysis_server/test/services/completion/dart/location/type_test_test.dart
@ -64,8 +64,6 @@ void f() { if (v i^ && false) {} }
 replacement
  left: 1
 suggestions
-  case
-    kind: keyword
  is
    kind: keyword
 ''');