lib.d.ts contains wrong typings for Number.isFinite()

[rogierschouten]

I'm not sure whether this is the repo to be putting this issue on, please let me know if it isn't.

TypeScript Version: 2.2.2

Code
Compiling with strict null checks:

 Number.isFinite(undefined);

Expected behavior:

No compile error. The argument to isFinite should be any, because the function is used to check the type. According to the spec at https://www.ecma-international.org/ecma-262/6.0/#sec-number.isfinite the type is checked by the function.

Actual behavior:

error TS2345: Argument of type 'undefined' is not assignable to parameter of type 'number'

[mhegazy]

Duplicate of #4002

[rogierschouten]

@mhegazy this is NOT a duplicate of #4002. That story is about the isFinite() function and not about Number.isFinite(). The spec for Number.isFinite() is different and the arguments brought forward in #4002 do not apply here. I still think I have a valid proposal.

[gcnew]

Yes, I think Number.isFinite() is intended to be used like Array.isArray, i.e. it is a guard for numbers, not a function that coerces its arguments to number prior to operation.

In this context the right signature for Number.isFinite is:

isFinite(arg: any): arg is number;

In my opinion, Number.isInteger and Number.isSafeInteger should not be changed, because TS doesn't distinguish between integers and numbers yet. When integers start having their own type at a later point, it would be a breaking change if the above mentioned functions were made is number guards now.

PS: the global isFinite is coercing and all arguments from #4002 apply to it. However, Number.isFinite is different.

Number.isFinite('123'); // false
isFinite('123'); // true

[RyanCavanaugh]

isFinite(arg: any): arg is number isn't correct because there are some things that are of type number that aren't finite. Type predicates must return true for all values of a type, not just some, to be correct.

[rogierschouten]

@gcnew: @RyanCavanaugh is right, the typing should be:

isFinite(arg: any): boolean;

[mhegazy]

not sure i understand why this issue is different from #4002.

[gcnew]

I think the logic should be the reverse. Every value for which isFinite returns true is definitely a number. But not every number is finite - the only exceptions are NaN, Infinity and -Infinity. It seems rather pedantic not to guard arg as number on the grounds that it can theoretically be one of the above three constants, which the type system doesn't even consider as literal types.

Edit: In my opinion a better approach is to model these known corner case constants as special cases. Number should not include them in a strict setting the same way types don't include null and undefined in strictNullChecks.

[mhegazy]

just to be clear, are we talking about the argument type for Number.isFinite (currently number) or the return type being a user-defined type guard (currently boolean)?

I am assuming this issue is about the earlier.

[gcnew]

From my perspective, if Number.isFinite doesn't become a guard for number, there is no reason to change the type of arg to any.

[gcnew]

@mhegazy I consider this issue different from #4002 as Number.isFinite should be usable to assert a value of unknown type as a number, the same way Array.isArray asserts that a value is actually an array.

[chandanch]

I guess it should should be

Number.isFinite(argument : any) : boolean

[RyanCavanaugh]

@gcnew see #15048 for type guards which don't cover their entire domains

Regarding accepting any, I don't understand why this is desirable. Any non-number is known to produce false from this function. You could say "Well I want to use this to test a number | string" but we can't narrow the input to number because it's not exhaustive (see above again), so it's not a useful function for this purpose (typeof x === "number" would be better).

[rogierschouten]

The reason that it is desirable is the following:

/**
 * @param n  A finite number (optional)
 */
function doSomething(n?: number) {
  if (Number.isFinite(n)) {
   // process user input
  }
}

is less to type and less to execute than

/**
 * @param n  A finite number (optional)
 */
function doSomething(n?: number) {
  if (n !== undefined && Number.isFinite(n)) {
   // process user input
  }
}

And the ECMA spec says I can simply use the first example but the typescript compiler simply won't compile it

I've seen people do all kinds of things like

if (n !== null && n !== undefined && isFinite(n))

and usually they'll forget the null check in a couple of cases. FINALLY I can give all my team a guideline that will always work without surprises and it's called Number.isFinite(). It just works. I only need it to compile with strict null checks on.

[RyanCavanaugh]

You could also write Number.isFinite(n!), or n != null, or typeof n === 'number', depending. I'm not sure why you're using isFinite for this case - you really want to act as if the parameter wasn't provided in the cases where it's Infinity or NaN ?

[RyanCavanaugh]

e.g. I'd be really surprised if I wrote doSomething(n / arr.length) without checking for an empty array and that got silently ignored because Infinity isn't isFinite

[rogierschouten]

Ok so maybe I constructed the wrong example but I've also seen a lot of this

if (!Number.isFinite(userInput)) {
  throw new Error("boo");
}

That's a very nice catch-all that checks exactly what you want to check

[chandanch]

We also do in this way:

if(userNumber typeOf number) {
    if(!Number isFinite(userNumber)) {
        console.log("Error")
    }
}
else {
    console.log("Not a number");
}

[mhegazy]

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.