Skip to content

Commit

Permalink
Fix: no-unused-vars false positive around callback (fixes #6576) (#…
Browse files Browse the repository at this point in the history 
…6579)

* Fix: `no-unused-vars` false positive around callback (fixes #6576)

* Fix: `no-unused-vars` false positive about reuse (fixes #6576)
  • Loading branch information
@mysticatea @nzakas
mysticatea authored and nzakas committed Jul 5, 2016
1 parent 124d8a3 commit 27700cf
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 9 deletions.
7 changes: 7 additions & 0 deletions docs/rules/no-unused-vars.md
View file
Expand Up @@ -11,6 +11,7 @@ A variable is considered to be used if any of the following are true:
* It represents a function that is called (`doSomething()`)
* It is read (`var y = x`)
* It is passed into a function as an argument (`doSomething(x)`)
* It is read inside of a function that is passed to another function (`doSomething(function() { foo(); })`)

A variable is *not* considered to be used if it is only ever assigned to (`var x = 5`) or declared.

Expand Down Expand Up @@ -61,6 +62,12 @@ myFunc(function foo() {
(function(foo) {
return foo;
})();

var myFunc;
myFunc = setTimeout(function() {
// myFunc is considered used
myFunc();
}, 50);
```

### exported
Expand Down
106 changes: 99 additions & 7 deletions lib/rules/no-unused-vars.js
View file
Expand Up @@ -10,6 +10,7 @@
//------------------------------------------------------------------------------

var lodash = require("lodash");
var astUtils = require("../ast-utils");

//------------------------------------------------------------------------------
// Rule Definition
Expand Down Expand Up @@ -95,6 +96,8 @@ module.exports = {
// Helpers
//--------------------------------------------------------------------------

var STATEMENT_TYPE = /(?:Statement|Declaration)$/;

/**
* Determines if a given variable is being exported from a module.
* @param {Variable} variable - EScope variable object.
Expand Down Expand Up @@ -152,6 +155,20 @@ module.exports = {
return false;
}

/**
* Checks the position of given nodes.
*
* @param {ASTNode} inner - A node which is expected as inside.
* @param {ASTNode} outer - A node which is expected as outside.
* @returns {boolean} `true` if the `inner` node exists in the `outer` node.
*/
function isInside(inner, outer) {
return (
inner.range[0] >= outer.range[0] &&
inner.range[1] <= outer.range[1]
);
}

/**
* If a given reference is left-hand side of an assignment, this gets
* the right-hand side node of the assignment.
Expand All @@ -164,27 +181,102 @@ module.exports = {
var id = ref.identifier;
var parent = id.parent;
var granpa = parent.parent;
var refScope = ref.from.variableScope;
var varScope = ref.resolved.scope.variableScope;
var canBeUsedLater = refScope !== varScope;

/*
* Inherits the previous node if this reference is in the node.
* This is for `a = a + a`-like code.
*/
if (prevRhsNode &&
prevRhsNode.range[0] <= id.range[0] &&
prevRhsNode.range[1] >= id.range[1]
) {
if (prevRhsNode && isInside(id, prevRhsNode)) {
return prevRhsNode;
}

if (parent.type === "AssignmentExpression" &&
granpa.type === "ExpressionStatement" &&
id === parent.left
id === parent.left &&
!canBeUsedLater
) {
return parent.right;
}
return null;
}

/**
* Checks whether a given function node is stored to somewhere or not.
* If the function node is stored, the function can be used later.
*
* @param {ASTNode} funcNode - A function node to check.
* @param {ASTNode} rhsNode - The RHS node of the previous assignment.
* @returns {boolean} `true` if under the following conditions:
* - the funcNode is assigned to a variable.
* - the funcNode is bound as an argument of a function call.
* - the function is bound to a property and the object satisfies above conditions.
*/
function isStorableFunction(funcNode, rhsNode) {
var node = funcNode;
var parent = funcNode.parent;

while (parent && isInside(parent, rhsNode)) {
switch (parent.type) {
case "SequenceExpression":
if (parent.expressions[parent.expressions.length - 1] !== node) {
return false;
}
break;

case "CallExpression":
case "NewExpression":
return parent.callee !== node;

case "AssignmentExpression":
case "TaggedTemplateExpression":
case "YieldExpression":
return true;

default:
if (STATEMENT_TYPE.test(parent.type)) {

/*
* If it encountered statements, this is a complex pattern.
* Since analyzeing complex patterns is hard, this returns `true` to avoid false positive.
*/
return true;
}
}

node = parent;
parent = parent.parent;
}

return false;
}

/**
* Checks whether a given Identifier node exists inside of a function node which can be used later.
*
* "can be used later" means:
* - the function is assigned to a variable.
* - the function is bound to a property and the object can be used later.
* - the function is bound as an argument of a function call.
*
* If a reference exists in a function which can be used later, the reference is read when the function is called.
*
* @param {ASTNode} id - An Identifier node to check.
* @param {ASTNode} rhsNode - The RHS node of the previous assignment.
* @returns {boolean} `true` if the `id` node exists inside of a function node which can be used later.
*/
function isInsideOfStorableFunction(id, rhsNode) {
var funcNode = astUtils.getUpperFunction(id);

return (
funcNode &&
isInside(funcNode, rhsNode) &&
isStorableFunction(funcNode, rhsNode)
);
}

/**
* Checks whether a given reference is a read to update itself or not.
*
Expand Down Expand Up @@ -213,8 +305,8 @@ module.exports = {
// in RHS of an assignment for itself. e.g. `a = a + 1`
(
rhsNode &&
rhsNode.range[0] <= id.range[0] &&
rhsNode.range[1] >= id.range[1]
isInside(id, rhsNode) &&
!isInsideOfStorableFunction(id, rhsNode)
)
);
}
Expand Down
56 changes: 54 additions & 2 deletions tests/lib/rules/no-unused-vars.js
View file
Expand Up @@ -165,7 +165,41 @@ ruleTester.run("no-unused-vars", rule, {
{code: "var a = 0, b; b = a++; foo(b);"},
{code: "function foo(a) { var b = a = a + 1; bar(b) } foo();"},
{code: "function foo(a) { var b = a += a + 1; bar(b) } foo();"},
{code: "function foo(a) { var b = a++; bar(b) } foo();"}
{code: "function foo(a) { var b = a++; bar(b) } foo();"},

// https://github.com/eslint/eslint/issues/6576
{
code: [
"var unregisterFooWatcher;",
"// ...",
"unregisterFooWatcher = $scope.$watch( \"foo\", function() {",
" // ...some code..",
" unregisterFooWatcher();",
"});"
].join("\n")
},
{
code: [
"var ref;",
"ref = setInterval(",
" function(){",
" clearInterval(ref);",
" }, 10);",
].join("\n")
},
{
code: [
"var _timer;",
"function f() {",
" _timer = setTimeout(function () {}, _timer ? 100 : 0);",
"}",
"f();",
].join("\n")
},
{code: "function foo(cb) { cb = function() { function something(a) { cb(1 + a); } register(something); }(); } foo();"},
{code: "function* foo(cb) { cb = yield function(a) { cb(1 + a); }; } foo();", parserOptions: {ecmaVersion: 6}},
{code: "function foo(cb) { cb = tag`hello${function(a) { cb(1 + a); }}`; } foo();", parserOptions: {ecmaVersion: 6}},
{code: "function foo(cb) { var b; cb = b = function(a) { cb(1 + a); }; b(); } foo();"}
],
invalid: [
{ code: "function foox() { return foox(); }", errors: [{ message: "'foox' is defined but never used", type: "Identifier"}] },
Expand Down Expand Up @@ -373,6 +407,24 @@ ruleTester.run("no-unused-vars", rule, {
{code: "function foo(a) { a += a + 1 } foo();", errors: [{message: "'a' is defined but never used"}]},
{code: "function foo(a) { a++ } foo();", errors: [{message: "'a' is defined but never used"}]},
{code: "var a = 3; a = a * 5 + 6;", errors: [{message: "'a' is defined but never used"}]},
{code: "var a = 2, b = 4; a = a * 2 + b;", errors: [{message: "'a' is defined but never used"}]}
{code: "var a = 2, b = 4; a = a * 2 + b;", errors: [{message: "'a' is defined but never used"}]},

// https://github.com/eslint/eslint/issues/6576 (For coverage)
{
code: "function foo(cb) { cb = function(a) { cb(1 + a); }; bar(not_cb); } foo();",
errors: [{message: "'cb' is defined but never used"}]
},
{
code: "function foo(cb) { cb = function(a) { return cb(1 + a); }(); } foo();",
errors: [{message: "'cb' is defined but never used"}]
},
{
code: "function foo(cb) { cb = (function(a) { cb(1 + a); }, cb); } foo();",
errors: [{message: "'cb' is defined but never used"}]
},
{
code: "function foo(cb) { cb = (0, function(a) { cb(1 + a); }); } foo();",
errors: [{message: "'cb' is defined but never used"}]
}
]
});

0 comments on commit 27700cf

Please sign in to comment.