From b069a139164339a6c78e872e2f454daf81211a25 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Sat, 18 Jan 2025 08:37:49 -0800 Subject: [PATCH 1/4] [analyzer] Use `TypeImpl` for expression types. The type of `ExpressionImpl.staticType` is changed from `DartType?` to `TypeImpl?`, and the type of `ExpressionExtension.typeOrThrow` is changed from `DartType` to `TypeImpl`. To reduce the amount of casting required by these changes, a few other changes are included: - An additional extension `ExpressionImplExtension.typeOrThrow` is added; this has the same behavior as `ExpressionExtension.typeOrThrow`, but it doesn't require a type cast. - The type of `ErrorVerifier._typeProvider` is changed from `TypeProvider` to `TypeProviderImpl`. This allows calling `TypeProviderImpl` methods that are known to return `TypeImpl`. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See https://github.com/dart-lang/sdk/issues/59763. Change-Id: I066262462bad32f4715e9a4b78b5d6697480c036 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405063 Commit-Queue: Paul Berry Reviewed-by: Konstantin Shcheglov --- pkg/analyzer/lib/src/dart/ast/ast.dart | 13 +++++++++---- pkg/analyzer/lib/src/dart/ast/extensions.dart | 12 +++++++++++- pkg/analyzer/lib/src/dart/constant/evaluation.dart | 2 +- .../function_expression_invocation_resolver.dart | 2 +- .../dart/resolver/function_reference_resolver.dart | 4 ++-- .../dart/resolver/method_invocation_resolver.dart | 3 ++- pkg/analyzer/lib/src/generated/error_verifier.dart | 5 ++--- pkg/analyzer/lib/src/generated/resolver.dart | 6 ++++-- .../lib/src/summary2/top_level_inference.dart | 4 +--- 9 files changed, 33 insertions(+), 18 deletions(-) diff --git a/pkg/analyzer/lib/src/dart/ast/ast.dart b/pkg/analyzer/lib/src/dart/ast/ast.dart index 949dae529ca3..214b6587980d 100644 --- a/pkg/analyzer/lib/src/dart/ast/ast.dart +++ b/pkg/analyzer/lib/src/dart/ast/ast.dart @@ -24,6 +24,7 @@ import 'package:analyzer/src/dart/ast/extensions.dart'; import 'package:analyzer/src/dart/ast/to_source_visitor.dart'; import 'package:analyzer/src/dart/element/element.dart'; import 'package:analyzer/src/dart/element/member.dart'; +import 'package:analyzer/src/dart/element/type.dart'; import 'package:analyzer/src/dart/element/type_schema.dart'; import 'package:analyzer/src/dart/resolver/body_inference_context.dart'; import 'package:analyzer/src/dart/resolver/typed_literal_resolver.dart'; @@ -6093,7 +6094,7 @@ final class ExpressionFunctionBodyImpl extends FunctionBodyImpl sealed class ExpressionImpl extends AstNodeImpl implements CollectionElementImpl, Expression { - DartType? _staticType; + TypeImpl? _staticType; @experimental @override @@ -6148,7 +6149,7 @@ sealed class ExpressionImpl extends AstNodeImpl } @override - DartType? get staticType => _staticType; + TypeImpl? get staticType => _staticType; @override ExpressionImpl get unParenthesized => this; @@ -6221,7 +6222,9 @@ sealed class ExpressionImpl extends AstNodeImpl /// @param expression the node whose type is to be recorded /// @param type the static type of the node void recordStaticType(DartType type, {required ResolverVisitor resolver}) { - _staticType = type; + // TODO(paulberry): remove this cast by changing the type of the parameter + // `type`. + _staticType = type as TypeImpl; if (type.isBottom) { resolver.flowAnalysis.flow?.handleExit(); } @@ -6253,7 +6256,9 @@ sealed class ExpressionImpl extends AstNodeImpl /// static type anyway (e.g. the [SimpleIdentifier] representing the method /// name in a method invocation). void setPseudoExpressionStaticType(DartType? type) { - _staticType = type; + // TODO(paulberry): remove this cast by changing the type of the parameter + // `type`. + _staticType = type as TypeImpl?; } } diff --git a/pkg/analyzer/lib/src/dart/ast/extensions.dart b/pkg/analyzer/lib/src/dart/ast/extensions.dart index 11ea40e5ecce..254d8aa48fbe 100644 --- a/pkg/analyzer/lib/src/dart/ast/extensions.dart +++ b/pkg/analyzer/lib/src/dart/ast/extensions.dart @@ -10,6 +10,7 @@ import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/element2.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/src/dart/ast/ast.dart'; +import 'package:analyzer/src/dart/element/type.dart'; import 'package:analyzer/src/utilities/extensions/element.dart'; import 'package:collection/collection.dart'; @@ -172,7 +173,16 @@ extension ExpressionExtension on Expression { /// This accessor should be used on expressions that are expected to /// be already resolved. Every such expression must have the type set, /// at least `dynamic`. - DartType get typeOrThrow { + TypeImpl get typeOrThrow => (this as ExpressionImpl).typeOrThrow; +} + +extension ExpressionImplExtension on ExpressionImpl { + /// Return the static type of this expression. + /// + /// This accessor should be used on expressions that are expected to + /// be already resolved. Every such expression must have the type set, + /// at least `dynamic`. + TypeImpl get typeOrThrow { var type = staticType; if (type == null) { throw StateError('No type: $this'); diff --git a/pkg/analyzer/lib/src/dart/constant/evaluation.dart b/pkg/analyzer/lib/src/dart/constant/evaluation.dart index 5ec4d1c21098..9d01b872050d 100644 --- a/pkg/analyzer/lib/src/dart/constant/evaluation.dart +++ b/pkg/analyzer/lib/src/dart/constant/evaluation.dart @@ -808,7 +808,7 @@ class ConstantVisitor extends UnifyingAstVisitor { @override Constant visitConstructorReference(ConstructorReference node) { var constructorFunctionType = node.typeOrThrow; - if (constructorFunctionType is! FunctionType) { + if (constructorFunctionType is! FunctionTypeImpl) { return InvalidConstant.forEntity( node, CompileTimeErrorCode.INVALID_CONSTANT); } diff --git a/pkg/analyzer/lib/src/dart/resolver/function_expression_invocation_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/function_expression_invocation_resolver.dart index 9ea594f8e485..d6cfe9049cd5 100644 --- a/pkg/analyzer/lib/src/dart/resolver/function_expression_invocation_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/function_expression_invocation_resolver.dart @@ -56,7 +56,7 @@ class FunctionExpressionInvocationResolver { } receiverType = _typeSystem.resolveToBound(receiverType); - if (receiverType is FunctionType) { + if (receiverType is FunctionTypeImpl) { _nullableDereferenceVerifier.expression( CompileTimeErrorCode.UNCHECKED_INVOCATION_OF_NULLABLE_VALUE, function, diff --git a/pkg/analyzer/lib/src/dart/resolver/function_reference_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/function_reference_resolver.dart index 5b54dbbcb4c9..130d91f7b51c 100644 --- a/pkg/analyzer/lib/src/dart/resolver/function_reference_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/function_reference_resolver.dart @@ -896,9 +896,9 @@ class FunctionReferenceResolver { var receiverType = receiver.staticType; if (receiverType == null) { return null; - } else if (receiverType is TypeParameterType) { + } else if (receiverType is TypeParameterTypeImpl) { return null; - } else if (receiverType is FunctionType) { + } else if (receiverType is FunctionTypeImpl) { if (name.name == FunctionElement.CALL_METHOD_NAME) { return receiverType; } diff --git a/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart index e0108f70dbbb..23c5713fd77d 100644 --- a/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart @@ -336,7 +336,8 @@ class MethodInvocationResolver with ScopeHelpers { argumentList: node.argumentList, contextType: contextType, whyNotPromotedArguments: whyNotPromotedArguments) - .resolveInvocation(rawType: rawType is FunctionType ? rawType : null); + .resolveInvocation( + rawType: rawType is FunctionTypeImpl ? rawType : null); node.recordStaticType(staticStaticType, resolver: _resolver); } diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart index 89228fc4331c..995ecd0a7c84 100644 --- a/pkg/analyzer/lib/src/generated/error_verifier.dart +++ b/pkg/analyzer/lib/src/generated/error_verifier.dart @@ -19,7 +19,6 @@ import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/element2.dart'; import 'package:analyzer/dart/element/type.dart'; -import 'package:analyzer/dart/element/type_provider.dart'; import 'package:analyzer/diagnostic/diagnostic.dart'; import 'package:analyzer/error/error.dart'; import 'package:analyzer/error/listener.dart'; @@ -189,7 +188,7 @@ class ErrorVerifier extends RecursiveAstVisitor final AnalysisOptions options; /// The object providing access to the types defined by the language. - final TypeProvider _typeProvider; + final TypeProviderImpl _typeProvider; /// The type system primitives @override @@ -296,7 +295,7 @@ class ErrorVerifier extends RecursiveAstVisitor _typeArgumentsVerifier = TypeArgumentsVerifier(options, _currentLibrary, errorReporter); _returnTypeVerifier = ReturnTypeVerifier( - typeProvider: _typeProvider as TypeProviderImpl, + typeProvider: _typeProvider, typeSystem: typeSystem, errorReporter: errorReporter, strictCasts: strictCasts, diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart index 585aa68369ec..cab8dbcfd85f 100644 --- a/pkg/analyzer/lib/src/generated/resolver.dart +++ b/pkg/analyzer/lib/src/generated/resolver.dart @@ -810,7 +810,9 @@ class ResolverVisitor extends ThrowingAstVisitor '(${replacementExpression.runtimeType}) $replacementExpression', ); } - staticType = operations.unknownType.unwrapTypeSchemaView(); + // TODO(paulberry): remove this cast by changing the type of + // `operations.unknownType` to `SharedTypeSchemaView`. + staticType = operations.unknownType.unwrapTypeSchemaView() as TypeImpl; } return ExpressionTypeAnalysisResult( type: SharedTypeView(staticType)); @@ -1208,7 +1210,7 @@ class ResolverVisitor extends ThrowingAstVisitor } var staticType = expression.staticType; - if (staticType is! FunctionType || staticType.typeFormals.isEmpty) { + if (staticType is! FunctionTypeImpl || staticType.typeFormals.isEmpty) { return expression; } diff --git a/pkg/analyzer/lib/src/summary2/top_level_inference.dart b/pkg/analyzer/lib/src/summary2/top_level_inference.dart index 223055008516..6b83bd6b1b54 100644 --- a/pkg/analyzer/lib/src/summary2/top_level_inference.dart +++ b/pkg/analyzer/lib/src/summary2/top_level_inference.dart @@ -276,9 +276,7 @@ class _PropertyInducingElementTypeInference } var initializerType = _node.initializer!.typeOrThrow; - // TODO(paulberry): eliminate this cast by changing the return type of - // `typeOrThrow` to `TypeImpl`. - return _refineType(initializerType as TypeImpl); + return _refineType(initializerType); } TypeImpl _refineType(TypeImpl type) { From 7dc5a42d5eaf3a2017e030cfca01372c5d832deb Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Sat, 18 Jan 2025 08:38:20 -0800 Subject: [PATCH 2/4] [analyzer] Change substitute methods to return `TypeImpl`. The following methods are changed so that they return `TypeImpl` rather than `DartType`: - `FreshTypeParameters.substitute` - `Substitution.substituteType` - `_NullSubstitution.substituteType` This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See https://github.com/dart-lang/sdk/issues/59763. Change-Id: Iaf4959cd7f6428d1dbe1b15089f7e2a30eff3495 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405080 Reviewed-by: Konstantin Shcheglov Commit-Queue: Paul Berry --- pkg/analyzer/lib/src/dart/element/element.dart | 6 +++--- pkg/analyzer/lib/src/dart/element/type_algebra.dart | 9 +++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/analyzer/lib/src/dart/element/element.dart b/pkg/analyzer/lib/src/dart/element/element.dart index 9dc6500703a3..85a12ff3ae46 100644 --- a/pkg/analyzer/lib/src/dart/element/element.dart +++ b/pkg/analyzer/lib/src/dart/element/element.dart @@ -11001,7 +11001,7 @@ class TypeAliasElementImpl extends _ExistingElementImpl ? NullabilitySuffix.question : nullabilitySuffix; - if (type is FunctionType) { + if (type is FunctionTypeImpl) { return FunctionTypeImpl( typeFormals: type.typeFormals, parameters: type.parameters, @@ -11032,7 +11032,7 @@ class TypeAliasElementImpl extends _ExistingElementImpl typeArguments: typeArguments, ), ); - } else if (type is TypeParameterType) { + } else if (type is TypeParameterTypeImpl) { return TypeParameterTypeImpl( element: type.element, nullabilitySuffix: resultNullability, @@ -11042,7 +11042,7 @@ class TypeAliasElementImpl extends _ExistingElementImpl ), ); } else { - return (type as TypeImpl).withNullability(resultNullability); + return type.withNullability(resultNullability); } } diff --git a/pkg/analyzer/lib/src/dart/element/type_algebra.dart b/pkg/analyzer/lib/src/dart/element/type_algebra.dart index b622147a5d2e..4176dc72973d 100644 --- a/pkg/analyzer/lib/src/dart/element/type_algebra.dart +++ b/pkg/analyzer/lib/src/dart/element/type_algebra.dart @@ -180,7 +180,7 @@ class FreshTypeParameters { ); } - DartType substitute(DartType type) => substitution.substituteType(type); + TypeImpl substitute(DartType type) => substitution.substituteType(type); } /// Substitution that is based on the [map]. @@ -205,9 +205,9 @@ abstract class Substitution { return types.map(mapInterfaceType); } - DartType substituteType(DartType type, {bool contravariant = false}) { + TypeImpl substituteType(DartType type, {bool contravariant = false}) { var visitor = _TopSubstitutor(this, contravariant); - return type.accept(visitor); + return type.accept(visitor) as TypeImpl; } /// Substitutes both variables from [first] and [second], favoring those from @@ -390,7 +390,8 @@ class _NullSubstitution extends MapSubstitution { } @override - DartType substituteType(DartType type, {bool contravariant = false}) => type; + TypeImpl substituteType(DartType type, {bool contravariant = false}) => + type as TypeImpl; @override String toString() => "Substitution.empty"; From b908274b8cab788936c10fa7db78402f169228be Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Sat, 18 Jan 2025 10:06:46 -0800 Subject: [PATCH 3/4] [analysis_server] Minor refactors to enable running EditArgument through legacy server This is some minor refactoring to support the next CL that will enable the EditArgument request (the one that actually edits arguments, not the one that gets the list of arguments) over legacy. It moves some code for sending LSP reverse-requests through the legacy server from test code into the actual server (and fixes that they weren't correctly wrapped in the 'lsp.handle' protocol classes) and updates the tests to use the shared interface ready. Change-Id: I06492138645538072fb12f2fc2d424e214f8055b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404824 Reviewed-by: Brian Wilkerson Reviewed-by: Konstantin Shcheglov Commit-Queue: Brian Wilkerson --- .../lib/src/legacy_analysis_server.dart | 46 +++++++++++++++ .../commands/simple_edit_handler.dart | 2 +- .../handler_edit_argument.dart | 2 +- .../lib/src/lsp/lsp_analysis_server.dart | 6 +- pkg/analysis_server/lib/src/lsp/progress.dart | 2 +- .../src/lsp/server_capabilities_computer.dart | 4 +- .../lib/src/utilities/mocks.dart | 56 +++++++++++++++---- .../test/domain_analysis_test.dart | 1 + .../test/domain_server_test.dart | 6 +- .../server/message_scheduler_test.dart | 2 + .../test/lsp/edit_argument_test.dart | 22 ++++---- .../test/lsp/server_abstract.dart | 5 +- .../test/lsp/workspace_apply_edit_test.dart | 6 -- .../abstract_lsp_over_legacy.dart | 54 +++++++----------- .../editable_arguments_test.dart | 5 -- .../workspace_apply_edit_test.dart | 2 +- .../shared_workspace_apply_edit_tests.dart | 4 +- 17 files changed, 143 insertions(+), 82 deletions(-) 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), ); From dd9a061f4e6ca3db3b81b8c274bd50b631045bde Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Sat, 18 Jan 2025 10:59:08 -0800 Subject: [PATCH 4/4] [analysis_server] Enable "EditArgument" request to work over the legacy protocol + move all related tests to a shared mixin so they run for both servers. Change-Id: I853e8f24948c07fdde2a5d6a64ddc65962357b6d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404841 Commit-Queue: Brian Wilkerson Reviewed-by: Brian Wilkerson Reviewed-by: Phil Quitslund --- .../lib/src/analysis_server.dart | 5 + .../lib/src/legacy_analysis_server.dart | 1 + .../handler_edit_argument.dart | 9 - .../lib/src/lsp/lsp_analysis_server.dart | 4 +- .../test/lsp/edit_argument_test.dart | 718 +---------------- .../lsp_over_legacy/edit_argument_test.dart | 28 + .../test/lsp_over_legacy/test_all.dart | 2 + .../shared/shared_edit_argument_tests.dart | 724 ++++++++++++++++++ 8 files changed, 767 insertions(+), 724 deletions(-) create mode 100644 pkg/analysis_server/test/lsp_over_legacy/edit_argument_test.dart create mode 100644 pkg/analysis_server/test/shared/shared_edit_argument_tests.dart diff --git a/pkg/analysis_server/lib/src/analysis_server.dart b/pkg/analysis_server/lib/src/analysis_server.dart index 84205ef0f822..d8d51deba229 100644 --- a/pkg/analysis_server/lib/src/analysis_server.dart +++ b/pkg/analysis_server/lib/src/analysis_server.dart @@ -1033,6 +1033,11 @@ abstract class AnalysisServer { /// 'lsp.notification' notification. void sendLspNotification(lsp.NotificationMessage notification); + /// Sends an LSP request with the given [params] to the client and waits for a + /// response. Completes with the raw [lsp.ResponseMessage] which could be an + /// error response. + Future sendLspRequest(lsp.Method method, Object params); + /// Sends an error notification to the user. void sendServerErrorNotification( String message, diff --git a/pkg/analysis_server/lib/src/legacy_analysis_server.dart b/pkg/analysis_server/lib/src/legacy_analysis_server.dart index 5ab95bd8196c..d0b21b618b44 100644 --- a/pkg/analysis_server/lib/src/legacy_analysis_server.dart +++ b/pkg/analysis_server/lib/src/legacy_analysis_server.dart @@ -719,6 +719,7 @@ 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. + @override Future sendLspRequest( lsp.Method method, Object params, 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 5a8a1991f1b7..265ec6383857 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 @@ -10,7 +10,6 @@ import 'package:analysis_server/src/lsp/constants.dart'; import 'package:analysis_server/src/lsp/error_or.dart'; import 'package:analysis_server/src/lsp/handlers/custom/editable_arguments/editable_arguments_mixin.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:analysis_server/src/lsp/source_edits.dart'; import 'package:analyzer/dart/analysis/results.dart'; @@ -286,14 +285,6 @@ class EditArgumentHandler extends SharedMessageHandler /// Sends [workspaceEdit] to the client and returns `null` if applied /// successfully or an error otherwise. Future> _sendEditToClient(WorkspaceEdit workspaceEdit) async { - var server = this.server; - if (server is! LspAnalysisServer) { - return error( - ErrorCodes.RequestFailed, - 'Sending edits is currently only supported for clients using LSP directly', - ); - } - var editDescription = 'Edit argument'; var editResponse = await server.sendLspRequest( Method.workspace_applyEdit, 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 0f3ce258fb6e..9357d9fa2a06 100644 --- a/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart +++ b/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart @@ -861,9 +861,7 @@ class LspAnalysisServer extends AnalysisServer { channel.sendNotification(notification); } - /// 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. + @override Future sendLspRequest(Method method, Object params) { var requestId = nextRequestId++; var completer = Completer(); diff --git a/pkg/analysis_server/test/lsp/edit_argument_test.dart b/pkg/analysis_server/test/lsp/edit_argument_test.dart index e7dac5aa8c8e..8b92399bc6a2 100644 --- a/pkg/analysis_server/test/lsp/edit_argument_test.dart +++ b/pkg/analysis_server/test/lsp/edit_argument_test.dart @@ -2,14 +2,11 @@ // 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.dart'; import 'package:analysis_server/src/lsp/handlers/custom/editable_arguments/handler_edit_argument.dart'; -import 'package:analyzer/src/test_utilities/test_code_format.dart'; import 'package:test/test.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; -import '../tool/lsp_spec/matchers.dart'; -import '../utils/test_code_extensions.dart'; +import '../shared/shared_edit_argument_tests.dart'; import 'server_abstract.dart'; // ignore_for_file: prefer_single_quotes @@ -218,718 +215,15 @@ class ComputeStringValueTest { } @reflectiveTest -class EditArgumentTest extends SharedAbstractLspAnalysisServerTest { - late TestCode code; - +class EditArgumentTest extends SharedAbstractLspAnalysisServerTest + // Tests are defined in SharedEditArgumentTests because they + // are shared and run for both LSP and Legacy servers. + with + SharedEditArgumentTests { @override void setUp() { super.setUp(); writeTestPackageConfig(flutter: true); } - - test_comma_addArg_addsIfExists() async { - await _expectSimpleArgumentEdit( - params: '({ int? x, int? y })', - originalArgs: '(x: 1,)', - edit: ArgumentEdit(name: 'y', newValue: 2), - expectedArgs: '(x: 1, y: 2,)', - ); - } - - test_comma_addArg_doesNotAddIfNotExists() async { - await _expectSimpleArgumentEdit( - params: '({ int? x, int? y })', - originalArgs: '(x: 1)', - edit: ArgumentEdit(name: 'y', newValue: 2), - expectedArgs: '(x: 1, y: 2)', - ); - } - - test_comma_editArg_doesNotAddIfNotExists() async { - await _expectSimpleArgumentEdit( - params: '({ int? x, int? y })', - originalArgs: '(x: 1, y: 1)', - edit: ArgumentEdit(name: 'y', newValue: 2), - expectedArgs: '(x: 1, y: 2)', - ); - } - - test_comma_editArg_retainsIfExists() async { - await _expectSimpleArgumentEdit( - params: '({ int? x, int? y })', - originalArgs: '(x: 1, y: 1,)', - edit: ArgumentEdit(name: 'y', newValue: 2), - expectedArgs: '(x: 1, y: 2,)', - ); - } - - test_named_addAfterNamed() async { - await _expectSimpleArgumentEdit( - params: '({ int? x, int? y })', - originalArgs: '(x: 1)', - edit: ArgumentEdit(name: 'y', newValue: 2), - expectedArgs: '(x: 1, y: 2)', - ); - } - - test_named_addAfterNamed_afterChildNotAtEnd() async { - await _expectSimpleArgumentEdit( - params: '({ int? x, int? y, Widget? child })', - originalArgs: '(child: null, x: 1)', - edit: ArgumentEdit(name: 'y', newValue: 2), - expectedArgs: '(child: null, x: 1, y: 2)', - ); - } - - test_named_addAfterNamed_beforeChildAtEnd() async { - await _expectSimpleArgumentEdit( - params: '({ int? x, int? y, Widget? child })', - originalArgs: '(x: 1, child: null)', - edit: ArgumentEdit(name: 'y', newValue: 2), - expectedArgs: '(x: 1, y: 2, child: null)', - ); - } - - test_named_addAfterNamed_beforeChildrenAtEnd() async { - await _expectSimpleArgumentEdit( - params: '({ int? x, int? y, List? children })', - originalArgs: '(x: 1, children: [])', - edit: ArgumentEdit(name: 'y', newValue: 2), - expectedArgs: '(x: 1, y: 2, children: [])', - ); - } - - test_named_addAfterPositional() async { - await _expectSimpleArgumentEdit( - params: '(int? x, { int? y })', - originalArgs: '(1)', - edit: ArgumentEdit(name: 'y', newValue: 2), - expectedArgs: '(1, y: 2)', - ); - } - - test_named_addAfterPositional_afterChildNotAtEnd() async { - await _expectSimpleArgumentEdit( - params: '(int? x, { int? y, Widget? child })', - originalArgs: '(child: null, 1)', - edit: ArgumentEdit(name: 'y', newValue: 2), - expectedArgs: '(child: null, 1, y: 2)', - ); - } - - test_named_addAfterPositional_beforeChildAtEnd() async { - await _expectSimpleArgumentEdit( - params: '(int? x, { int? y, Widget? child })', - originalArgs: '(1, child: null)', - edit: ArgumentEdit(name: 'y', newValue: 2), - expectedArgs: '(1, y: 2, child: null)', - ); - } - - test_named_addAfterPositional_beforeChildrenAtEnd() async { - await _expectSimpleArgumentEdit( - params: '(int? x, { int? y, List? children })', - originalArgs: '(1, children: [])', - edit: ArgumentEdit(name: 'y', newValue: 2), - expectedArgs: '(1, y: 2, children: [])', - ); - } - - test_optionalPositional_addAfterPositional() async { - await _expectSimpleArgumentEdit( - params: '([int? x, int? y])', - originalArgs: '(1)', - edit: ArgumentEdit(name: 'y', newValue: 2), - expectedArgs: '(1, 2)', - ); - } - - test_optionalPositional_notNext_afterPositional() async { - await _expectFailedEdit( - params: '([int? x, int y = 10, int? z])', - originalArgs: '(1)', - edit: ArgumentEdit(name: 'z', newValue: 2), - message: - "Parameter 'z' is not editable: " - "A value for the 3rd parameter can't be added until a value for all preceding positional parameters have been added.", - ); - } - - test_optionalPositional_notNext_solo() async { - await _expectFailedEdit( - params: '([int? x = 10, int? y])', - originalArgs: '()', - edit: ArgumentEdit(name: 'y', newValue: 2), - message: - "Parameter 'y' is not editable: " - "A value for the 2nd parameter can't be added until a value for all preceding positional parameters have been added.", - ); - } - - test_requiredPositional_addAfterNamed() async { - failTestOnErrorDiagnostic = false; // Tests with missing positional. - await _expectSimpleArgumentEdit( - params: '(int? x, { int? y })', - originalArgs: '(y: 1)', - edit: ArgumentEdit(name: 'x', newValue: 2), - expectedArgs: '(y: 1, 2)', - ); - } - - test_requiredPositional_addAfterPositional() async { - failTestOnErrorDiagnostic = false; // Tests with missing positional. - await _expectSimpleArgumentEdit( - params: '(int? x, int? y)', - originalArgs: '(1)', - edit: ArgumentEdit(name: 'y', newValue: 2), - expectedArgs: '(1, 2)', - ); - } - - test_requiredPositional_notNext_afterPositional() async { - failTestOnErrorDiagnostic = false; // Tests with missing positional. - await _expectFailedEdit( - params: '(int? x, int? y, int? z)', - originalArgs: '(1)', - edit: ArgumentEdit(name: 'z', newValue: 2), - message: - "Parameter 'z' is not editable: " - "A value for the 3rd parameter can't be added until a value for all preceding positional parameters have been added.", - ); - } - - test_requiredPositional_notNext_noExisting() async { - failTestOnErrorDiagnostic = false; // Tests with missing positional. - await _expectFailedEdit( - params: '(int? x, int? y)', - originalArgs: '()', - edit: ArgumentEdit(name: 'y', newValue: 2), - message: - "Parameter 'y' is not editable: " - "A value for the 2nd parameter can't be added until a value for all preceding positional parameters have been added.", - ); - } - - test_requiredPositional_notNext_onlyNamed() async { - failTestOnErrorDiagnostic = false; // Tests with missing positional. - await _expectFailedEdit( - params: '(int? x, int? y, { int? z })', - originalArgs: '(z: 1)', - edit: ArgumentEdit(name: 'y', newValue: 2), - message: - "Parameter 'y' is not editable: " - "A value for the 2nd parameter can't be added until a value for all preceding positional parameters have been added.", - ); - } - - test_soloArgument_addNamed() async { - await _expectSimpleArgumentEdit( - params: '({int? x })', - originalArgs: '()', - edit: ArgumentEdit(name: 'x', newValue: 2), - expectedArgs: '(x: 2)', - ); - } - - test_soloArgument_addOptionalPositional() async { - await _expectSimpleArgumentEdit( - params: '([int? x])', - originalArgs: '()', - edit: ArgumentEdit(name: 'x', newValue: 2), - expectedArgs: '(2)', - ); - } - - test_soloArgument_addRequiredPositional() async { - failTestOnErrorDiagnostic = false; // Tests with missing positional. - await _expectSimpleArgumentEdit( - params: '(int? x)', - originalArgs: '()', - edit: ArgumentEdit(name: 'x', newValue: 2), - expectedArgs: '(2)', - ); - } - - test_soloArgument_editNamed() async { - await _expectSimpleArgumentEdit( - params: '({int? x })', - originalArgs: '(x: 1)', - edit: ArgumentEdit(name: 'x', newValue: 2), - expectedArgs: '(x: 2)', - ); - } - - test_soloArgument_editOptionalPositional() async { - await _expectSimpleArgumentEdit( - params: '([int? x])', - originalArgs: '(1)', - edit: ArgumentEdit(name: 'x', newValue: 2), - expectedArgs: '(2)', - ); - } - - test_soloArgument_editRequiredPositional() async { - await _expectSimpleArgumentEdit( - params: '(int? x)', - originalArgs: '(1)', - edit: ArgumentEdit(name: 'x', newValue: 2), - expectedArgs: '(2)', - ); - } - - test_type_bool_invalidType() async { - await _expectFailedEdit( - params: '({ bool? x })', - originalArgs: '(x: true)', - edit: ArgumentEdit(name: 'x', newValue: 'invalid'), - message: 'Value for parameter "x" should be bool? but was String', - ); - } - - test_type_bool_null_allowed() async { - await _expectSimpleArgumentEdit( - params: '({ bool? x })', - originalArgs: '(x: true)', - edit: ArgumentEdit(name: 'x'), - expectedArgs: '(x: null)', - ); - } - - test_type_bool_null_notAllowed() async { - await _expectFailedEdit( - params: '({ required bool x })', - originalArgs: '(x: true)', - edit: ArgumentEdit(name: 'x'), - message: 'Value for non-nullable parameter "x" cannot be null', - ); - } - - test_type_bool_replaceLiteral() async { - await _expectSimpleArgumentEdit( - params: '({ bool? x })', - originalArgs: '(x: true)', - edit: ArgumentEdit(name: 'x', newValue: false), - expectedArgs: '(x: false)', - ); - } - - test_type_bool_replaceNonLiteral() async { - await _expectSimpleArgumentEdit( - params: '({ bool? x })', - originalArgs: '(x: 1 == 1)', - edit: ArgumentEdit(name: 'x', newValue: false), - expectedArgs: '(x: false)', - ); - } - - test_type_double_invalidType() async { - await _expectFailedEdit( - params: '({ double? x })', - originalArgs: '(x: 1.1)', - edit: ArgumentEdit(name: 'x', newValue: 'invalid'), - message: 'Value for parameter "x" should be double? but was String', - ); - } - - test_type_double_null_allowed() async { - await _expectSimpleArgumentEdit( - params: '({ double? x })', - originalArgs: '(x: 1.0)', - edit: ArgumentEdit(name: 'x'), - expectedArgs: '(x: null)', - ); - } - - test_type_double_null_notAllowed() async { - await _expectFailedEdit( - params: '({ required double x })', - originalArgs: '(x: 1.0)', - edit: ArgumentEdit(name: 'x'), - message: 'Value for non-nullable parameter "x" cannot be null', - ); - } - - test_type_double_replaceInt() async { - await _expectSimpleArgumentEdit( - params: '({ double? x })', - originalArgs: '(x: 1)', - edit: ArgumentEdit(name: 'x', newValue: 2.2), - expectedArgs: '(x: 2.2)', - ); - } - - test_type_double_replaceLiteral() async { - await _expectSimpleArgumentEdit( - params: '({ double? x })', - originalArgs: '(x: 1.1)', - edit: ArgumentEdit(name: 'x', newValue: 2.2), - expectedArgs: '(x: 2.2)', - ); - } - - test_type_double_replaceNonLiteral() async { - await _expectSimpleArgumentEdit( - params: '({ double? x })', - originalArgs: '(x: 1.1 + 0.1)', - edit: ArgumentEdit(name: 'x', newValue: 2.2), - expectedArgs: '(x: 2.2)', - ); - } - - test_type_double_replaceWithInt() async { - await _expectSimpleArgumentEdit( - params: '({ double? x })', - originalArgs: '(x: 1.1)', - edit: ArgumentEdit(name: 'x', newValue: 2), - expectedArgs: '(x: 2)', - ); - } - - test_type_enum_invalidType() async { - await _expectFailedEdit( - additionalCode: 'enum E { one, two }', - params: '({ E? x })', - originalArgs: '(x: E.one)', - edit: ArgumentEdit(name: 'x', newValue: 'invalid'), - message: - 'Value for parameter "x" should be one of "E.one", "E.two" but was "invalid"', - ); - } - - test_type_enum_null_allowed() async { - await _expectSimpleArgumentEdit( - additionalCode: 'enum E { one, two }', - params: '({ E? x })', - originalArgs: '(x: E.one)', - edit: ArgumentEdit(name: 'x'), - expectedArgs: '(x: null)', - ); - } - - test_type_enum_null_notAllowed() async { - await _expectFailedEdit( - additionalCode: 'enum E { one, two }', - params: '({ required E x })', - originalArgs: '(x: E.one)', - edit: ArgumentEdit(name: 'x'), - message: 'Value for non-nullable parameter "x" cannot be null', - ); - } - - test_type_enum_replaceLiteral() async { - await _expectSimpleArgumentEdit( - additionalCode: 'enum E { one, two }', - params: '({ E? x })', - originalArgs: '(x: E.one)', - edit: ArgumentEdit(name: 'x', newValue: 'E.two'), - expectedArgs: '(x: E.two)', - ); - } - - test_type_enum_replaceNonLiteral() async { - await _expectSimpleArgumentEdit( - additionalCode: ''' -enum E { one, two } -const myConst = E.one; -''', - params: '({ E? x })', - originalArgs: '(x: myConst)', - edit: ArgumentEdit(name: 'x', newValue: 'E.two'), - expectedArgs: '(x: E.two)', - ); - } - - test_type_int_invalidType() async { - await _expectFailedEdit( - params: '({ int? x })', - originalArgs: '(x: 1)', - edit: ArgumentEdit(name: 'x', newValue: 'invalid'), - message: 'Value for parameter "x" should be int? but was String', - ); - } - - test_type_int_null_allowed() async { - await _expectSimpleArgumentEdit( - params: '({ int? x })', - originalArgs: '(x: 1)', - edit: ArgumentEdit(name: 'x'), - expectedArgs: '(x: null)', - ); - } - - test_type_int_null_notAllowed() async { - await _expectFailedEdit( - params: '({ required int x })', - originalArgs: '(x: 1)', - edit: ArgumentEdit(name: 'x'), - message: 'Value for non-nullable parameter "x" cannot be null', - ); - } - - test_type_int_replaceLiteral() async { - await _expectSimpleArgumentEdit( - params: '({ int? x })', - originalArgs: '(x: 1)', - edit: ArgumentEdit(name: 'x', newValue: 2), - expectedArgs: '(x: 2)', - ); - } - - test_type_int_replaceNonLiteral() async { - await _expectSimpleArgumentEdit( - params: '({ int? x })', - originalArgs: '(x: 1 + 0)', - edit: ArgumentEdit(name: 'x', newValue: 2), - expectedArgs: '(x: 2)', - ); - } - - test_type_string_containsBackslashes() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: "(x: 'a')", - edit: ArgumentEdit(name: 'x', newValue: r'a\b'), - expectedArgs: r"(x: 'a\\b')", - ); - } - - test_type_string_containsBothQuotes() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: "(x: 'a')", - edit: ArgumentEdit(name: 'x', newValue: '''a'b"c'''), - expectedArgs: r'''(x: 'a\'b"c')''', - ); - } - - test_type_string_containsSingleQuotes() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: "(x: 'a')", - edit: ArgumentEdit(name: 'x', newValue: "a'b"), - expectedArgs: r'''(x: 'a\'b')''', - ); - } - - test_type_string_invalidType() async { - await _expectFailedEdit( - params: '({ String? x })', - originalArgs: "(x: 'a')", - edit: ArgumentEdit(name: 'x', newValue: 123), - message: 'Value for parameter "x" should be String? but was int', - ); - } - - test_type_string_multiline() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: "(x: 'a')", - edit: ArgumentEdit(name: 'x', newValue: 'a\nb'), - expectedArgs: r'''(x: 'a\nb')''', - ); - } - - test_type_string_null_allowed() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: "(x: 'a')", - edit: ArgumentEdit(name: 'x'), - expectedArgs: '(x: null)', - ); - } - - test_type_string_null_notAllowed() async { - await _expectFailedEdit( - params: '({ required String x })', - originalArgs: "(x: 'a')", - edit: ArgumentEdit(name: 'x'), - message: 'Value for non-nullable parameter "x" cannot be null', - ); - } - - test_type_string_quotes_dollar_escapedNonRaw() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: "(x: '')", - edit: ArgumentEdit(name: 'x', newValue: r'$'), - expectedArgs: r"(x: '\$')", - ); - } - - test_type_string_quotes_dollar_notEscapedRaw() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: "(x: r'')", - edit: ArgumentEdit(name: 'x', newValue: r'$'), - expectedArgs: r"(x: r'$')", - ); - } - - test_type_string_quotes_usesExistingDouble() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: '(x: "a")', - edit: ArgumentEdit(name: 'x', newValue: 'a'), - expectedArgs: '(x: "a")', - ); - } - - test_type_string_quotes_usesExistingSingle() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: "(x: 'a')", - edit: ArgumentEdit(name: 'x', newValue: 'a'), - expectedArgs: "(x: 'a')", - ); - } - - test_type_string_quotes_usesExistingTripleDouble() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: '(x: """a""")', - edit: ArgumentEdit(name: 'x', newValue: 'a'), - expectedArgs: '(x: """a""")', - ); - } - - test_type_string_quotes_usesExistingTripleSingle() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: "(x: '''a''')", - edit: ArgumentEdit(name: 'x', newValue: 'a'), - expectedArgs: "(x: '''a''')", - ); - } - - test_type_string_replaceLiteral() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: "(x: 'a')", - edit: ArgumentEdit(name: 'x', newValue: 'b'), - expectedArgs: "(x: 'b')", - ); - } - - test_type_string_replaceLiteral_raw() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: "(x: r'a')", - edit: ArgumentEdit(name: 'x', newValue: 'b'), - expectedArgs: "(x: r'b')", - ); - } - - test_type_string_replaceLiteral_tripleQuoted() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: "(x: '''a''')", - edit: ArgumentEdit(name: 'x', newValue: 'b'), - expectedArgs: "(x: '''b''')", - ); - } - - test_type_string_replaceNonLiteral() async { - await _expectSimpleArgumentEdit( - params: '({ String? x })', - originalArgs: "(x: 'a' + 'a')", - edit: ArgumentEdit(name: 'x', newValue: 'b'), - expectedArgs: "(x: 'b')", - ); - } - - /// Initializes the server with [content] and tries to apply the argument - /// [edit] at the marked location. Verifies the changes made match - /// [expectedContent]. - Future _expectArgumentEdit( - String content, - ArgumentEdit edit, - String expectedContent, { - bool open = true, - }) async { - code = TestCode.parse(content); - createFile(testFilePath, code.code); - await initializeServer(); - if (open) { - await openFile(testFileUri, code.code); - } - await currentAnalysis; - var verifier = await executeForEdits( - () => editArgument(testFileUri, code.position.position, edit), - ); - - verifier.verifyFiles(expectedContent); - } - - /// Initializes the server and verifies a simple argument edit fails with - /// a given message. - Future _expectFailedEdit({ - required String params, - required String originalArgs, - required ArgumentEdit edit, - required String message, - String? additionalCode, - }) async { - additionalCode ??= ''; - var content = ''' -import 'package:flutter/widgets.dart'; - -$additionalCode - -class MyWidget extends StatelessWidget { - const MyWidget$params; - - @override - Widget build(BuildContext context) => MyW^idget$originalArgs; -} -'''; - - code = TestCode.parse(content); - createFile(testFilePath, code.code); - await initializeServer(); - await currentAnalysis; - - await expectLater( - editArgument(testFileUri, code.position.position, edit), - throwsA(isResponseError(ErrorCodes.RequestFailed, message: message)), - ); - } - - /// Initializes the server and verifies a simple argument edit. - Future _expectSimpleArgumentEdit({ - required String params, - required String originalArgs, - required ArgumentEdit edit, - required String expectedArgs, - String? additionalCode, - }) async { - additionalCode ??= ''; - var content = ''' -import 'package:flutter/widgets.dart'; - -$additionalCode - -class MyWidget extends StatelessWidget { - const MyWidget$params; - - @override - Widget build(BuildContext context) => MyW^idget$originalArgs; -} -'''; - var expectedContent = ''' ->>>>>>>>>> lib/test.dart -import 'package:flutter/widgets.dart'; - -$additionalCode - -class MyWidget extends StatelessWidget { - const MyWidget$params; - - @override - Widget build(BuildContext context) => MyWidget$expectedArgs; -} -'''; - - await _expectArgumentEdit(content, edit, expectedContent); - } } diff --git a/pkg/analysis_server/test/lsp_over_legacy/edit_argument_test.dart b/pkg/analysis_server/test/lsp_over_legacy/edit_argument_test.dart new file mode 100644 index 000000000000..c3132e9ba36b --- /dev/null +++ b/pkg/analysis_server/test/lsp_over_legacy/edit_argument_test.dart @@ -0,0 +1,28 @@ +// Copyright (c) 2025, 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:test_reflective_loader/test_reflective_loader.dart'; + +import '../shared/shared_edit_argument_tests.dart'; +import 'abstract_lsp_over_legacy.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(EditArgumentTest); + }); +} + +@reflectiveTest +class EditArgumentTest extends SharedLspOverLegacyTest + with + // Tests are defined in SharedEditArgumentTests because they + // are shared and run for both LSP and Legacy servers. + SharedEditArgumentTests { + @override + Future setUp() async { + await super.setUp(); + + writeTestPackageConfig(flutter: true); + } +} diff --git a/pkg/analysis_server/test/lsp_over_legacy/test_all.dart b/pkg/analysis_server/test/lsp_over_legacy/test_all.dart index afc8547e3e10..504b2802372d 100644 --- a/pkg/analysis_server/test/lsp_over_legacy/test_all.dart +++ b/pkg/analysis_server/test/lsp_over_legacy/test_all.dart @@ -10,6 +10,7 @@ import 'dart_text_document_content_provider_test.dart' import 'document_color_test.dart' as document_color; import 'document_highlights_test.dart' as document_highlights; import 'document_symbols_test.dart' as document_symbols; +import 'edit_argument_test.dart' as edit_argument; import 'editable_arguments_test.dart' as editable_arguments; import 'format_test.dart' as format; import 'hover_test.dart' as hover; @@ -28,6 +29,7 @@ void main() { document_color.main; document_highlights.main(); document_symbols.main(); + edit_argument.main(); editable_arguments.main(); format.main(); hover.main(); diff --git a/pkg/analysis_server/test/shared/shared_edit_argument_tests.dart b/pkg/analysis_server/test/shared/shared_edit_argument_tests.dart new file mode 100644 index 000000000000..fc543b26d06b --- /dev/null +++ b/pkg/analysis_server/test/shared/shared_edit_argument_tests.dart @@ -0,0 +1,724 @@ +// Copyright (c) 2025, 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/lsp_protocol/protocol.dart'; +import 'package:analyzer/src/test_utilities/test_code_format.dart'; +import 'package:test/test.dart'; + +import '../lsp/request_helpers_mixin.dart'; +import '../tool/lsp_spec/matchers.dart'; +import '../utils/test_code_extensions.dart'; +import 'shared_test_interface.dart'; + +/// Shared edit argument tests that are used by both LSP + Legacy server +/// tests. +mixin SharedEditArgumentTests + on SharedTestInterface, LspRequestHelpersMixin, LspVerifyEditHelpersMixin { + late TestCode code; + + test_comma_addArg_addsIfExists() async { + await _expectSimpleArgumentEdit( + params: '({ int? x, int? y })', + originalArgs: '(x: 1,)', + edit: ArgumentEdit(name: 'y', newValue: 2), + expectedArgs: '(x: 1, y: 2,)', + ); + } + + test_comma_addArg_doesNotAddIfNotExists() async { + await _expectSimpleArgumentEdit( + params: '({ int? x, int? y })', + originalArgs: '(x: 1)', + edit: ArgumentEdit(name: 'y', newValue: 2), + expectedArgs: '(x: 1, y: 2)', + ); + } + + test_comma_editArg_doesNotAddIfNotExists() async { + await _expectSimpleArgumentEdit( + params: '({ int? x, int? y })', + originalArgs: '(x: 1, y: 1)', + edit: ArgumentEdit(name: 'y', newValue: 2), + expectedArgs: '(x: 1, y: 2)', + ); + } + + test_comma_editArg_retainsIfExists() async { + await _expectSimpleArgumentEdit( + params: '({ int? x, int? y })', + originalArgs: '(x: 1, y: 1,)', + edit: ArgumentEdit(name: 'y', newValue: 2), + expectedArgs: '(x: 1, y: 2,)', + ); + } + + test_named_addAfterNamed() async { + await _expectSimpleArgumentEdit( + params: '({ int? x, int? y })', + originalArgs: '(x: 1)', + edit: ArgumentEdit(name: 'y', newValue: 2), + expectedArgs: '(x: 1, y: 2)', + ); + } + + test_named_addAfterNamed_afterChildNotAtEnd() async { + await _expectSimpleArgumentEdit( + params: '({ int? x, int? y, Widget? child })', + originalArgs: '(child: null, x: 1)', + edit: ArgumentEdit(name: 'y', newValue: 2), + expectedArgs: '(child: null, x: 1, y: 2)', + ); + } + + test_named_addAfterNamed_beforeChildAtEnd() async { + await _expectSimpleArgumentEdit( + params: '({ int? x, int? y, Widget? child })', + originalArgs: '(x: 1, child: null)', + edit: ArgumentEdit(name: 'y', newValue: 2), + expectedArgs: '(x: 1, y: 2, child: null)', + ); + } + + test_named_addAfterNamed_beforeChildrenAtEnd() async { + await _expectSimpleArgumentEdit( + params: '({ int? x, int? y, List? children })', + originalArgs: '(x: 1, children: [])', + edit: ArgumentEdit(name: 'y', newValue: 2), + expectedArgs: '(x: 1, y: 2, children: [])', + ); + } + + test_named_addAfterPositional() async { + await _expectSimpleArgumentEdit( + params: '(int? x, { int? y })', + originalArgs: '(1)', + edit: ArgumentEdit(name: 'y', newValue: 2), + expectedArgs: '(1, y: 2)', + ); + } + + test_named_addAfterPositional_afterChildNotAtEnd() async { + await _expectSimpleArgumentEdit( + params: '(int? x, { int? y, Widget? child })', + originalArgs: '(child: null, 1)', + edit: ArgumentEdit(name: 'y', newValue: 2), + expectedArgs: '(child: null, 1, y: 2)', + ); + } + + test_named_addAfterPositional_beforeChildAtEnd() async { + await _expectSimpleArgumentEdit( + params: '(int? x, { int? y, Widget? child })', + originalArgs: '(1, child: null)', + edit: ArgumentEdit(name: 'y', newValue: 2), + expectedArgs: '(1, y: 2, child: null)', + ); + } + + test_named_addAfterPositional_beforeChildrenAtEnd() async { + await _expectSimpleArgumentEdit( + params: '(int? x, { int? y, List? children })', + originalArgs: '(1, children: [])', + edit: ArgumentEdit(name: 'y', newValue: 2), + expectedArgs: '(1, y: 2, children: [])', + ); + } + + test_optionalPositional_addAfterPositional() async { + await _expectSimpleArgumentEdit( + params: '([int? x, int? y])', + originalArgs: '(1)', + edit: ArgumentEdit(name: 'y', newValue: 2), + expectedArgs: '(1, 2)', + ); + } + + test_optionalPositional_notNext_afterPositional() async { + await _expectFailedEdit( + params: '([int? x, int y = 10, int? z])', + originalArgs: '(1)', + edit: ArgumentEdit(name: 'z', newValue: 2), + message: + "Parameter 'z' is not editable: " + "A value for the 3rd parameter can't be added until a value for all preceding positional parameters have been added.", + ); + } + + test_optionalPositional_notNext_solo() async { + await _expectFailedEdit( + params: '([int? x = 10, int? y])', + originalArgs: '()', + edit: ArgumentEdit(name: 'y', newValue: 2), + message: + "Parameter 'y' is not editable: " + "A value for the 2nd parameter can't be added until a value for all preceding positional parameters have been added.", + ); + } + + test_requiredPositional_addAfterNamed() async { + failTestOnErrorDiagnostic = false; // Tests with missing positional. + await _expectSimpleArgumentEdit( + params: '(int? x, { int? y })', + originalArgs: '(y: 1)', + edit: ArgumentEdit(name: 'x', newValue: 2), + expectedArgs: '(y: 1, 2)', + ); + } + + test_requiredPositional_addAfterPositional() async { + failTestOnErrorDiagnostic = false; // Tests with missing positional. + await _expectSimpleArgumentEdit( + params: '(int? x, int? y)', + originalArgs: '(1)', + edit: ArgumentEdit(name: 'y', newValue: 2), + expectedArgs: '(1, 2)', + ); + } + + test_requiredPositional_notNext_afterPositional() async { + failTestOnErrorDiagnostic = false; // Tests with missing positional. + await _expectFailedEdit( + params: '(int? x, int? y, int? z)', + originalArgs: '(1)', + edit: ArgumentEdit(name: 'z', newValue: 2), + message: + "Parameter 'z' is not editable: " + "A value for the 3rd parameter can't be added until a value for all preceding positional parameters have been added.", + ); + } + + test_requiredPositional_notNext_noExisting() async { + failTestOnErrorDiagnostic = false; // Tests with missing positional. + await _expectFailedEdit( + params: '(int? x, int? y)', + originalArgs: '()', + edit: ArgumentEdit(name: 'y', newValue: 2), + message: + "Parameter 'y' is not editable: " + "A value for the 2nd parameter can't be added until a value for all preceding positional parameters have been added.", + ); + } + + test_requiredPositional_notNext_onlyNamed() async { + failTestOnErrorDiagnostic = false; // Tests with missing positional. + await _expectFailedEdit( + params: '(int? x, int? y, { int? z })', + originalArgs: '(z: 1)', + edit: ArgumentEdit(name: 'y', newValue: 2), + message: + "Parameter 'y' is not editable: " + "A value for the 2nd parameter can't be added until a value for all preceding positional parameters have been added.", + ); + } + + test_soloArgument_addNamed() async { + await _expectSimpleArgumentEdit( + params: '({int? x })', + originalArgs: '()', + edit: ArgumentEdit(name: 'x', newValue: 2), + expectedArgs: '(x: 2)', + ); + } + + test_soloArgument_addOptionalPositional() async { + await _expectSimpleArgumentEdit( + params: '([int? x])', + originalArgs: '()', + edit: ArgumentEdit(name: 'x', newValue: 2), + expectedArgs: '(2)', + ); + } + + test_soloArgument_addRequiredPositional() async { + failTestOnErrorDiagnostic = false; // Tests with missing positional. + await _expectSimpleArgumentEdit( + params: '(int? x)', + originalArgs: '()', + edit: ArgumentEdit(name: 'x', newValue: 2), + expectedArgs: '(2)', + ); + } + + test_soloArgument_editNamed() async { + await _expectSimpleArgumentEdit( + params: '({int? x })', + originalArgs: '(x: 1)', + edit: ArgumentEdit(name: 'x', newValue: 2), + expectedArgs: '(x: 2)', + ); + } + + test_soloArgument_editOptionalPositional() async { + await _expectSimpleArgumentEdit( + params: '([int? x])', + originalArgs: '(1)', + edit: ArgumentEdit(name: 'x', newValue: 2), + expectedArgs: '(2)', + ); + } + + test_soloArgument_editRequiredPositional() async { + await _expectSimpleArgumentEdit( + params: '(int? x)', + originalArgs: '(1)', + edit: ArgumentEdit(name: 'x', newValue: 2), + expectedArgs: '(2)', + ); + } + + test_type_bool_invalidType() async { + await _expectFailedEdit( + params: '({ bool? x })', + originalArgs: '(x: true)', + edit: ArgumentEdit(name: 'x', newValue: 'invalid'), + message: 'Value for parameter "x" should be bool? but was String', + ); + } + + test_type_bool_null_allowed() async { + await _expectSimpleArgumentEdit( + params: '({ bool? x })', + originalArgs: '(x: true)', + edit: ArgumentEdit(name: 'x'), + expectedArgs: '(x: null)', + ); + } + + test_type_bool_null_notAllowed() async { + await _expectFailedEdit( + params: '({ required bool x })', + originalArgs: '(x: true)', + edit: ArgumentEdit(name: 'x'), + message: 'Value for non-nullable parameter "x" cannot be null', + ); + } + + test_type_bool_replaceLiteral() async { + await _expectSimpleArgumentEdit( + params: '({ bool? x })', + originalArgs: '(x: true)', + edit: ArgumentEdit(name: 'x', newValue: false), + expectedArgs: '(x: false)', + ); + } + + test_type_bool_replaceNonLiteral() async { + await _expectSimpleArgumentEdit( + params: '({ bool? x })', + originalArgs: '(x: 1 == 1)', + edit: ArgumentEdit(name: 'x', newValue: false), + expectedArgs: '(x: false)', + ); + } + + test_type_double_invalidType() async { + await _expectFailedEdit( + params: '({ double? x })', + originalArgs: '(x: 1.1)', + edit: ArgumentEdit(name: 'x', newValue: 'invalid'), + message: 'Value for parameter "x" should be double? but was String', + ); + } + + test_type_double_null_allowed() async { + await _expectSimpleArgumentEdit( + params: '({ double? x })', + originalArgs: '(x: 1.0)', + edit: ArgumentEdit(name: 'x'), + expectedArgs: '(x: null)', + ); + } + + test_type_double_null_notAllowed() async { + await _expectFailedEdit( + params: '({ required double x })', + originalArgs: '(x: 1.0)', + edit: ArgumentEdit(name: 'x'), + message: 'Value for non-nullable parameter "x" cannot be null', + ); + } + + test_type_double_replaceInt() async { + await _expectSimpleArgumentEdit( + params: '({ double? x })', + originalArgs: '(x: 1)', + edit: ArgumentEdit(name: 'x', newValue: 2.2), + expectedArgs: '(x: 2.2)', + ); + } + + test_type_double_replaceLiteral() async { + await _expectSimpleArgumentEdit( + params: '({ double? x })', + originalArgs: '(x: 1.1)', + edit: ArgumentEdit(name: 'x', newValue: 2.2), + expectedArgs: '(x: 2.2)', + ); + } + + test_type_double_replaceNonLiteral() async { + await _expectSimpleArgumentEdit( + params: '({ double? x })', + originalArgs: '(x: 1.1 + 0.1)', + edit: ArgumentEdit(name: 'x', newValue: 2.2), + expectedArgs: '(x: 2.2)', + ); + } + + test_type_double_replaceWithInt() async { + await _expectSimpleArgumentEdit( + params: '({ double? x })', + originalArgs: '(x: 1.1)', + edit: ArgumentEdit(name: 'x', newValue: 2), + expectedArgs: '(x: 2)', + ); + } + + test_type_enum_invalidType() async { + await _expectFailedEdit( + additionalCode: 'enum E { one, two }', + params: '({ E? x })', + originalArgs: '(x: E.one)', + edit: ArgumentEdit(name: 'x', newValue: 'invalid'), + message: + 'Value for parameter "x" should be one of "E.one", "E.two" but was "invalid"', + ); + } + + test_type_enum_null_allowed() async { + await _expectSimpleArgumentEdit( + additionalCode: 'enum E { one, two }', + params: '({ E? x })', + originalArgs: '(x: E.one)', + edit: ArgumentEdit(name: 'x'), + expectedArgs: '(x: null)', + ); + } + + test_type_enum_null_notAllowed() async { + await _expectFailedEdit( + additionalCode: 'enum E { one, two }', + params: '({ required E x })', + originalArgs: '(x: E.one)', + edit: ArgumentEdit(name: 'x'), + message: 'Value for non-nullable parameter "x" cannot be null', + ); + } + + test_type_enum_replaceLiteral() async { + await _expectSimpleArgumentEdit( + additionalCode: 'enum E { one, two }', + params: '({ E? x })', + originalArgs: '(x: E.one)', + edit: ArgumentEdit(name: 'x', newValue: 'E.two'), + expectedArgs: '(x: E.two)', + ); + } + + test_type_enum_replaceNonLiteral() async { + await _expectSimpleArgumentEdit( + additionalCode: ''' +enum E { one, two } +const myConst = E.one; +''', + params: '({ E? x })', + originalArgs: '(x: myConst)', + edit: ArgumentEdit(name: 'x', newValue: 'E.two'), + expectedArgs: '(x: E.two)', + ); + } + + test_type_int_invalidType() async { + await _expectFailedEdit( + params: '({ int? x })', + originalArgs: '(x: 1)', + edit: ArgumentEdit(name: 'x', newValue: 'invalid'), + message: 'Value for parameter "x" should be int? but was String', + ); + } + + test_type_int_null_allowed() async { + await _expectSimpleArgumentEdit( + params: '({ int? x })', + originalArgs: '(x: 1)', + edit: ArgumentEdit(name: 'x'), + expectedArgs: '(x: null)', + ); + } + + test_type_int_null_notAllowed() async { + await _expectFailedEdit( + params: '({ required int x })', + originalArgs: '(x: 1)', + edit: ArgumentEdit(name: 'x'), + message: 'Value for non-nullable parameter "x" cannot be null', + ); + } + + test_type_int_replaceLiteral() async { + await _expectSimpleArgumentEdit( + params: '({ int? x })', + originalArgs: '(x: 1)', + edit: ArgumentEdit(name: 'x', newValue: 2), + expectedArgs: '(x: 2)', + ); + } + + test_type_int_replaceNonLiteral() async { + await _expectSimpleArgumentEdit( + params: '({ int? x })', + originalArgs: '(x: 1 + 0)', + edit: ArgumentEdit(name: 'x', newValue: 2), + expectedArgs: '(x: 2)', + ); + } + + test_type_string_containsBackslashes() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: "(x: 'a')", + edit: ArgumentEdit(name: 'x', newValue: r'a\b'), + expectedArgs: r"(x: 'a\\b')", + ); + } + + test_type_string_containsBothQuotes() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: "(x: 'a')", + edit: ArgumentEdit(name: 'x', newValue: '''a'b"c'''), + expectedArgs: r'''(x: 'a\'b"c')''', + ); + } + + test_type_string_containsSingleQuotes() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: "(x: 'a')", + edit: ArgumentEdit(name: 'x', newValue: "a'b"), + expectedArgs: r'''(x: 'a\'b')''', + ); + } + + test_type_string_invalidType() async { + await _expectFailedEdit( + params: '({ String? x })', + originalArgs: "(x: 'a')", + edit: ArgumentEdit(name: 'x', newValue: 123), + message: 'Value for parameter "x" should be String? but was int', + ); + } + + test_type_string_multiline() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: "(x: 'a')", + edit: ArgumentEdit(name: 'x', newValue: 'a\nb'), + expectedArgs: r'''(x: 'a\nb')''', + ); + } + + test_type_string_null_allowed() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: "(x: 'a')", + edit: ArgumentEdit(name: 'x'), + expectedArgs: '(x: null)', + ); + } + + test_type_string_null_notAllowed() async { + await _expectFailedEdit( + params: '({ required String x })', + originalArgs: "(x: 'a')", + edit: ArgumentEdit(name: 'x'), + message: 'Value for non-nullable parameter "x" cannot be null', + ); + } + + test_type_string_quotes_dollar_escapedNonRaw() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: "(x: '')", + edit: ArgumentEdit(name: 'x', newValue: r'$'), + expectedArgs: r"(x: '\$')", + ); + } + + test_type_string_quotes_dollar_notEscapedRaw() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: "(x: r'')", + edit: ArgumentEdit(name: 'x', newValue: r'$'), + expectedArgs: r"(x: r'$')", + ); + } + + test_type_string_quotes_usesExistingDouble() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: '(x: "a")', + edit: ArgumentEdit(name: 'x', newValue: 'a'), + expectedArgs: '(x: "a")', + ); + } + + test_type_string_quotes_usesExistingSingle() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: "(x: 'a')", + edit: ArgumentEdit(name: 'x', newValue: 'a'), + expectedArgs: "(x: 'a')", + ); + } + + test_type_string_quotes_usesExistingTripleDouble() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: '(x: """a""")', + edit: ArgumentEdit(name: 'x', newValue: 'a'), + expectedArgs: '(x: """a""")', + ); + } + + test_type_string_quotes_usesExistingTripleSingle() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: "(x: '''a''')", + edit: ArgumentEdit(name: 'x', newValue: 'a'), + expectedArgs: "(x: '''a''')", + ); + } + + test_type_string_replaceLiteral() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: "(x: 'a')", + edit: ArgumentEdit(name: 'x', newValue: 'b'), + expectedArgs: "(x: 'b')", + ); + } + + test_type_string_replaceLiteral_raw() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: "(x: r'a')", + edit: ArgumentEdit(name: 'x', newValue: 'b'), + expectedArgs: "(x: r'b')", + ); + } + + test_type_string_replaceLiteral_tripleQuoted() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: "(x: '''a''')", + edit: ArgumentEdit(name: 'x', newValue: 'b'), + expectedArgs: "(x: '''b''')", + ); + } + + test_type_string_replaceNonLiteral() async { + await _expectSimpleArgumentEdit( + params: '({ String? x })', + originalArgs: "(x: 'a' + 'a')", + edit: ArgumentEdit(name: 'x', newValue: 'b'), + expectedArgs: "(x: 'b')", + ); + } + + /// Initializes the server with [content] and tries to apply the argument + /// [edit] at the marked location. Verifies the changes made match + /// [expectedContent]. + Future _expectArgumentEdit( + String content, + ArgumentEdit edit, + String expectedContent, { + bool open = true, + }) async { + code = TestCode.parse(content); + createFile(testFilePath, code.code); + await initializeServer(); + if (open) { + await openFile(testFileUri, code.code); + } + await currentAnalysis; + var verifier = await executeForEdits( + () => editArgument(testFileUri, code.position.position, edit), + ); + + verifier.verifyFiles(expectedContent); + } + + /// Initializes the server and verifies a simple argument edit fails with + /// a given message. + Future _expectFailedEdit({ + required String params, + required String originalArgs, + required ArgumentEdit edit, + required String message, + String? additionalCode, + }) async { + additionalCode ??= ''; + var content = ''' +import 'package:flutter/widgets.dart'; + +$additionalCode + +class MyWidget extends StatelessWidget { + const MyWidget$params; + + @override + Widget build(BuildContext context) => MyW^idget$originalArgs; +} +'''; + + code = TestCode.parse(content); + createFile(testFilePath, code.code); + await initializeServer(); + await currentAnalysis; + + await expectLater( + editArgument(testFileUri, code.position.position, edit), + throwsA(isResponseError(ErrorCodes.RequestFailed, message: message)), + ); + } + + /// Initializes the server and verifies a simple argument edit. + Future _expectSimpleArgumentEdit({ + required String params, + required String originalArgs, + required ArgumentEdit edit, + required String expectedArgs, + String? additionalCode, + }) async { + additionalCode ??= ''; + var content = ''' +import 'package:flutter/widgets.dart'; + +$additionalCode + +class MyWidget extends StatelessWidget { + const MyWidget$params; + + @override + Widget build(BuildContext context) => MyW^idget$originalArgs; +} +'''; + var expectedContent = ''' +>>>>>>>>>> lib/test.dart +import 'package:flutter/widgets.dart'; + +$additionalCode + +class MyWidget extends StatelessWidget { + const MyWidget$params; + + @override + Widget build(BuildContext context) => MyWidget$expectedArgs; +} +'''; + + await _expectArgumentEdit(content, edit, expectedContent); + } +}