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.

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

feat: use-isnan report NaN in indexOf and lastIndexOf with fromIndex #18225

Merged
merged 3 commits into from Mar 23, 2024

Conversation

Tanujkanti4441
Copy link
Contributor

@Tanujkanti4441 Tanujkanti4441 commented Mar 23, 2024

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

made the rule use-isnan to report NaN in indexOf and lastIndexOf with fromIndex
now rule will report following code-

var foo = myArray.indexOf(NaN, 1);

var bar = myArray.lastIndexOf(NaN, 1);

Is there anything you'd like reviewers to focus on?

Fixes: #18161

@Tanujkanti4441 Tanujkanti4441 requested a review from a team as a code owner March 23, 2024 10:04
@eslint-github-bot eslint-github-bot bot added the bug ESLint is working incorrectly label Mar 23, 2024
@github-actions github-actions bot added the rule Relates to ESLint's core rules label Mar 23, 2024
Copy link

netlify bot commented Mar 23, 2024

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit d651b55
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/65fec3d08b04d000096b7f33

@mdjermanovic mdjermanovic added the accepted There is consensus among the team that this change meets the criteria for inclusion label Mar 23, 2024
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

Code and docs LGTM.

* code: "foo.lastIndexOf(NaN, b)",
* options: [{ enforceForIndexOf: true }]
* },
*/
Copy link
Member

Choose a reason for hiding this comment

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

Can you uncomment these tests and add 3rd argument that would make them valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mdjermanovic, in this rule following code

myArray.indexOf(NaN)

fixes to

myArray.findIndex(Number.isNaN)

but both cases return -1 so is this fix correct or there is something i am not aware of?

Copy link
Member

Choose a reason for hiding this comment

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

The fix is correct. For example:

const myArray = [0, 1, 2, NaN, 4, 5];

console.log(myArray.indexOf(NaN)); // -1

console.log(myArray.findIndex(Number.isNaN)); // 3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, thanks!

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mdjermanovic mdjermanovic changed the title fix: report NaN in indexOf and lastIndexOf with fromIndex feat: report NaN in indexOf and lastIndexOf with fromIndex Mar 23, 2024
@eslint-github-bot eslint-github-bot bot added the feature This change adds a new feature to ESLint label Mar 23, 2024
@mdjermanovic mdjermanovic changed the title feat: report NaN in indexOf and lastIndexOf with fromIndex feat: use-isnan report NaN in indexOf and lastIndexOf with fromIndex Mar 23, 2024
@mdjermanovic mdjermanovic merged commit d85c436 into eslint:main Mar 23, 2024
18 of 19 checks passed
@Tanujkanti4441 Tanujkanti4441 deleted the report-argument branch March 23, 2024 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly feature This change adds a new feature to ESLint rule Relates to ESLint's core rules
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

Bug: use-isnan doesn't report NaN in indexOf or lastIndexOf when fromIndex is given
2 participants