fixes #232, handling of arguments

lib/src/codegen/js_names.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_names;
 
 import 'dart:collection';
 import 'package:dev_compiler/src/js/js_ast.dart';
 
 /// Unique instance for temporary variables. Will be renamed consistently
@@ skipping 42 matching lines @@
 /// * rename JS identifiers to avoid keywords.
 /// * rename temporary variables to avoid colliding with user-specified names,
 ///   or other temporaries
 ///
 /// Each instance of [TemporaryId] is treated as a unique variable, with its
 /// `name` field simply the suggestion of what name to use. By contrast
 /// [Identifiers] are never renamed unless they are an invalid identifier, like
 /// `function` or `instanceof`, and their `name` field controls whether they
 /// refer to the same variable.
 class TemporaryNamer extends LocalNamer {
-  final Map<Object, String> renames;
+  _FunctionScope scope;
 
-  TemporaryNamer(Node node) : renames = new _RenameVisitor.build(node).renames;
+  TemporaryNamer(Node node) : scope = new _RenameVisitor.build(node).rootScope;
 
   String getName(Identifier node) {
-    var rename = renames[identifierKey(node)];
+    var rename = scope.renames[identifierKey(node)];
     if (rename != null) return rename;
-
-    assert(!needsRename(node));
     return node.name;
   }
 
-  void enterScope(FunctionExpression node) {}
-  void leaveScope() {}
+  void enterScope(FunctionExpression node) {
+    scope = scope.functions[node];
+  }
+  void leaveScope() {
+    scope = scope.parent;
+  }
 }
 
 /// Represents a complete function scope in JS.
 ///
 /// We don't currently track ES6 block scopes, because we don't represent them
 /// in js_ast yet.
 class _FunctionScope {
   /// The parent scope.
   final _FunctionScope parent;
 
   /// All names declared in this scope.
   final declared = new HashSet<Object>();
 
   /// All names [declared] in this scope or its [parent]s, that is used in this
   /// scope and/or children. This is exactly the set of variable names we must
   /// not collide with inside this scope.
   final used = new HashSet<String>();
 
   /// Nested functions, these are visited after everything else so the names
   /// they might need are in scope.
-  final functions = new List<FunctionExpression>();
+  final functions = new Map<FunctionExpression, _FunctionScope>();
+
+  /// New names assigned for temps and identifiers.
+  final renames = new HashMap<Object, String>();
 
   _FunctionScope(this.parent);
 }
 
 /// Collects all names used in the visited tree.
 class _RenameVisitor extends VariableDeclarationVisitor {
   final pendingRenames = new Map<Object, Set<_FunctionScope>>();
-  final renames = new HashMap<Object, String>();
 
   final _FunctionScope rootScope = new _FunctionScope(null);
   _FunctionScope scope;
 
   _RenameVisitor.build(Node root) {
     scope = rootScope;
     root.accept(this);
     _finishFunctions();
     _finishNames();
   }
@@ skipping 20 matching lines @@
       declScope = rootScope;
       declScope.declared.add(id);
     }
     _markUsed(node, id, declScope);
   }
 
   _markUsed(Identifier node, Object id, _FunctionScope declScope) {
     // If it needs rename, we can't add it to the used name set yet, instead we
     // will record all scopes it is visible in.
     Set<_FunctionScope> usedIn = null;
-    if (needsRename(node)) {
+    var rename = declScope != rootScope && needsRename(node);
+    if (rename) {
       usedIn = pendingRenames.putIfAbsent(id, () => new HashSet());
     }
     for (var s = scope, end = declScope.parent; s != end; s = s.parent) {
       if (usedIn != null) {
         usedIn.add(s);
       } else {
         s.used.add(node.name);
       }
     }
   }
 
   visitFunctionExpression(FunctionExpression node) {
     // Visit nested functions after all identifiers are declared.
-    scope.functions.add(node);
+    scope.functions[node] = new _FunctionScope(scope);
   }
 
   void _finishFunctions() {
-    for (var f in scope.functions) {
-      scope = new _FunctionScope(scope);
+    scope.functions.forEach((FunctionExpression f, _FunctionScope s) {
+      scope = s;
       super.visitFunctionExpression(f);
       _finishFunctions();
       scope = scope.parent;
-    }
+    });
   }
 
   void _finishNames() {
     var allNames = new Set<String>();
     pendingRenames.forEach((id, scopes) {
       allNames.clear();
       for (var s in scopes) allNames.addAll(s.used);
 
       var name = _findName(id, allNames);
-      renames[id] = name;
-
-      for (var s in scopes) s.used.add(name);
+      for (var s in scopes) {
+        s.used.add(name);
+        s.renames[id] = name;
+      }
     });
   }
 
   static String _findName(Object id, Set<String> usedNames) {
     String name;
     bool valid;
     if (id is TemporaryId) {
       name = id.name;
       valid = !invalidVariableName(name);
     } else {
@@ skipping 77 matching lines @@
 /// In particular, "caller" "callee" and "arguments" cannot be used.
 bool invalidStaticFieldName(String name) {
   switch (name) {
     case "arguments":
     case "caller":
     case "callee":
       return true;
   }
   return false;
 }