From 697ed7056d27507985a6cbd4225ba6b5566a2d4d Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Fri, 23 Apr 2021 07:52:09 +0530 Subject: [PATCH 1/7] Fix: report error for sequence expression in no-unused-vars (fixes #14325) --- lib/rules/no-unused-vars.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/no-unused-vars.js b/lib/rules/no-unused-vars.js index 7619be331fa..7ee178d797a 100644 --- a/lib/rules/no-unused-vars.js +++ b/lib/rules/no-unused-vars.js @@ -433,7 +433,7 @@ module.exports = { ) || ( parent.type === "UpdateExpression" && - grandparent.type === "ExpressionStatement" + (grandparent.type === "ExpressionStatement" || grandparent.type === "SequenceExpression") ) || rhsNode && isInside(id, rhsNode) && !isInsideOfStorableFunction(id, rhsNode))) From a11d639a699e671d96f36065dfcfeb8e6569d84c Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Fri, 23 Apr 2021 07:58:54 +0530 Subject: [PATCH 2/7] Chore: add tests --- tests/lib/rules/no-unused-vars.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/lib/rules/no-unused-vars.js b/tests/lib/rules/no-unused-vars.js index 423afa167b8..8244afe9d01 100644 --- a/tests/lib/rules/no-unused-vars.js +++ b/tests/lib/rules/no-unused-vars.js @@ -995,6 +995,27 @@ ruleTester.run("no-unused-vars", rule, { definedError("c") ] }, + { + code: `let x = 0; + x++; x = 0;`, + parserOptions: { ecmaVersion: 2015 }, + errors: [{ ...assignedError("x"), line: 2, column: 18 }] + }, + + // https://github.com/eslint/eslint/issues/14325 + { + code: `let x = 0; + x++, x = 0;`, + parserOptions: { ecmaVersion: 2015 }, + errors: [{ ...assignedError("x"), line: 2, column: 18 }] + }, + { + code: `let x = 0; + x++, x = 0; + x=3;`, + parserOptions: { ecmaVersion: 2015 }, + errors: [{ ...assignedError("x"), line: 3, column: 13 }] + }, { code: "(function ({ a, b }, { c } ) { return b; })();", parserOptions: { ecmaVersion: 2015 }, From 3ca8b5b07c05accb936205ea5df0c8181acb2be7 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Tue, 11 May 2021 18:50:54 +0530 Subject: [PATCH 3/7] Update: suggestions --- lib/rules/no-unused-vars.js | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/rules/no-unused-vars.js b/lib/rules/no-unused-vars.js index 7ee178d797a..965515e2d43 100644 --- a/lib/rules/no-unused-vars.js +++ b/lib/rules/no-unused-vars.js @@ -410,6 +410,31 @@ module.exports = { ); } + /** + * Checks whether a given node is unused expression or not. + * @param {ASTNode} node The node itself + * @returns {boolean} The node is an unused expression. + * @private + */ + function isUnusedExpression(node) { + const isLastExpression = node.end === node.range[1]; + const parent = node.parent; + + if (parent.type === "ExpressionStatement") { + return true; + } + + if (parent.type === "SequenceExpression") { + if (!isLastExpression) { + return true; + } + return isUnusedExpression(parent); + + } + + return false; + } + /** * Checks whether a given reference is a read to update itself or not. * @param {eslint-scope.Reference} ref A reference to check. @@ -433,7 +458,7 @@ module.exports = { ) || ( parent.type === "UpdateExpression" && - (grandparent.type === "ExpressionStatement" || grandparent.type === "SequenceExpression") + isUnusedExpression(parent) ) || rhsNode && isInside(id, rhsNode) && !isInsideOfStorableFunction(id, rhsNode))) From 476b2b0d86b504b6b76f85dfa70fcfbe19aa63fb Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Tue, 11 May 2021 19:06:12 +0530 Subject: [PATCH 4/7] Chore: refactor --- lib/rules/no-unused-vars.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rules/no-unused-vars.js b/lib/rules/no-unused-vars.js index 965515e2d43..37c3a3acd27 100644 --- a/lib/rules/no-unused-vars.js +++ b/lib/rules/no-unused-vars.js @@ -417,7 +417,6 @@ module.exports = { * @private */ function isUnusedExpression(node) { - const isLastExpression = node.end === node.range[1]; const parent = node.parent; if (parent.type === "ExpressionStatement") { @@ -425,11 +424,12 @@ module.exports = { } if (parent.type === "SequenceExpression") { + const isLastExpression = node.end === node.range[1]; + if (!isLastExpression) { return true; } return isUnusedExpression(parent); - } return false; From ad5c413994d59272df940c1ad2b8ad9578599306 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Tue, 11 May 2021 19:34:22 +0530 Subject: [PATCH 5/7] Chore: refactor --- lib/rules/no-unused-vars.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/no-unused-vars.js b/lib/rules/no-unused-vars.js index 37c3a3acd27..fea634b8e48 100644 --- a/lib/rules/no-unused-vars.js +++ b/lib/rules/no-unused-vars.js @@ -424,7 +424,7 @@ module.exports = { } if (parent.type === "SequenceExpression") { - const isLastExpression = node.end === node.range[1]; + const isLastExpression = node.expressions && node.expressions[node.expressions.length - 1] === node; if (!isLastExpression) { return true; From a83ce385930e611fbbaa2dd57bb1bc47a7520cc9 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Thu, 13 May 2021 17:43:09 +0530 Subject: [PATCH 6/7] Fix: logic --- lib/rules/no-unused-vars.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/rules/no-unused-vars.js b/lib/rules/no-unused-vars.js index fea634b8e48..adf465905c2 100644 --- a/lib/rules/no-unused-vars.js +++ b/lib/rules/no-unused-vars.js @@ -424,7 +424,7 @@ module.exports = { } if (parent.type === "SequenceExpression") { - const isLastExpression = node.expressions && node.expressions[node.expressions.length - 1] === node; + const isLastExpression = parent.expressions[parent.expressions.length - 1] === node; if (!isLastExpression) { return true; @@ -445,7 +445,6 @@ module.exports = { function isReadForItself(ref, rhsNode) { const id = ref.identifier; const parent = id.parent; - const grandparent = parent.parent; return ref.isRead() && ( @@ -453,7 +452,7 @@ module.exports = { (// in RHS of an assignment for itself. e.g. `a = a + 1` (( parent.type === "AssignmentExpression" && - grandparent.type === "ExpressionStatement" && + isUnusedExpression(parent) && parent.left === id ) || ( From d60a59a9992ab10986f165e6acd37f319120c3f0 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Thu, 13 May 2021 17:59:22 +0530 Subject: [PATCH 7/7] Chore: add tests --- tests/lib/rules/no-unused-vars.js | 60 +++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 6 deletions(-) diff --git a/tests/lib/rules/no-unused-vars.js b/tests/lib/rules/no-unused-vars.js index 8244afe9d01..48ccdb1d42f 100644 --- a/tests/lib/rules/no-unused-vars.js +++ b/tests/lib/rules/no-unused-vars.js @@ -177,6 +177,10 @@ ruleTester.run("no-unused-vars", rule, { { code: "(function(obj) { for ( const name in obj ) { return true } })({})", parserOptions: { ecmaVersion: 6 } }, { code: "(function(obj) { for ( const name in obj ) return true })({})", parserOptions: { ecmaVersion: 6 } }, + // Sequence Expressions (See https://github.com/eslint/eslint/issues/14325) + { code: "let x = 0; foo = (0, x++);", parserOptions: { ecmaVersion: 6 } }, + { code: "let x = 0; foo = (0, x += 1);", parserOptions: { ecmaVersion: 6 } }, + // caughtErrors { code: "try{}catch(err){console.error(err);}", @@ -995,12 +999,6 @@ ruleTester.run("no-unused-vars", rule, { definedError("c") ] }, - { - code: `let x = 0; - x++; x = 0;`, - parserOptions: { ecmaVersion: 2015 }, - errors: [{ ...assignedError("x"), line: 2, column: 18 }] - }, // https://github.com/eslint/eslint/issues/14325 { @@ -1016,6 +1014,56 @@ ruleTester.run("no-unused-vars", rule, { parserOptions: { ecmaVersion: 2015 }, errors: [{ ...assignedError("x"), line: 3, column: 13 }] }, + { + code: "let x = 0; x++, 0;", + parserOptions: { ecmaVersion: 2015 }, + errors: [{ ...assignedError("x"), line: 1, column: 12 }] + }, + { + code: "let x = 0; 0, x++;", + parserOptions: { ecmaVersion: 2015 }, + errors: [{ ...assignedError("x"), line: 1, column: 15 }] + }, + { + code: "let x = 0; 0, (1, x++);", + parserOptions: { ecmaVersion: 2015 }, + errors: [{ ...assignedError("x"), line: 1, column: 19 }] + }, + { + code: "let x = 0; foo = (x++, 0);", + parserOptions: { ecmaVersion: 2015 }, + errors: [{ ...assignedError("x"), line: 1, column: 19 }] + }, + { + code: "let x = 0; foo = ((0, x++), 0);", + parserOptions: { ecmaVersion: 2015 }, + errors: [{ ...assignedError("x"), line: 1, column: 23 }] + }, + { + code: "let x = 0; x += 1, 0;", + parserOptions: { ecmaVersion: 2015 }, + errors: [{ ...assignedError("x"), line: 1, column: 12 }] + }, + { + code: "let x = 0; 0, x += 1;", + parserOptions: { ecmaVersion: 2015 }, + errors: [{ ...assignedError("x"), line: 1, column: 15 }] + }, + { + code: "let x = 0; 0, (1, x += 1);", + parserOptions: { ecmaVersion: 2015 }, + errors: [{ ...assignedError("x"), line: 1, column: 19 }] + }, + { + code: "let x = 0; foo = (x += 1, 0);", + parserOptions: { ecmaVersion: 2015 }, + errors: [{ ...assignedError("x"), line: 1, column: 19 }] + }, + { + code: "let x = 0; foo = ((0, x += 1), 0);", + parserOptions: { ecmaVersion: 2015 }, + errors: [{ ...assignedError("x"), line: 1, column: 23 }] + }, { code: "(function ({ a, b }, { c } ) { return b; })();", parserOptions: { ecmaVersion: 2015 },