Issue 18895. Convert an expression function body into the block function body when Extract Local.

pkg/analysis_server/lib/src/services/refactoring/extract_local.dart

Index: pkg/analysis_server/lib/src/services/refactoring/extract_local.dart
diff --git a/pkg/analysis_server/lib/src/services/refactoring/extract_local.dart b/pkg/analysis_server/lib/src/services/refactoring/extract_local.dart
index ba189d25c6cb657e02930577cac1d3c5d8324ff3..1494558f20c93a04c3a6e8656f05a2f76ca47142 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/extract_local.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/extract_local.dart
@@ -129,22 +129,54 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl implements
     {
       String declarationSource;
       if (stringLiteralPart != null) {
-        declarationSource = "var ${name} = '${stringLiteralPart}';";
+        declarationSource = "var $name = '$stringLiteralPart';";
       } else {
         String keyword = _declarationKeyword;
         String initializerSource = utils.getRangeText(selectionRange);
-        declarationSource = "${keyword} ${name} = ${initializerSource};";
+        declarationSource = "$keyword $name = $initializerSource;";
       }
+      String eol = utils.endOfLine;
       // prepare location for declaration
-      Statement targetStatement = _findTargetStatement(occurrences);
-      String prefix = utils.getNodePrefix(targetStatement);
+      Statement targetStatement;

[Paul Berry, 2014/09/15 14:45:23]

This code is confusing because of the implicit convention that either
targetStatement or targetBody will be null, depending whether the target is a
statement or a body.

How about something like this instead?

AstNode target;
{
  ...
  if (commonParent is Block) {
    ...
    target = firstParents[commonIndex + 1];
  } else {
    target = _getEnclosingExpressionBody(commonParent);
    if (target == null) {
      target = commonParent.getAncestor((node) => node is Statement);
    }
  }
  // insert variable declaration
  if (target is Statement) {
    ...
  } else if (target is ExpressionFunctionBody) {
    ...
  }

[scheglov, 2014/09/15 15:28:12]

Done.

+      ExpressionFunctionBody targetBody;
+      {
+        List<AstNode> nodes = _findNodes(occurrences);
+        AstNode commonParent = getNearestCommonAncestor(nodes);
+        if (commonParent is Block) {
+          List<AstNode> firstParents = getParents(nodes[0]);
+          int commonIndex = firstParents.indexOf(commonParent);
+          targetStatement = firstParents[commonIndex + 1];
+        } else {
+          targetBody = _getEnclosingExpressionBody(commonParent);
+          if (targetBody == null) {
+            targetStatement =
+                commonParent.getAncestor((node) => node is Statement);
+          }
+        }
+      }
       // insert variable declaration
-      String eol = utils.endOfLine;
-      SourceEdit edit = new SourceEdit(
-          targetStatement.offset,
-          0,
-          '${declarationSource}${eol}${prefix}');
-      change.addEdit(file, edit);
+      if (targetStatement != null) {
+        String prefix = utils.getNodePrefix(targetStatement);
+        SourceEdit edit =
+            new SourceEdit(targetStatement.offset, 0, declarationSource + eol + prefix);
+        change.addEdit(file, edit);
+      }
+      if (targetBody != null) {
+        String prefix = utils.getNodePrefix(targetBody.parent);
+        String indent = utils.getIndent(1);
+        String declStatement = prefix + indent + declarationSource + eol;
+        String exprStatement = prefix + indent + 'return ';
+        Expression expr = targetBody.expression;
+        change.addEdit(
+            file,
+            new SourceEdit(
+                targetBody.offset,
+                expr.offset - targetBody.offset,
+                '{' + eol + declStatement + exprStatement));
+        change.addEdit(
+            file,
+            new SourceEdit(expr.end, 0, ';' + eol + prefix + '}'));
+      }
     }
     // prepare replacement
     String occurrenceReplacement = name;
@@ -241,19 +273,20 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl implements
   }
 
   /**
-   * Returns the [Statement] such that variable declaration added before it is
-   * visible at all given occurrences.
+   * Returns the [ExpressionFunctionBody] that encloses [node], or `null`
+   * if [node] is not enclosed with an [ExpressionFunctionBody].
    */
-  Statement _findTargetStatement(List<SourceRange> occurrences) {
-    List<AstNode> nodes = _findNodes(occurrences);
-    List<AstNode> firstParents = getParents(nodes[0]);
-    AstNode commonParent = getNearestCommonAncestor(nodes);
-    if (commonParent is Block) {
-      int commonIndex = firstParents.indexOf(commonParent);
-      return firstParents[commonIndex + 1] as Statement;
-    } else {
-      return commonParent.getAncestor((node) => node is Statement);
+  ExpressionFunctionBody _getEnclosingExpressionBody(AstNode node) {
+    while (node != null) {
+      if (node is Statement) {
+        return null;
+      }
+      if (node is ExpressionFunctionBody) {
+        return node;
+      }
+      node = node.parent;
     }
+    return null;
   }
 
   /**