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

feat: add suggestions to use-isnan in indexOf & lastIndexOf calls #18063

Merged
merged 12 commits into from
Feb 14, 2024
Merged

feat: add suggestions to use-isnan in indexOf & lastIndexOf calls #18063

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9b61b58
feat: add suggestions to `use-isnan` in `indexOf` & `lastIndexOf` calls
StyleShit Jan 31, 2024
5eaeead
wip
StyleShit Feb 1, 2024
c9ba917
wip
StyleShit Feb 1, 2024
8ffcf1e
shorten names
StyleShit Feb 1, 2024
1be3f8c
fix text
StyleShit Feb 1, 2024
356b608
use const
StyleShit Feb 1, 2024
05d1b41
Merge remote-tracking branch 'eslint/main' into feat/use-isnan-sugges…
StyleShit Feb 8, 2024
5754839
wip
StyleShit Feb 8, 2024
a7d5ed9
isSuggestable
StyleShit Feb 11, 2024
a96f7e2
Merge remote-tracking branch 'eslint/main' into feat/use-isnan-sugges…
StyleShit Feb 11, 2024
f7359ec
computed
StyleShit Feb 14, 2024
ff77c61
Merge remote-tracking branch 'eslint/main' into feat/use-isnan-sugges…
StyleShit Feb 14, 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
32 changes: 30 additions & 2 deletions lib/rules/use-isnan.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ module.exports = {
caseNaN: "'case NaN' can never match. Use Number.isNaN before the switch.",
indexOfNaN: "Array prototype method '{{ methodName }}' cannot find NaN.",
replaceWithIsNaN: "Replace with Number.isNaN.",
replaceWithCastingAndIsNaN: "Replace with Number.isNaN cast to a Number."
replaceWithCastingAndIsNaN: "Replace with Number.isNaN cast to a Number.",
replaceWithFindIndex: "Replace with Array.prototype.{{ methodName }}."
}
},

