Rule proposal: Prefer Number methods

[sindresorhus]

Fail

parseInt(…);
parseFloat(…);
isNaN(…);
isFinite(…);

Pass

Number.parseInt(…);
Number.parseFloat(…);
Number.isNaN(…);
Number.isFinite(…);

This rule cannot be auto-fixable as the globals methods have slightly different behavior. However, it should use the ESLint suggestions API.

Should we also enforce Number.NaN over NaN? They equivalent. I'm leaning towards "yes", for consistency, and also less use of floating globals.

[fisker]

I never see anyone use Number.NaN, but we can make a option to enable it.

Relate issue on eslint eslint/eslint#8894

Question: should this rule auto fixable ? Those methods are not total equal. didn't see

[sindresorhus]

Maybe we could just try enforcing Number.NaN without an option and see if anyone cares enough to complain?

[fisker]

One note, please use this isShadowed check

eslint-plugin-unicorn/rules/new-for-builtins.js

Line 20 in cdfedd8

if (enforceNew.has(name) && !isShadowed(context.getScope(), callee)) {

when implementing.

[fisker]

Name suggestion? How about no-global-number-method? But Number.NaN is not a method

[fisker]

prefer-number-member?
prefer-number-properties? The spec calls them Properties of the Number Constructor

[fisker]

Should we suggest Number.POSITIVE_INFINITY and Number.NEGATIVE_INFINITY over Infinity?

[sindresorhus]

prefer-number-properties

👍

Should we suggest Number.POSITIVE_INFINITY and Number.NEGATIVE_INFINITY over Infinity?

No, Infinity is still better. The difference between positive/negative Infinity doesn't matter in 99% of cases.

[jimmywarting]

I would like to see a reasoning for why number properties are better, (and not just because of global-phobia)
If you can't argument for a good reason, then don't break something that isn't broken.

I came here for the Number.NaN reason... never seen anyone write that
...or using Number Number.NEGATIVE_INFINITY That's just too long and unpleasant to write
Some functions are the same, parseInt === Number.parseInt, Number.parseFloat === parseFloat, So why should you use Number[xxx]?

isNaN i can understand it's not the same as Number.isNaN and dose things differently

The global isNaN() function converts the tested value to a Number, then tests it.

There may just be usecases where you would prefer to use one over the other, just because of it's differences

[sindresorhus]

If you can't argument for a good reason, then don't break something that isn't broken.

No one is forcing you to use this rule. You can opt out of it.

...or using Number Number.NEGATIVE_INFINITY That's just too long and unpleasant to write

There's an option to not check Infinity. I still personally prefer it for consistency.

Some functions are the same, parseInt === Number.parseInt, Number.parseFloat === parseFloat

Consistency. When there are some that are better and should be preferred, it's better for readability to consistency use the Number methods ones.

[jimmywarting]

No one is forcing you to use this rule. You can opt out of it.

That's what all linter say.
Think it should be the reverse, You can opt in for non-essential rules

[jimmywarting]

Reason agains Number.parseInt

Don't work in IE
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/parseInt

But it's pretty much dead now anyway

[fisker]

Isn't all these properties not available in IE? If you are targeting IE, you can't check any of them.

[fisker]

I think we can add option for Number.NaN, but not othere properties, otherwise this rule won't make sense to enable.

[jean-moldovan]

There are cases when you, for example, parse query params and want to be able to convert a string into a number if possible: 0, invalid, 123.

So you would do:

  if (isNaN(score)) { <-- Number.isNaN works differently here
    return null
  }

  return Number(score)

[fisker]

This is why we didn't auto fix this case https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prefer-number-properties.md

[jean-moldovan]

Yes, but the difference is that the linter will still complain causing confusion unless you tell it to ignore that line.

Making it configurable can solve this.

[fisker]

That's the point of this rule, you should refactor to use Number.isNaN()

[fregante]

you should refactor to use Number.isNaN()

What is the suggested refactor though? This?

- if (isNaN(numberOrIdk)) {
+ if (typeof numberOrIdk !== number || Number.isNaN(numberOrIdk)) {

If so, it's quite verbose. The lack of an autofix doesn't exactly justify a "bad" rule. In this case, if my code is equivalent, then it could still technically autofix it.

• Related: Show exact solutions to incorrect code in the documentation #1751

[fisker]

I think

- if (isNaN(numberOrIdk)) {
+ if (Number.isNaN(Number(numberOrIdk))) {

is more correct.