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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 7 commits into from Feb 6, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9f9afd5
fix: `use-isnan` doesn't report on `SequenceExpression`s
StyleShit Jan 31, 2024
c528495
kinda docs?
StyleShit Jan 31, 2024
1fde508
Merge branch 'main' into fix/use-isnan-sequence-expression
amareshsm Feb 1, 2024
25c37eb
fix suggestions
StyleShit Feb 1, 2024
20cb687
more cases
StyleShit Feb 1, 2024
fc98c02
Merge remote-tracking branch 'eslint/main' into fix/use-isnan-sequenc…
StyleShit Feb 5, 2024
172657c
fix tests
StyleShit Feb 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/src/rules/use-isnan.md
View file
Open in desktop
Expand Up @@ -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));
```

:::
Expand Down
31 changes: 21 additions & 10 deletions lib/rules/use-isnan.js
View file
Open in desktop
Expand Up @@ -21,9 +21,17 @@ const astUtils = require("./utils/ast-utils");
* @returns {boolean} `true` if the node is 'NaN' identifier.
*/
function isNaNIdentifier(node) {
StyleShit marked this conversation as resolved.
Show resolved Hide resolved
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")
);
}

Expand Down Expand Up @@ -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({
Expand Down
93 changes: 93 additions & 0 deletions tests/lib/rules/use-isnan.js
View file
Open in desktop
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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: [
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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" } }]
}
]
});