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

 // Copyright (c) 2014, 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.
 
 library services.src.refactoring.extract_local;
 
 import 'dart:async';
 
 import 'package:analysis_server/src/protocol.dart' hide Element;
 import 'package:analysis_server/src/services/correction/name_suggestion.dart';
@@ skipping 111 matching lines @@
       String declarationSource = '$keyword $name = ';
       SourceEdit edit =
           new SourceEdit(singleExpression.offset, 0, declarationSource);
       change.addEdit(file, edit);
       return new Future.value(change);
     }
     // add variable declaration
     {
       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;
     if (stringLiteralPart != null) {
       occurrenceReplacement = "\${$name}";
     }
     // replace occurrences with variable reference
     for (SourceRange range in occurrences) {
       SourceEdit edit = new SourceEdit.range(range, occurrenceReplacement);
       change.addEdit(file, edit);
@@ skipping 76 matching lines @@
   List<AstNode> _findNodes(List<SourceRange> ranges) {
     List<AstNode> nodes = <AstNode>[];
     for (SourceRange range in ranges) {
       AstNode node = new NodeLocator.con1(range.offset).searchWithin(unit);
       nodes.add(node);
     }
     return nodes;
   }
 
   /**
-   * 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;
   }
 
   /**
    * Checks if it is OK to extract the node with the given [SourceRange].
    */
   bool _isExtractable(SourceRange range) {
     _ExtractExpressionAnalyzer analyzer = new _ExtractExpressionAnalyzer(range);
     utils.unit.accept(analyzer);
     return analyzer.status.isOK;
   }
@@ skipping 261 matching lines @@
       Token startToken = nodeTokens[startTokenIndex];
       Token endToken = nodeTokens[endTokenIndex];
       // add occurrence range
       int occuStart = nodeOffset + startToken.offset;
       int occuEnd = nodeOffset + endToken.end;
       SourceRange occuRange = rangeStartEnd(occuStart, occuEnd);
       _addOccurrence(occuRange);
     }
   }
 }