From e159bfee6405ebd1aa75d67ccdbf841c0224ffb4 Mon Sep 17 00:00:00 2001 From: Brian Wilkerson Date: Mon, 24 Jun 2024 16:04:53 +0000 Subject: [PATCH] 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 Commit-Queue: Brian Wilkerson --- .../completion/dart/completion_manager.dart | 82 ++++++++++++++++++- .../declaration/imported_reference_test.dart | 6 -- .../dart/declaration/library_prefix_test.dart | 2 - .../declaration/local_reference_test.dart | 6 -- .../dart/declaration/type_member_test.dart | 6 -- .../completion/dart/declaration/uri_test.dart | 4 - .../dart/location/class_declaration_test.dart | 12 --- .../location/function_declaration_test.dart | 2 - .../location/function_expression_test.dart | 6 -- .../dart/location/import_directive_test.dart | 8 -- .../location/method_declaration_test.dart | 2 - .../dart/location/switch_statement_test.dart | 2 - .../dart/location/type_test_test.dart | 2 - 13 files changed, 78 insertions(+), 62 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/completion/dart/completion_manager.dart b/pkg/analysis_server/lib/src/services/completion/dart/completion_manager.dart index 9519e7673ea..3e755735548 100644 --- 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, ''); + } +} diff --git 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 index 85493c62941..872648e487f 100644 --- 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 '''); diff --git 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 index 24bafab0249..aabce6119bb 100644 --- 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 '''); diff --git 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 index 77d5d110a70..1c7040fafb7 100644 --- 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 '''); diff --git 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 index 08a97273e22..27c3ffd9e7a 100644 --- 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 '''); diff --git a/pkg/analysis_server/test/services/completion/dart/declaration/uri_test.dart b/pkg/analysis_server/test/services/completion/dart/declaration/uri_test.dart index f44c1984798..78579f890a4 100644 --- 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 diff --git 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 index 89f4cf8327d..7761a0050e5 100644 --- 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 '''); } diff --git 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 index 22b9ccd27a2..fd868e346d7 100644 --- 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 '''); } diff --git 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 index 80dbb240259..3cd4b988644 100644 --- 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 '''); } diff --git 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 index a7f3d20c65f..6e9ef9c0dee 100644 --- 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 '''); } } diff --git 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 index d4a24101e09..ab8d28a61ba 100644 --- 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 '''); } diff --git 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 index 38659406360..13e44cc1a67 100644 --- 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 '''); } diff --git 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 index b22886226f6..468132092c9 100644 --- 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 ''');