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

Support Number.NaN within use-isnan #14715

Closed
esdmr opened this issue Jun 16, 2021 · 3 comments · Fixed by #14718 · 4 remaining pull requests
Closed

Support Number.NaN within use-isnan #14715

esdmr opened this issue Jun 16, 2021 · 3 comments · Fixed by #14718 · 4 remaining pull requests
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly repro:yes Issues with a reproducible example rule Relates to ESLint's core rules

Comments

@esdmr
Copy link

esdmr commented Jun 16, 2021

What rule do you want to change?
use-isnan

Does this change cause the rule to produce more or fewer warnings?
More.

How will the change be implemented? (New option, new default behavior, etc.)?
Perhaps as a default behaviour, as NaN and Number.NaN are equivalent.

Please provide some example code that this change will affect:

x === NaN;
x === Number.NaN;

What does the rule currently do for this code?
Only the first line is reported as a NaN comparison. Second line does not report.

What will the rule do after it's changed?
Both lines will report as NaN comparisons. (Maybe recommending Number.isNaN instead of isNaN but that is aside)

Are you willing to submit a pull request to implement this change?
I would prefer if someone else send a PR.

At lib/rules/use-isnan.js:24:

-    return Boolean(node) && node.type === "Identifier" && node.name === "NaN";
+    return Boolean(node) && (node.type === "Identifier" && node.name === "NaN" ||
+        node.type === "MemberExpression" &&
+        node.object.type === "Identifier" && node.object.name === "Number" &&
+        node.property.type === "Identifier" && node.property.name === "NaN");
@esdmr esdmr added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules triage An ESLint team member will look at this issue soon labels Jun 16, 2021
@snitin315
Copy link
Contributor

snitin315 commented Jun 17, 2021

Looks like a bug to me. Online Demo

I would prefer if someone else send a PR.

I can send a PR if this issue is accepted.

@snitin315 snitin315 added bug ESLint is working incorrectly repro:yes Issues with a reproducible example and removed enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Jun 17, 2021
@aladdin-add
Copy link
Member

agreed it was a bug.

@nzakas nzakas added the accepted There is consensus among the team that this change meets the criteria for inclusion label Jun 17, 2021
@nzakas
Copy link
Member

nzakas commented Jun 17, 2021

Agree, looks like a bug.

snitin315 added a commit that referenced this issue Jun 18, 2021

Verified

This commit was signed with the committer’s verified signature.
allanlewis Allan Lewis
)
mdjermanovic added a commit that referenced this issue Jun 26, 2021

Verified

This commit was signed with the committer’s verified signature.
allanlewis Allan Lewis
…14718)

* Update: improve `isNaNIdentifier` to detect `Number.isNaN` (fixes #14715)

* Chore: add test cases for `Number.NaN`

* Docs: add more examples for `use-isnan`

* Chore: improve logic and add more test cases

* Docs: Update docs/rules/use-isnan.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Dec 24, 2021
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Dec 24, 2021
@nzakas nzakas moved this to Complete in Triage Jan 3, 2023
@nzakas nzakas added this to Triage Jan 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.