From 9f9afd5bab583c066bca7b3e6f9f8af1be7ba605 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 31 Jan 2024 20:44:01 +0200 Subject: [PATCH 1/5] fix: `use-isnan` doesn't report on `SequenceExpression`s Closes #18058 --- lib/rules/use-isnan.js | 14 +++++++++++--- tests/lib/rules/use-isnan.js | 28 ++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/lib/rules/use-isnan.js b/lib/rules/use-isnan.js index b00a701c6bd..e4c1c2761c1 100644 --- a/lib/rules/use-isnan.js +++ b/lib/rules/use-isnan.js @@ -21,9 +21,17 @@ const astUtils = require("./utils/ast-utils"); * @returns {boolean} `true` if the node is 'NaN' identifier. */ function isNaNIdentifier(node) { - return Boolean(node) && ( - astUtils.isSpecificId(node, "NaN") || - astUtils.isSpecificMemberAccess(node, "Number", "NaN") + if (!node) { + return false; + } + + const nodeToCheck = node.type === "SequenceExpression" + ? node.expressions.at(-1) + : node; + + return ( + astUtils.isSpecificId(nodeToCheck, "NaN") || + astUtils.isSpecificMemberAccess(nodeToCheck, "Number", "NaN") ); } diff --git a/tests/lib/rules/use-isnan.js b/tests/lib/rules/use-isnan.js index 26c887a1975..807c02f99fd 100644 --- a/tests/lib/rules/use-isnan.js +++ b/tests/lib/rules/use-isnan.js @@ -183,6 +183,10 @@ ruleTester.run("use-isnan", rule, { "foo.lastIndexOf(NaN)", "foo.indexOf(Number.NaN)", "foo.lastIndexOf(Number.NaN)", + "foo.indexOf((NaN, 1))", + "foo.lastIndexOf((NaN, 1))", + "foo.indexOf((Number.NaN, 1))", + "foo.lastIndexOf((Number.NaN, 1))", { code: "foo.indexOf(NaN)", options: [{}] @@ -1010,6 +1014,30 @@ ruleTester.run("use-isnan", rule, { options: [{ enforceForIndexOf: true }], languageOptions: { ecmaVersion: 2020 }, errors: [{ messageId: "indexOfNaN", data: { methodName: "indexOf" } }] + }, + { + code: "foo.indexOf((1, NaN))", + options: [{ enforceForIndexOf: true }], + languageOptions: { ecmaVersion: 2020 }, + errors: [{ messageId: "indexOfNaN", data: { methodName: "indexOf" } }] + }, + { + code: "foo.indexOf((1, Number.NaN))", + options: [{ enforceForIndexOf: true }], + languageOptions: { ecmaVersion: 2020 }, + errors: [{ messageId: "indexOfNaN", data: { methodName: "indexOf" } }] + }, + { + code: "foo.lastIndexOf((1, NaN))", + options: [{ enforceForIndexOf: true }], + languageOptions: { ecmaVersion: 2020 }, + errors: [{ messageId: "indexOfNaN", data: { methodName: "lastIndexOf" } }] + }, + { + code: "foo.lastIndexOf((1, Number.NaN))", + options: [{ enforceForIndexOf: true }], + languageOptions: { ecmaVersion: 2020 }, + errors: [{ messageId: "indexOfNaN", data: { methodName: "lastIndexOf" } }] } ] }); From c5284953bb660f50c0697ba11ad3db70aadbec4c Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 31 Jan 2024 21:07:53 +0200 Subject: [PATCH 2/5] kinda docs? --- docs/src/rules/use-isnan.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/src/rules/use-isnan.md b/docs/src/rules/use-isnan.md index d6eda9d5d91..f3de5919d18 100644 --- a/docs/src/rules/use-isnan.md +++ b/docs/src/rules/use-isnan.md @@ -224,6 +224,8 @@ var hasNaN = myArray.indexOf(NaN) >= 0; var firstIndex = myArray.indexOf(NaN); var lastIndex = myArray.lastIndexOf(NaN); + +var indexWithSequenceExpression = myArray.indexOf((doStuff(), NaN)); ``` ::: From 25c37eb253c881c69aa5a6a4d7e26462f476d627 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Thu, 1 Feb 2024 18:57:06 +0200 Subject: [PATCH 3/5] fix suggestions --- lib/rules/use-isnan.js | 17 +++++++++------- tests/lib/rules/use-isnan.js | 39 ++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/lib/rules/use-isnan.js b/lib/rules/use-isnan.js index e4c1c2761c1..357c51d13d6 100644 --- a/lib/rules/use-isnan.js +++ b/lib/rules/use-isnan.js @@ -123,7 +123,10 @@ module.exports = { (isNaNIdentifier(node.left) || isNaNIdentifier(node.right)) ) { const suggestedFixes = []; - const isFixable = fixableOperators.has(node.operator); + const NaNNode = isNaNIdentifier(node.left) ? node.left : node.right; + + const isSequenceExpression = NaNNode.type === "SequenceExpression"; + const isFixable = fixableOperators.has(node.operator) && !isSequenceExpression; const isCastable = castableOperators.has(node.operator); if (isFixable) { @@ -131,13 +134,13 @@ module.exports = { messageId: "replaceWithIsNaN", fix: getBinaryExpressionFixer(node, value => `Number.isNaN(${value})`) }); - } - if (isCastable) { - suggestedFixes.push({ - messageId: "replaceWithCastingAndIsNaN", - fix: getBinaryExpressionFixer(node, value => `Number.isNaN(Number(${value}))`) - }); + if (isCastable) { + suggestedFixes.push({ + messageId: "replaceWithCastingAndIsNaN", + fix: getBinaryExpressionFixer(node, value => `Number.isNaN(Number(${value}))`) + }); + } } context.report({ diff --git a/tests/lib/rules/use-isnan.js b/tests/lib/rules/use-isnan.js index 807c02f99fd..bcfc28e4bbf 100644 --- a/tests/lib/rules/use-isnan.js +++ b/tests/lib/rules/use-isnan.js @@ -49,6 +49,9 @@ ruleTester.run("use-isnan", rule, { "foo(2 / Number.NaN)", "var x; if (x = Number.NaN) { }", "x === Number[NaN];", + "x === (NaN, 1)", + "x === (doStuff(), NaN, 1)", + "x === (doStuff(), Number.NaN, 1)", //------------------------------------------------------------------------------ // enforceForSwitchCase @@ -174,6 +177,14 @@ ruleTester.run("use-isnan", rule, { code: "switch(foo) { case foo.Number.NaN: break }", options: [{ enforceForSwitchCase: true }] }, + { + code: "switch((NaN, doStuff(), 1)) {}", + options: [{ enforceForSwitchCase: true }] + }, + { + code: "switch((Number.NaN, doStuff(), 1)) {}", + options: [{ enforceForSwitchCase: true }] + }, //------------------------------------------------------------------------------ // enforceForIndexOf @@ -751,6 +762,20 @@ ruleTester.run("use-isnan", rule, { ] }] }, + { + code: "x === (doStuff(), NaN);", + errors: [{ + ...comparisonError, + suggestions: [] + }] + }, + { + code: "x === (doStuff(), Number.NaN);", + errors: [{ + ...comparisonError, + suggestions: [] + }] + }, //------------------------------------------------------------------------------ // enforceForSwitchCase @@ -914,6 +939,20 @@ ruleTester.run("use-isnan", rule, { { messageId: "caseNaN", type: "SwitchCase", column: 22 } ] }, + { + code: "switch((doStuff(), NaN)) {}", + options: [{ enforceForSwitchCase: true }], + errors: [ + { messageId: "switchNaN", type: "SwitchStatement", column: 1 } + ] + }, + { + code: "switch((doStuff(), Number.NaN)) {}", + options: [{ enforceForSwitchCase: true }], + errors: [ + { messageId: "switchNaN", type: "SwitchStatement", column: 1 } + ] + }, //------------------------------------------------------------------------------ // enforceForIndexOf From 20cb6879493589964f793a68a1685d24a576f61a Mon Sep 17 00:00:00 2001 From: StyleShit Date: Thu, 1 Feb 2024 18:59:40 +0200 Subject: [PATCH 4/5] more cases --- tests/lib/rules/use-isnan.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/lib/rules/use-isnan.js b/tests/lib/rules/use-isnan.js index bcfc28e4bbf..f171cb47399 100644 --- a/tests/lib/rules/use-isnan.js +++ b/tests/lib/rules/use-isnan.js @@ -776,6 +776,20 @@ ruleTester.run("use-isnan", rule, { suggestions: [] }] }, + { + code: "x == (doStuff(), NaN);", + errors: [{ + ...comparisonError, + suggestions: [] + }] + }, + { + code: "x == (doStuff(), Number.NaN);", + errors: [{ + ...comparisonError, + suggestions: [] + }] + }, //------------------------------------------------------------------------------ // enforceForSwitchCase From 172657c1d1923577322395b1336025898e5119ce Mon Sep 17 00:00:00 2001 From: StyleShit Date: Mon, 5 Feb 2024 18:48:44 +0200 Subject: [PATCH 5/5] fix tests --- tests/lib/rules/use-isnan.js | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/lib/rules/use-isnan.js b/tests/lib/rules/use-isnan.js index f171cb47399..337f1f4d146 100644 --- a/tests/lib/rules/use-isnan.js +++ b/tests/lib/rules/use-isnan.js @@ -194,10 +194,6 @@ ruleTester.run("use-isnan", rule, { "foo.lastIndexOf(NaN)", "foo.indexOf(Number.NaN)", "foo.lastIndexOf(Number.NaN)", - "foo.indexOf((NaN, 1))", - "foo.lastIndexOf((NaN, 1))", - "foo.indexOf((Number.NaN, 1))", - "foo.lastIndexOf((Number.NaN, 1))", { code: "foo.indexOf(NaN)", options: [{}] @@ -359,6 +355,22 @@ ruleTester.run("use-isnan", rule, { { code: "foo.lastIndexOf(Number.NaN())", options: [{ enforceForIndexOf: true }] + }, + { + code: "foo.indexOf((NaN, 1))", + options: [{ enforceForIndexOf: true }] + }, + { + code: "foo.lastIndexOf((NaN, 1))", + options: [{ enforceForIndexOf: true }] + }, + { + code: "foo.indexOf((Number.NaN, 1))", + options: [{ enforceForIndexOf: true }] + }, + { + code: "foo.lastIndexOf((Number.NaN, 1))", + options: [{ enforceForIndexOf: true }] } ], invalid: [