fix: use-isnan doesn't report on SequenceExpressions (#18059)

* fix: `use-isnan` doesn't report on `SequenceExpression`s Closes #18058 * kinda docs? * fix suggestions * more cases * fix tests --------- Co-authored-by: Amaresh S M <amareshsm13@gmail.com>
eslint · Feb 6, 2024 · c4d26fd · c4d26fd
1 parent 6ea339e
commit c4d26fd
Show file tree

Hide file tree

Showing 3 changed files with 116 additions and 10 deletions.
diff --git 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));
 ```
 
 :::

diff --git 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")
     );
 }
 
@@ -115,21 +123,24 @@ 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) {
                     suggestedFixes.push({
                         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
@@ -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
@@ -344,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: [
@@ -747,6 +774,34 @@ ruleTester.run("use-isnan", rule, {
                 ]
             }]
         },
+        {
+            code: "x === (doStuff(), NaN);",
+            errors: [{
+                ...comparisonError,
+                suggestions: []
+            }]
+        },
+        {
+            code: "x === (doStuff(), Number.NaN);",
+            errors: [{
+                ...comparisonError,
+                suggestions: []
+            }]
+        },
+        {
+            code: "x == (doStuff(), NaN);",
+            errors: [{
+                ...comparisonError,
+                suggestions: []
+            }]
+        },
+        {
+            code: "x == (doStuff(), Number.NaN);",
+            errors: [{
+                ...comparisonError,
+                suggestions: []
+            }]
+        },
 
         //------------------------------------------------------------------------------
         // enforceForSwitchCase
@@ -910,6 +965,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
@@ -1010,6 +1079,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" } }]
         }
     ]
 });