From 7ad1786e45a8056269103576b05d0d4d69b38f13 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 26 Jan 2022 20:07:28 +0000 Subject: [PATCH] [analysis_server] Refactor completion handlers in preparation for using NotImportedContributor Change-Id: I18608b110093e87887f8e9118340e323fe829248 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/230081 Reviewed-by: Brian Wilkerson Commit-Queue: Brian Wilkerson --- .../protocol_custom_generated.dart | 400 +++++++----------- .../src/lsp/handlers/handler_completion.dart | 130 +++--- .../handlers/handler_completion_resolve.dart | 195 +++++---- .../lib/src/lsp/handlers/handler_states.dart | 16 +- pkg/analysis_server/lib/src/lsp/mapping.dart | 39 +- .../test/lsp/completion_dart_test.dart | 44 +- .../test/lsp/server_abstract.dart | 9 +- .../tool/lsp_spec/generate_all.dart | 10 +- 8 files changed, 404 insertions(+), 439 deletions(-) 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 5c222c3a91f..7d095d71836 100644 --- a/pkg/analysis_server/lib/lsp_protocol/protocol_custom_generated.dart +++ b/pkg/analysis_server/lib/lsp_protocol/protocol_custom_generated.dart @@ -169,29 +169,167 @@ class CompletionItemResolutionInfo implements ToJsonable { CompletionItemResolutionInfo.canParse, CompletionItemResolutionInfo.fromJson); - CompletionItemResolutionInfo({required this.file, required this.offset}); static CompletionItemResolutionInfo fromJson(Map json) { - if (DartCompletionItemResolutionInfo.canParse(json, nullLspJsonReporter)) { - return DartCompletionItemResolutionInfo.fromJson(json); + if (DartSuggestionSetCompletionItemResolutionInfo.canParse( + json, nullLspJsonReporter)) { + return DartSuggestionSetCompletionItemResolutionInfo.fromJson(json); } if (PubPackageCompletionItemResolutionInfo.canParse( json, nullLspJsonReporter)) { return PubPackageCompletionItemResolutionInfo.fromJson(json); } + return CompletionItemResolutionInfo(); + } + + Map toJson() { + var __result = {}; + return __result; + } + + static bool canParse(Object? obj, LspJsonReporter reporter) { + if (obj is Map) { + return true; + } else { + reporter.reportError('must be of type CompletionItemResolutionInfo'); + return false; + } + } + + @override + bool operator ==(Object other) { + if (other is CompletionItemResolutionInfo && + other.runtimeType == CompletionItemResolutionInfo) { + return true; + } + return false; + } + + @override + int get hashCode => 42; + + @override + String toString() => jsonEncoder.convert(toJson()); +} + +class DartDiagnosticServer implements ToJsonable { + static const jsonHandler = LspJsonHandler( + DartDiagnosticServer.canParse, DartDiagnosticServer.fromJson); + + DartDiagnosticServer({required this.port}); + static DartDiagnosticServer fromJson(Map json) { + final portJson = json['port']; + final port = portJson as int; + return DartDiagnosticServer(port: port); + } + + final int port; + + Map toJson() { + var __result = {}; + __result['port'] = port; + return __result; + } + + static bool canParse(Object? obj, LspJsonReporter reporter) { + if (obj is Map) { + reporter.push('port'); + try { + if (!obj.containsKey('port')) { + reporter.reportError('must not be undefined'); + return false; + } + final port = obj['port']; + if (port == null) { + reporter.reportError('must not be null'); + return false; + } + if (!(port is int)) { + reporter.reportError('must be of type int'); + return false; + } + } finally { + reporter.pop(); + } + return true; + } else { + reporter.reportError('must be of type DartDiagnosticServer'); + return false; + } + } + + @override + bool operator ==(Object other) { + if (other is DartDiagnosticServer && + other.runtimeType == DartDiagnosticServer) { + return port == other.port && true; + } + return false; + } + + @override + int get hashCode => port.hashCode; + + @override + String toString() => jsonEncoder.convert(toJson()); +} + +class DartSuggestionSetCompletionItemResolutionInfo + implements CompletionItemResolutionInfo, ToJsonable { + static const jsonHandler = LspJsonHandler( + DartSuggestionSetCompletionItemResolutionInfo.canParse, + DartSuggestionSetCompletionItemResolutionInfo.fromJson); + + DartSuggestionSetCompletionItemResolutionInfo( + {required this.file, + required this.offset, + required this.libId, + required this.displayUri, + required this.rOffset, + required this.iLength, + required this.rLength}); + static DartSuggestionSetCompletionItemResolutionInfo fromJson( + Map json) { final fileJson = json['file']; final file = fileJson as String; final offsetJson = json['offset']; final offset = offsetJson as int; - return CompletionItemResolutionInfo(file: file, offset: offset); + final libIdJson = json['libId']; + final libId = libIdJson as int; + final displayUriJson = json['displayUri']; + final displayUri = displayUriJson as String; + final rOffsetJson = json['rOffset']; + final rOffset = rOffsetJson as int; + final iLengthJson = json['iLength']; + final iLength = iLengthJson as int; + final rLengthJson = json['rLength']; + final rLength = rLengthJson as int; + return DartSuggestionSetCompletionItemResolutionInfo( + file: file, + offset: offset, + libId: libId, + displayUri: displayUri, + rOffset: rOffset, + iLength: iLength, + rLength: rLength); } + final String displayUri; final String file; + final int iLength; + final int libId; final int offset; + final int rLength; + final int rOffset; Map toJson() { var __result = {}; __result['file'] = file; __result['offset'] = offset; + __result['libId'] = libId; + __result['displayUri'] = displayUri; + __result['rOffset'] = rOffset; + __result['iLength'] = iLength; + __result['rLength'] = rLength; return __result; } @@ -233,90 +371,6 @@ class CompletionItemResolutionInfo implements ToJsonable { } finally { reporter.pop(); } - return true; - } else { - reporter.reportError('must be of type CompletionItemResolutionInfo'); - return false; - } - } - - @override - bool operator ==(Object other) { - if (other is CompletionItemResolutionInfo && - other.runtimeType == CompletionItemResolutionInfo) { - return file == other.file && offset == other.offset && true; - } - return false; - } - - @override - int get hashCode => Object.hash(file, offset); - - @override - String toString() => jsonEncoder.convert(toJson()); -} - -class DartCompletionItemResolutionInfo - implements CompletionItemResolutionInfo, ToJsonable { - static const jsonHandler = LspJsonHandler( - DartCompletionItemResolutionInfo.canParse, - DartCompletionItemResolutionInfo.fromJson); - - DartCompletionItemResolutionInfo( - {required this.libId, - required this.displayUri, - required this.rOffset, - required this.iLength, - required this.rLength, - required this.file, - required this.offset}); - static DartCompletionItemResolutionInfo fromJson(Map json) { - final libIdJson = json['libId']; - final libId = libIdJson as int; - final displayUriJson = json['displayUri']; - final displayUri = displayUriJson as String; - final rOffsetJson = json['rOffset']; - final rOffset = rOffsetJson as int; - final iLengthJson = json['iLength']; - final iLength = iLengthJson as int; - final rLengthJson = json['rLength']; - final rLength = rLengthJson as int; - final fileJson = json['file']; - final file = fileJson as String; - final offsetJson = json['offset']; - final offset = offsetJson as int; - return DartCompletionItemResolutionInfo( - libId: libId, - displayUri: displayUri, - rOffset: rOffset, - iLength: iLength, - rLength: rLength, - file: file, - offset: offset); - } - - final String displayUri; - final String file; - final int iLength; - final int libId; - final int offset; - final int rLength; - final int rOffset; - - Map toJson() { - var __result = {}; - __result['libId'] = libId; - __result['displayUri'] = displayUri; - __result['rOffset'] = rOffset; - __result['iLength'] = iLength; - __result['rLength'] = rLength; - __result['file'] = file; - __result['offset'] = offset; - return __result; - } - - static bool canParse(Object? obj, LspJsonReporter reporter) { - if (obj is Map) { reporter.push('libId'); try { if (!obj.containsKey('libId')) { @@ -407,60 +461,25 @@ class DartCompletionItemResolutionInfo } finally { reporter.pop(); } - reporter.push('file'); - try { - if (!obj.containsKey('file')) { - reporter.reportError('must not be undefined'); - return false; - } - final file = obj['file']; - if (file == null) { - reporter.reportError('must not be null'); - return false; - } - if (!(file is String)) { - reporter.reportError('must be of type String'); - return false; - } - } finally { - reporter.pop(); - } - reporter.push('offset'); - try { - if (!obj.containsKey('offset')) { - reporter.reportError('must not be undefined'); - return false; - } - final offset = obj['offset']; - if (offset == null) { - reporter.reportError('must not be null'); - return false; - } - if (!(offset is int)) { - reporter.reportError('must be of type int'); - return false; - } - } finally { - reporter.pop(); - } return true; } else { - reporter.reportError('must be of type DartCompletionItemResolutionInfo'); + reporter.reportError( + 'must be of type DartSuggestionSetCompletionItemResolutionInfo'); return false; } } @override bool operator ==(Object other) { - if (other is DartCompletionItemResolutionInfo && - other.runtimeType == DartCompletionItemResolutionInfo) { - return libId == other.libId && + if (other is DartSuggestionSetCompletionItemResolutionInfo && + other.runtimeType == DartSuggestionSetCompletionItemResolutionInfo) { + return file == other.file && + offset == other.offset && + libId == other.libId && displayUri == other.displayUri && rOffset == other.rOffset && iLength == other.iLength && rLength == other.rLength && - file == other.file && - offset == other.offset && true; } return false; @@ -468,69 +487,7 @@ class DartCompletionItemResolutionInfo @override int get hashCode => - Object.hash(libId, displayUri, rOffset, iLength, rLength, file, offset); - - @override - String toString() => jsonEncoder.convert(toJson()); -} - -class DartDiagnosticServer implements ToJsonable { - static const jsonHandler = LspJsonHandler( - DartDiagnosticServer.canParse, DartDiagnosticServer.fromJson); - - DartDiagnosticServer({required this.port}); - static DartDiagnosticServer fromJson(Map json) { - final portJson = json['port']; - final port = portJson as int; - return DartDiagnosticServer(port: port); - } - - final int port; - - Map toJson() { - var __result = {}; - __result['port'] = port; - return __result; - } - - static bool canParse(Object? obj, LspJsonReporter reporter) { - if (obj is Map) { - reporter.push('port'); - try { - if (!obj.containsKey('port')) { - reporter.reportError('must not be undefined'); - return false; - } - final port = obj['port']; - if (port == null) { - reporter.reportError('must not be null'); - return false; - } - if (!(port is int)) { - reporter.reportError('must be of type int'); - return false; - } - } finally { - reporter.pop(); - } - return true; - } else { - reporter.reportError('must be of type DartDiagnosticServer'); - return false; - } - } - - @override - bool operator ==(Object other) { - if (other is DartDiagnosticServer && - other.runtimeType == DartDiagnosticServer) { - return port == other.port && true; - } - return false; - } - - @override - int get hashCode => port.hashCode; + Object.hash(file, offset, libId, displayUri, rOffset, iLength, rLength); @override String toString() => jsonEncoder.convert(toJson()); @@ -1211,29 +1168,19 @@ class PubPackageCompletionItemResolutionInfo PubPackageCompletionItemResolutionInfo.canParse, PubPackageCompletionItemResolutionInfo.fromJson); - PubPackageCompletionItemResolutionInfo( - {required this.packageName, required this.file, required this.offset}); + PubPackageCompletionItemResolutionInfo({required this.packageName}); static PubPackageCompletionItemResolutionInfo fromJson( Map json) { final packageNameJson = json['packageName']; final packageName = packageNameJson as String; - final fileJson = json['file']; - final file = fileJson as String; - final offsetJson = json['offset']; - final offset = offsetJson as int; - return PubPackageCompletionItemResolutionInfo( - packageName: packageName, file: file, offset: offset); + return PubPackageCompletionItemResolutionInfo(packageName: packageName); } - final String file; - final int offset; final String packageName; Map toJson() { var __result = {}; __result['packageName'] = packageName; - __result['file'] = file; - __result['offset'] = offset; return __result; } @@ -1257,42 +1204,6 @@ class PubPackageCompletionItemResolutionInfo } finally { reporter.pop(); } - reporter.push('file'); - try { - if (!obj.containsKey('file')) { - reporter.reportError('must not be undefined'); - return false; - } - final file = obj['file']; - if (file == null) { - reporter.reportError('must not be null'); - return false; - } - if (!(file is String)) { - reporter.reportError('must be of type String'); - return false; - } - } finally { - reporter.pop(); - } - reporter.push('offset'); - try { - if (!obj.containsKey('offset')) { - reporter.reportError('must not be undefined'); - return false; - } - final offset = obj['offset']; - if (offset == null) { - reporter.reportError('must not be null'); - return false; - } - if (!(offset is int)) { - reporter.reportError('must be of type int'); - return false; - } - } finally { - reporter.pop(); - } return true; } else { reporter.reportError( @@ -1305,16 +1216,13 @@ class PubPackageCompletionItemResolutionInfo bool operator ==(Object other) { if (other is PubPackageCompletionItemResolutionInfo && other.runtimeType == PubPackageCompletionItemResolutionInfo) { - return packageName == other.packageName && - file == other.file && - offset == other.offset && - true; + return packageName == other.packageName && true; } return false; } @override - int get hashCode => Object.hash(packageName, file, offset); + int get hashCode => packageName.hashCode; @override String toString() => jsonEncoder.convert(toJson()); 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 d2bd74d4979..736ab90bbaa 100644 --- a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart +++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart @@ -31,13 +31,14 @@ import 'package:analyzer_plugin/protocol/protocol_common.dart'; import 'package:analyzer_plugin/protocol/protocol_generated.dart' as plugin; import 'package:analyzer_plugin/src/utilities/completion/completion_target.dart'; -class CompletionHandler - extends MessageHandler> +class CompletionHandler extends MessageHandler with LspPluginRequestHandlerMixin { + /// Whether to include symbols from libraries that have not been imported. final bool suggestFromUnimportedLibraries; - CompletionHandler( - LspAnalysisServer server, this.suggestFromUnimportedLibraries) - : super(server); + + CompletionHandler(LspAnalysisServer server, LspInitializationOptions options) + : suggestFromUnimportedLibraries = options.suggestFromUnimportedLibraries, + super(server); @override Method get handlesMessage => Method.textDocument_completion; @@ -47,7 +48,7 @@ class CompletionHandler CompletionParams.jsonHandler; @override - Future>> handle( + Future> handle( CompletionParams params, CancellationToken token) async { final clientCapabilities = server.clientCapabilities; if (clientCapabilities == null) { @@ -56,9 +57,6 @@ class CompletionHandler 'Requests not before server is initilized'); } - final includeSuggestionSets = - suggestFromUnimportedLibraries && clientCapabilities.applyEdit; - final triggerCharacter = params.context?.triggerCharacter; final pos = params.position; final path = pathOfDoc(params.textDocument); @@ -75,14 +73,13 @@ class CompletionHandler await lineInfo.mapResult((lineInfo) => toOffset(lineInfo, pos)); return offset.mapResult((offset) async { - Future>>? serverResultsFuture; + Future>? serverResultsFuture; final pathContext = server.resourceProvider.pathContext; final fileExtension = pathContext.extension(path.result); if (fileExtension == '.dart' && !unit.isError) { serverResultsFuture = _getServerDartItems( clientCapabilities, - includeSuggestionSets, unit.result, offset, triggerCharacter, @@ -110,7 +107,8 @@ class CompletionHandler } } - serverResultsFuture ??= Future.value(success(const [])); + serverResultsFuture ??= + Future.value(success(CompletionList(isIncomplete: false, items: []))); final pluginResultsFuture = _getPluginResults( clientCapabilities, lineInfo.result, path.result, offset); @@ -125,9 +123,15 @@ class CompletionHandler if (serverResults.isError) return serverResults; if (pluginResults.isError) return pluginResults; - return success( - serverResults.result.followedBy(pluginResults.result).toList(), - ); + return success(CompletionList( + // If any set of the results is incomplete, the whole batch must be + // marked as such. + isIncomplete: serverResults.result.isIncomplete || + pluginResults.result.isIncomplete, + items: serverResults.result.items + .followedBy(pluginResults.result.items) + .toList(), + )); }); } @@ -174,7 +178,7 @@ class CompletionHandler String _createImportedSymbolKey(String name, Uri declaringUri) => '$name/$declaringUri'; - Future>> _getPluginResults( + Future> _getPluginResults( LspClientCapabilities capabilities, LineInfo lineInfo, String path, @@ -188,22 +192,27 @@ class CompletionHandler .map((e) => plugin.CompletionGetSuggestionsResult.fromResponse(e)) .toList(); - return success(_pluginResultsToItems( - capabilities, - lineInfo, - offset, - pluginResults, - ).toList()); + return success(CompletionList( + isIncomplete: false, + items: _pluginResultsToItems( + capabilities, + lineInfo, + offset, + pluginResults, + ).toList(), + )); } - Future>> _getServerDartItems( + Future> _getServerDartItems( LspClientCapabilities capabilities, - bool includeSuggestionSets, ResolvedUnitResult unit, int offset, String? triggerCharacter, CancellationToken token, ) async { + final useSuggestionSets = + suggestFromUnimportedLibraries && capabilities.applyEdit; + var performance = OperationPerformanceImpl(''); return await performance.runAsync( 'request', @@ -226,14 +235,14 @@ class CompletionHandler if (triggerCharacter != null) { if (!_triggerCharacterValid(offset, triggerCharacter, target)) { - return success([]); + return success(CompletionList(isIncomplete: false, items: [])); } } Set? includedElementKinds; Set? includedElementNames; List? includedSuggestionRelevanceTags; - if (includeSuggestionSets) { + if (useSuggestionSets) { includedElementKinds = {}; includedElementNames = {}; includedSuggestionRelevanceTags = []; @@ -284,18 +293,22 @@ class CompletionHandler offset, itemReplacementOffset, itemInsertLength); } + // Convert to LSP ranges using the LineInfo. + Range? replacementRange = toRange( + unit.lineInfo, itemReplacementOffset, itemReplacementLength); + Range? insertionRange = toRange( + unit.lineInfo, itemReplacementOffset, itemInsertLength); + return toCompletionItem( capabilities, unit.lineInfo, item, - itemReplacementOffset, - itemInsertLength, - itemReplacementLength, - // TODO(dantup): Including commit characters in every completion - // increases the payload size. The LSP spec is ambigious - // about how this should be handled (and VS Code requires it) but - // this should be removed (or made conditional based on a capability) - // depending on how the spec is updated. + replacementRange: replacementRange, + insertionRange: insertionRange, + // TODO(dantup): Move commit characters to the main response + // and remove from each individual item (to reduce payload size) + // once the following change ships (and the Dart VS Code + // extension is updated to use it). // https://github.com/microsoft/vscode-languageserver-node/issues/673 includeCommitCharacters: server.clientConfiguration.global.previewCommitCharacters, @@ -390,11 +403,10 @@ class CompletionHandler completionRequest.replacementOffset, insertLength, completionRequest.replacementLength, - // TODO(dantup): Including commit characters in every completion - // increases the payload size. The LSP spec is ambigious - // about how this should be handled (and VS Code requires it) but - // this should be removed (or made conditional based on a capability) - // depending on how the spec is updated. + // TODO(dantup): Move commit characters to the main response + // and remove from each individual item (to reduce payload size) + // once the following change ships (and the Dart VS Code + // extension is updated to use it). // https://github.com/microsoft/vscode-languageserver-node/issues/673 includeCommitCharacters: server .clientConfiguration.global.previewCommitCharacters, @@ -415,15 +427,16 @@ class CompletionHandler completionPerformance.suggestionCount = results.length; - return success(matchingResults); + return success( + CompletionList(isIncomplete: false, items: matchingResults)); } on AbortCompletion { - return success([]); + return success(CompletionList(isIncomplete: false, items: [])); } }, ); } - Future>> _getServerYamlItems( + Future> _getServerYamlItems( YamlCompletionGenerator generator, LspClientCapabilities capabilities, String path, @@ -437,23 +450,25 @@ class CompletionHandler suggestions.replacementOffset, suggestions.replacementLength, ); + final replacementRange = toRange( + lineInfo, suggestions.replacementOffset, suggestions.replacementLength); + final insertionRange = + toRange(lineInfo, suggestions.replacementOffset, insertLength); + final completionItems = suggestions.suggestions .map( (item) => toCompletionItem( capabilities, lineInfo, item, - suggestions.replacementOffset, - insertLength, - suggestions.replacementLength, + replacementRange: replacementRange, + insertionRange: insertionRange, includeCommitCharacters: false, completeFunctionCalls: false, // Add on any completion-kind-specific resolution data that will be // used during resolve() calls to provide additional information. resolutionData: item.kind == CompletionSuggestionKind.PACKAGE_NAME ? PubPackageCompletionItemResolutionInfo( - file: path, - offset: offset, // The completion for package names may contain a trailing // ': ' for convenience, so if it's there, trim it off. packageName: item.completion.split(':').first, @@ -462,7 +477,7 @@ class CompletionHandler ), ) .toList(); - return success(completionItems); + return success(CompletionList(isIncomplete: false, items: completionItems)); } /// Returns true if [node] is part of an invocation and already has an argument @@ -495,18 +510,23 @@ class CompletionHandler List pluginResults, ) { return pluginResults.expand((result) { + final insertLength = _computeInsertLength( + offset, + result.replacementOffset, + result.replacementLength, + ); + final replacementRange = + toRange(lineInfo, result.replacementOffset, result.replacementLength); + final insertionRange = + toRange(lineInfo, result.replacementOffset, insertLength); + return result.results.map( (item) => toCompletionItem( capabilities, lineInfo, item, - result.replacementOffset, - _computeInsertLength( - offset, - result.replacementOffset, - result.replacementLength, - ), - result.replacementLength, + replacementRange: replacementRange, + insertionRange: insertionRange, // Plugins cannot currently contribute commit characters and we should // not assume that the Dart ones would be correct for all of their // completions. 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 72903a854ee..d2d2beca198 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 @@ -2,16 +2,19 @@ // 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/lsp_protocol/protocol_custom_generated.dart'; +import 'package:analysis_server/lsp_protocol/protocol_custom_generated.dart' + hide Element; import 'package:analysis_server/lsp_protocol/protocol_generated.dart'; import 'package:analysis_server/lsp_protocol/protocol_special.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/lsp_analysis_server.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' as analyzer; +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/source/line_info.dart'; import 'package:analyzer/src/util/comment.dart' as analyzer; import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; @@ -39,8 +42,8 @@ class CompletionResolveHandler ) async { final resolutionInfo = item.data; - if (resolutionInfo is DartCompletionItemResolutionInfo) { - return resolveDartCompletion(item, resolutionInfo, token); + if (resolutionInfo is DartSuggestionSetCompletionItemResolutionInfo) { + return resolveDartSuggestionSetCompletion(item, resolutionInfo, token); } else if (resolutionInfo is PubPackageCompletionItemResolutionInfo) { return resolvePubPackageCompletion(item, resolutionInfo, token); } else { @@ -50,51 +53,14 @@ class CompletionResolveHandler Future> resolveDartCompletion( CompletionItem item, - DartCompletionItemResolutionInfo 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 lineInfo = server.getLineInfo(file); - if (lineInfo == null) { - return error( - ErrorCodes.InternalError, - 'Line info not available for $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.libId); - if (library == null) { - return error( - ErrorCodes.InvalidParams, - 'Library ID is not valid: ${data.libId}', - data.libId.toString(), - ); - } - - // If filterText is different to the label, it's because label has parens/args - // appended and we should take the basic label. We cannot use insertText as - // it may include snippets, whereas filterText is always just the pure string. - var requestedName = item.filterText ?? item.label; - // The label might be `MyEnum.myValue`, but we import only `MyEnum`. - if (requestedName.contains('.')) { - requestedName = requestedName.substring( - 0, - requestedName.indexOf('.'), - ); - } - + LspClientCapabilities clientCapabilities, + LineInfo lineInfo, + CancellationToken token, { + required String file, + required Uri libraryUri, + required Range insertionRange, + required Range replacementRange, + }) async { const timeout = Duration(milliseconds: 1000); var timer = Stopwatch()..start(); _latestCompletionItem = item; @@ -110,38 +76,23 @@ class CompletionResolveHandler return cancelled(); } - analyzer.LibraryElement requestedLibraryElement; - { - final result = await session.getLibraryByUri(library.uriStr); - if (result is LibraryElementResult) { - requestedLibraryElement = result.element; - } else { - return error( - ErrorCodes.InvalidParams, - 'Invalid library URI: ${library.uriStr}', - '${result.runtimeType}', - ); - } + final element = await _getElement(session, libraryUri, item); + if (element == null) { + return error( + ErrorCodes.InvalidParams, + 'No such element: ${item.label} in $libraryUri', + item.label, + ); } if (token.isCancellationRequested) { return cancelled(); } - var requestedElement = - requestedLibraryElement.exportNamespace.get(requestedName); - if (requestedElement == null) { - return error( - ErrorCodes.InvalidParams, - 'No such element: $requestedName in ${library.uriStr}', - requestedName, - ); - } - var newInsertText = item.insertText ?? item.label; final builder = ChangeBuilder(session: session); await builder.addDartFileEdit(file, (builder) { - final result = builder.importLibraryElement(library.uri); + final result = builder.importLibraryElement(libraryUri); if (result.prefix != null) { newInsertText = '${result.prefix}.$newInsertText'; } @@ -169,21 +120,30 @@ class CompletionResolveHandler arguments: [workspaceEdit]); } - // Documentation is added on during resolve for LSP. final formats = clientCapabilities.completionDocumentationFormats; - final supportsInsertReplace = - clientCapabilities.insertReplaceCompletionRanges; final dartDoc = - analyzer.getDartDocPlainText(requestedElement.documentationComment); + analyzer.getDartDocPlainText(element.documentationComment); final documentation = dartDoc != null ? asStringOrMarkupContent(formats, dartDoc) : null; + final supportsInsertReplace = + clientCapabilities.insertReplaceCompletionRanges; + + // If the only URI we have is a file:// URI, display it as relative to + // the file we're importing into, rather than the full URI. + final pathContext = server.resourceProvider.pathContext; + final autoImportDisplayUri = libraryUri.isScheme('file') + ? pathContext.relative( + libraryUri.toFilePath(), + from: pathContext.dirname(file), + ) + : libraryUri.toString(); return success(CompletionItem( label: item.label, kind: item.kind, tags: item.tags, detail: thisFilesChanges.isNotEmpty - ? "Auto import from '${data.displayUri}'\n\n${item.detail ?? ''}" + ? "Auto import from '$autoImportDisplayUri'\n\n${item.detail ?? ''}" .trim() : item.detail, documentation: documentation, @@ -193,17 +153,17 @@ class CompletionResolveHandler filterText: item.filterText, insertText: newInsertText, insertTextFormat: item.insertTextFormat, - textEdit: supportsInsertReplace && data.iLength != data.rLength + textEdit: supportsInsertReplace && insertionRange != replacementRange ? Either2.t2( InsertReplaceEdit( - insert: toRange(lineInfo, data.rOffset, data.iLength), - replace: toRange(lineInfo, data.rOffset, data.rLength), + insert: insertionRange, + replace: replacementRange, newText: newInsertText, ), ) : Either2.t1( TextEdit( - range: toRange(lineInfo, data.rOffset, data.rLength), + range: replacementRange, newText: newInsertText, ), ), @@ -229,6 +189,52 @@ class CompletionResolveHandler ); } + Future> resolveDartSuggestionSetCompletion( + CompletionItem item, + DartSuggestionSetCompletionItemResolutionInfo 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 lineInfo = server.getLineInfo(file); + if (lineInfo == null) { + return error( + ErrorCodes.InternalError, + 'Line info not available for $file', + null, + ); + } + + var library = server.declarationsTracker?.getLibrary(data.libId); + if (library == null) { + return error( + ErrorCodes.InvalidParams, + 'Library ID is not valid: ${data.libId}', + data.libId.toString(), + ); + } + + final insertionRange = toRange(lineInfo, data.rOffset, data.iLength); + final replacementRange = toRange(lineInfo, data.rOffset, data.rLength); + + return resolveDartCompletion( + item, + clientCapabilities, + lineInfo, + token, + file: file, + libraryUri: library.uri, + insertionRange: insertionRange, + replacementRange: replacementRange, + ); + } + Future> resolvePubPackageCompletion( CompletionItem item, PubPackageCompletionItemResolutionInfo data, @@ -265,4 +271,31 @@ class CompletionResolveHandler data: item.data, )); } + + /// Gets the [Element] for the completion item [item] in [libraryUri]. + Future _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. + + 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/lsp/handlers/handler_states.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_states.dart index 0d9eb196b15..abc53cf98ec 100644 --- a/pkg/analysis_server/lib/src/lsp/handlers/handler_states.dart +++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_states.dart @@ -61,6 +61,7 @@ class InitializedStateMessageHandler extends ServerStateMessageHandler { InitializedStateMessageHandler( LspAnalysisServer server, ) : super(server) { + final options = server.initializationOptions; reject(Method.initialize, ServerErrorCodes.ServerAlreadyInitialized, 'Server already initialized'); reject(Method.initialized, ServerErrorCodes.ServerAlreadyInitialized, @@ -75,13 +76,10 @@ class InitializedStateMessageHandler extends ServerStateMessageHandler { TextDocumentCloseHandler(server), ); registerHandler(HoverHandler(server)); - registerHandler(CompletionHandler( - server, - server.initializationOptions.suggestFromUnimportedLibraries, - )); + registerHandler(CompletionHandler(server, options)); + registerHandler(CompletionResolveHandler(server)); registerHandler(DocumentColorHandler(server)); registerHandler(DocumentColorPresentationHandler(server)); - registerHandler(CompletionResolveHandler(server)); registerHandler(SignatureHelpHandler(server)); registerHandler(DefinitionHandler(server)); registerHandler(SuperHandler(server)); @@ -94,12 +92,8 @@ class InitializedStateMessageHandler extends ServerStateMessageHandler { registerHandler(DocumentSymbolHandler(server)); registerHandler(CodeActionHandler(server)); registerHandler(ExecuteCommandHandler(server)); - registerHandler( - WorkspaceFoldersHandler( - server, - !server.initializationOptions.onlyAnalyzeProjectsWithOpenFiles, - ), - ); + registerHandler(WorkspaceFoldersHandler( + server, !options.onlyAnalyzeProjectsWithOpenFiles)); registerHandler(PrepareRenameHandler(server)); registerHandler(RenameHandler(server)); registerHandler(FoldingHandler(server)); diff --git a/pkg/analysis_server/lib/src/lsp/mapping.dart b/pkg/analysis_server/lib/src/lsp/mapping.dart index 305bc674299..c182eeef082 100644 --- a/pkg/analysis_server/lib/src/lsp/mapping.dart +++ b/pkg/analysis_server/lib/src/lsp/mapping.dart @@ -418,7 +418,7 @@ lsp.CompletionItem declarationToCompletionItem( ? InsertTextMode.asIs : null, // data, used for completionItem/resolve. - data: lsp.DartCompletionItemResolutionInfo( + data: lsp.DartSuggestionSetCompletionItemResolutionInfo( file: file, offset: offset, libId: includedSuggestionSet.id, @@ -1007,10 +1007,9 @@ lsp.CodeActionKind toCodeActionKind( lsp.CompletionItem toCompletionItem( LspClientCapabilities capabilities, server.LineInfo lineInfo, - server.CompletionSuggestion suggestion, - int replacementOffset, - int insertLength, - int replacementLength, { + server.CompletionSuggestion suggestion, { + Range? replacementRange, + Range? insertionRange, required bool includeCommitCharacters, required bool completeFunctionCalls, CompletionItemResolutionInfo? resolutionData, @@ -1127,20 +1126,22 @@ lsp.CompletionItem toCompletionItem( insertTextMode: supportsAsIsInsertMode && isMultilineCompletion ? InsertTextMode.asIs : null, - textEdit: supportsInsertReplace && insertLength != replacementLength - ? Either2.t2( - InsertReplaceEdit( - insert: toRange(lineInfo, replacementOffset, insertLength), - replace: toRange(lineInfo, replacementOffset, replacementLength), - newText: insertText, - ), - ) - : Either2.t1( - TextEdit( - range: toRange(lineInfo, replacementOffset, replacementLength), - newText: insertText, - ), - ), + textEdit: (insertionRange == null || replacementRange == null) + ? null + : supportsInsertReplace && insertionRange != replacementRange + ? Either2.t2( + InsertReplaceEdit( + insert: insertionRange, + replace: replacementRange, + newText: insertText, + ), + ) + : Either2.t1( + TextEdit( + range: replacementRange, + newText: insertText, + ), + ), ); } diff --git a/pkg/analysis_server/test/lsp/completion_dart_test.dart b/pkg/analysis_server/test/lsp/completion_dart_test.dart index cbacc55f5a2..01f2b097ef7 100644 --- a/pkg/analysis_server/test/lsp/completion_dart_test.dart +++ b/pkg/analysis_server/test/lsp/completion_dart_test.dart @@ -1268,7 +1268,7 @@ void f() { } expect(res.any((c) => c.label == 'UniqueNamedClassForLspThree'), isTrue); } - Future test_suggestionSets() async { + Future test_unimportedSymbols() async { newFile( join(projectFolderPath, 'other_file.dart'), content: ''' @@ -1342,7 +1342,7 @@ void f() { } Future - test_suggestionSets_doesNotDuplicate_importedViaMultipleLibraries() async { + test_unimportedSymbols_doesNotDuplicate_importedViaMultipleLibraries() async { // An item that's already imported through multiple libraries that // export it should not result in multiple entries. newFile( @@ -1387,7 +1387,7 @@ void f() { } Future - test_suggestionSets_doesNotDuplicate_importedViaSingleLibrary() async { + test_unimportedSymbols_doesNotDuplicate_importedViaSingleLibrary() async { // An item that's already imported through a library that exports it // should not result in multiple entries. newFile( @@ -1430,7 +1430,7 @@ void f() { expect(completions, hasLength(1)); } - Future test_suggestionSets_doesNotFilterSymbolsWithSameName() async { + Future test_unimportedSymbols_doesNotFilterSymbolsWithSameName() async { // Classes here are not re-exports, so should not be filtered out. newFile( join(projectFolderPath, 'source_file1.dart'), @@ -1472,7 +1472,7 @@ void f() { expectAutoImportCompletion(resolvedCompletions, '../source_file3.dart'); } - Future test_suggestionSets_enumValues() async { + Future test_unimportedSymbols_enumValues() async { newFile( join(projectFolderPath, 'source_file.dart'), content: ''' @@ -1536,7 +1536,7 @@ void f() { ''')); } - Future test_suggestionSets_enumValuesAlreadyImported() async { + Future test_unimportedSymbols_enumValuesAlreadyImported() async { newFile( join(projectFolderPath, 'lib', 'source_file.dart'), content: ''' @@ -1577,10 +1577,10 @@ void f() { expect(completions, hasLength(1)); final resolved = await resolveCompletion(completions.first); // It should not include auto-import text since it's already imported. - expect(resolved.detail, isNull); + expect(resolved.detail, isNot(contains('Auto import from'))); } - Future test_suggestionSets_filtersOutAlreadyImportedSymbols() async { + Future test_unimportedSymbols_filtersOutAlreadyImportedSymbols() async { newFile( join(projectFolderPath, 'lib', 'source_file.dart'), content: ''' @@ -1623,7 +1623,7 @@ void f() { expect(resolved.detail, isNull); } - Future test_suggestionSets_importsPackageUri() async { + Future test_unimportedSymbols_importsPackageUri() async { newFile( join(projectFolderPath, 'lib', 'my_class.dart'), content: 'class MyClass {}', @@ -1654,7 +1654,7 @@ void f() { } Future - test_suggestionSets_includesReexportedSymbolsForEachFile() async { + test_unimportedSymbols_includesReexportedSymbolsForEachFile() async { newFile( join(projectFolderPath, 'source_file.dart'), content: ''' @@ -1700,7 +1700,7 @@ void f() { expectAutoImportCompletion(resolvedCompletions, '../reexport2.dart'); } - Future test_suggestionSets_insertReplaceRanges() async { + Future test_unimportedSymbols_insertReplaceRanges() async { newFile( join(projectFolderPath, 'other_file.dart'), content: ''' @@ -1792,7 +1792,7 @@ void f() { ''')); } - Future test_suggestionSets_insertsIntoPartFiles() async { + Future test_unimportedSymbols_insertsIntoPartFiles() async { // File we'll be adding an import for. newFile( join(projectFolderPath, 'other_file.dart'), @@ -1883,7 +1883,7 @@ import '../other_file.dart'; part 'main.dart';''')); } - Future test_suggestionSets_members() async { + Future test_unimportedSymbols_members() async { newFile( join(projectFolderPath, 'source_file.dart'), content: ''' @@ -1961,7 +1961,7 @@ void f() { /// (as it always used 0 as the modification stamp) which would prevent /// completion including items from files that were open (had overlays). /// https://github.com/Dart-Code/Dart-Code/issues/2286#issuecomment-658597532 - Future test_suggestionSets_modifiedFiles() async { + Future test_unimportedSymbols_modifiedFiles() async { final otherFilePath = join(projectFolderPath, 'lib', 'other_file.dart'); final otherFileUri = Uri.file(otherFilePath); @@ -1991,7 +1991,7 @@ void f() { expect(matching, hasLength(1)); } - Future test_suggestionSets_namedConstructors() async { + Future test_unimportedSymbols_namedConstructors() async { newFile( join(projectFolderPath, 'other_file.dart'), content: ''' @@ -2050,7 +2050,8 @@ void f() { ''')); } - Future test_suggestionSets_preferRelativeImportsLib_insideLib() async { + Future + test_unimportedSymbols_preferRelativeImportsLib_insideLib() async { _enableLints([LintNames.prefer_relative_imports]); final importingFilePath = join(projectFolderPath, 'lib', 'nested1', 'main.dart'); @@ -2085,7 +2086,8 @@ void f() { ); } - Future test_suggestionSets_preferRelativeImportsLib_outsideLib() async { + Future + test_unimportedSymbols_preferRelativeImportsLib_outsideLib() async { // Files outside of the lib folder should still get absolute imports to // files inside lib, even with the lint enabled. _enableLints([LintNames.prefer_relative_imports]); @@ -2122,7 +2124,7 @@ void f() { ); } - Future test_suggestionSets_unavailableIfDisabled() async { + Future test_unimportedSymbols_unavailableIfDisabled() async { newFile( join(projectFolderPath, 'other_file.dart'), content: 'class InOtherFile {}', @@ -2137,7 +2139,9 @@ void f() { final initialAnalysis = waitForAnalysisComplete(); // Support applyEdit, but explicitly disable the suggestions. await initialize( - initializationOptions: {'suggestFromUnimportedLibraries': false}, + initializationOptions: { + 'suggestFromUnimportedLibraries': false, + }, workspaceCapabilities: withApplyEditSupport(emptyWorkspaceClientCapabilities), ); @@ -2151,7 +2155,7 @@ void f() { expect(completion, isNull); } - Future test_suggestionSets_unavailableWithoutApplyEdit() async { + Future test_unimportedSymbols_unavailableWithoutApplyEdit() async { // If client doesn't advertise support for workspace/applyEdit, we won't // include suggestion sets. newFile( diff --git a/pkg/analysis_server/test/lsp/server_abstract.dart b/pkg/analysis_server/test/lsp/server_abstract.dart index 851966ef04e..4bc57a023e9 100644 --- a/pkg/analysis_server/test/lsp/server_abstract.dart +++ b/pkg/analysis_server/test/lsp/server_abstract.dart @@ -1055,6 +1055,12 @@ mixin LspAnalysisServerTestMixin implements ClientCapabilitiesHelperMixin { } Future> getCompletion(Uri uri, Position pos, + {CompletionContext? context}) async { + final response = await getCompletionList(uri, pos, context: context); + return response.items; + } + + Future getCompletionList(Uri uri, Position pos, {CompletionContext? context}) { final request = makeRequest( Method.textDocument_completion, @@ -1064,8 +1070,7 @@ mixin LspAnalysisServerTestMixin implements ClientCapabilitiesHelperMixin { position: pos, ), ); - return expectSuccessfulResponseTo( - request, _fromJsonList(CompletionItem.fromJson)); + return expectSuccessfulResponseTo(request, CompletionList.fromJson); } Future, List>> getDefinition( diff --git a/pkg/analysis_server/tool/lsp_spec/generate_all.dart b/pkg/analysis_server/tool/lsp_spec/generate_all.dart index f1783c251a8..58a60e23c3b 100644 --- a/pkg/analysis_server/tool/lsp_spec/generate_all.dart +++ b/pkg/analysis_server/tool/lsp_spec/generate_all.dart @@ -251,17 +251,17 @@ List getCustomClasses() { ], ), interface( + // Used as a base class for all resolution data classes. 'CompletionItemResolutionInfo', - [ - field('file', type: 'string'), - field('offset', type: 'int'), - ], + [], ), interface( - 'DartCompletionItemResolutionInfo', + 'DartSuggestionSetCompletionItemResolutionInfo', [ // These fields have short-ish names because they're on the payload // for all suggestion-set backed completions. + field('file', type: 'string'), + field('offset', type: 'int'), field('libId', type: 'int'), field('displayUri', type: 'string'), field('rOffset', type: 'int'), // replacementOffset