fix readability regressions from previous block-scope CL

lib/src/codegen/js_codegen.dart

 // Copyright (c) 2015, 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 dev_compiler.src.codegen.js_codegen;
 
 import 'dart:collection' show HashSet, HashMap, SplayTreeSet;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
 import 'package:analyzer/src/generated/ast.dart' hide ConstantEvaluator;
@@ skipping 1749 matching lines @@
     var ret = new JS.Return(_visit(node.expression));
     return new JS.Block(initArgs != null ? [initArgs, ret] : [ret]);
   }
 
   @override
   JS.Block visitEmptyFunctionBody(EmptyFunctionBody node) => new JS.Block([]);
 
   @override
   JS.Block visitBlockFunctionBody(BlockFunctionBody node) {
     var initArgs = _emitArgumentInitializers(node.parent);
-    var block = visitBlock(node.block);
-    if (initArgs != null) return new JS.Block([initArgs, block]);
-    return block;
+    var stmts = _visitList(node.block.statements) as List<JS.Statement>;
+    if (initArgs != null) stmts.insert(0, initArgs);
+    return new JS.Block(stmts);
   }
 
   @override
   JS.Block visitBlock(Block node) =>
       new JS.Block(_visitList(node.statements) as List<JS.Statement>,
           isScope: true);
 
   @override
   visitMethodInvocation(MethodInvocation node) {
     if (node.operator != null && node.operator.lexeme == '?.') {
@@ skipping 1142 matching lines @@
             'let # = #;', [_visit(name), _visit(_catchParameter)]));
         _catchParameter = name;
       }
       if (node.stackTraceParameter != null) {
         var stackVar = node.stackTraceParameter.name;
         body.add(js.statement(
             'let # = dart.stackTrace(#);', [stackVar, _visit(name)]));
       }
     }
 
-    body.add(_visit(node.body));
+    body.add(
+        new JS.Block(_visitList(node.body.statements) as List<JS.Statement>));

[Leaf, 2015/09/09 21:29:45]

Will JS be ok with this hoisting if either the exception parameter or the stack
trace parameter share the same name as a variable declared in the list of
statements being hoisted out?

[Jennifer Messerly, 2015/09/09 21:48:39]

JS wouldn't care, unless we end up desugaring (_catchParameter != null or
node.stackTraceParameter != null) ... but in that case, we have a duplicate
declaration.

If Dart really supports having a local variable that shadows and the exception
and/or stack trace (ugh, why?!?!) then we'll need to fix it, or make it an error
(because that seems like pretty useless behavior) :|.

Assuming that behavior is specified, we've had this bug forever though, until I
"fixed" it unintentionally last Friday. So I'd be happiest to land this and then
investigate that issue in a follow up, if that works for you. I think the fix
will be unrelated: to avoid a large readability regression, we'll need to do
some smarts to detect the conflict specifically, and that's unlike the other
places. Or, we need to change how variable declaration is generated in this case
to make it be an assignment.

[Jennifer Messerly, 2015/09/09 22:02:57]

confirmed that it's spec'd that way.

     _catchParameter = savedCatch;
     return _statement(body);
   }
 
   @override
   JS.Case visitSwitchCase(SwitchCase node) {
     var expr = _visit(node.expression);
     var body = _visitList(node.statements) as List<JS.Statement>;
     if (node.labels.isNotEmpty) {
       body.insert(0, js.comment('Unimplemented case labels: ${node.labels}'));
@@ skipping 362 matching lines @@
 
 /// A special kind of element created by the compiler, signifying a temporary
 /// variable. These objects use instance equality, and should be shared
 /// everywhere in the tree where they are treated as the same variable.
 class TemporaryVariableElement extends LocalVariableElementImpl {
   TemporaryVariableElement.forNode(Identifier name) : super.forNode(name);
 
   int get hashCode => identityHashCode(this);
   bool operator ==(Object other) => identical(this, other);
 }