Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested loop at the end of function fail on Don't make functions within a loop #25

Closed
jakubzitny opened this issue Mar 10, 2016 · 2 comments

Comments

@jakubzitny
Copy link
Contributor

This coffee code fails on ?:?: Don't make functions within a loop.

letts = [ 'a', 'b', 'c' ]
nums = [1, 2, 3]

fun = ->
  for lett of letts
    for num of nums
      console.log(lett, num)

fun()

The fact that the for loop is last statement means, coffee wants to return its value. If the loop has another loop in it, there is anonymoous function inside the loop that "collets" the values to return. Transpiled js looks like this:

(function() {
  var fun, letts, nums;
  letts = ['a', 'b', 'c'];
  nums = [1, 2, 3];

  fun = function() {
    var lett, num, results;
    results = [];
    for (lett in letts) {
      results.push((function() {
        var results1;
        results1 = [];
        for (num in nums) {
          results1.push(console.log(lett, num));
        }
        return results1;
      })());
    }
    return results;
  };

  fun();
}).call(this);

Do you guys think, it would make sense to add "Don't make functions within a loop." to errorsToSkip in hint.coffee?

@cueedee
Copy link
Member

cueedee commented Oct 13, 2017

While I agree that your particular example seems to make a case in favour of adding this warning to the ignore list, doing so would also cause very legitimate reports to start slipping under the radar; such as in:

funcs = (( ( j ) -> i + j ) for i in [0..10] )

I feel that it is rather the responsibility of jshint itself to intelligently make the distinction between legitimate uses such as in your example and the real problematic cases such as the one above.

@cueedee
Copy link
Member

cueedee commented Oct 25, 2017

I would like to revoke my previous assertion.

Little did I realise the pervasiveness of this warning until I found that my generator had been confidingly inserting jshint's relaxing loopfunc option into its generated grunt-coffee-jshint config.

Btw, this warning currently comes in the form of "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.".

So, instead of adding this warning to the errorsToSkip list, which basically is just an escape-hatch for suppressing warnings for which no relaxing options exist, I would rather include loopfunc into the list of jshint's relaxing options that coffee-jshint turns on by default.

@cueedee cueedee reopened this Oct 25, 2017
@cueedee cueedee removed the wontfix label Oct 25, 2017
cueedee added a commit that referenced this issue Oct 25, 2017
Fixes #25

Please _do_ note that this will now also hide legitimate reports.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants