Ensure LSP completion filterText doesn't include '()' or '(…)'

DanTup · commit-bot@chromium.org · commit 31cd2bd75ac0 · 2019-08-07T15:25:40.000Z
Change-Id: I01c380ee3a36dbdd15706cb55b108899cacd9fc5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112249 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Commit-Queue: Danny Tuppeny <dantup@google.com>
diff --git a/pkg/analysis_server/lib/src/lsp/mapping.dart b/pkg/analysis_server/lib/src/lsp/mapping.dart
@@ -153,10 +153,12 @@ lsp.CompletionItem declarationToCompletionItem(
   int replacementOffset,
   int replacementLength,
 ) {
-  // Build display labels and text to insert. insertText may differ from label
-  // if the label includes things like (…).
+  // Build display labels and text to insert. insertText and filterText may
+  // differ from label (for ex. if the label includes things like (…)). If
+  // either are missing then label will be used by the client.
   String label;
   String insertText;
+  String filterText;
   switch (declaration.kind) {
     case DeclarationKind.ENUM_CONSTANT:
       label = '${declaration.parent.name}.${declaration.name}';
@@ -167,11 +169,13 @@ lsp.CompletionItem declarationToCompletionItem(
         label += '.${declaration.name}';
       }
       insertText = label;
+      filterText = label;
       label += declaration.parameterNames?.isNotEmpty ?? false ? '(…)' : '()';
       break;
     case DeclarationKind.FUNCTION:
       label = declaration.name;
       insertText = label;
+      filterText = label;
       label += declaration.parameterNames?.isNotEmpty ?? false ? '(…)' : '()';
       break;
     default:
@@ -206,8 +210,8 @@ lsp.CompletionItem declarationToCompletionItem(
     //  10 -> 999990
     //   1 -> 999999
     (1000000 - itemRelevance).toString(),
-    null, // filterText uses label if not set
-    insertText, // insertText uses label if not set
+    filterText != label ? filterText : null, // filterText uses label if not set
+    insertText != label ? insertText : null, // insertText uses label if not set
     null, // insertTextFormat (we always use plain text so can ommit this)
     null, // textEdit - added on during resolve
     null, // additionalTextEdits, used for adding imports, etc.
@@ -564,11 +568,12 @@ lsp.CompletionItem toCompletionItem(
   int replacementOffset,
   int replacementLength,
 ) {
-  // Build display labels and text to insert. insertText may differ from label
-  // if the label includes things like (…). If insertText is left as null then
-  // label is used.
+  // Build display labels and text to insert. insertText and filterText may
+  // differ from label (for ex. if the label includes things like (…)). If
+  // either are missing then label will be used by the client.
   String label;
   String insertText;
+  String filterText;
   if (suggestion.displayText != null) {
     label = suggestion.displayText;
     insertText = suggestion.completion;
@@ -580,6 +585,7 @@ lsp.CompletionItem toCompletionItem(
         label = suggestion.completion;
         // Label is the insert text plus the parens to indicate it's callable.
         insertText = label;
+        filterText = label;
         label += suggestion.parameterNames?.isNotEmpty ?? false ? '(…)' : '()';
         break;
       default:
@@ -613,7 +619,7 @@ lsp.CompletionItem toCompletionItem(
     //  10 -> 999990
     //   1 -> 999999
     (1000000 - suggestion.relevance).toString(),
-    null, // filterText uses label if not set
+    filterText != label ? filterText : null, // filterText uses label if not set
     insertText != label ? insertText : null, // insertText uses label if not set
     null, // insertTextFormat (we always use plain text so can ommit this)
     new lsp.TextEdit(
diff --git a/pkg/analysis_server/test/lsp/completion_test.dart b/pkg/analysis_server/test/lsp/completion_test.dart
@@ -229,12 +229,29 @@ class CompletionTest extends AbstractLspAnalysisServerTest {
     final item = res.singleWhere((c) => c.label == 'abcdefghij');
     expect(item.insertTextFormat,
         anyOf(equals(InsertTextFormat.PlainText), isNull));
-    // ignore: deprecated_member_use_from_same_package
     expect(item.insertText, anyOf(equals('abcdefghij'), isNull));
     final updated = applyTextEdits(withoutMarkers(content), [item.textEdit]);
     expect(updated, contains('a.abcdefghij'));
   }
 
+  test_parensNotInFilterTextInsertText() async {
+    final content = '''
+    class MyClass {}
+
+    main() {
+      MyClass a = new MyCla^
+    }
+    ''';
+
+    await initialize();
+    await openFile(mainFileUri, withoutMarkers(content));
+    final res = await getCompletion(mainFileUri, positionFromMarker(content));
+    expect(res.any((c) => c.label == 'MyClass()'), isTrue);
+    final item = res.singleWhere((c) => c.label == 'MyClass()');
+    expect(item.filterText, equals('MyClass'));
+    expect(item.insertText, equals('MyClass'));
+  }
+
   test_suggestionSets() async {
     newFile(
       join(projectFolderPath, 'other_file.dart'),
@@ -768,7 +785,6 @@ main() {
     final item = res.singleWhere((c) => c.label == 'abcdefghij');
     expect(item.insertTextFormat,
         anyOf(equals(InsertTextFormat.PlainText), isNull));
-    // ignore: deprecated_member_use_from_same_package
     expect(item.insertText, anyOf(equals('abcdefghij'), isNull));
     final updated = applyTextEdits(withoutMarkers(content), [item.textEdit]);
     expect(updated, contains('a.abcdefghij'));
