[dart2js] Simplifying arrow functions that return a single expression

expressions like this:
() => { return 0; }
become:
() => 0

May have sourcemap ramifications down the line. See: https://github.com/dart-lang/sdk/issues/47354

Change-Id: I66c2794a73fbe2013c3352250f501abcdb0a002c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/210288
Reviewed-by: Stephen Adams <sra@google.com>
Commit-Queue: Mark Zhou <markzipan@google.com>

pkg/compiler/lib/src/js/size_estimator.dart

@@ -843,6 +843,13 @@ class SizeEstimator implements NodeVisitor {
     out("=>");
     int closingPosition;
     Node body = fun.body;
+    // Simplify arrow functions that return a single expression.
+    if (fun.implicitReturnAllowed && body is Block) {
+      final statement = unwrapBlockIfSingleStatement(body);
+      if (statement is Return) {
+        body = statement.value;
+      }
+    }
     if (body is Block) {
       closingPosition = blockOut(body);
     } else {
@@ -854,7 +861,7 @@ class SizeEstimator implements NodeVisitor {
       visitNestedExpression(body, ASSIGNMENT,
           newInForInit: false, newAtStatementBegin: false);
       if (needsParens) out(")");
-      closingPosition = charCount - 1;
+      closingPosition = charCount;
     }
     return closingPosition;
   }

pkg/compiler/test/js/js_parser_test.dart

@@ -18,6 +18,32 @@ testExpression(String expression, [String expect = ""]) {
   }
 }
 
+/// Tests an arrow expression with implicit returns allowed and disallowed.
+///
+/// Only checks the immediate, outermost arrow function.
+testArrowFunction(String arrowExpression,
+    [String implicitReturnExpect = "", String noImplicitReturnExpect = ""]) {
+  jsAst.ArrowFunction fun = js(arrowExpression);
+  jsAst.ArrowFunction implicitReturnFun = jsAst.ArrowFunction(
+      fun.params, fun.body,
+      asyncModifier: fun.asyncModifier, implicitReturnAllowed: true);
+  jsAst.ArrowFunction noImplicitReturnFun = jsAst.ArrowFunction(
+      fun.params, fun.body,
+      asyncModifier: fun.asyncModifier, implicitReturnAllowed: false);
+  String implicitReturnText =
+      jsAst.prettyPrint(implicitReturnFun, allowVariableMinification: false);
+  String noImplicitReturnText =
+      jsAst.prettyPrint(noImplicitReturnFun, allowVariableMinification: false);
+  String comparison =
+      implicitReturnExpect == "" ? arrowExpression : implicitReturnExpect;
+  Expect.stringEquals(comparison, implicitReturnText);
+  if (noImplicitReturnExpect == "") {
+    Expect.stringEquals(comparison, noImplicitReturnText);
+  } else {
+    Expect.stringEquals(noImplicitReturnExpect, noImplicitReturnText);
+  }
+}
+
 testError(String expression, [String expect = ""]) {
   bool doCheck(exception) {
     final exceptionText = '$exception';
@@ -193,13 +219,17 @@ void main() {
   testExpression("a = b = c");
   testExpression("var a = b = c");
   // Arrow functions.
-  testExpression("(x) => x", "x => x");
-  testExpression("(x, y) => {\n  return x + y;\n}");
-  testExpression("() => 42");
-  testExpression('() => ({foo: "bar"})');
-  testExpression("() => {}", """
+  testArrowFunction("(x) => x", "x => x");
+  testArrowFunction(
+      "(x) => {\n  return x;\n}", "x => x", "x => {\n  return x;\n}");
+  testArrowFunction("(x, y) => {\n  return x + y;\n}", "(x, y) => x + y",
+      "(x, y) => {\n  return x + y;\n}");
+  testArrowFunction("() => 42");
+  testArrowFunction('() => ({foo: "bar"})');
+  testArrowFunction("() => {}", """
 () => {
 }""");
+  // Arrow function invocation.
   testExpression("(() => 1)()");
   testExpression("((x) => x)(y)", "(x => x)(y)");
   testExpression("(() => {x = 1;})()", """

pkg/js_ast/lib/src/nodes.dart

@@ -1516,8 +1516,13 @@ class ArrowFunction extends FunctionExpression {
   @override
   final AsyncModifier asyncModifier;
 
+  /// Indicates whether it is permissible to try to emit this arrow function
+  /// in a form with an implicit 'return'.
+  final bool implicitReturnAllowed;
+
   ArrowFunction(this.params, this.body,
-      {this.asyncModifier = AsyncModifier.sync});
+      {this.asyncModifier = AsyncModifier.sync,
+      this.implicitReturnAllowed = true});
 
   T accept<T>(NodeVisitor<T> visitor) => visitor.visitArrowFunction(this);
 
@@ -1534,8 +1539,9 @@ class ArrowFunction extends FunctionExpression {
     body.accept1(visitor, arg);
   }
 
-  ArrowFunction _clone() =>
-      ArrowFunction(params, body, asyncModifier: asyncModifier);
+  ArrowFunction _clone() => ArrowFunction(params, body,
+      asyncModifier: asyncModifier,
+      implicitReturnAllowed: implicitReturnAllowed);
 
   int get precedenceLevel => ASSIGNMENT;
 }

pkg/js_ast/lib/src/printer.dart

@@ -1128,6 +1128,15 @@ class Printer implements NodeVisitor {
     spaceOut();
     int closingPosition;
     Node body = fun.body;
+    // Simplify arrow functions that return a single expression.
+    // Note that this can result in some sourcemapped positions disappearing
+    // around the elided Return. See http://dartbug.com/47354
+    if (fun.implicitReturnAllowed && body is Block) {
+      final statement = unwrapBlockIfSingleStatement(body);
+      if (statement is Return) {
+        body = statement.value;
+      }
+    }
     if (body is Block) {
       closingPosition =
           blockOut(body, shouldIndent: false, needsNewline: false);
@@ -1140,7 +1149,7 @@ class Printer implements NodeVisitor {
       visitNestedExpression(body, ASSIGNMENT,
           newInForInit: false, newAtStatementBegin: false);
       if (needsParens) out(")");
-      closingPosition = _charCount - 1;
+      closingPosition = _charCount;
     }
     localNamer.leaveScope();
     return closingPosition;