DAS: Make CreateMethodOrFunction.fixKind final
With this change, CreateMethodOrFunction complies with the contract
that `fixKind` not change before/after `compute()` is called.
This change includes some tidying to simplify the code:
* The decision of whether to use `DartFixKind.CREATE_METHOD` or
`DartFixKind.CREATE_FUNCTION` is solely based on whether the
"target element" is an InterfaceElement. So in the new factory
constructor, we compute the "target element", and save that in a
final field, as well as the fixKind.
* `isStatic` was computed eagerly with other variables in a nest of
else-if bodies. But it is only needed when creating a _method_, and
so it's value can be computed nearer to where it is used.
* `compute()` was ~100 lines long, and a lot of that code was dedicated
to computing the parameter type. Intermediate values like `argument`
and `parameterElement` sort of clutter the local variables; all of
that code can be moved to a function that just computes the
parameter type.
Change-Id: Ibd5d96b752e7e03e52f585bb594c34331b08cdf1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/436861
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
diff --git a/pkg/analysis_server/lib/src/services/correction/dart/create_method_or_function.dart b/pkg/analysis_server/lib/src/services/correction/dart/create_method_or_function.dart
index 9f6c550..a751b0a 100644
--- a/pkg/analysis_server/lib/src/services/correction/dart/create_method_or_function.dart
+++ b/pkg/analysis_server/lib/src/services/correction/dart/create_method_or_function.dart
@@ -17,11 +17,57 @@
import 'package:analyzer_plugin/utilities/range_factory.dart';
class CreateMethodOrFunction extends ResolvedCorrectionProducer {
- FixKind _fixKind = DartFixKind.CREATE_METHOD;
+ @override
+ final FixKind fixKind;
String _functionName = '';
- CreateMethodOrFunction({required super.context});
+ /// The [Element] for the target of the node's parent, if that is a
+ /// [PrefixedIdentifier] or [PropertyAccess], and `null` otherwise.
+ final Element? _targetElement;
+
+ factory CreateMethodOrFunction({required CorrectionProducerContext context}) {
+ if (context is StubCorrectionProducerContext) {
+ return CreateMethodOrFunction._(context: context);
+ }
+
+ if (context.node case SimpleIdentifier node) {
+ // Prepare the argument expression (to get the parameter).
+ Element? targetElement;
+ var target = getQualifiedPropertyTarget(node);
+ if (target == null) {
+ targetElement = node.enclosingInterfaceElement;
+ } else {
+ var targetType = target.staticType;
+ if (targetType is InterfaceType) {
+ targetElement = targetType.element;
+ } else {
+ targetElement = switch (target) {
+ SimpleIdentifier() => target.element,
+ PrefixedIdentifier() => target.identifier.element,
+ _ => null,
+ };
+ }
+ }
+
+ return CreateMethodOrFunction._(
+ context: context,
+ targetElement: targetElement,
+ fixKind:
+ targetElement is InterfaceElement
+ ? DartFixKind.CREATE_METHOD
+ : DartFixKind.CREATE_FUNCTION,
+ );
+ }
+
+ return CreateMethodOrFunction._(context: context);
+ }
+
+ CreateMethodOrFunction._({
+ required super.context,
+ Element? targetElement,
+ this.fixKind = DartFixKind.CREATE_METHOD,
+ }) : _targetElement = targetElement;
@override
CorrectionApplicability get applicability =>
@@ -32,102 +78,26 @@
List<String> get fixArguments => [_functionName];
@override
- FixKind get fixKind => _fixKind;
-
- @override
Future<void> compute(ChangeBuilder builder) async {
- var isStatic = false;
- var nameNode = node;
- if (nameNode is SimpleIdentifier) {
- // prepare argument expression (to get parameter)
- InterfaceElement? targetElement;
- Expression argument;
+ if (node case SimpleIdentifier node) {
var target = getQualifiedPropertyTarget(node);
- if (target != null) {
- var targetType = target.staticType;
- if (targetType is InterfaceType) {
- targetElement = targetType.element;
- argument = target.parent as Expression;
- } else if (target case SimpleIdentifier(
- :InterfaceElement? element,
- :Expression parent,
- )) {
- isStatic = true;
- targetElement = element;
- argument = parent;
- } else if (target
- case SimpleIdentifier identifier ||
- PrefixedIdentifier(:var identifier)) {
- if (identifier.element case InterfaceElement element) {
- isStatic = true;
- targetElement = element;
- argument = target.parent as Expression;
- } else {
- return;
- }
- } else {
- return;
- }
- } else {
- targetElement = node.enclosingInterfaceElement;
- argument = nameNode;
- }
- argument = stepUpNamedExpression(argument);
- // should be argument of some invocation
- // or child of an expression that is one
- var parameterElement = argument.correspondingParameter;
- int? recordFieldIndex;
- if (argument.parent case ConditionalExpression parent) {
- if (argument == parent.condition) {
- return;
- }
- parameterElement = parent.correspondingParameter;
- } else if (argument.parent case RecordLiteral record) {
- parameterElement = record.correspondingParameter;
- for (var (index, field)
- in record.fields.whereNotType<NamedExpression>().indexed) {
- if (field == argument) {
- recordFieldIndex = index;
- break;
- }
- }
- }
- if (parameterElement == null) {
- return;
- }
- // should be parameter of function type
- var parameterType = parameterElement.type;
- if (parameterType is RecordType) {
- // Finds the corresponding field for argument
- if (argument is NamedExpression) {
- var fieldName = argument.name.label.name;
- for (var field in parameterType.namedFields) {
- if (field.name == fieldName) {
- parameterType = field.type;
- break;
- }
- }
- } else if (recordFieldIndex != null) {
- var field = parameterType.positionalFields[recordFieldIndex];
- parameterType = field.type;
- }
- }
- if (parameterType is InterfaceType && parameterType.isDartCoreFunction) {
- parameterType = FunctionTypeImpl(
- typeFormals: const [],
- parameters: const [],
- returnType: DynamicTypeImpl.instance,
- nullabilitySuffix: NullabilitySuffix.none,
- );
- }
+
+ // In order to fix this by creating a function or a method,
+ // `parameterType` should be be a function type.
+ var parameterType = _computeParameterType(node, target);
if (parameterType is! FunctionType) {
return;
}
- // add proposal
- if (targetElement != null) {
+
+ if (_targetElement is InterfaceElement) {
+ var isStatic =
+ (target is SimpleIdentifier &&
+ target.element is InterfaceElement) ||
+ (target is PrefixedIdentifier &&
+ target.identifier.element is InterfaceElement);
await _createMethod(
builder,
- targetElement,
+ _targetElement,
parameterType,
isStatic: isStatic,
);
@@ -137,6 +107,69 @@
}
}
+ DartType? _computeParameterType(SimpleIdentifier node, Expression? target) {
+ // `argument` should be an argument of some invocation or child of an
+ // expression that is one.
+ Expression argument;
+ if (target == null) {
+ argument = node;
+ } else {
+ var targetParent = target.parent;
+ if (targetParent is! Expression) {
+ return null;
+ }
+ argument = targetParent;
+ }
+ argument = stepUpNamedExpression(argument);
+
+ var parameterElement = argument.correspondingParameter;
+ int? recordFieldIndex;
+ if (argument.parent case ConditionalExpression parent) {
+ if (argument == parent.condition) {
+ return null;
+ }
+ parameterElement = parent.correspondingParameter;
+ } else if (argument.parent case RecordLiteral record) {
+ parameterElement = record.correspondingParameter;
+ for (var (index, field)
+ in record.fields.whereNotType<NamedExpression>().indexed) {
+ if (field == argument) {
+ recordFieldIndex = index;
+ break;
+ }
+ }
+ }
+ if (parameterElement == null) {
+ return null;
+ }
+
+ var parameterType = parameterElement.type;
+ if (parameterType is RecordType) {
+ // Finds the corresponding field for argument
+ if (argument is NamedExpression) {
+ var fieldName = argument.name.label.name;
+ for (var field in parameterType.namedFields) {
+ if (field.name == fieldName) {
+ parameterType = field.type;
+ break;
+ }
+ }
+ } else if (recordFieldIndex != null) {
+ var field = parameterType.positionalFields[recordFieldIndex];
+ parameterType = field.type;
+ }
+ }
+ if (parameterType is InterfaceType && parameterType.isDartCoreFunction) {
+ return FunctionTypeImpl(
+ typeFormals: const [],
+ parameters: const [],
+ returnType: DynamicTypeImpl.instance,
+ nullabilitySuffix: NullabilitySuffix.none,
+ );
+ }
+ return parameterType;
+ }
+
/// Prepares proposal for creating function corresponding to the given
/// [FunctionType].
Future<void> _createExecutable(
@@ -211,7 +244,6 @@
sourcePrefix,
sourceSuffix,
);
- _fixKind = DartFixKind.CREATE_FUNCTION;
_functionName = name;
}
@@ -270,7 +302,6 @@
sourcePrefix,
sourceSuffix,
);
- _fixKind = DartFixKind.CREATE_METHOD;
_functionName = name;
}
}
diff --git a/pkg/analysis_server/lib/src/services/correction/util.dart b/pkg/analysis_server/lib/src/services/correction/util.dart
index b79ea69..4ae647d 100644
--- a/pkg/analysis_server/lib/src/services/correction/util.dart
+++ b/pkg/analysis_server/lib/src/services/correction/util.dart
@@ -258,22 +258,11 @@
/// If given [node] is name of qualified property extraction, returns target
/// from which this property is extracted, otherwise `null`.
-Expression? getQualifiedPropertyTarget(AstNode node) {
- var parent = node.parent;
- if (parent is PrefixedIdentifier) {
- var prefixed = parent;
- if (prefixed.identifier == node) {
- return parent.prefix;
- }
- }
- if (parent is PropertyAccess) {
- var access = parent;
- if (access.propertyName == node) {
- return access.realTarget;
- }
- }
- return null;
-}
+Expression? getQualifiedPropertyTarget(AstNode node) => switch (node.parent) {
+ PrefixedIdentifier parent when parent.identifier == node => parent.prefix,
+ PropertyAccess parent when parent.propertyName == node => parent.realTarget,
+ _ => null,
+};
/// Returns the given [statement] if not a block, or the first child statement
/// if a block, or `null` if more than one child.