Expand Down Expand Up @@ -173,7 +174,34 @@ module.exports = {
node.arguments.length === 1 &&
isNaNIdentifier(node.arguments[0])
) {
context.report({ node, messageId: "indexOfNaN", data: { methodName } });

/*
* We can't fix things like `arr.indexOf((doStuff(), NaN))` because we'll need to somehow call the expressions
* before `NaN` in order to preserve side effects, which is not possible with fixes like `arr.findIndex(Number.isNaN)`.
StyleShit marked this conversation as resolved.
Show resolved Hide resolved
*/
const isFixable = node.arguments[0].type !== "SequenceExpression";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think isFixable actually means isSuggestable? (To disambiguate between fixes and suggestions, it's probably better to keep the naming in line.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh... I always mix them up...
fixed (pun intended 😄)

let suggestedFixes = [];

if (isFixable) {
const findIndexMethodSuggestion = methodName === "indexOf" ? "findIndex" : "findLastIndex";
const shouldWrap = callee.property.type === "Literal";
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved

suggestedFixes = [{
messageId: "replaceWithFindIndex",
data: { methodName: findIndexMethodSuggestion },
fix: fixer => [
fixer.replaceText(callee.property, shouldWrap ? `"${findIndexMethodSuggestion}"` : findIndexMethodSuggestion),
fixer.replaceText(node.arguments[0], "Number.isNaN")
]
}];
}

context.report({
node,
messageId: "indexOfNaN",
data: { methodName },
suggest: suggestedFixes
});
}
}
}
Expand Down
192 changes: 174 additions & 18 deletions tests/lib/rules/use-isnan.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -918,98 +918,254 @@ ruleTester.run("use-isnan", rule, {
{
code: "foo.indexOf(NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "indexOf" } }]
errors: [{
messageId: "indexOfNaN",
type: "CallExpression",
data: { methodName: "indexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findIndex" },
output: "foo.findIndex(Number.isNaN)"
}]
}]
},
{
code: "foo.lastIndexOf(NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "lastIndexOf" } }]
errors: [{
messageId: "indexOfNaN",
type: "CallExpression",
data: { methodName: "lastIndexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findLastIndex" },
output: "foo.findLastIndex(Number.isNaN)"
}]
}]
},
{
code: "foo['indexOf'](NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "indexOf" } }]
errors: [{
messageId: "indexOfNaN",
type: "CallExpression",
data: { methodName: "indexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findIndex" },
output: 'foo["findIndex"](Number.isNaN)'
}]
}]
},
{
code: "foo['lastIndexOf'](NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "lastIndexOf" } }]
errors: [{
messageId: "indexOfNaN",
type: "CallExpression",
data: { methodName: "lastIndexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findLastIndex" },
output: 'foo["findLastIndex"](Number.isNaN)'
}]
}]
},
{
code: "foo().indexOf(NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "indexOf" } }]
errors: [{
messageId: "indexOfNaN",
type: "CallExpression",
data: { methodName: "indexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findIndex" },
output: "foo().findIndex(Number.isNaN)"
}]
}]
},
{
code: "foo.bar.lastIndexOf(NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "lastIndexOf" } }]
errors: [{
messageId: "indexOfNaN",
type: "CallExpression",
data: { methodName: "lastIndexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findLastIndex" },
output: "foo.bar.findLastIndex(Number.isNaN)"
}]
}]
},
{
code: "foo.indexOf?.(NaN)",
options: [{ enforceForIndexOf: true }],
languageOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "indexOfNaN", data: { methodName: "indexOf" } }]
errors: [{
messageId: "indexOfNaN",
data: { methodName: "indexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findIndex" },
output: "foo.findIndex?.(Number.isNaN)"
}]
}]
},
{
code: "foo?.indexOf(NaN)",
options: [{ enforceForIndexOf: true }],
languageOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "indexOfNaN", data: { methodName: "indexOf" } }]
errors: [{
messageId: "indexOfNaN",
data: { methodName: "indexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findIndex" },
output: "foo?.findIndex(Number.isNaN)"
}]
}]
},
{
code: "(foo?.indexOf)(NaN)",
options: [{ enforceForIndexOf: true }],
languageOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "indexOfNaN", data: { methodName: "indexOf" } }]
errors: [{
messageId: "indexOfNaN",
data: { methodName: "indexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findIndex" },
output: "(foo?.findIndex)(Number.isNaN)"
}]
}]
},
{
code: "foo.indexOf(Number.NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "indexOf" } }]
errors: [{
messageId: "indexOfNaN",
type: "CallExpression",
data: { methodName: "indexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findIndex" },
output: "foo.findIndex(Number.isNaN)"
}]
}]
},
{
code: "foo.lastIndexOf(Number.NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "lastIndexOf" } }]
errors: [{
messageId: "indexOfNaN",
type: "CallExpression",
data: { methodName: "lastIndexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findLastIndex" },
output: "foo.findLastIndex(Number.isNaN)"
}]
}]
},
{
code: "foo['indexOf'](Number.NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "indexOf" } }]
errors: [{
messageId: "indexOfNaN",
type: "CallExpression",
data: { methodName: "indexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findIndex" },
output: 'foo["findIndex"](Number.isNaN)'
}]
}]
},
{
code: "foo['lastIndexOf'](Number.NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "lastIndexOf" } }]
errors: [{
messageId: "indexOfNaN",
type: "CallExpression",
data: { methodName: "lastIndexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findLastIndex" },
output: 'foo["findLastIndex"](Number.isNaN)'
}]
}]
},
{
code: "foo().indexOf(Number.NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "indexOf" } }]
errors: [{
messageId: "indexOfNaN",
type: "CallExpression",
data: { methodName: "indexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findIndex" },
output: "foo().findIndex(Number.isNaN)"
}]
}]
},
{
code: "foo.bar.lastIndexOf(Number.NaN)",
options: [{ enforceForIndexOf: true }],
errors: [{ messageId: "indexOfNaN", type: "CallExpression", data: { methodName: "lastIndexOf" } }]
errors: [{
messageId: "indexOfNaN",
type: "CallExpression",
data: { methodName: "lastIndexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findLastIndex" },
output: "foo.bar.findLastIndex(Number.isNaN)"
}]
}]
},
{
code: "foo.indexOf?.(Number.NaN)",
options: [{ enforceForIndexOf: true }],
languageOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "indexOfNaN", data: { methodName: "indexOf" } }]
errors: [{
messageId: "indexOfNaN",
data: { methodName: "indexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findIndex" },
output: "foo.findIndex?.(Number.isNaN)"
}]
}]
},
{
code: "foo?.indexOf(Number.NaN)",
options: [{ enforceForIndexOf: true }],
languageOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "indexOfNaN", data: { methodName: "indexOf" } }]
errors: [{
messageId: "indexOfNaN",
data: { methodName: "indexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findIndex" },
output: "foo?.findIndex(Number.isNaN)"
}]
}]
},
{
code: "(foo?.indexOf)(Number.NaN)",
options: [{ enforceForIndexOf: true }],
languageOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "indexOfNaN", data: { methodName: "indexOf" } }]
errors: [{
messageId: "indexOfNaN",
data: { methodName: "indexOf" },
suggestions: [{
messageId: "replaceWithFindIndex",
data: { methodName: "findIndex" },
output: "(foo?.findIndex)(Number.isNaN)"
}]
}]
}
]
});