implement new and improved let scoping - fixes #91, fixes #102, fixes #124

lib/6to5/scope.js

@@ -6,24 +6,23 @@ var _        = require("lodash");
 
 function Scope(parent, block) {
   this.parent = parent;
+  this.block  = block;
   this.ids    = block._scopeIds = block._scopeIds || Scope.getIds(block);
 }
 
 Scope.getIds = function (block) {
-  var ids = [];
+  var ids = {};
 
   if (t.isBlockStatement(block)) {
     _.each(block.body, function (node) {
       if (t.isVariableDeclaration(node) && node.kind !== "var") {
-        ids = ids.concat(t.getIds(node));
+        ids = t.getIds(node, true);
       }
     });
-  }
-
-  if (t.isProgram(block) || t.isFunction(block)) {
+  } else if (t.isProgram(block) || t.isFunction(block)) {
     traverse(block, function (node) {
       if (t.isVariableDeclaration(node) && node.kind === "var") {
-        ids = ids.concat(t.getIds(node));
+        ids = t.getIds(node, true);
       } else if (t.isFunction(node)) {
         return false;
       }
@@ -32,16 +31,33 @@ Scope.getIds = function (block) {
 
   if (t.isFunction(block)) {
     _.each(block.params, function (param) {
-      ids = ids.concat(t.getIds(param));
+      _.merge(ids, t.getIds(param, true));
     });
   }
 
   return ids;
 };
 
-Scope.prototype.has = function (id, noParent) {
-  if (!id) return false;
-  if (_.contains(this.ids, id)) return true;
-  if (noParent !== false && this.parent) return this.parent.has(id);
-  return false;
+Scope.prototype.get = function (id) {
+  return id && (this.getOwn(id) || this.parentGet(id));
+};
+
+Scope.prototype.getOwn = function (id) {
+  return _.has(this.ids, id) && this.ids[id];
+};
+
+Scope.prototype.parentGet = function (id) {
+  return this.parent && this.parent.get(id);
+};
+
+Scope.prototype.has = function (id) {
+  return id && (this.hasOwn(id) || this.parentHas(id));
+};
+
+Scope.prototype.hasOwn = function (id) {
+  return !!this.getOwn(id);
+};
+
+Scope.prototype.parentHas = function (id) {
+  return this.parent && this.parent.has(id);
 };

lib/6to5/transformers/let-scoping.js

@@ -3,58 +3,276 @@ var util     = require("../util");
 var t        = require("../types");
 var _        = require("lodash");
 
-exports.VariableDeclaration = function (node, parent, file) {
-  if (node.kind !== "let") return;
+var isLet = function (node) {
+  if (!t.isVariableDeclaration(node)) return false;
+  if (node._let) return true;
+  if (node.kind !== "let") return false;
+
+  node._let = true;
   node.kind = "var";
+  return true;
+};
 
-  var ids = {};
+var isVar = function (node) {
+  return t.isVariableDeclaration(node) && node.kind === "var" && !isLet(node);
+};
 
-  _.each(node.declarations, function (declar) {
-    _.each(util.getIds(declar.id), function (id) {
-      ids[id] = t.identifier(file.generateUid(id));
+exports.VariableDeclaration = function (node) {
+  isLet(node);
+};
+
+exports.For = function (node, parent, file, scope) {
+  var init = node.left || node.init;
+  if (isLet(init)) {
+    t.ensureBlock(node);
+    node.body._letDeclars = [init];
+  }
+
+  if (t.isLabeledStatement(parent)) {
+    // set label so `run` has access to it
+    node.label = parent.label;
+  }
+
+  run(node, node.body, parent, file, scope);
+
+  if (node.label && !t.isLabeledStatement(parent)) {
+    // we've been given a label so let's wrap ourself
+    return t.labeledStatement(node.label, node);
+  }
+};
+
+exports.BlockStatement = function (block, parent, file, scope) {
+  if (!t.isFor(parent)) {
+    run(false, block, parent, file, scope);
+  }
+};
+
+var noClosure = function (letDeclars, block, replacements) {
+  standardiseLets(letDeclars);
+
+  if (_.isEmpty(replacements)) return;
+
+  traverse(block, function (node, parent) {
+    if (!t.isIdentifier(node)) return;
+    if (!t.isReferenced(node, parent)) return;
+    node.name = replacements[node.name] || node.name;
+  });
+};
+
+var standardiseLets = function (declars) {
+  _.each(declars, function (declar) {
+    delete declar._let;
+  });
+};
+
+var getInfo = function (block, file, scope) {
+  var opts = {
+    outsideKeys: [],
+    declarators:  block._letDeclars || [],
+    duplicates:  {},
+    keys:        []
+  };
+
+  _.each(opts.declarators, function (declar) {
+    opts.declarators.push(declar);
+
+    var keys = t.getIds(declar);
+    opts.outsideKeys = opts.outsideKeys.concat(keys);
+    opts.keys = opts.keys.concat(keys);
+  });
+
+  _.each(block.body, function (declar) {
+    if (!isLet(declar)) return;
+
+    _.each(t.getIds(declar, true), function (id, key) {
+      var has = scope.parentGet(key);
+      if (has && has !== id) {
+        opts.duplicates[key] = id.name = file.generateUid(key, scope);
+      }
+
+      opts.keys.push(key);
     });
   });
 
-  var replaceId = function (node, parent) {
-    // not an identifier so we have no use for this node
-    if (node.type !== "Identifier") return;
+  return opts;
+};
 
-    // not a let reference
-    var id = ids[node.name];
-    if (!id) return;
+var run = function (forParent, block, parent, file, scope) {
+  if (block._letDone) return;
+  block._letDone = true;
 
-    if (t.isReferenced(node, parent)) return id;
+  var info = getInfo(block, file, scope);
+  var declarators = info.declarators;
+  var letKeys = info.keys;
+
+  //
+
+  if (!letKeys.length) return noClosure(declarators, block, info.duplicates);
+
+  //
+
+  var letReferences = {};
+  var closurify = false;
+
+  traverse(block, function (node, parent, opts) {
+    if (t.isFunction(node)) {
+      traverse(node, function (node, parent, opts2) {
+        if (!t.isIdentifier(node)) return;
+        if (!t.isReferenced(node, parent)) return;
+        if (opts.scope.hasOwn(node.name)) return;
+
+        closurify = true;
+        if (!_.contains(info.outsideKeys, node.name)) return;
+        if (_.has(letReferences, node.name)) return;
+
+        letReferences[node.name] = node;
+      });
+      return false;
+    } else if (t.isFor(node)) {
+      return false;
+    }
+  });
+
+  letReferences = _.values(letReferences);
+
+  if (!closurify) return noClosure(declarators, block, info.duplicates);
+
+  //
+
+  var has = {
+    hasContinue: false,
+    hasReturn:   false,
+    hasBreak:    false,
   };
 
-  var isProgram = t.isProgram(parent);
+  if (forParent) {
+    traverse(block, function (node) {
+      var replace;
 
-  var replace = function (node, parent) {
-    if (!isProgram && _.contains(t.FUNCTION_TYPES, node.type)) {
-      var letReferences = [];
-
-      traverse(node, function (node, parent) {
-        var id = replaceId(node, parent);
-        if (id && !_.contains(letReferences, id)) letReferences.push(id);
-        return id;
-      });
-
-      if (letReferences.length) {
-        if (t.isFunctionDeclaration(node)) {
-          throw new Error("`FunctionDeclaration`s that use `let` and `constant` references aren't allowed outside of the root scope");
-        } else {
-          var func = t.functionExpression(null, letReferences, t.blockStatement([
-            t.returnStatement(node)
-          ]));
-          func._aliasFunction = true;
-          return t.callExpression(func, letReferences);
-        }
-      } else {
+      if (t.isFunction(node) || t.isFor(node)) {
         return false;
+      } else if (t.isBreakStatement(node) && !node.label) {
+        has.hasBreak = true;
+        replace = t.returnStatement(t.literal("break"));
+      } else if (t.isContinueStatement(node) && !node.label) {
+        has.hasContinue = true;
+        replace = t.returnStatement(t.literal("continue"));
+      } else if (t.isReturnStatement(node)) {
+        has.hasReturn = true;
+        replace = t.returnStatement(t.objectExpression([
+          t.property("init", t.identifier("v"), node.argument)
+        ]));
+      }
+
+      if (replace) return t.inherits(replace, node);
+    });
+  }
+
+  //
+
+  var body = [];
+
+  var pushDeclar = function (node) {
+    body.push(t.variableDeclaration("var", node.declarations.map(function (declar) {
+      return t.variableDeclarator(declar.id);
+    })));
+
+    var replace = [];
+
+    _.each(node.declarations, function (declar) {
+      if (declar.init) replace.push(buildDeclarAssign(declar));
+    });
+
+    return replace;
+  };
+
+  var buildDeclarAssign = function (declar) {
+    var expr = t.assignmentExpression("=", declar.id, declar.init);
+    return t.inherits(expr, declar);
+  };
+
+  // hoist `var` declarations
+  traverse(block, function (node, parent) {
+    if (t.isForStatement(node)) {
+      if (isVar(node.init)) {
+        node.init = t.sequenceExpression(pushDeclar(node.init));
+      }
+    } else if (t.isFor(node)) {
+      if (isVar(node.left)) {
+        body.push()
+        node.left = node.left.declarations[0].id;
+      }
+    } else if (isVar(node)) {
+      return pushDeclar(node).map(t.expressionStatement);
+    } else if (t.isFunction(node)) {
+      return false;
+    }
+  });
+
+  standardiseLets(declarators);
+
+  //
+
+  if (t.isFunction(parent)) return;
+
+  //
+
+  var fn = t.functionExpression(null, letReferences, t.blockStatement(block.body));
+  fn._aliasFunction = true;
+
+  var params = _.cloneDeep(letReferences);
+
+  _.each(params, function (param) {
+    param.name = info.duplicates[param.name] || param.name;
+  });
+
+  var call = t.callExpression(fn, params);
+
+  block.body = body;
+
+  //
+
+  var ret = t.identifier(file.generateUid("ret", scope));
+
+  var retCheck;
+
+  if (has.hasReturn || has.hasBreak || has.hasContinue) {
+    body.push(t.variableDeclaration("var", [
+      t.variableDeclarator(ret, call)
+    ]));
+
+    if (has.hasReturn) {
+      retCheck = t.ifStatement(
+        t.binaryExpression("===", t.unaryExpression("typeof", ret, true), t.literal("object")),
+        t.returnStatement(t.memberExpression(ret, t.identifier("v")))
+      );
+
+      if (!has.hasBreak && !has.hasContinue) {
+        body.push(retCheck);
       }
     }
 
-    return replaceId(node, parent);
-  };
+    if (has.hasBreak || has.hasContinue) {
+      var label = forParent.label = forParent.label || t.identifier(file.generateUid("loop", scope));
 
-  traverse(parent, replace);
+      var cases = [];
+
+      if (has.hasBreak) {
+        cases.push(t.switchCase(t.literal("break"), [t.breakStatement(label)]));
+      }
+
+      if (has.hasContinue) {
+        cases.push(t.switchCase(t.literal("continue"), [t.continueStatement(label)]));
+      }
+
+      if (has.hasReturn) {
+        cases.push(t.switchCase(null, [retCheck]));
+      }
+
+      body.push(t.switchStatement(ret, cases));
+    }
+
+  } else {
+    body.push(t.expressionStatement(call));
+  }
 };

lib/6to5/traverse.js

@@ -48,9 +48,13 @@ function traverse(parent, callbacks, opts) {
         if (result != null) node = obj[key] = result;
       };
 
+      //
+      var opts2 = _.clone(opts);
+      if (t.isScope(node)) opts2.scope = new Scope(opts.scope, node);
+
       // enter
       if (callbacks.enter) {
-        result = callbacks.enter(node, parent, opts);
+        result = callbacks.enter(node, parent, opts2);
 
         // stop iteration
         if (result === false) return;
@@ -59,13 +63,11 @@ function traverse(parent, callbacks, opts) {
       }
 
       // traverse node
-      var opts2 = _.clone(opts);
-      if (t.isScope(node)) opts2.scope = new Scope(opts.scope, node);
       traverse(node, callbacks, opts2);
 
       // exit
       if (callbacks.exit) {
-        maybeReplace(callbacks.exit(node, parent, opts));
+        maybeReplace(callbacks.exit(node, parent, opts2));
       }
     };
 

lib/6to5/types/index.js

@@ -94,15 +94,15 @@ t.toBlock = function (node, parent) {
   return t.blockStatement(node);
 };
 
-t.getIds = function (node) {
+t.getIds = function (node, map) {
   var search = [node];
-  var ids  = [];
+  var ids    = {};
 
   while (search.length) {
     var id = search.shift();
 
     if (t.isIdentifier(id)) {
-      ids.push(id.name);
+      ids[id.name] = id;
     } else if (t.isArrayPattern(id)) {
       _.each(id.elements, function (elem) {
         search.push(elem);
@@ -122,6 +122,7 @@ t.getIds = function (node) {
     }
   }
 
+  if (!map) ids = _.keys(ids);
   return ids;
 };
 

test/fixtures/transformation/constants/block-statement/expected.js

@@ -1,6 +1,6 @@
 "use strict";
 
-for (var _i in arr) {
-  var _MULTIPLIER = 5;
-  console.log(arr[_i] * _MULTIPLIER);
-}
+for (var i in arr) {
+  var MULTIPLIER = 5;
+  console.log(arr[i] * MULTIPLIER);
+}

test/fixtures/transformation/constants/destructuring/expected.js

@@ -1,20 +1,20 @@
 "use strict";
 
 var _ref = [1, 2];
-var _a = _ref[0];
-var _b = _ref[1];
+var a = _ref[0];
+var b = _ref[1];
 var _ref2 = [3, 4];
-var _c = _ref2[0];
-var _d = _ref2[1];
+var c = _ref2[0];
+var d = _ref2[1];
 var _ref3 = {
   e: 5,
   f: 6
 };
-var _e = _ref3.e;
-var _f = _ref3.f;
+var e = _ref3.e;
+var f = _ref3.f;
 var _ref4 = {
   a: 7,
   b: 8
 };
-var _g = _ref4.a;
-var _h = _ref4.b;
+var g = _ref4.a;
+var h = _ref4.b;

test/fixtures/transformation/constants/program/expected.js

@@ -1,7 +1,7 @@
 "use strict";
 
-var _MULTIPLIER = 5;
+var MULTIPLIER = 5;
 
 for (var i in arr) {
-  console.log(arr[i] * _MULTIPLIER);
-}
+  console.log(arr[i] * MULTIPLIER);
+}

test/fixtures/transformation/for-of/let/expected.js

@@ -1,5 +1,5 @@
 "use strict";
 
 for (var _iterator = arr[Symbol.iterator](), _step; !(_step = _iterator.next()).done;) {
-  var _i = _step.value;
-}
+  var i = _step.value;
+}

test/fixtures/transformation/let-scoping/closure/actual.js

@@ -0,0 +1,9 @@
+(function() {
+  let foo = 'bar';
+
+  [true].forEach(function() {
+    foo = 'baz';
+  });
+
+  console.log(foo);
+});

test/fixtures/transformation/let-scoping/closure/expected.js

@@ -0,0 +1,11 @@
+"use strict";
+
+(function () {
+  var foo = "bar";
+
+  [true].forEach(function () {
+    foo = "baz";
+  });
+
+  console.log(foo);
+});

test/fixtures/transformation/let-scoping/deep-nested/exec.js

@@ -2,11 +2,11 @@ var a = 'var a';
 {
   var b = 'var b';
   {
-  var c = 'var c';
-  let d = 'let d';
-  assert.equal(d, 'let d');
+    var c = 'var c';
+    let d = 'let d';
+    assert.equal(d, 'let d');
   }
   assert.throws(function () {
-  d;
+    d;
   }, /d is not defined/);
 }

test/fixtures/transformation/let-scoping/for-in-initializers/exec.js

@@ -3,11 +3,11 @@ var result;
   let let_result = [];
   let let_array = ['one', 'two', 'three'];
   for (var index in let_array) {
-  let let_index = index;
-  let let_value = let_array[let_index];
-  let_result.push(function () {
-    return [let_index, let_value];
-  });
+    let let_index = index;
+    let let_value = let_array[let_index];
+    let_result.push(function () {
+      return [let_index, let_value];
+    });
   }
   result = let_result;
 }

test/fixtures/transformation/let-scoping/for-initializers/exec.js

@@ -3,10 +3,10 @@ var result;
   let let_x = 'let x';
   let let_l = [];
   for (var var_x = 1, var_y = 2, var_z = 3; var_x < 10; var_x ++) {
-  let l_x = var_x, l_y = var_y, l_z = var_z;
-  let_l.push(function () {
-    return [l_x, l_y, l_z];
-  });
+    let l_x = var_x, l_y = var_y, l_z = var_z;
+    let_l.push(function () {
+      return [l_x, l_y, l_z];
+    });
   }
   result = let_l;
 }

test/fixtures/transformation/let-scoping/function-declaration/actual.js

@@ -0,0 +1,9 @@
+domready(function () {
+  let a = 1;
+  const b = 1;
+  runIt();
+
+  function runIt() {
+    console.log(a + b);
+  }
+});

test/fixtures/transformation/let-scoping/function-declaration/expected.js

@@ -0,0 +1,11 @@
+"use strict";
+
+domready(function () {
+  var a = 1;
+  var b = 1;
+  runIt();
+
+  function runIt() {
+    console.log(a + b);
+  }
+});

test/fixtures/transformation/let-scoping/in-closure/exec.js

@@ -1,13 +1,13 @@
 function letInClosure(n) {
   var l = [];
   for (var i = 0; i < n; i ++) {
-  let let_i = i;
-  if (i % 3 == 0) {
-    continue;
-  }
-  l.push(function () {
-    return let_i;
-  });
+    let let_i = i;
+    if (i % 3 == 0) {
+      continue;
+    }
+    l.push(function () {
+      return let_i;
+    });
   }
   return l;
 }

test/fixtures/transformation/let-scoping/no-nested-function-declarations/actual.js

@@ -1,7 +0,0 @@
-var numbers = [1, 2, 3];
-
-for (let i in numbers) {
-  function foo() {
-    return i;
-  }
-}

test/fixtures/transformation/let-scoping/no-nested-function-declarations/options.json

@@ -1,3 +0,0 @@
-{
-  "throws": "`FunctionDeclaration`s that use `let` and `constant` references aren't allowed outside of the root scope"
-}

test/fixtures/transformation/let-scoping/no-renaming/actual.js

@@ -0,0 +1,6 @@
+function foo (n) {
+  let bar = n;
+  eval('console.log(bar)');
+}
+
+foo(42);

test/fixtures/transformation/let-scoping/no-renaming/expected.js

@@ -0,0 +1,9 @@
+"use strict";
+
+function foo(n) {
+  var bar = n;
+  eval("console.log(bar)");
+}
+
+foo(42);
+

test/fixtures/transformation/modules-amd/exports-variable/expected.js

@@ -8,12 +8,12 @@ define(["exports"], function (exports) {
   exports.foo2 = foo2;
   var foo3;
   exports.foo3 = foo3;
-  var _foo4 = 2;
-  exports.foo4 = _foo4;
-  var _foo5;
-  exports.foo5 = _foo5;
-  var _foo6 = 3;
-  exports.foo6 = _foo6;
+  var foo4 = 2;
+  exports.foo4 = foo4;
+  var foo5;
+  exports.foo5 = foo5;
+  var foo6 = 3;
+  exports.foo6 = foo6;
   function foo7() {}
 
   var foo8 = function foo8() {};

test/fixtures/transformation/modules-common/exports-variable/expected.js

@@ -7,12 +7,12 @@ var foo2 = function () {};
 exports.foo2 = foo2;
 var foo3;
 exports.foo3 = foo3;
-var _foo4 = 2;
-exports.foo4 = _foo4;
-var _foo5;
-exports.foo5 = _foo5;
-var _foo6 = 3;
-exports.foo6 = _foo6;
+var foo4 = 2;
+exports.foo4 = foo4;
+var foo5;
+exports.foo5 = foo5;
+var foo6 = 3;
+exports.foo6 = foo6;
 function foo7() {}
 
 var foo8 = function foo8() {};

test/fixtures/transformation/modules-umd/exports-variable/expected.js

@@ -14,12 +14,12 @@
   exports.foo2 = foo2;
   var foo3;
   exports.foo3 = foo3;
-  var _foo4 = 2;
-  exports.foo4 = _foo4;
-  var _foo5;
-  exports.foo5 = _foo5;
-  var _foo6 = 3;
-  exports.foo6 = _foo6;
+  var foo4 = 2;
+  exports.foo4 = foo4;
+  var foo5;
+  exports.foo5 = foo5;
+  var foo6 = 3;
+  exports.foo6 = foo6;
   function foo7() {}
 
   var foo8 = function foo8() {};