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

Function parameters referenced in closure not properly handled by no-unused-vars #584

Closed
burnnat opened this issue Feb 4, 2014 · 3 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly rule Relates to ESLint's core rules
Milestone

Comments

@burnnat
Copy link
Contributor

burnnat commented Feb 4, 2014

Consider the following javascript:

var doStuff = function(fn) {
    fn();
};

var callStuff = function(first, second) {
    doStuff(function() {
        console.log(second);
    });
};

callStuff('foo', 'bar');

Running ESLint on this sample code yields an error from the no-unused-vars rule:

sample.js: line 5, col 25, Error - first is defined but never used (no-unused-vars)

1 problem

Normally, the no-unused-vars rule will ignore any function parameters that precede a "used" parameter. But this seems to break when the parameter is used inside a closure, as in this example - even though the second parameter is properly detected as being used, the first parameter is not ignored and causes a spurious error.

@nzakas
Copy link
Member

nzakas commented Feb 4, 2014

You're totally correct. Looks like the closure is confusing the rule.

@nzakas
Copy link
Member

nzakas commented Feb 11, 2014

I think this is tied to the markIgnorableUnusedVariables() only looking up one level, to the nearest function. @ilyavolodin?

@ilyavolodin
Copy link
Member

I'm not 100% sure, markIgnorableUnusedVariables was added by somebody else. But yes, based on the code I'm pretty sure that's due to findFirstAncestorThatIsFunctionExpressionOrDeclaration() function. I think that this function should be removed and instead, we shouldn't use ancestors in markIgnorableUnusedVariables but use usedVariable.node to find function. If direct parent of usedVariable.node is function, the we need to mark all previous arguments as used, if not, don't do anything.

If nobody else will be able to get to it soon-ish, I'll try to fix it at some point (sorry, still don't have enough free time).

@nzakas nzakas closed this as completed in 9fe55c8 Feb 12, 2014
nzakas added a commit that referenced this issue Feb 12, 2014
Fix: Make sure no-unused-vars doesn't get confused by nested functions (fixes #584)
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 7, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

3 participants