This might be a bit finicky since it would catch const isNaN = Number.isNaN; isNaN(val); too? But maybe that's not a real problem... 😆

No objections, but one question and one observation.

Question: What's the reason for avoiding the isNaN() global?

Observation: I think this can probably be implemented using no-restricted-syntax rather than a custom rule.

@Trott

Question: What's the reason for avoiding the isNaN() global?

For personal reasons, I think it's more clear to use Number.isNaN. Also, several Code and Learn PRs have been submitted to move towards Number.isNaN, so I figured that people had come to consensus that it was a good idea.

Observation: I think this can probably be implemented using no-restricted-syntax rather than a custom rule.

Interesting -- refactored down to use no-restricted-syntax

Unfortunately this won't work as it overrides all the no-restricted-syntax rules declared within .eslintrc.yaml in the root directory.

Oh, yeah, I assumed it was a lint rule we'd want for the entire code base...

Oh, yeah, I assumed it was a lint rule we'd want for the entire code base...

Yeah, I think we could do that? I don't see any reason why we would be stricter about this in test than lib.

Are we sure we want to necessarily encourage people to change isNaN() to Number.isNan(). They behave differently.

isNaN('a string is not a nubmer'); //true
Number.isNaN('a string is not a number'); // false

Number.isNaN should be the new standard way of doing this as per ES2015. There's weird coercion in isNaN.

updated @apapirovski @Trott

For context, I was initially hesitant to add the rule to lib/, since we are generally performance sensitive, but I don't think the change will massively alter performance. Should benchmarks be run anyways?

@maclover7 Sorry to make this tedious, but this is what I'd recommend:

• Revert to your initial custom rule and apply it just to test
• If you wish, in a separate PR, propose changing isNaN() to Number.isNaN() in lib and prepare to run benchmarks and have a fair amount of conversation about it. :-D
• If the above bullet point suggestion lands, then use the restricted-syntax thing to replace your custom rule.

(And yeah, had I not said anything, first bullet point would have happened and all of this would have been avoided. Sorry!)

updated @Trott, PTAL

CI: https://ci.nodejs.org/job/node-test-pull-request/12184/

Will land tomorrow unless any objections

Landed in e554bc8