[analysis_server] Use ElementLocation to properly resolve documentation in LSP completions
Fixes https://github.com/Dart-Code/Dart-Code/issues/3787.
Change-Id: Iffd83fbd96f2ff8dc136c2a3f5ec7824e7451ca2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/269960
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/lsp_protocol/protocol_custom_generated.dart b/pkg/analysis_server/lib/lsp_protocol/protocol_custom_generated.dart
index 78f1364..6d690e4c 100644
--- a/pkg/analysis_server/lib/lsp_protocol/protocol_custom_generated.dart
+++ b/pkg/analysis_server/lib/lsp_protocol/protocol_custom_generated.dart
@@ -307,6 +307,7 @@
DartNotImportedCompletionResolutionInfo({
required this.file,
required this.libraryUri,
+ this.ref,
});
static DartNotImportedCompletionResolutionInfo fromJson(
Map<String, Object?> json) {
@@ -314,9 +315,12 @@
final file = fileJson as String;
final libraryUriJson = json['libraryUri'];
final libraryUri = libraryUriJson as String;
+ final refJson = json['ref'];
+ final ref = refJson as String?;
return DartNotImportedCompletionResolutionInfo(
file: file,
libraryUri: libraryUri,
+ ref: ref,
);
}
@@ -328,11 +332,19 @@
/// The URI to be imported if this completion is selected.
final String libraryUri;
+ /// The ElementLocation of the item being completed.
+ ///
+ /// This is used to provide documentation in the resolved response.
+ final String? ref;
+
@override
Map<String, Object?> toJson() {
var result = <String, Object?>{};
result['file'] = file;
result['libraryUri'] = libraryUri;
+ if (ref != null) {
+ result['ref'] = ref;
+ }
return result;
}
@@ -342,8 +354,12 @@
allowsUndefined: false, allowsNull: false)) {
return false;
}
- return _canParseString(obj, reporter, 'libraryUri',
- allowsUndefined: false, allowsNull: false);
+ if (!_canParseString(obj, reporter, 'libraryUri',
+ allowsUndefined: false, allowsNull: false)) {
+ return false;
+ }
+ return _canParseString(obj, reporter, 'ref',
+ allowsUndefined: true, allowsNull: false);
} else {
reporter.reportError(
'must be of type DartNotImportedCompletionResolutionInfo');
@@ -356,13 +372,15 @@
return other is DartNotImportedCompletionResolutionInfo &&
other.runtimeType == DartNotImportedCompletionResolutionInfo &&
file == other.file &&
- libraryUri == other.libraryUri;
+ libraryUri == other.libraryUri &&
+ ref == other.ref;
}
@override
int get hashCode => Object.hash(
file,
libraryUri,
+ ref,
);
@override
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart
index 3cd41fb..0e7a233 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart
@@ -13,6 +13,7 @@
import 'package:analysis_server/src/provisional/completion/completion_core.dart';
import 'package:analysis_server/src/services/completion/completion_performance.dart';
import 'package:analysis_server/src/services/completion/dart/completion_manager.dart';
+import 'package:analysis_server/src/services/completion/dart/dart_completion_suggestion.dart';
import 'package:analysis_server/src/services/completion/filtering/fuzzy_matcher.dart';
import 'package:analysis_server/src/services/completion/yaml/analysis_options_generator.dart';
import 'package:analysis_server/src/services/completion/yaml/fix_data_generator.dart';
@@ -455,12 +456,17 @@
// call later.
CompletionItemResolutionInfo? resolutionInfo;
final libraryUri = item.libraryUri;
+
if (useNotImportedCompletions &&
libraryUri != null &&
(item.isNotImported ?? false)) {
+ final dartElement =
+ item is DartCompletionSuggestion ? item.dartElement : null;
+
resolutionInfo = DartNotImportedCompletionResolutionInfo(
file: unit.path,
libraryUri: libraryUri,
+ ref: dartElement?.location?.encoding,
);
}
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
index fa3d892..f33ce80 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion_resolve.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion_resolve.dart
@@ -4,13 +4,13 @@
import 'package:analysis_server/lsp_protocol/protocol.dart' hide Element;
import 'package:analysis_server/src/computer/computer_hover.dart';
-import 'package:analysis_server/src/lsp/client_capabilities.dart';
import 'package:analysis_server/src/lsp/constants.dart';
import 'package:analysis_server/src/lsp/handlers/handlers.dart';
import 'package:analysis_server/src/lsp/mapping.dart';
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/analysis/session.dart';
-import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/src/dart/element/element.dart';
+import 'package:analyzer/src/utilities/extensions/analysis_session.dart';
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
class CompletionResolveHandler
@@ -47,13 +47,25 @@
}
}
- Future<ErrorOr<CompletionItem>> resolveDartCompletion(
+ Future<ErrorOr<CompletionItem>> resolveDartNotImportedCompletion(
CompletionItem item,
- LspClientCapabilities clientCapabilities,
- CancellationToken token, {
- required String file,
- required Uri libraryUri,
- }) async {
+ DartNotImportedCompletionResolutionInfo data,
+ CancellationToken token,
+ ) async {
+ final clientCapabilities = server.clientCapabilities;
+ if (clientCapabilities == null) {
+ // This should not happen unless a client misbehaves.
+ return error(ErrorCodes.ServerNotInitialized,
+ 'Requests not before server is initilized');
+ }
+
+ final file = data.file;
+ final libraryUri = Uri.parse(data.libraryUri);
+ final elementLocationReference = data.ref;
+ final elementLocation = elementLocationReference != null
+ ? ElementLocationImpl.con2(elementLocationReference)
+ : null;
+
const timeout = Duration(milliseconds: 1000);
var timer = Stopwatch()..start();
_latestCompletionItem = item;
@@ -108,7 +120,9 @@
// Look up documentation if we can get an element for this item.
Either2<MarkupContent, String>? documentation;
- final element = await _getElement(session, libraryUri, item);
+ final element = elementLocation != null
+ ? await session.locateElement(elementLocation)
+ : null;
if (element != null) {
final formats = clientCapabilities.completionDocumentationFormats;
final dartDocInfo = server.getDartdocDirectiveInfoForSession(session);
@@ -172,27 +186,6 @@
);
}
- Future<ErrorOr<CompletionItem>> resolveDartNotImportedCompletion(
- CompletionItem item,
- DartNotImportedCompletionResolutionInfo data,
- CancellationToken token,
- ) async {
- final clientCapabilities = server.clientCapabilities;
- if (clientCapabilities == null) {
- // This should not happen unless a client misbehaves.
- return error(ErrorCodes.ServerNotInitialized,
- 'Requests not before server is initilized');
- }
-
- return resolveDartCompletion(
- item,
- clientCapabilities,
- token,
- file: data.file,
- libraryUri: Uri.parse(data.libraryUri),
- );
- }
-
Future<ErrorOr<CompletionItem>> resolvePubPackageCompletion(
CompletionItem item,
PubPackageCompletionItemResolutionInfo data,
@@ -228,32 +221,4 @@
data: item.data,
));
}
-
- /// Gets the [Element] for the completion item [item] in [libraryUri].
- Future<Element?> _getElement(
- AnalysisSession session,
- Uri libraryUri,
- CompletionItem item,
- ) async {
- // If filterText is different to the label, it's because label has
- // parens/args appended so we should take the filterText to get the
- // elements name without. We cannot use insertText as it may include
- // snippets, whereas filterText is always just the pure string.
- var name = item.filterText ?? item.label;
-
- // The label might be `MyEnum.myValue`, but we need to find `MyEnum`.
- if (name.contains('.')) {
- name = name.substring(0, name.indexOf('.'));
- }
-
- // TODO(dantup): This is not handling default constructors or enums
- // correctly, so they will both show dart docs from the class/enum and not
- // the constructor/enum member. Extension members are not found at all and
- // will provide no docs.
-
- final result = await session.getLibraryByUri(libraryUri.toString());
- return result is LibraryElementResult
- ? result.element.exportNamespace.get(name)
- : null;
- }
}
diff --git a/pkg/analysis_server/lib/src/services/completion/dart/dart_completion_suggestion.dart b/pkg/analysis_server/lib/src/services/completion/dart/dart_completion_suggestion.dart
new file mode 100644
index 0000000..cff4b25
--- /dev/null
+++ b/pkg/analysis_server/lib/src/services/completion/dart/dart_completion_suggestion.dart
@@ -0,0 +1,42 @@
+// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analysis_server/src/protocol_server.dart'
+ show CompletionSuggestion;
+import 'package:analyzer/dart/element/element.dart';
+
+/// An extension of [CompletionSuggestion] that includes additional
+/// Dart-specific fields that are not part of the JSON protocol.
+class DartCompletionSuggestion extends CompletionSuggestion {
+ final Element? dartElement;
+
+ DartCompletionSuggestion(
+ super.kind,
+ super.relevance,
+ super.completion,
+ super.selectionOffset,
+ super.selectionLength,
+ super.isDeprecated,
+ super.isPotential, {
+ super.displayText,
+ super.replacementOffset,
+ super.replacementLength,
+ super.docSummary,
+ super.docComplete,
+ super.declaringType,
+ super.defaultArgumentListString,
+ super.defaultArgumentListTextRanges,
+ super.element,
+ super.returnType,
+ super.parameterNames,
+ super.parameterTypes,
+ super.requiredParameterCount,
+ super.hasNamedParameters,
+ super.parameterName,
+ super.parameterType,
+ super.libraryUri,
+ super.isNotImported,
+ required this.dartElement,
+ });
+}
diff --git a/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart b/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart
index 6b34277..b20f618 100644
--- a/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart
+++ b/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart
@@ -11,6 +11,7 @@
hide Element, ElementKind;
import 'package:analysis_server/src/provisional/completion/dart/completion_dart.dart';
import 'package:analysis_server/src/services/completion/dart/completion_manager.dart';
+import 'package:analysis_server/src/services/completion/dart/dart_completion_suggestion.dart';
import 'package:analysis_server/src/services/completion/dart/feature_computer.dart';
import 'package:analysis_server/src/services/completion/dart/utilities.dart';
import 'package:analysis_server/src/utilities/extensions/ast.dart';
@@ -350,7 +351,7 @@
required String displayText,
required int selectionOffset,
}) {
- return CompletionSuggestion(
+ return DartCompletionSuggestion(
CompletionSuggestionKind.INVOCATION,
Relevance.closure,
completion,
@@ -359,6 +360,7 @@
false,
false,
displayText: displayText,
+ dartElement: type.element,
);
}
@@ -724,7 +726,7 @@
buffer.write('$indent});');
_addSuggestion(
- CompletionSuggestion(
+ DartCompletionSuggestion(
kind,
relevance,
buffer.toString(),
@@ -734,6 +736,7 @@
false,
// Let the user know that we are going to insert a complete statement.
displayText: 'setState(() {});',
+ dartElement: method,
),
textToMatchOverride: 'setState',
);
@@ -812,7 +815,7 @@
relevance = Relevance.namedArgument;
}
- var suggestion = CompletionSuggestion(
+ var suggestion = DartCompletionSuggestion(
CompletionSuggestionKind.NAMED_ARGUMENT,
relevance,
completion,
@@ -822,7 +825,8 @@
false,
parameterName: name,
parameterType: type,
- replacementLength: replacementLength);
+ replacementLength: replacementLength,
+ dartElement: parameter);
if (parameter is FieldFormalParameterElement) {
_setDocumentation(suggestion, parameter);
suggestion.element =
@@ -916,7 +920,7 @@
var offsetDelta = targetId.offset + replacement.indexOf(completion);
var displayText =
displayTextBuffer.isNotEmpty ? displayTextBuffer.toString() : null;
- var suggestion = CompletionSuggestion(
+ var suggestion = DartCompletionSuggestion(
CompletionSuggestionKind.OVERRIDE,
Relevance.override,
completion,
@@ -924,7 +928,8 @@
selectionRange.length,
element.hasDeprecated,
false,
- displayText: displayText);
+ displayText: displayText,
+ dartElement: element);
suggestion.element = protocol.convertElement(element,
withNullability: _isNonNullableByDefault);
_addSuggestion(
@@ -1418,6 +1423,7 @@
documentation: documentation,
defaultArgumentList: defaultArgumentList,
element: suggestedElement,
+ dartElement: element,
);
}
@@ -1651,7 +1657,7 @@
@override
CompletionSuggestion build() {
- return CompletionSuggestion(
+ return DartCompletionSuggestion(
kind,
relevance,
completion,
@@ -1672,6 +1678,7 @@
defaultArgumentListTextRanges: element.defaultArgumentList?.ranges,
libraryUri: libraryUriStr,
isNotImported: isNotImported ? true : null,
+ dartElement: element.dartElement,
);
}
}
@@ -1691,6 +1698,7 @@
CompletionDefaultArgumentList? defaultArgumentList;
final _ElementDocumentation? documentation;
final protocol.Element element;
+ final Element dartElement;
_ElementCompletionData({
required this.completion,
@@ -1704,6 +1712,7 @@
required this.defaultArgumentList,
required this.documentation,
required this.element,
+ required this.dartElement,
});
}
diff --git a/pkg/analysis_server/test/lsp/completion_dart_test.dart b/pkg/analysis_server/test/lsp/completion_dart_test.dart
index 138d951..1e54c63 100644
--- a/pkg/analysis_server/test/lsp/completion_dart_test.dart
+++ b/pkg/analysis_server/test/lsp/completion_dart_test.dart
@@ -38,16 +38,16 @@
void main() {
defineReflectiveSuite(() {
defineReflectiveTests(CompletionTest);
+ defineReflectiveTests(CompletionDocumentationResolutionTest);
defineReflectiveTests(DartSnippetCompletionTest);
defineReflectiveTests(FlutterSnippetCompletionTest);
defineReflectiveTests(FlutterSnippetCompletionWithoutNullSafetyTest);
});
}
-@reflectiveTest
-class CompletionTest extends AbstractLspAnalysisServerTest
+abstract class AbstractCompletionTest extends AbstractLspAnalysisServerTest
with CompletionTestMixin {
- CompletionTest() {
+ AbstractCompletionTest() {
defaultInitializationOptions = {
// Default to a high budget for tests because everything is cold and
// may take longer to return.
@@ -55,6 +55,174 @@
};
}
+ void expectDocumentation(CompletionItem completion, Matcher matcher) {
+ final docs = completion.documentation?.map(
+ (markup) => markup.value,
+ (string) => string,
+ );
+ expect(docs, matcher);
+ }
+}
+
+@reflectiveTest
+class CompletionDocumentationResolutionTest extends AbstractCompletionTest {
+ late String content;
+
+ Future<CompletionItem> getCompletionItem(String label) async {
+ final completions =
+ await getCompletion(mainFileUri, positionFromMarker(content));
+ return completions.singleWhere((c) => c.label == label);
+ }
+
+ Future<void> initializeServer() async {
+ final initialAnalysis = waitForAnalysisComplete();
+ await initialize(
+ workspaceCapabilities:
+ withApplyEditSupport(emptyWorkspaceClientCapabilities));
+ await openFile(mainFileUri, withoutMarkers(content));
+ await initialAnalysis;
+ }
+
+ Future<void> test_class() async {
+ newFile(
+ join(projectFolderPath, 'my_class.dart'),
+ '''
+/// Class.
+class MyClass {}
+ ''',
+ );
+
+ content = '''
+void f() {
+ MyClass^
+}
+ ''';
+
+ await initializeServer();
+
+ final completion = await getCompletionItem('MyClass');
+ expectDocumentation(completion, isNull);
+
+ final resolved = await resolveCompletion(completion);
+ expectDocumentation(resolved, contains('Class.'));
+ }
+
+ Future<void> test_class_constructor() async {
+ newFile(
+ join(projectFolderPath, 'my_class.dart'),
+ '''
+class MyClass {
+ /// Constructor.
+ MyClass();
+}
+ ''',
+ );
+
+ content = '''
+void f() {
+ MyClass^
+}
+ ''';
+
+ await initializeServer();
+
+ final completion = await getCompletionItem('MyClass()');
+ expectDocumentation(completion, isNull);
+
+ final resolved = await resolveCompletion(completion);
+ expectDocumentation(resolved, contains('Constructor.'));
+ }
+
+ Future<void> test_class_constructorNamed() async {
+ newFile(
+ join(projectFolderPath, 'my_class.dart'),
+ '''
+class MyClass {
+ /// Named Constructor.
+ MyClass.named();
+}
+ ''',
+ );
+
+ content = '''
+void f() {
+ MyClass^
+}
+ ''';
+
+ await initializeServer();
+
+ final completion = await getCompletionItem('MyClass.named()');
+ expectDocumentation(completion, isNull);
+
+ final resolved = await resolveCompletion(completion);
+ expectDocumentation(resolved, contains('Named Constructor.'));
+ }
+
+ Future<void> test_enum() async {
+ newFile(
+ join(projectFolderPath, 'my_enum.dart'),
+ '''
+/// Enum.
+enum MyEnum {}
+ ''',
+ );
+
+ content = '''
+void f() {
+ MyEnum^
+}
+ ''';
+
+ await initializeServer();
+
+ final completion = await getCompletionItem('MyEnum');
+ expectDocumentation(completion, isNull);
+
+ final resolved = await resolveCompletion(completion);
+ expectDocumentation(resolved, contains('Enum.'));
+ }
+
+ Future<void> test_enum_member() async {
+ // Function used to provide type context in main file without importing
+ // the enum.
+ newFile(
+ join(projectFolderPath, 'lib', 'func.dart'),
+ '''
+import 'my_enum.dart';
+void enumFunc(MyEnum e) {}
+ ''',
+ );
+
+ newFile(
+ join(projectFolderPath, 'lib', 'my_enum.dart'),
+ '''
+enum MyEnum {
+ /// Enum Member.
+ one,
+}
+ ''',
+ );
+
+ content = '''
+import 'func.dart';
+void f() {
+ enumFunc(MyEnum^)
+}
+ ''';
+
+ await initializeServer();
+
+ final completion = await getCompletionItem('MyEnum.one');
+ expectDocumentation(completion, isNull);
+
+ final resolved = await resolveCompletion(completion);
+ expectDocumentation(resolved, contains('Enum Member.'));
+ }
+}
+
+@reflectiveTest
+class CompletionTest extends AbstractCompletionTest {
/// Checks whether the correct types of documentation are returned for
/// completions based on [preference].
Future<void> assertDocumentation(
@@ -85,21 +253,17 @@
final res = await getCompletion(mainFileUri, positionFromMarker(content));
final completion = res.singleWhere((c) => c.label == 'A');
- final docs = completion.documentation?.map(
- (markup) => markup.value,
- (string) => string,
- );
if (includesSummary) {
- expect(docs, contains('Summary.'));
+ expectDocumentation(completion, contains('Summary.'));
} else {
- expect(docs, isNot(contains('Summary.')));
+ expectDocumentation(completion, isNot(contains('Summary.')));
}
if (includesFull) {
- expect(docs, contains('Full.'));
+ expectDocumentation(completion, contains('Full.'));
} else {
- expect(docs, isNot(contains('Full.')));
+ expectDocumentation(completion, isNot(contains('Full.')));
}
}
@@ -143,23 +307,19 @@
// Expect no docs in original response and correct type of docs added
// during resolve.
- expect(completion.documentation, isNull);
+ expectDocumentation(completion, isNull);
final resolved = await resolveCompletion(completion);
- final docs = resolved.documentation?.map(
- (markup) => markup.value,
- (string) => string,
- );
if (includesSummary) {
- expect(docs, contains('Summary.'));
+ expectDocumentation(resolved, contains('Summary.'));
} else {
- expect(docs, isNot(contains('Summary.')));
+ expectDocumentation(resolved, isNot(contains('Summary.')));
}
if (includesFull) {
- expect(docs, contains('Full.'));
+ expectDocumentation(resolved, contains('Full.'));
} else {
- expect(docs, isNot(contains('Full.')));
+ expectDocumentation(resolved, isNot(contains('Full.')));
}
}
@@ -1982,7 +2142,7 @@
final originalTextEdit = completion.textEdit;
// Expect no docs, this is added during resolve.
- expect(completion.documentation, isNull);
+ expectDocumentation(completion, isNull);
// Resolve the completion item (via server) to get any additional edits.
// This is LSP's equiv of getSuggestionDetails() and is invoked by LSP
@@ -1998,10 +2158,7 @@
);
// Ensure the doc comment was added.
- expect(
- resolved.documentation!.valueEquals('This class is in another file.'),
- isTrue,
- );
+ expectDocumentation(resolved, equals('This class is in another file.'));
// Ensure the edit did not change.
expect(resolved.textEdit, originalTextEdit);
@@ -2485,7 +2642,7 @@
final originalTextEdit = completion.textEdit;
// Expect no docs, this is added during resolve.
- expect(completion.documentation, isNull);
+ expectDocumentation(completion, isNull);
// Resolve the completion item (via server) to get any additional edits.
// This is LSP's equiv of getSuggestionDetails() and is invoked by LSP
@@ -2501,10 +2658,7 @@
);
// Ensure the doc comment was added.
- expect(
- resolved.documentation!.valueEquals('This class is in another file.'),
- isTrue,
- );
+ expectDocumentation(resolved, equals('This class is in another file.'));
// Ensure the edit did not change.
expect(resolved.textEdit, originalTextEdit);
@@ -2762,7 +2916,7 @@
expect(completion.textEdit, isNotNull);
// Expect no docs, this is added during resolve.
- expect(completion.documentation, isNull);
+ expectDocumentation(completion, isNull);
// Resolve the completion item (via server) to get any additional edits.
// This is LSP's equiv of getSuggestionDetails() and is invoked by LSP
@@ -2887,6 +3041,7 @@
);
await openFile(mainFileUri, withoutMarkers(content));
await initialAnalysis;
+
final res = await getCompletion(mainFileUri, positionFromMarker(content));
// Ensure the item doesn't appear in the results (because we might not
diff --git a/pkg/analysis_server/tool/lsp_spec/generate_all.dart b/pkg/analysis_server/tool/lsp_spec/generate_all.dart
index 1f4720c..111adb9 100644
--- a/pkg/analysis_server/tool/lsp_spec/generate_all.dart
+++ b/pkg/analysis_server/tool/lsp_spec/generate_all.dart
@@ -319,6 +319,13 @@
type: 'string',
comment: 'The URI to be imported if this completion is selected.',
),
+ field(
+ 'ref',
+ type: 'string',
+ canBeUndefined: true,
+ comment: 'The ElementLocation of the item being completed.\n\n'
+ 'This is used to provide documentation in the resolved response.',
+ ),
],
baseType: 'CompletionItemResolutionInfo',
),
