diff --git a/pkg/analysis_server/lib/src/legacy_analysis_server.dart b/pkg/analysis_server/lib/src/legacy_analysis_server.dart index 475186542005..5ab95bd8196c 100644 --- a/pkg/analysis_server/lib/src/legacy_analysis_server.dart +++ b/pkg/analysis_server/lib/src/legacy_analysis_server.dart @@ -76,6 +76,7 @@ import 'package:analysis_server/src/handler/legacy/server_shutdown.dart'; import 'package:analysis_server/src/handler/legacy/unsupported_request.dart'; import 'package:analysis_server/src/lsp/client_capabilities.dart' as lsp; import 'package:analysis_server/src/lsp/client_configuration.dart' as lsp; +import 'package:analysis_server/src/lsp/constants.dart' as lsp; import 'package:analysis_server/src/lsp/handlers/handler_states.dart'; import 'package:analysis_server/src/operation/operation_analysis.dart'; import 'package:analysis_server/src/plugin/notification_manager.dart'; @@ -716,6 +717,51 @@ class LegacyAnalysisServer extends AnalysisServer { ); } + /// Sends an LSP request to the server (wrapped in 'lsp.handle') and unwraps + /// the LSP response from the result of the legacy response. + Future sendLspRequest( + lsp.Method method, + Object params, + ) async { + var id = nextServerRequestId++; + + // Build the complete LSP RequestMessage to send. + var lspMessage = lsp.RequestMessage( + id: lsp.Either2.t1(id), + jsonrpc: lsp.jsonRpcVersion, + method: method, + params: params, + ); + + // Wrap the LSP message inside a call to lsp.handle. + var response = await sendRequest( + Request( + id.toString(), + LSP_REQUEST_HANDLE, + LspHandleParams(lspMessage).toJson(clientUriConverter: uriConverter), + ), + ); + + // Unwrap the LSP response from the legacy response. + var result = LspHandleResult.fromResponse( + response, + clientUriConverter: uriConverter, + ); + var lspResponse = result.lspResponse; + + return lspResponse is Map + ? lsp.ResponseMessage.fromJson(lspResponse) + : lsp.ResponseMessage( + jsonrpc: lsp.jsonRpcVersion, + error: lsp.ResponseError( + code: lsp.ServerErrorCodes.UnhandledError, + message: + "The client responded to a '$method' LSP request but" + ' did not include a valid response in the lspResponse field', + ), + ); + } + /// Send the given [notification] to the client. void sendNotification(Notification notification) { channel.sendNotification(notification); diff --git a/pkg/analysis_server/lib/src/lsp/handlers/commands/simple_edit_handler.dart b/pkg/analysis_server/lib/src/lsp/handlers/commands/simple_edit_handler.dart index df7bad886c77..245904202c73 100644 --- a/pkg/analysis_server/lib/src/lsp/handlers/commands/simple_edit_handler.dart +++ b/pkg/analysis_server/lib/src/lsp/handlers/commands/simple_edit_handler.dart @@ -67,7 +67,7 @@ abstract class SimpleEditCommandHandler ) async { // Send the edit to the client via a applyEdit request (this is a request // from server -> client and the client will provide a response). - var editResponse = await server.sendRequest( + var editResponse = await server.sendLspRequest( Method.workspace_applyEdit, ApplyWorkspaceEditParams(label: commandName, edit: workspaceEdit), ); diff --git a/pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_edit_argument.dart b/pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_edit_argument.dart index 3c55664fb465..5a8a1991f1b7 100644 --- a/pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_edit_argument.dart +++ b/pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_edit_argument.dart @@ -295,7 +295,7 @@ class EditArgumentHandler extends SharedMessageHandler } var editDescription = 'Edit argument'; - var editResponse = await server.sendRequest( + var editResponse = await server.sendLspRequest( Method.workspace_applyEdit, ApplyWorkspaceEditParams(label: editDescription, edit: workspaceEdit), ); diff --git a/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart b/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart index 1bf225d9ac2b..0f3ce258fb6e 100644 --- a/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart +++ b/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart @@ -329,7 +329,7 @@ class LspAnalysisServer extends AnalysisServer { // Fetch all configuration we care about from the client. This is just // "dart" for now, but in future this may be extended to include // others (for example "flutter"). - var response = await sendRequest( + var response = await sendLspRequest( Method.workspace_configuration, ConfigurationParams( items: [ @@ -864,7 +864,7 @@ class LspAnalysisServer extends AnalysisServer { /// Sends a request with the given [params] to the client and wait for a /// response. Completes with the raw [ResponseMessage] which could be an /// error response. - Future sendRequest(Method method, Object params) { + Future sendLspRequest(Method method, Object params) { var requestId = nextRequestId++; var completer = Completer(); completers[requestId] = completer; @@ -1020,7 +1020,7 @@ class LspAnalysisServer extends AnalysisServer { List actions, ) async { assert(supportsShowMessageRequest); - var response = await sendRequest( + var response = await sendLspRequest( Method.window_showMessageRequest, ShowMessageRequestParams( type: type.forLsp, diff --git a/pkg/analysis_server/lib/src/lsp/progress.dart b/pkg/analysis_server/lib/src/lsp/progress.dart index 9d25d53b72cc..43c53df6fe4b 100644 --- a/pkg/analysis_server/lib/src/lsp/progress.dart +++ b/pkg/analysis_server/lib/src/lsp/progress.dart @@ -63,7 +63,7 @@ class _ServerCreatedProgressReporter extends _TokenProgressReporter { // begin is sent (which could happen because create is async), end will // not be sent/return too early. _tokenBeginRequest = _server - .sendRequest( + .sendLspRequest( Method.window_workDoneProgress_create, WorkDoneProgressCreateParams(token: _token), ) diff --git a/pkg/analysis_server/lib/src/lsp/server_capabilities_computer.dart b/pkg/analysis_server/lib/src/lsp/server_capabilities_computer.dart index f9aa2b076593..e3b91329aeed 100644 --- a/pkg/analysis_server/lib/src/lsp/server_capabilities_computer.dart +++ b/pkg/analysis_server/lib/src/lsp/server_capabilities_computer.dart @@ -305,7 +305,7 @@ class ServerCapabilitiesComputer { // we cannot re-enter this method until we have sent both the unregister // and register requests to the client atomically. // https://github.com/dart-lang/sdk/issues/47851#issuecomment-988093109 - unregistrationRequest = _server.sendRequest( + unregistrationRequest = _server.sendLspRequest( Method.client_unregisterCapability, UnregistrationParams(unregisterations: unregistrations), ); @@ -316,7 +316,7 @@ class ServerCapabilitiesComputer { // otherwise we don't know that the client supports registerCapability). if (registrationsToAdd.isNotEmpty) { registrationRequest = _server - .sendRequest( + .sendLspRequest( Method.client_registerCapability, RegistrationParams(registrations: registrationsToAdd), ) diff --git a/pkg/analysis_server/lib/src/utilities/mocks.dart b/pkg/analysis_server/lib/src/utilities/mocks.dart index 7de4355e2924..128338e4aecc 100644 --- a/pkg/analysis_server/lib/src/utilities/mocks.dart +++ b/pkg/analysis_server/lib/src/utilities/mocks.dart @@ -18,16 +18,31 @@ import 'package:watcher/watcher.dart'; class MockServerChannel implements ServerCommunicationChannel { /// A controller for the stream of requests and responses from the client to /// the server. + /// + /// Messages added to this stream should be converted to/from JSON to ensure + /// they are fully serialized/deserialized as they would be in a real server + /// otherwise tests may receive real instances where in reality they would be + /// maps. StreamController requestController = StreamController(); /// A controller for the stream of requests and responses from the server to /// the client. + /// + /// Messages added to this stream should be converted to/from JSON to ensure + /// they are fully serialized/deserialized as they would be in a real server + /// otherwise tests may receive real instances where in reality they would be + /// maps. StreamController responseController = StreamController.broadcast(); /// A controller for the stream of notifications from the server to the /// client. + /// + /// Unlike [requestController] and [responseController], notifications added + /// here are not round-tripped through JSON but instead have real class + /// instances as their `params`. This is because they are only used by tests + /// which will cast and/or switch on the types for convenience. StreamController notificationController = StreamController.broadcast(sync: true); @@ -67,7 +82,10 @@ class MockServerChannel implements ServerCommunicationChannel { if (_closed) { return; } + notificationsReceived.add(notification); + notificationController.add(notification); + var errorCompleter = this.errorCompleter; if (errorCompleter != null && notification.event == 'server.error') { var params = notification.params!; @@ -77,14 +95,17 @@ class MockServerChannel implements ServerCommunicationChannel { StackTrace.fromString(params['stackTrace'] as String), ); } - // Wrap send notification in future to simulate websocket - // TODO(scheglov): ask Dan why and decide what to do - // new Future(() => notificationController.add(notification)); - notificationController.add(notification); } @override void sendRequest(Request request) { + // Round-trip via JSON to ensure all types are fully serialized as they + // would be in a real setup. + request = + Request.fromJson( + jsonDecode(jsonEncode(request.toJson())) as Map, + )!; + serverRequestsSent.add(request); responseController.add(request); } @@ -95,9 +116,16 @@ class MockServerChannel implements ServerCommunicationChannel { if (_closed) { return; } + + // Round-trip via JSON to ensure all types are fully serialized as they + // would be in a real setup. + response = + Response.fromJson( + jsonDecode(jsonEncode(response.toJson())) as Map, + )!; + responsesReceived.add(response); - // Wrap send response in future to simulate WebSocket. - Future(() => responseController.add(response)); + responseController.add(response); } /// Send the given [request] to the server as if it had been sent from the @@ -117,8 +145,7 @@ class MockServerChannel implements ServerCommunicationChannel { jsonDecode(jsonEncode(request)) as Map, )!; - // Wrap send request in future to simulate WebSocket. - unawaited(Future(() => requestController.add(request))); + requestController.add(request); var response = await waitForResponse(request); // Round-trip via JSON to ensure all types are fully serialized as they @@ -133,13 +160,20 @@ class MockServerChannel implements ServerCommunicationChannel { /// Send the given [response] to the server as if it had been sent from the /// client. - Future simulateResponseFromClient(Response response) { + void simulateResponseFromClient(Response response) { // No further requests should be sent after the connection is closed. if (_closed) { throw Exception('simulateRequestFromClient after connection closed'); } - // Wrap send request in future to simulate WebSocket. - return Future(() => requestController.add(response)); + + // Round-trip via JSON to ensure all types are fully serialized as they + // would be in a real setup. + response = + Response.fromJson( + jsonDecode(jsonEncode(response)) as Map, + )!; + + requestController.add(response); } /// Return a future that will complete when a response associated with the diff --git a/pkg/analysis_server/test/domain_analysis_test.dart b/pkg/analysis_server/test/domain_analysis_test.dart index eee3359deba0..31dbbfb30737 100644 --- a/pkg/analysis_server/test/domain_analysis_test.dart +++ b/pkg/analysis_server/test/domain_analysis_test.dart @@ -1787,6 +1787,7 @@ analyzer: newFile(path, ''); await setRoots(included: [workspaceRootPath], excluded: []); + await waitForTasksFinished(); // No touch-screen. assertNotificationsText(r''' diff --git a/pkg/analysis_server/test/domain_server_test.dart b/pkg/analysis_server/test/domain_server_test.dart index b2698214c8a8..7f2d470d3e86 100644 --- a/pkg/analysis_server/test/domain_server_test.dart +++ b/pkg/analysis_server/test/domain_server_test.dart @@ -92,7 +92,7 @@ class ServerDomainTest extends PubPackageAnalysisServerTest { // Simulate the response. var request = serverChannel.serverRequestsSent[0]; - await serverChannel.simulateResponseFromClient( + serverChannel.simulateResponseFromClient( ServerOpenUrlRequestResult().toResponse( request.id, clientUriConverter: server.uriConverter, @@ -383,7 +383,7 @@ class ServerDomainTest extends PubPackageAnalysisServerTest { // Simulate the response. var request = serverChannel.serverRequestsSent[0]; - await serverChannel.simulateResponseFromClient( + serverChannel.simulateResponseFromClient( ServerShowMessageRequestResult( action: 'a', ).toResponse(request.id, clientUriConverter: server.uriConverter), @@ -405,7 +405,7 @@ class ServerDomainTest extends PubPackageAnalysisServerTest { // Simulate the response. var request = serverChannel.serverRequestsSent[0]; - await serverChannel.simulateResponseFromClient( + serverChannel.simulateResponseFromClient( ServerShowMessageRequestResult().toResponse( request.id, clientUriConverter: server.uriConverter, diff --git a/pkg/analysis_server/test/integration/server/message_scheduler_test.dart b/pkg/analysis_server/test/integration/server/message_scheduler_test.dart index 2e50de86e1ba..3bc4d591aedd 100644 --- a/pkg/analysis_server/test/integration/server/message_scheduler_test.dart +++ b/pkg/analysis_server/test/integration/server/message_scheduler_test.dart @@ -57,6 +57,7 @@ class LegacyServerMessageSchedulerTest extends PubPackageAnalysisServerTest { Future test_initialize() async { await setRoots(included: [workspaceRootPath], excluded: []); + await waitForTasksFinished(); _assertLogContents(messageScheduler, r''' Incoming LegacyMessage: analysis.setAnalysisRoots Entering process messages loop @@ -74,6 +75,7 @@ Exit process messages loop ).toRequest('0', clientUriConverter: server.uriConverter); futures.add(handleSuccessfulRequest(request)); await Future.wait(futures); + await waitForTasksFinished(); _assertLogContents(messageScheduler, r''' Incoming LegacyMessage: analysis.setAnalysisRoots Entering process messages loop diff --git a/pkg/analysis_server/test/lsp/edit_argument_test.dart b/pkg/analysis_server/test/lsp/edit_argument_test.dart index 5e2a9977bdf1..e7dac5aa8c8e 100644 --- a/pkg/analysis_server/test/lsp/edit_argument_test.dart +++ b/pkg/analysis_server/test/lsp/edit_argument_test.dart @@ -218,7 +218,7 @@ class ComputeStringValueTest { } @reflectiveTest -class EditArgumentTest extends AbstractLspAnalysisServerTest { +class EditArgumentTest extends SharedAbstractLspAnalysisServerTest { late TestCode code; @override @@ -848,14 +848,14 @@ const myConst = E.one; bool open = true, }) async { code = TestCode.parse(content); - newFile(mainFilePath, code.code); - await initialize(); + createFile(testFilePath, code.code); + await initializeServer(); if (open) { - await openFile(mainFileUri, code.code); + await openFile(testFileUri, code.code); } - await initialAnalysis; + await currentAnalysis; var verifier = await executeForEdits( - () => editArgument(mainFileUri, code.position.position, edit), + () => editArgument(testFileUri, code.position.position, edit), ); verifier.verifyFiles(expectedContent); @@ -885,12 +885,12 @@ class MyWidget extends StatelessWidget { '''; code = TestCode.parse(content); - newFile(mainFilePath, code.code); - await initialize(); - await initialAnalysis; + createFile(testFilePath, code.code); + await initializeServer(); + await currentAnalysis; await expectLater( - editArgument(mainFileUri, code.position.position, edit), + editArgument(testFileUri, code.position.position, edit), throwsA(isResponseError(ErrorCodes.RequestFailed, message: message)), ); } @@ -917,7 +917,7 @@ class MyWidget extends StatelessWidget { } '''; var expectedContent = ''' ->>>>>>>>>> lib/main.dart +>>>>>>>>>> lib/test.dart import 'package:flutter/widgets.dart'; $additionalCode diff --git a/pkg/analysis_server/test/lsp/server_abstract.dart b/pkg/analysis_server/test/lsp/server_abstract.dart index 1b67adaa77e6..6850f9027b8e 100644 --- a/pkg/analysis_server/test/lsp/server_abstract.dart +++ b/pkg/analysis_server/test/lsp/server_abstract.dart @@ -249,8 +249,8 @@ abstract class AbstractLspAnalysisServerTest _previousContextBuilds = server.contextBuilds; } - Future sendLspRequestToClient(Method method, Object params) { - return server.sendRequest(method, params); + Future sendLspRequest(Method method, Object params) { + return server.sendLspRequest(method, params); } @override @@ -1707,5 +1707,6 @@ abstract class SharedAbstractLspAnalysisServerTest @override Future initializeServer() async { await initialize(); + await currentAnalysis; } } diff --git a/pkg/analysis_server/test/lsp/workspace_apply_edit_test.dart b/pkg/analysis_server/test/lsp/workspace_apply_edit_test.dart index 2d6165247a92..4b114d15bb83 100644 --- a/pkg/analysis_server/test/lsp/workspace_apply_edit_test.dart +++ b/pkg/analysis_server/test/lsp/workspace_apply_edit_test.dart @@ -19,12 +19,6 @@ class WorkspaceApplyEditTest extends SharedAbstractLspAnalysisServerTest // Tests are defined in SharedWorkspaceApplyEditTests because they // are shared and run for both LSP and Legacy servers. SharedWorkspaceApplyEditTests { - @override - Future initializeServer() async { - await initialize(); - await currentAnalysis; - } - @override Future setUp() async { super.setUp(); diff --git a/pkg/analysis_server/test/lsp_over_legacy/abstract_lsp_over_legacy.dart b/pkg/analysis_server/test/lsp_over_legacy/abstract_lsp_over_legacy.dart index b9c753b61d61..7c770e337aac 100644 --- a/pkg/analysis_server/test/lsp_over_legacy/abstract_lsp_over_legacy.dart +++ b/pkg/analysis_server/test/lsp_over_legacy/abstract_lsp_over_legacy.dart @@ -183,7 +183,15 @@ abstract class LspOverLegacyTest extends PubPackageAnalysisServerTest Stream get requestsFromServer => serverChannel .serverToClientRequests .where((request) => request.method == LSP_REQUEST_HANDLE) - .map((request) => RequestMessage.fromJson(request.params)); + .map((request) { + var params = LspHandleParams.fromRequest( + request, + clientUriConverter: uriConverter, + ); + return RequestMessage.fromJson( + params.lspMessage as Map, + ); + }); /// The URI for the macro-generated content for [testFileUri]. Uri get testFileMacroUri => @@ -338,6 +346,10 @@ abstract class LspOverLegacyTest extends PubPackageAnalysisServerTest await handleSuccessfulRequest(request); } + Future sendLspRequest(Method method, Object params) { + return server.sendLspRequest(method, params); + } + @override void sendResponseToServer(ResponseMessage response) { serverChannel.simulateResponseFromClient( @@ -347,7 +359,9 @@ abstract class LspOverLegacyTest extends PubPackageAnalysisServerTest // A client-provided response to an LSP reverse-request is always // a full LSP result payload as the "result". The legacy request should // always succeed and any errors handled as LSP error responses within. - result: response.toJson(), + result: LspHandleResult( + response, + ).toJson(clientUriConverter: uriConverter), ), ); } @@ -399,6 +413,11 @@ abstract class SharedLspOverLegacyTest extends LspOverLegacyTest newFile(path, content); } + @override + Future initializeServer() async { + await waitForTasksFinished(); + } + @override Future openFile(Uri uri, String content, {int version = 1}) async { // TODO(dantup): Add version here when legacy protocol supports. @@ -430,35 +449,4 @@ abstract class SharedLspOverLegacyTest extends LspOverLegacyTest // For legacy, we can use addOverlay to replace the whole file. await addOverlay(fromUri(uri), content); } - - /// Wraps an LSP request up and sends it from the server to the client. - Future sendLspRequestToClient( - Method method, - Object params, - ) async { - var id = server.nextServerRequestId++; - // Round-trip through JSON to ensure everything becomes basic types and we - // don't have instances of classes like `Either2<>` in the JSON. - var lspRequest = - jsonDecode( - jsonEncode( - RequestMessage( - id: Either2.t1(id), - jsonrpc: jsonRpcVersion, - method: method, - params: params, - ), - ), - ) - as Map; - var legacyResponse = await server.sendRequest( - Request(id.toString(), LSP_REQUEST_HANDLE, lspRequest), - ); - - // Round-trip through JSON to ensure everything becomes basic types and we - // don't have instances of classes like `Either2<>` in the JSON. - var lspResponse = - jsonDecode(jsonEncode(legacyResponse.result)) as Map; - return ResponseMessage.fromJson(lspResponse); - } } diff --git a/pkg/analysis_server/test/lsp_over_legacy/editable_arguments_test.dart b/pkg/analysis_server/test/lsp_over_legacy/editable_arguments_test.dart index d12ff0665942..4944fd84fbfe 100644 --- a/pkg/analysis_server/test/lsp_over_legacy/editable_arguments_test.dart +++ b/pkg/analysis_server/test/lsp_over_legacy/editable_arguments_test.dart @@ -22,11 +22,6 @@ class EditableArgumentsTest extends SharedLspOverLegacyTest // Tests are defined in SharedEditableArgumentsTests because they // are shared and run for both LSP and Legacy servers. SharedEditableArgumentsTests { - @override - Future initializeServer() async { - await waitForTasksFinished(); - } - @override Future setUp() async { await super.setUp(); diff --git a/pkg/analysis_server/test/lsp_over_legacy/workspace_apply_edit_test.dart b/pkg/analysis_server/test/lsp_over_legacy/workspace_apply_edit_test.dart index 45495f5f5f7c..071f82ae2a8c 100644 --- a/pkg/analysis_server/test/lsp_over_legacy/workspace_apply_edit_test.dart +++ b/pkg/analysis_server/test/lsp_over_legacy/workspace_apply_edit_test.dart @@ -21,7 +21,7 @@ class WorkspaceApplyEditTest extends SharedLspOverLegacyTest SharedWorkspaceApplyEditTests { @override Future initializeServer() async { - await waitForTasksFinished(); + await super.initializeServer(); await sendClientCapabilities(); } diff --git a/pkg/analysis_server/test/shared/shared_workspace_apply_edit_tests.dart b/pkg/analysis_server/test/shared/shared_workspace_apply_edit_tests.dart index 3cb3a19afb04..ced3997b395d 100644 --- a/pkg/analysis_server/test/shared/shared_workspace_apply_edit_tests.dart +++ b/pkg/analysis_server/test/shared/shared_workspace_apply_edit_tests.dart @@ -31,7 +31,7 @@ mixin SharedWorkspaceApplyEditTests /// Overridden by test subclasses to send LSP requests from the server to /// the client. - Future sendLspRequestToClient(Method method, Object params); + Future sendLspRequest(Method method, Object params); test_applyEdit_existingFile() async { var code = TestCode.parse(''' @@ -147,7 +147,7 @@ inserted<<<<<<<<<< ApplyWorkspaceEditResult? receivedApplyEditResult; var verifier = await executeForEdits(() async { - var result = await sendLspRequestToClient( + var result = await sendLspRequest( Method.workspace_applyEdit, ApplyWorkspaceEditParams(edit: workspaceEdit), );