Repository navigation
Number.isInteger and others should accept any and act as type guards #21199
Description
TypeScript Version: 2.7.0-dev.20180116
Code
function ensureInt(x: number | string): number {
if (Number.isInteger(x)) { // Same with isSafeInteger, isNan and isFinite
return x;
} else {
throw Error("Type error");
}
}Expected behavior:
It should compile without errors.
Actual behavior:
The compiler complains as follow:
file.ts(2,26): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
Type 'string' is not assignable to type 'number'.
file.ts(3,9): error TS2322: Type 'string | number' is not assignable to type 'number'.
Type 'string' is not assignable to type 'number'.
Proposed solution:
Rewrite es6 type definition of NumberConstructor.isInteger (et al.) like this:
isInteger(number: any): number is number;
Activity
jcalz commentedon Jan 16, 2018
Seems like one of many duplicates of #4002, and maybe a duplicate of #19557, and another opportunity to think about how to prevent such duplicates. Maybe the text under
<!-- BUGS: Please use this template. -->in the issue reporting template should be very explicit about how to check for existing issues? (Instead of just relying on the link at the top saying "read the contributing guidelines"?)You don't want anything to act as
arg is Tunlesstruemeans it is andfalsemeans it isn't. Do you wantNumber.isInteger(0.5) === falseto imply that0.5is not anumber?cyco130 commentedon Jan 16, 2018
I see, it makes sense. I did search for isInteger before submitting but it didn't occur to me to search for isNaN etc.
jcalz commentedon Jan 16, 2018
Sure, it happens.
The relevant advice in the contributing guidelines says "Check for synonyms. For example, if your bug involves an interface, it likely also occurs with type aliases or classes." (I guess the word "synonyms" should probably be replaced with "related terms", since the example isn't showing literal synonyms anyway.)
The question is: is the stuff in that document the best we can do to guide potential issue-filers to find potential duplicates without scaring them away entirely? A prominent warning like "You're probably not the first person to encounter this issue; you'd better spend at least an hour searching through the existing issues for every possible related term before filing; if you file a duplicate, we will send monsters to devour you." would probably cut down on the false positives, but false negatives are bad too. Oh well, who knows?
cyco130 commentedon Jan 16, 2018
Heh, thanks for the advice. I'll make sure to search for at least an hour before requesting something like
isInteger(n: any): implies n is number:)RyanCavanaugh commentedon Jan 18, 2018
Joe Calzaretta (@jcalz) didn't quite go as far as what you have up there, but #21238 puts some tough love in the template
MicahZoltu commentedon May 27, 2018
I opened #24436 without finding this (apparently I also searched for the wrong keywords). However, this (and #24436) are not duplicates of #4002. Despite the similarity in naming,
Number.isFiniteandisFiniteare not the same function and all of the arguments made in #4002 do not apply to the functions on theNumberconstructor. If this (and #24436) are to remain closed, I would like to see new arguments formulated/expressed as to why to ensure they aren't being closed just because they look like duplicates of #4002.