[[FIX]] Allow closing over immutable bindings

The risk of confusion for functions declared within loops only pertains to references to mutable bindings. Relax the wanting regarding "confusing semantics" to only occur in the presence of one or more references to mutable bindings.
jshint · Aug 21, 2017 · 7091685 · 7091685
1 parent 9fe8c94
commit 7091685
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 6 deletions.
diff --git a/src/scope-manager.js b/src/scope-manager.js
@@ -262,7 +262,7 @@ var scopeManager = function(state, predefined, exported, declared) {
         isUnstackingFunctionParams = _current["(type)"] === "functionparams",
         isUnstackingFunctionOuter = _current["(type)"] === "functionouter";
 
-      var i, j;
+      var i, j, isImmutable;
       var currentUsages = _current["(usages)"];
       var currentLabels = _current["(labels)"];
       var usedLabelNameList = Object.keys(currentUsages);
@@ -278,7 +278,7 @@ var scopeManager = function(state, predefined, exported, declared) {
         var usedLabel = currentLabels[usedLabelName];
         if (usedLabel) {
           var usedLabelType = usedLabel["(type)"];
-          var isImmutable = usedLabelType === "const" || usedLabelType === "import";
+          isImmutable = usedLabelType === "const" || usedLabelType === "import";
 
           if (usedLabel["(useOutsideOfScope)"] && !state.option.funcscope) {
             var usedTokens = usage["(tokens)"];
@@ -314,11 +314,14 @@ var scopeManager = function(state, predefined, exported, declared) {
           continue;
         }
 
-        if (isUnstackingFunctionOuter) {
-          state.funct["(isCapturing)"] = true;
-        }
-
         if (subScope) {
+          var labelType = this.labeltype(usedLabelName);
+          isImmutable = labelType === "const" ||
+            (labelType === null && _scopeStack[0]["(predefined)"][usedLabelName] === false);
+          if (isUnstackingFunctionOuter && !isImmutable) {
+            state.funct["(isCapturing)"] = true;
+          }
+
           // not exiting the global scope, so copy the usage down in case its an out of scope usage
           if (!subScope["(usages)"][usedLabelName]) {
             subScope["(usages)"][usedLabelName] = usage;

diff --git a/tests/unit/options.js b/tests/unit/options.js
@@ -1408,6 +1408,74 @@ exports.loopfunc = function (test) {
     .addError(3, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
     .test(src2, { es3: true, loopfunc: false, boss: true });
 
+  TestRun(test, "Allows closing over immutable bindings (ES5)")
+    .addError(6, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .addError(7, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .test([
+        "var outerVar;",
+        "",
+        "while (false) {",
+        "  var innerVar;",
+        "",
+        "  void function() { var localVar; return outerVar; };",
+        "  void function() { var localVar; return innerVar; };",
+        "  void function() { var localVar; return localVar; };",
+        "",
+        "}",
+      ]);
+
+  TestRun(test, "Allows closing over immutable bindings (globals)")
+    .addError(8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .addError(15, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .test([
+        "/* globals immutableGlobal: false, mutableGlobal: true */",
+        "while (false) {",
+        "  void function() { return eval; };",
+        "  void function() { return Infinity; };",
+        "  void function() { return NaN; };",
+        "  void function() { return undefined; };",
+        "  void function() { return immutableGlobal; };",
+        "  void function() { return mutableGlobal; };",
+        "}",
+        "",
+        "// Should recognize shadowing",
+        "(function() {",
+        "  var immutableGlobal;",
+        "  while (false) {",
+        "    void function() { return immutableGlobal; };",
+        "  }",
+        "}());"
+      ]);
+
+  TestRun(test, "Allows closing over immutable bindings (ES2015)")
+    .addError(10, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .addError(11, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .addError(18, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .addError(19, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
+    .test([
+        "let outerLet;",
+        "const outerConst = 0;",
+        "class OuterClass {}",
+        "",
+        "while (false) {",
+        "  let innerLet;",
+        "  const innerConst = 0;",
+        "  class InnerClass {}",
+        "",
+        "  void function() { let localLet; return outerLet; };",
+        "  void function() { let localLet; return innerLet; };",
+        "  void function() { let localLet; return localLet; };",
+        "",
+        "  void function() { const localConst = 0; return outerConst; };",
+        "  void function() { const localConst = 0; return innerConst; };",
+        "  void function() { const localConst = 0; return localConst; };",
+        "",
+        "  void function() { class LocalClass {} return OuterClass; };",
+        "  void function() { class LocalClass {} return InnerClass; };",
+        "  void function() { class LocalClass {} return LocalClass; };",
+        "}"
+      ], { esversion: 2015 });
+
   test.done();
 };