Implement using suggestion sets for LSP completions
Change-Id: I66d725c09492040be5e621d80270b12a21bb8192
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/102704
Commit-Queue: Danny Tuppeny <dantup@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/domains/completion/available_suggestions.dart b/pkg/analysis_server/lib/src/domains/completion/available_suggestions.dart
index bee0613..ce8b297 100644
--- a/pkg/analysis_server/lib/src/domains/completion/available_suggestions.dart
+++ b/pkg/analysis_server/lib/src/domains/completion/available_suggestions.dart
@@ -148,7 +148,7 @@
protocol.Element _protocolElement(Declaration declaration) {
return protocol.Element(
- _protocolElementKind(declaration.kind),
+ protocolElementKind(declaration.kind),
declaration.name,
_protocolElementFlags(declaration),
location: protocol.Location(
@@ -173,7 +173,12 @@
);
}
-protocol.ElementKind _protocolElementKind(DeclarationKind kind) {
+// TODO(dantup): We need to expose this because the Declarations code currently
+// returns declarations with DeclarationKinds but the DartCompletionManager
+// gives us a list of "included ElementKinds". Maybe it would be better to expose
+// includedDeclarationKinds and then just map that list to ElementKinds once in
+// domain_completion for the original protocol?
+protocol.ElementKind protocolElementKind(DeclarationKind kind) {
switch (kind) {
case DeclarationKind.CLASS:
return protocol.ElementKind.CLASS;
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 e4c9b61..ea2b0a2 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart
@@ -7,6 +7,8 @@
import 'package:analysis_server/lsp_protocol/protocol_generated.dart';
import 'package:analysis_server/lsp_protocol/protocol_special.dart';
+import 'package:analysis_server/protocol/protocol_generated.dart';
+import 'package:analysis_server/src/domains/completion/available_suggestions.dart';
import 'package:analysis_server/src/lsp/handlers/handlers.dart';
import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
import 'package:analysis_server/src/lsp/mapping.dart';
@@ -15,6 +17,7 @@
import 'package:analysis_server/src/services/completion/completion_performance.dart';
import 'package:analysis_server/src/services/completion/dart/completion_manager.dart';
import 'package:analyzer/dart/analysis/results.dart';
+import 'package:analyzer_plugin/protocol/protocol_common.dart';
// If the client does not provide capabilities.completion.completionItemKind.valueSet
// then we must never send a kind that's not in this list.
@@ -97,12 +100,20 @@
final completionRequest =
new CompletionRequestImpl(unit, offset, performance);
- try {
- CompletionContributor contributor = new DartCompletionManager();
+ Set<ElementKind> includedElementKinds;
+ List<IncludedSuggestionRelevanceTag> includedSuggestionRelevanceTags;
+ if (includeSuggestionSets) {
+ includedElementKinds = Set<ElementKind>();
+ includedSuggestionRelevanceTags = <IncludedSuggestionRelevanceTag>[];
+ }
+ try {
+ CompletionContributor contributor = new DartCompletionManager(
+ includedElementKinds: includedElementKinds,
+ includedSuggestionRelevanceTags: includedSuggestionRelevanceTags,
+ );
final suggestions =
await contributor.computeSuggestions(completionRequest);
-
final results = suggestions
.map((item) => toCompletionItem(
completionCapabilities,
@@ -114,6 +125,46 @@
))
.toList();
+ // Now compute items in suggestion sets.
+ List<IncludedSuggestionSet> includedSuggestionSets =
+ includedElementKinds == null || unit == null
+ ? const []
+ : computeIncludedSetList(
+ server.declarationsTracker,
+ unit,
+ );
+
+ includedSuggestionSets.forEach((includedSet) {
+ final library = server.declarationsTracker.getLibrary(includedSet.id);
+ if (library == null) {
+ return;
+ }
+
+ // Make a fast lookup for tag relevance.
+ final tagBoosts = <String, int>{};
+ includedSuggestionRelevanceTags
+ .forEach((t) => tagBoosts[t.tag] = t.relevanceBoost);
+
+ final setResults = library.declarations
+ // Filter to only the kinds we should return.
+ .where((item) =>
+ includedElementKinds.contains(protocolElementKind(item.kind)))
+ .map((item) => declarationToCompletionItem(
+ completionCapabilities,
+ clientSupportedCompletionKinds,
+ unit.path,
+ offset,
+ includedSet,
+ library,
+ tagBoosts,
+ unit.lineInfo,
+ item,
+ completionRequest.replacementOffset,
+ completionRequest.replacementLength,
+ ));
+ results.addAll(setResults);
+ });
+
performance.notificationCount = 1;
performance.suggestionCountFirst = results.length;
performance.suggestionCountLast = results.length;
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 c32ec91..df26ab8 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
@@ -6,11 +6,22 @@
import 'package:analysis_server/lsp_protocol/protocol_generated.dart';
import 'package:analysis_server/lsp_protocol/protocol_special.dart';
+import 'package:analysis_server/src/lsp/constants.dart';
import 'package:analysis_server/src/lsp/handlers/handlers.dart';
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_plugin/utilities/change_builder/change_builder_dart.dart';
class CompletionResolveHandler
extends MessageHandler<CompletionItem, CompletionItem> {
+ ///
+ /// The latest completion item we were asked to resolve. We use it to abort
+ /// previous requests.
+ ///
+ CompletionItem _latestCompletionItem;
+
CompletionResolveHandler(LspAnalysisServer server) : super(server);
Method get handlesMessage => Method.completionItem_resolve;
@@ -19,7 +30,143 @@
LspJsonHandler<CompletionItem> get jsonHandler => CompletionItem.jsonHandler;
Future<ErrorOr<CompletionItem>> handle(CompletionItem item) async {
- // TODO: Implement resolution. For now just always return the same item back.
- return success(item);
+ // If this isn't an item with resolution data, return the same item back.
+ if (item.data == null) {
+ return success(item);
+ }
+
+ final data = item.data;
+ final lineInfo = server.getLineInfo(data.file);
+ if (lineInfo == null) {
+ return error(
+ ErrorCodes.InternalError,
+ 'Line info not available for ${data.file}',
+ null,
+ );
+ }
+
+ // TODO(dantup): This logic is all repeated from domain_completion and needs
+ // extracting (with support for the different types of responses between
+ // the servers). Where is an appropriate place to put it?
+
+ var library = server.declarationsTracker.getLibrary(data.libraryId);
+ if (library == null) {
+ return error(
+ ErrorCodes.InvalidParams,
+ 'Library ID is not valid: ${data.libraryId}',
+ data.libraryId.toString(),
+ );
+ }
+
+ // The label might be `MyEnum.myValue`, but we import only `MyEnum`.
+ var requestedName = item.label;
+ if (requestedName.contains('.')) {
+ requestedName = requestedName.substring(
+ 0,
+ requestedName.indexOf('.'),
+ );
+ }
+
+ const timeout = Duration(milliseconds: 1000);
+ var timer = Stopwatch()..start();
+ _latestCompletionItem = item;
+ while (item == _latestCompletionItem && timer.elapsed < timeout) {
+ try {
+ var analysisDriver = server.getAnalysisDriver(data.file);
+ var session = analysisDriver.currentSession;
+
+ var fileElement = await session.getUnitElement(data.file);
+ var libraryPath = fileElement.element.librarySource.fullName;
+
+ var resolvedLibrary = await session.getResolvedLibrary(libraryPath);
+
+ analyzer.LibraryElement requestedLibraryElement;
+ try {
+ requestedLibraryElement = await session.getLibraryByUri(
+ library.uriStr,
+ );
+ } on ArgumentError catch (e) {
+ return error(
+ ErrorCodes.InvalidParams,
+ 'Invalid library URI: ${library.uriStr}',
+ '$e',
+ );
+ }
+
+ var requestedElement =
+ requestedLibraryElement.exportNamespace.get(requestedName);
+ if (requestedElement == null) {
+ return error(
+ ErrorCodes.InvalidParams,
+ 'No such element: $requestedName',
+ requestedName,
+ );
+ }
+
+ var newLabel = item.label;
+ final builder = DartChangeBuilder(session);
+ await builder.addFileEdit(libraryPath, (builder) {
+ final result = builder.importLibraryElement(
+ targetLibrary: resolvedLibrary,
+ targetPath: libraryPath,
+ targetOffset: data.offset,
+ requestedLibrary: requestedLibraryElement,
+ requestedElement: requestedElement,
+ );
+ if (result.prefix != null) {
+ newLabel = '${result.prefix}.$newLabel';
+ }
+ });
+
+ final changes = builder.sourceChange;
+ final thisFilesChanges =
+ changes.edits.where((e) => e.file == data.file).toList();
+ final otherFilesChanges =
+ changes.edits.where((e) => e.file != data.file).toList();
+
+ // If this completion involves editing other files, we'll need to build
+ // a command that the client will call to apply those edits later.
+ Command command;
+ if (otherFilesChanges.isNotEmpty) {
+ final workspaceEdit = createWorkspaceEdit(server, otherFilesChanges);
+ command = Command(
+ 'Add import', Commands.sendWorkspaceEdit, [workspaceEdit]);
+ }
+
+ return success(CompletionItem(
+ newLabel,
+ item.kind,
+ data.autoImportDisplayUri != null
+ ? "Auto import from '${data.autoImportDisplayUri}'\n\n${item.detail ?? ''}"
+ .trim()
+ : item.detail,
+ item.documentation,
+ item.deprecated,
+ item.preselect,
+ item.sortText,
+ item.filterText,
+ newLabel,
+ item.insertTextFormat,
+ item.textEdit,
+ thisFilesChanges
+ .expand((change) =>
+ change.edits.map((edit) => toTextEdit(lineInfo, edit)))
+ .toList(),
+ item.commitCharacters,
+ command ?? item.command,
+ item.data,
+ ));
+ } on InconsistentAnalysisException {
+ // Loop around to try again.
+ }
+ }
+
+ // Timeout or abort, send the empty response.
+
+ return error(
+ ErrorCodes.RequestCancelled,
+ 'Request was cancelled for taking too long or another request being received',
+ null,
+ );
}
}
diff --git a/pkg/analysis_server/lib/src/lsp/mapping.dart b/pkg/analysis_server/lib/src/lsp/mapping.dart
index 3719bb2..4970325 100644
--- a/pkg/analysis_server/lib/src/lsp/mapping.dart
+++ b/pkg/analysis_server/lib/src/lsp/mapping.dart
@@ -14,6 +14,7 @@
import 'package:analysis_server/lsp_protocol/protocol_special.dart' as lsp;
import 'package:analysis_server/lsp_protocol/protocol_special.dart'
show ErrorOr, Either2, Either4;
+import 'package:analysis_server/protocol/protocol_generated.dart';
import 'package:analysis_server/src/lsp/constants.dart' as lsp;
import 'package:analysis_server/src/lsp/dartdoc.dart';
import 'package:analysis_server/src/lsp/lsp_analysis_server.dart' as lsp;
@@ -26,6 +27,8 @@
import 'package:analyzer/src/dart/analysis/search.dart' as server
show DeclarationKind;
import 'package:analyzer/src/generated/source.dart' as server;
+import 'package:analyzer/src/services/available_declarations.dart';
+import 'package:analyzer/src/services/available_declarations.dart' as dec;
import 'package:analyzer_plugin/utilities/fixes/fixes.dart' as server;
const languageSourceName = 'dart';
@@ -57,6 +60,42 @@
.toList());
}
+lsp.CompletionItemKind declarationKindToCompletionItemKind(
+ HashSet<lsp.CompletionItemKind> clientSupportedCompletionKinds,
+ dec.DeclarationKind kind,
+) {
+ bool isSupported(lsp.CompletionItemKind kind) =>
+ clientSupportedCompletionKinds.contains(kind);
+
+ List<lsp.CompletionItemKind> getKindPreferences() {
+ switch (kind) {
+ case dec.DeclarationKind.CLASS:
+ case dec.DeclarationKind.CLASS_TYPE_ALIAS:
+ case dec.DeclarationKind.MIXIN:
+ return const [lsp.CompletionItemKind.Class];
+ case dec.DeclarationKind.CONSTRUCTOR:
+ return const [lsp.CompletionItemKind.Constructor];
+ case dec.DeclarationKind.ENUM:
+ case dec.DeclarationKind.ENUM_CONSTANT:
+ return const [lsp.CompletionItemKind.Enum];
+ case dec.DeclarationKind.FUNCTION:
+ return const [lsp.CompletionItemKind.Function];
+ case dec.DeclarationKind.FUNCTION_TYPE_ALIAS:
+ return const [lsp.CompletionItemKind.Class];
+ case dec.DeclarationKind.GETTER:
+ return const [lsp.CompletionItemKind.Property];
+ case dec.DeclarationKind.SETTER:
+ return const [lsp.CompletionItemKind.Property];
+ case dec.DeclarationKind.VARIABLE:
+ return const [lsp.CompletionItemKind.Variable];
+ default:
+ return null;
+ }
+ }
+
+ return getKindPreferences().firstWhere(isSupported, orElse: () => null);
+}
+
lsp.SymbolKind declarationKindToSymbolKind(
HashSet<lsp.SymbolKind> clientSupportedSymbolKinds,
server.DeclarationKind kind,
@@ -99,6 +138,69 @@
return getKindPreferences().firstWhere(isSupported, orElse: () => null);
}
+lsp.CompletionItem declarationToCompletionItem(
+ lsp.TextDocumentClientCapabilitiesCompletion completionCapabilities,
+ HashSet<lsp.CompletionItemKind> supportedCompletionItemKinds,
+ String file,
+ int offset,
+ IncludedSuggestionSet includedSuggestionSet,
+ Library library,
+ Map<String, int> tagBoosts,
+ server.LineInfo lineInfo,
+ dec.Declaration declaration,
+ int replacementOffset,
+ int replacementLength,
+) {
+ final label = declaration.name;
+
+ final useDeprecated =
+ completionCapabilities?.completionItem?.deprecatedSupport == true;
+ final formats = completionCapabilities?.completionItem?.documentationFormat;
+
+ final completionKind = declarationKindToCompletionItemKind(
+ supportedCompletionItemKinds, declaration.kind);
+
+ var itemRelevance = includedSuggestionSet.relevance;
+ if (declaration.relevanceTags != null)
+ declaration.relevanceTags
+ .forEach((t) => itemRelevance += (tagBoosts[t] ?? 0));
+
+ return new lsp.CompletionItem(
+ label,
+ completionKind,
+ getDeclarationCompletionDetail(declaration, completionKind, useDeprecated),
+ asStringOrMarkupContent(formats, cleanDartdoc(declaration.docComplete)),
+ useDeprecated ? declaration.isDeprecated : null,
+ false, // preselect
+ // Relevance is a number, highest being best. LSP does text sort so subtract
+ // from a large number so that a text sort will result in the correct order.
+ // 555 -> 999455
+ // 10 -> 999990
+ // 1 -> 999999
+ (1000000 - itemRelevance).toString(),
+ null, // filterText uses label if not set
+ null, // insertText is deprecated, but also uses label if not set
+ // We don't have completions that use snippets, so we always return PlainText.
+ lsp.InsertTextFormat.PlainText,
+ new lsp.TextEdit(
+ // TODO(dantup): If `clientSupportsSnippets == true` then we should map
+ // `selection` in to a snippet (see how Dart Code does this).
+ toRange(lineInfo, replacementOffset, replacementLength),
+ label,
+ ),
+ [], // additionalTextEdits, used for adding imports, etc.
+ [], // commitCharacters
+ null, // command
+ // data, used for completionItem/resolve.
+ new lsp.CompletionItemResolutionInfo(
+ file,
+ offset,
+ includedSuggestionSet.id,
+ includedSuggestionSet.displayUri ?? library.uri?.toString(),
+ ),
+ );
+}
+
lsp.CompletionItemKind elementKindToCompletionItemKind(
HashSet<lsp.CompletionItemKind> clientSupportedCompletionKinds,
server.ElementKind kind,
@@ -276,6 +378,45 @@
}
}
+String getDeclarationCompletionDetail(
+ dec.Declaration declaration,
+ lsp.CompletionItemKind completionKind,
+ bool clientSupportsDeprecated,
+) {
+ final hasParameters =
+ declaration.parameters != null && declaration.parameters.isNotEmpty;
+ final hasReturnType =
+ declaration.returnType != null && declaration.returnType.isNotEmpty;
+
+ final prefix = clientSupportsDeprecated || !declaration.isDeprecated
+ ? ''
+ : '(Deprecated) ';
+
+ if (completionKind == lsp.CompletionItemKind.Property) {
+ // Setters appear as methods with one arg but they also cause getters to not
+ // appear in the completion list, so displaying them as setters is misleading.
+ // To avoid this, always show only the return type, whether it's a getter
+ // or a setter.
+ return prefix +
+ (declaration.kind == dec.DeclarationKind.GETTER
+ ? declaration.returnType
+ // Don't assume setters always have parameters
+ // See https://github.com/dart-lang/sdk/issues/27747
+ : declaration.parameters != null &&
+ declaration.parameters.isNotEmpty
+ // Extract the type part from '(MyType value)`
+ ? declaration.parameters
+ .substring(1, declaration.parameters.lastIndexOf(" "))
+ : '');
+ } else if (hasParameters && hasReturnType) {
+ return '$prefix${declaration.parameters} → ${declaration.returnType}';
+ } else if (hasReturnType) {
+ return '$prefix${declaration.returnType}';
+ } else {
+ return prefix;
+ }
+}
+
bool isDartDocument(lsp.TextDocumentIdentifier doc) =>
doc?.uri?.endsWith('.dart');
diff --git a/pkg/analysis_server/test/lsp/completion_test.dart b/pkg/analysis_server/test/lsp/completion_test.dart
index c910b60..8a378f4 100644
--- a/pkg/analysis_server/test/lsp/completion_test.dart
+++ b/pkg/analysis_server/test/lsp/completion_test.dart
@@ -224,7 +224,6 @@
expect(updated, contains('a.abcdefghij'));
}
- @failingTest
test_suggestionSets() async {
newFile(
join(projectFolderPath, 'other_file.dart'),
@@ -280,7 +279,6 @@
'''));
}
- @failingTest
test_suggestionSets_insertsIntoPartFiles() async {
// File we'll be adding an import for.
newFile(
diff --git a/pkg/analysis_server/tool/lsp_spec/README.md b/pkg/analysis_server/tool/lsp_spec/README.md
index 659921c..5cfc02b 100644
--- a/pkg/analysis_server/tool/lsp_spec/README.md
+++ b/pkg/analysis_server/tool/lsp_spec/README.md
@@ -19,6 +19,7 @@
## Initialization Options
- `onlyAnalyzeProjectsWithOpenFiles`: When set to `true`, analysis will only be performed for projects that have open files rather than the root workspace folder. Defaults to `false`.
+- `suggestFromUnimportedLibraries`: When set to `false`, completion will not include synbols that are not already imported into the current file. Defaults to `true`, though the client must additionally support `workspace/applyEdit` for these completions to be included.
## Method Status