Remove docs from LSP (suggestion set) completions and provide during …

…resolve Change-Id: I1e3c99e3076e829822b418ffaf19dcb891e93f81 Bug: #37163 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/104861 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
dart-lang · Jun 5, 2019 · 2f0c990 · 2f0c990
1 parent 91b3281
commit 2f0c990
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 4 deletions.
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion_resolve.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion_resolve.dart
@@ -12,6 +12,7 @@ import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
 import 'package:analysis_server/src/lsp/mapping.dart';
 import 'package:analyzer/dart/analysis/session.dart';
 import 'package:analyzer/dart/element/element.dart' as analyzer;
+import 'package:analyzer/src/util/comment.dart' as analyzer;
 import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart';
 
 class CompletionResolveHandler
@@ -146,14 +147,21 @@ class CompletionResolveHandler
               'Add import', Commands.sendWorkspaceEdit, [workspaceEdit]);
         }
 
+        // Documentation is added on during resolve for LSP.
+        final formats = server.clientCapabilities?.textDocument?.completion
+            ?.completionItem?.documentationFormat;
+        final dartDoc =
+            analyzer.getDartDocPlainText(requestedElement.documentationComment);
+        final documentation = asStringOrMarkupContent(formats, dartDoc);
+
         return success(CompletionItem(
           newLabel,
           item.kind,
           data.autoImportDisplayUri != null
               ? "Auto import from '${data.autoImportDisplayUri}'\n\n${item.detail ?? ''}"
                   .trim()
               : item.detail,
-          item.documentation,
+          documentation,
           item.deprecated,
           item.preselect,
           item.sortText,

diff --git a/pkg/analysis_server/lib/src/lsp/mapping.dart b/pkg/analysis_server/lib/src/lsp/mapping.dart
@@ -155,7 +155,6 @@ lsp.CompletionItem declarationToCompletionItem(
 
   final useDeprecated =
       completionCapabilities?.completionItem?.deprecatedSupport == true;
-  final formats = completionCapabilities?.completionItem?.documentationFormat;
 
   final completionKind = declarationKindToCompletionItemKind(
       supportedCompletionItemKinds, declaration.kind);
@@ -172,7 +171,7 @@ lsp.CompletionItem declarationToCompletionItem(
     label,
     completionKind,
     getDeclarationCompletionDetail(declaration, completionKind, useDeprecated),
-    asStringOrMarkupContent(formats, cleanDartdoc(declaration.docComplete)),
+    null, // documentation - will be added during resolve.
     useDeprecated && declaration.isDeprecated ? true : null,
     null, // preselect
     // Relevance is a number, highest being best. LSP does text sort so subtract

diff --git a/pkg/analysis_server/test/lsp/completion_test.dart b/pkg/analysis_server/test/lsp/completion_test.dart
@@ -228,7 +228,10 @@ class CompletionTest extends AbstractLspAnalysisServerTest {
   test_suggestionSets() async {
     newFile(
       join(projectFolderPath, 'other_file.dart'),
-      content: 'class InOtherFile {}',
+      content: '''
+      /// This class is in another file.
+      class InOtherFile {}
+      ''',
     );
 
     final content = '''
@@ -249,6 +252,9 @@ main() {
     final completion = res.singleWhere((c) => c.label == 'InOtherFile');
     expect(completion, isNotNull);
 
+    // Expect no docs, since these are added during resolve.
+    expect(completion.documentation, isNull);
+
     // Resolve the completion item (via server) to get its edits. This is the
     // LSP's equiv of getSuggestionDetails() and is invoked by LSP clients to
     // populate additional info (in our case, the additional edits for inserting
@@ -260,6 +266,12 @@ main() {
     expect(
         resolved.detail, startsWith("Auto import from '../other_file.dart'"));
 
+    // Ensure the doc comment was added.
+    expect(
+      resolved.documentation.valueEquals('This class is in another file.'),
+      isTrue,
+    );
+
     // There should be no command for this item because it doesn't need imports
     // in other files. Same-file completions are in additionalEdits.
     expect(resolved.command, isNull);