From ed49d04e2e00ec3df7f596100444e2bdd01db705 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Mon, 2 Dec 2024 20:06:13 +0000 Subject: [PATCH] [analysis_server] Extract some shared code from editableArguments to be used by editArgument This doesn't change any functionality, it just extracts some code from the editableArguments handler into a mixin because the editArgument handler will want to reuse some of this logic to locate the argument/parameter and ensure it is allowed to be edited. Change-Id: Ibe9f1350c977b470847cebe2ecf7a7bec5256000 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/398440 Reviewed-by: Elliott Brooks Reviewed-by: Brian Wilkerson Commit-Queue: Brian Wilkerson --- .../lib/src/lsp/constants.dart | 3 + .../editable_arguments_mixin.dart | 165 +++++++++++++++++ .../handler_editable_arguments.dart | 168 +++--------------- .../lib/src/lsp/handlers/handler_states.dart | 2 +- 4 files changed, 195 insertions(+), 143 deletions(-) create mode 100644 pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/editable_arguments_mixin.dart rename pkg/analysis_server/lib/src/lsp/handlers/custom/{ => editable_arguments}/handler_editable_arguments.dart (58%) diff --git a/pkg/analysis_server/lib/src/lsp/constants.dart b/pkg/analysis_server/lib/src/lsp/constants.dart index 9ae4e652c59b..1c288207b89d 100644 --- a/pkg/analysis_server/lib/src/lsp/constants.dart +++ b/pkg/analysis_server/lib/src/lsp/constants.dart @@ -153,6 +153,9 @@ abstract final class CustomMethods { static const dartTextDocumentContentDidChange = Method( 'dart/textDocumentContentDidChange', ); + + /// Method for requesting the set of editable arguments at a location in a + /// document. static const dartTextDocumentEditableArguments = Method( 'experimental/dart/textDocument/editableArguments', ); diff --git a/pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/editable_arguments_mixin.dart b/pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/editable_arguments_mixin.dart new file mode 100644 index 000000000000..08195efcc2b1 --- /dev/null +++ b/pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/editable_arguments_mixin.dart @@ -0,0 +1,165 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analysis_server/src/utilities/extensions/numeric.dart'; +import 'package:analyzer/dart/analysis/results.dart'; +import 'package:analyzer/dart/element/element2.dart'; +import 'package:analyzer/src/dart/ast/ast.dart'; +import 'package:analyzer/src/dart/element/element.dart'; +import 'package:analyzer/src/utilities/extensions/ast.dart'; +import 'package:analyzer/src/utilities/extensions/flutter.dart'; + +/// Information about the arguments and parameters for an invocation. +typedef EditableInvocationInfo = + ({ + List parameters, + Map parameterArguments, + Map positionalParameterIndexes, + ArgumentList argumentList, + int numPositionals, + int numSuppliedPositionals, + }); + +/// A mixin that provides functionality for locating arguments and associated +/// parameters in a document to allow a client to provide editing capabilities. +mixin EditableArgumentsMixin { + /// Gets information about an invocation at [offset] in [result] that can be + /// edited. + EditableInvocationInfo? getInvocationInfo( + ResolvedUnitResult result, + int offset, + ) { + var node = result.unit.nodeCovering(offset: offset); + // Walk up to find an invocation that is widget creation. + var invocation = node?.thisOrAncestorMatching((node) { + return switch (node) { + InstanceCreationExpression() => node.isWidgetCreation, + InvocationExpressionImpl() => node.isWidgetFactory, + _ => false, + }; + }); + + // Return the related argument list. + var (parameters, argumentList) = switch (invocation) { + InstanceCreationExpression() => ( + invocation.constructorName.element?.formalParameters, + invocation.argumentList, + ), + MethodInvocation( + methodName: Identifier(element: ExecutableElement2 element), + ) => + (element.formalParameters, invocation.argumentList), + _ => (null, null), + }; + + if (parameters == null || argumentList == null) { + return null; + } + + var numPositionals = parameters.where((p) => p.isPositional).length; + var numSuppliedPositionals = + argumentList.arguments + .where((argument) => argument is! NamedExpression) + .length; + + // Build a map of parameters to their positional index so we can tell + // whether a parameter that doesn't already have an argument will be + // editable (positionals can only be added if all previous positionals + // exist). + var currentParameterIndex = 0; + var positionalParameterIndexes = { + for (var parameter in parameters) + if (parameter.isPositional) parameter: currentParameterIndex++, + }; + + // Build a map of the parameters that have arguments so we can put them + // first or look up whether a parameter is editable based on the argument. + var parameterArguments = { + for (var argument in argumentList.arguments) + if (argument.correspondingParameter case var parameter?) + parameter: argument, + }; + + return ( + parameters: parameters, + positionalParameterIndexes: positionalParameterIndexes, + parameterArguments: parameterArguments, + argumentList: argumentList, + numPositionals: numPositionals, + numSuppliedPositionals: numSuppliedPositionals, + ); + } + + /// Checks whether [argument] is editable and if not, returns a human-readable + /// description why. + String? getNotEditableReason({ + required Expression? argument, + required int? positionalIndex, + required int numPositionals, + required int numSuppliedPositionals, + }) { + // If the argument has an existing value, editability is based only on that + // value. + if (argument != null) { + return switch (argument) { + AdjacentStrings() => "Adjacent strings can't be edited", + StringInterpolation() => "Interpolated strings can't be edited", + SimpleStringLiteral() when argument.value.contains('\n') => + "Strings containing newlines can't be edited", + _ => null, + }; + } + + // If we are missing positionals, we can only add this one if it is the next + // (first missing) one. + if (positionalIndex != null && numSuppliedPositionals < numPositionals) { + // To be allowed, we must be the next one. Eg. our index is equal to the + // length/count of the existing ones. + if (positionalIndex != numSuppliedPositionals) { + return 'A value for the ${(positionalIndex + 1).toStringWithSuffix()} ' + "parameter can't be added until a value for all preceding " + 'positional parameters have been added.'; + } + } + + return null; + } + + /// Returns the name of an enum constant prefixed with the enum name. + String? getQualifiedEnumConstantName(FieldElement2 enumConstant) { + var enumName = enumConstant.enclosingElement2.name3; + var name = enumConstant.name3; + return enumName != null && name != null ? '$enumName.$name' : null; + } + + /// Returns a list of the constants of an enum constant prefixed with the enum + /// name. + List getQualifiedEnumConstantNames(EnumElement2 element3) => + element3.constants2.map(getQualifiedEnumConstantName).nonNulls.toList(); +} + +extension on InvocationExpressionImpl { + /// Whether this is an invocation for an extension method that has the + /// `@widgetFactory` annotation. + bool get isWidgetFactory { + // Only consider functions that return widgets. + if (!staticType.isWidgetType) { + return false; + } + + // We only support @widgetFactory on extension methods. + var element = switch (function) { + Identifier(:var element) + when element?.enclosingElement2 is ExtensionElement2 => + element, + _ => null, + }; + + return switch (element) { + FragmentedAnnotatableElementMixin(:var metadata2) => + metadata2.hasWidgetFactory, + _ => false, + }; + } +} diff --git a/pkg/analysis_server/lib/src/lsp/handlers/custom/handler_editable_arguments.dart b/pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_editable_arguments.dart similarity index 58% rename from pkg/analysis_server/lib/src/lsp/handlers/custom/handler_editable_arguments.dart rename to pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_editable_arguments.dart index ed1839702e27..0b86ca478b53 100644 --- a/pkg/analysis_server/lib/src/lsp/handlers/custom/handler_editable_arguments.dart +++ b/pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_editable_arguments.dart @@ -7,30 +7,24 @@ import 'dart:async'; import 'package:analysis_server/lsp_protocol/protocol.dart' hide Element; 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/mapping.dart'; -import 'package:analysis_server/src/utilities/extensions/numeric.dart'; import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/dart/constant/value.dart'; import 'package:analyzer/dart/element/element2.dart'; import 'package:analyzer/dart/element/nullability_suffix.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/src/dart/ast/ast.dart'; -import 'package:analyzer/src/dart/element/element.dart'; import 'package:analyzer/src/lint/constants.dart'; -import 'package:analyzer/src/utilities/extensions/ast.dart'; -import 'package:analyzer/src/utilities/extensions/flutter.dart'; - -/// Information about the arguments and parameters for an invocation. -typedef _InvocationInfo = (List?, ArgumentList?); /// Information about the values for a parameter/argument. typedef _Values = ({bool isDefault, DartObject? parameterValue, DartObject? argumentValue}); class EditableArgumentsHandler - extends - SharedMessageHandler { + extends SharedMessageHandler + with EditableArgumentsMixin { EditableArgumentsHandler(super.server); @override @@ -93,57 +87,41 @@ class EditableArgumentsHandler TextDocumentIdentifier textDocument, int offset, ) { - var (parameters, argumentList) = _getInvocationInfo(result, offset); - if (parameters == null || argumentList == null) { + var invocation = getInvocationInfo(result, offset); + if (invocation == null) { return null; } var textDocument = server.getVersionedDocumentIdentifier(result.path); - // Build a map of parameters to their positional index so we can tell - // whether a parameter that doesn't already have an argument will be - // editable (positionals can only be added if all previous positionals - // exist). - var currentParameterIndex = 0; - var parametersWithIndex = parameters.map( - (parameter) => ( - parameter, - parameter.isPositional ? currentParameterIndex++ : null, - ), - ); - var numPositionals = parameters.where((p) => p.isPositional).length; - var numSuppliedPositionals = - argumentList.arguments - .where((argument) => argument is! NamedExpression) - .length; - - // Build a map of the parameters that have arguments so we can put them - // first. - var parametersWithArguments = { - for (var argument in argumentList.arguments) - argument.correspondingParameter: argument, - }; + var ( + :parameters, + :positionalParameterIndexes, + :parameterArguments, + :argumentList, + :numPositionals, + :numSuppliedPositionals, + ) = invocation; // Build the complete list of editable arguments. var editableArguments = [ // First arguments that exist in the order they were specified. for (var MapEntry(key: parameter, value: argument) - in parametersWithArguments.entries) - if (parameter != null) - _toEditableArgument( - parameter, - argument, - numPositionals: numPositionals, - numSuppliedPositionals: numSuppliedPositionals, - ), + in parameterArguments.entries) + _toEditableArgument( + parameter, + argument, + numPositionals: numPositionals, + numSuppliedPositionals: numSuppliedPositionals, + ), // Then the remaining parameters that don't have existing arguments. - for (var (parameter, index) in parametersWithIndex.where( - (p) => !parametersWithArguments.containsKey(p.$1), + for (var parameter in parameters.where( + (p) => !parameterArguments.containsKey(p), )) _toEditableArgument( parameter, null, - positionalIndex: index, + positionalIndex: positionalParameterIndexes[parameter], numPositionals: numPositionals, numSuppliedPositionals: numSuppliedPositionals, ), @@ -155,67 +133,6 @@ class EditableArgumentsHandler ); } - /// Gets the argument list at [offset] that can be edited. - _InvocationInfo _getInvocationInfo(ResolvedUnitResult result, int offset) { - var node = result.unit.nodeCovering(offset: offset); - // Walk up to find an invocation that is widget creation. - var invocation = node?.thisOrAncestorMatching((node) { - return switch (node) { - InstanceCreationExpression() => node.isWidgetCreation, - InvocationExpressionImpl() => node.isWidgetFactory, - _ => false, - }; - }); - - // Return the related argument list. - return switch (invocation) { - InstanceCreationExpression() => ( - invocation.constructorName.element?.formalParameters, - invocation.argumentList, - ), - MethodInvocation( - methodName: Identifier(element: ExecutableElement2 element), - ) => - (element.formalParameters, invocation.argumentList), - _ => (null, null), - }; - } - - /// Checks whether [argument] is editable and if not, returns a human-readable - /// description why. - String? _getNotEditableReason({ - required Expression? argument, - required int? positionalIndex, - required int numPositionals, - required int numSuppliedPositionals, - }) { - // If the argument has an existing value, editability is based only on that - // value. - if (argument != null) { - return switch (argument) { - AdjacentStrings() => "Adjacent strings can't be edited", - StringInterpolation() => "Interpolated strings can't be edited", - SimpleStringLiteral() when argument.value.contains('\n') => - "Strings containing newlines can't be edited", - _ => null, - }; - } - - // If we are missing positionals, we can only add this one if it is the next - // (first missing) one. - if (positionalIndex != null && numSuppliedPositionals < numPositionals) { - // To be allowed, we must be the next one. Eg. our index is equal to the - // length/count of the existing ones. - if (positionalIndex != numSuppliedPositionals) { - return 'A value for the ${(positionalIndex + 1).toStringWithSuffix()} ' - "parameter can't be added until a value for all preceding " - 'positional parameters have been added.'; - } - } - - return null; - } - /// Computes the values for a parameter and argument and returns them along /// with a flag indicating if the default parameter value is being used. _Values _getValues( @@ -238,13 +155,6 @@ class EditableArgumentsHandler ); } - /// Returns the name of an enum constant prefixed with the enum name. - String? _qualifiedEnumConstant(FieldElement2 enumConstant) { - var enumName = enumConstant.enclosingElement2.name3; - var name = enumConstant.name3; - return enumName != null && name != null ? '$enumName.$name' : null; - } - /// Converts a [parameter]/[argument] pair into an [EditableArgument] if it /// is an argument that can be edited. EditableArgument? _toEditableArgument( @@ -265,7 +175,7 @@ class EditableArgumentsHandler List? options; // Determine whether a value for this parameter is editable. - var notEditableReason = _getNotEditableReason( + var notEditableReason = getNotEditableReason( argument: valueExpression, positionalIndex: positionalIndex, numPositionals: numPositionals, @@ -288,8 +198,7 @@ class EditableArgumentsHandler value = (values.argumentValue ?? values.parameterValue)?.toStringValue(); } else if (parameter.type case InterfaceType(:EnumElement2 element3)) { type = 'enum'; - options = - element3.constants2.map(_qualifiedEnumConstant).nonNulls.toList(); + options = getQualifiedEnumConstantNames(element3); // Try to match the argument value up with the enum. var valueObject = values.argumentValue ?? values.parameterValue; @@ -300,7 +209,7 @@ class EditableArgumentsHandler if (index != null) { var enumConstant = element3.constants2.elementAtOrNull(index); if (enumConstant != null) { - value = _qualifiedEnumConstant(enumConstant); + value = getQualifiedEnumConstantName(enumConstant); } } } @@ -345,28 +254,3 @@ class EditableArgumentsHandler ); } } - -extension on InvocationExpressionImpl { - /// Whether this is an invocation for an extension method that has the - /// `@widgetFactory` annotation. - bool get isWidgetFactory { - // Only consider functions that return widgets. - if (!staticType.isWidgetType) { - return false; - } - - // We only support @widgetFactory on extension methods. - var element = switch (function) { - Identifier(:var element) - when element?.enclosingElement2 is ExtensionElement2 => - element, - _ => null, - }; - - return switch (element) { - FragmentedAnnotatableElementMixin(:var metadata2) => - metadata2.hasWidgetFactory, - _ => false, - }; - } -} diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_states.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_states.dart index 6003bda45022..38b7c405eb40 100644 --- a/pkg/analysis_server/lib/src/lsp/handlers/handler_states.dart +++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_states.dart @@ -8,12 +8,12 @@ import 'package:analysis_server/lsp_protocol/protocol.dart'; import 'package:analysis_server/src/analysis_server.dart'; 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/handler_editable_arguments.dart'; import 'package:analysis_server/src/lsp/handlers/custom/handler_augmentation.dart'; import 'package:analysis_server/src/lsp/handlers/custom/handler_augmented.dart'; import 'package:analysis_server/src/lsp/handlers/custom/handler_connect_to_dtd.dart'; import 'package:analysis_server/src/lsp/handlers/custom/handler_dart_text_document_content_provider.dart'; import 'package:analysis_server/src/lsp/handlers/custom/handler_diagnostic_server.dart'; -import 'package:analysis_server/src/lsp/handlers/custom/handler_editable_arguments.dart'; import 'package:analysis_server/src/lsp/handlers/custom/handler_experimental_echo.dart'; import 'package:analysis_server/src/lsp/handlers/custom/handler_imports.dart'; import 'package:analysis_server/src/lsp/handlers/custom/handler_reanalyze.dart';