Ticket #423. When functions are generated within comprehensions ... the comprehensions should close over the element instead of sharing it.

lib/command.js

@@ -58,41 +58,44 @@
   // compile them. If a directory is passed, recursively compile all
   // '.coffee' extension source files in it and all subdirectories.
   compileScripts = function() {
-    var _b, _c, _d, _e, base, compile, source;
+    var _b, _c, _d, _e;
     _b = []; _d = sources;
     for (_c = 0, _e = _d.length; _c < _e; _c++) {
-      source = _d[_c];
+      (function() {
-      _b.push((function() {
+        var base, compile;
-        base = source;
+        var source = _d[_c];
-        compile = function(source, topLevel) {
+        return _b.push((function() {
-          return path.exists(source, function(exists) {
+          base = source;
-            if (!(exists)) {
+          compile = function(source, topLevel) {
-              throw new Error(("File not found: " + source));
+            return path.exists(source, function(exists) {
-            }
+              if (!(exists)) {
-            return fs.stat(source, function(err, stats) {
+                throw new Error(("File not found: " + source));
-              if (stats.isDirectory()) {
-                return fs.readdir(source, function(err, files) {
-                  var _f, _g, _h, _i, file;
-                  _f = []; _h = files;
-                  for (_g = 0, _i = _h.length; _g < _i; _g++) {
-                    file = _h[_g];
-                    _f.push(compile(path.join(source, file)));
-                  }
-                  return _f;
-                });
-              } else if (topLevel || path.extname(source) === '.coffee') {
-                fs.readFile(source, function(err, code) {
-                  return compileScript(source, code.toString(), base);
-                });
-                if (options.watch) {
-                  return watch(source, base);
-                }
               }
+              return fs.stat(source, function(err, stats) {
+                if (stats.isDirectory()) {
+                  return fs.readdir(source, function(err, files) {
+                    var _f, _g, _h, _i, file;
+                    _f = []; _h = files;
+                    for (_g = 0, _i = _h.length; _g < _i; _g++) {
+                      file = _h[_g];
+                      _f.push(compile(path.join(source, file)));
+                    }
+                    return _f;
+                  });
+                } else if (topLevel || path.extname(source) === '.coffee') {
+                  fs.readFile(source, function(err, code) {
+                    return compileScript(source, code.toString(), base);
+                  });
+                  if (options.watch) {
+                    return watch(source, base);
+                  }
+                }
+              });
             });
-          });
+          };
-        };
+          return compile(source, true);
-        return compile(source, true);
+        })());
-      })());
+      })();
     }
     return _b;
   };

lib/nodes.js

@@ -1590,20 +1590,22 @@
     // comprehensions. Some of the generated code can be shared in common, and
     // some cannot.
     ForNode.prototype.compileNode = function(o) {
-      var body, bodyDent, close, forPart, index, ivar, lvar, name, range, returnResult, rvar, scope, source, sourcePart, stepPart, svar, topLevel, varPart, vars;
+      var body, close, codeInBody, forPart, index, ivar, lvar, name, namePart, range, returnResult, rvar, scope, source, sourcePart, stepPart, svar, topLevel, varPart, vars;
       topLevel = del(o, 'top') && !this.returns;
       range = this.source instanceof ValueNode && this.source.base instanceof RangeNode && !this.source.properties.length;
       source = range ? this.source.base : this.source;
+      codeInBody = this.body.contains(function(n) {
+        return n instanceof CodeNode;
+      });
       scope = o.scope;
       name = this.name && this.name.compile(o);
       index = this.index && this.index.compile(o);
-      if (name && !this.pattern) {
+      if (name && !this.pattern && !codeInBody) {
         scope.find(name);
       }
-      if (index) {
+      if (index && !codeInBody) {
         scope.find(index);
       }
-      bodyDent = this.idt(1);
       if (!(topLevel)) {
         rvar = scope.freeVariable();
       }
@@ -1620,13 +1622,13 @@
         svar = scope.freeVariable();
         sourcePart = ("" + svar + " = " + (this.source.compile(o)) + ";");
         if (this.pattern) {
-          varPart = new AssignNode(this.name, literal(("" + svar + "[" + ivar + "]"))).compile(merge(o, {
+          namePart = new AssignNode(this.name, literal(("" + svar + "[" + ivar + "]"))).compile(merge(o, {
             indent: this.idt(1),
             top: true
           })) + "\n";
         } else {
           if (name) {
-            varPart = ("" + bodyDent + name + " = " + svar + "[" + ivar + "];\n");
+            namePart = ("" + name + " = " + svar + "[" + ivar + "]");
           }
         }
         if (!(this.object)) {
@@ -1638,18 +1640,23 @@
       sourcePart = (rvar ? ("" + rvar + " = []; ") : '') + sourcePart;
       sourcePart = sourcePart ? ("" + this.tab + sourcePart + "\n" + this.tab) : this.tab;
       returnResult = this.compileReturnValue(rvar, o);
-      if (topLevel && body.contains(function(n) {
-        return n instanceof CodeNode;
-      })) {
-        body = ClosureNode.wrap(body, true);
-      }
       if (!(topLevel)) {
         body = PushNode.wrap(rvar, body);
       }
       this.guard ? (body = Expressions.wrap([new IfNode(this.guard, body)])) : null;
+      if (codeInBody) {
+        if (namePart) {
+          body.unshift(literal(("var " + namePart)));
+        }
+        body = ClosureNode.wrap(body, true);
+      } else {
+        if (namePart) {
+          varPart = ("" + (this.idt(1)) + namePart + ";\n");
+        }
+      }
       this.object ? (forPart = ("" + ivar + " in " + svar + ") { if (" + (utility('hasProp')) + ".call(" + svar + ", " + ivar + ")")) : null;
       body = body.compile(merge(o, {
-        indent: bodyDent,
+        indent: this.idt(1),
         top: true
       }));
       vars = range ? name : ("" + name + ", " + ivar);

src/nodes.coffee

@@ -1188,44 +1188,48 @@ exports.ForNode: class ForNode extends BaseNode
   # comprehensions. Some of the generated code can be shared in common, and
   # some cannot.
   compileNode: (o) ->
-    topLevel:      del(o, 'top') and not @returns
+    topLevel:       del(o, 'top') and not @returns
     range:          @source instanceof ValueNode and @source.base instanceof RangeNode and not @source.properties.length
     source:         if range then @source.base else @source
+    codeInBody:     @body.contains (n) -> n instanceof CodeNode
     scope:          o.scope
     name:           @name and @name.compile o
     index:          @index and @index.compile o
-    scope.find name  if name and not @pattern
+    scope.find name  if name and not @pattern and not codeInBody
-    scope.find index if index
+    scope.find index if index and not codeInBody
-    bodyDent:      @idt 1
     rvar:           scope.freeVariable() unless topLevel
     ivar:           if range then name else index or scope.freeVariable()
-    varPart:       ''
+    varPart:        ''
     body:           Expressions.wrap([@body])
     if range
-      sourcePart:  source.compileVariables o
+      sourcePart:   source.compileVariables o
-      forPart:     source.compile merge o, {index: ivar, step: @step}
+      forPart:      source.compile merge o, {index: ivar, step: @step}
     else
       svar:         scope.freeVariable()
-      sourcePart:  "$svar = ${ @source.compile(o) };"
+      sourcePart:   "$svar = ${ @source.compile(o) };"
       if @pattern
-        varPart:   new AssignNode(@name, literal("$svar[$ivar]")).compile(merge o, {indent: @idt(1), top: true}) + "\n"
+        namePart:   new AssignNode(@name, literal("$svar[$ivar]")).compile(merge o, {indent: @idt(1), top: true}) + "\n"
       else
-        varPart:   "$bodyDent$name = $svar[$ivar];\n" if name
+        namePart:   "$name = $svar[$ivar]" if name
       unless @object
         lvar:       scope.freeVariable()
-        stepPart:  if @step then "$ivar += ${ @step.compile(o) }" else "$ivar++"
+        stepPart:   if @step then "$ivar += ${ @step.compile(o) }" else "$ivar++"
-        forPart:   "$ivar = 0, $lvar = ${svar}.length; $ivar < $lvar; $stepPart"
+        forPart:    "$ivar = 0, $lvar = ${svar}.length; $ivar < $lvar; $stepPart"
-    sourcePart:    (if rvar then "$rvar = []; " else '') + sourcePart
+    sourcePart:     (if rvar then "$rvar = []; " else '') + sourcePart
-    sourcePart:    if sourcePart then "$@tab$sourcePart\n$@tab" else @tab
+    sourcePart:     if sourcePart then "$@tab$sourcePart\n$@tab" else @tab
-    returnResult:  @compileReturnValue(rvar, o)
+    returnResult:   @compileReturnValue(rvar, o)
 
-    body:           ClosureNode.wrap(body, true) if topLevel and body.contains (n) -> n instanceof CodeNode
     body:           PushNode.wrap(rvar, body) unless topLevel
     if @guard
       body:         Expressions.wrap([new IfNode(@guard, body)])
+    if codeInBody
+      body.unshift  literal "var $namePart" if namePart
+      body:         ClosureNode.wrap(body, true)
+    else
+      varPart:      "${@idt(1)}$namePart;\n" if namePart
     if @object
-      forPart: "$ivar in $svar) { if (${utility('hasProp')}.call($svar, $ivar)"
+      forPart:      "$ivar in $svar) { if (${utility('hasProp')}.call($svar, $ivar)"
-    body:           body.compile(merge(o, {indent: bodyDent, top: true}))
+    body:           body.compile(merge(o, {indent: @idt(1), top: true}))
     vars:           if range then name else "$name, $ivar"
     close:          if @object then '}}' else '}'
     "${sourcePart}for ($forPart) {\n$varPart$body\n$@tab$close$returnResult"

test/test_comprehensions.coffee

@@ -62,18 +62,26 @@ ok 2 in evens
 # Ensure that the closure wrapper preserves local variables.
 obj: {}
 
-methods: ['one', 'two', 'three']
+for method in ['one', 'two', 'three']
-
+  obj[method]: ->
-for method in methods
+    "I'm " + method
-  name: method
-  obj[name]: ->
-    "I'm " + name
 
 ok obj.one()   is "I'm one"
 ok obj.two()   is "I'm two"
 ok obj.three() is "I'm three"
 
 
+# Even when referenced in the filter.
+list: ['one', 'two', 'three']
+
+methods: for num in list when num isnt 'two'
+  -> num
+
+ok methods.length is 2
+ok methods[0]() is 'one'
+ok methods[1]() is 'three'
+
+
 # Naked ranges are expanded into arrays.
 array: [0..10]
 ok(num % 2 is 0 for num in array by 2)