Include scope.depth in temporary variable names.

This elegantly ensures that temporary names will not collide with names in
nested or enclosing scopes, without requiring any actual scanning of those
other scopes.

Author: benjamn

lib/scope.js

@@ -2,7 +2,6 @@ var assert = require("assert");
 var types = require("./types");
 var Type = types.Type;
 var namedTypes = types.namedTypes;
-var builders = types.builders;
 var Node = namedTypes.Node;
 var isArray = types.builtInTypes.array;
 var hasOwn = Object.prototype.hasOwnProperty;
@@ -62,17 +61,25 @@ Sp.declares = function(name) {
 };
 
 Sp.declareTemporary = function(prefix) {
-    assert.ok(/^[a-z$_]/i.test(prefix), prefix);
+    if (prefix) {
+        assert.ok(/^[a-z$_]/i.test(prefix), prefix);
+    } else {
+        prefix = "t$";
+    }
+
+    // Include this.depth in the name to make sure the name does not
+    // collide with any variables in nested/enclosing scopes.
+    prefix += this.depth.toString(36) + "$";
+
     this.scan();
 
     var index = 0;
     while (this.declares(prefix + index)) {
         ++index;
     }
 
-    var id = builders.identifier(prefix + index);
-    this.bindings[prefix + index] = id;
-    return id;
+    var name = prefix + index;
+    return this.bindings[name] = types.builders.identifier(name);
 };
 
 Sp.scan = function(force) {

test/run.js

@@ -726,7 +726,7 @@ describe("global scope", function() {
     });
 });
 
-describe("scope.getBindings", function () {
+describe("scope methods", function () {
     var traverse = types.traverse;
 
     var scope = [
@@ -739,8 +739,9 @@ describe("scope.getBindings", function () {
         "};"
     ];
 
-    var ast = parse(scope.join("\n"));
-    it("should get local and global scope bindings", function() {
+    it("getBindings should get local and global scope bindings", function() {
+        var ast = parse(scope.join("\n"));
+
         traverse(ast, function(node) {
             var bindings;
             if (n.Program.check(node)) {
@@ -760,6 +761,64 @@ describe("scope.getBindings", function () {
             }
         });
     });
+
+    it("declareTemporary should use distinct names in nested scopes", function() {
+        var ast = parse(scope.join("\n"));
+        var globalVarDecl;
+        var barVarDecl;
+        var romVarDecl;
+
+        types.visit(ast, {
+            visitProgram: function(path) {
+                path.get("body").unshift(
+                    globalVarDecl = b.variableDeclaration("var", [
+                        b.variableDeclarator(
+                            path.scope.declareTemporary("$"),
+                            b.literal("global")
+                        ),
+                        b.variableDeclarator(
+                            path.scope.declareTemporary("$"),
+                            b.literal("global")
+                        )
+                    ])
+                );
+
+                this.traverse(path);
+            },
+
+            visitFunction: function(path) {
+                var funcId = path.value.id;
+
+                var varDecl = b.variableDeclaration("var", [
+                    b.variableDeclarator(
+                        path.scope.declareTemporary("$"),
+                        b.literal(funcId.name + 1)
+                    ),
+                    b.variableDeclarator(
+                        path.scope.declareTemporary("$"),
+                        b.literal(funcId.name + 2)
+                    )
+                ]);
+
+                path.get("body", "body").unshift(varDecl);
+
+                if (funcId.name === "bar") {
+                    barVarDecl = varDecl;
+                } else if (funcId.name === "rom") {
+                    romVarDecl = varDecl;
+                }
+
+                this.traverse(path);
+            }
+        });
+
+        assert.strictEqual(globalVarDecl.declarations[0].id.name, "$0$0");
+        assert.strictEqual(globalVarDecl.declarations[1].id.name, "$0$1");
+        assert.strictEqual(barVarDecl.declarations[0].id.name, "$1$0");
+        assert.strictEqual(barVarDecl.declarations[1].id.name, "$1$1");
+        assert.strictEqual(romVarDecl.declarations[0].id.name, "$1$0");
+        assert.strictEqual(romVarDecl.declarations[1].id.name, "$1$1");
+    });
 });
 
 describe("catch block scope", function() {