DAS: Make CreateMethodOrFunction.fixKind final

srawlins · Commit Queue · commit 04e11b83c00d · 2025-06-25T12:15:41.000-07:00
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
@@ -17,11 +17,57 @@ import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
 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 =>
@@ -31,103 +77,27 @@ class CreateMethodOrFunction extends ResolvedCorrectionProducer {
   @override
   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 @@ class CreateMethodOrFunction extends ResolvedCorrectionProducer {
     }
   }
 
+  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 @@ class CreateMethodOrFunction extends ResolvedCorrectionProducer {
       sourcePrefix,
       sourceSuffix,
     );
-    _fixKind = DartFixKind.CREATE_FUNCTION;
     _functionName = name;
   }
 
@@ -270,7 +302,6 @@ class CreateMethodOrFunction extends ResolvedCorrectionProducer {
       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
@@ -258,22 +258,11 @@ List<AstNode> getParents(AstNode node) {
 
 /// 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.
