Repository navigation
Implicit any issue with overloaded methods #11936
Description
Activity
I can confirm with
2.1.0-dev.20161029. Seems like it tries to match with only the first overload.Removing the first overload fixes it. Changing the callback to
(a) => {}also fixes this.Probably caused by #11905
this indeed is caused by the change #11905. we should reconsider our fix there.
- added this to the milestone
on Oct 31, 2016 - changed the title
[-]Implicit any issue with overflowed methods[/-][+]Implicit any issue with overloaded methods[/+]on Oct 31, 2016 This is actually an improvement —
bwas (mostly) an unreported implicit any before. In 2.0 (or earlier, all the way back to 1.1) try compilingb.blah()inside the body of the lambda. You won't get an error becausebis implicitly any.Note that
- The playground gives the type of
bas number, so there's an additional bug in the language service. - The
anys are a limitation of the current compiler design, which fixes types while resolving the first overload, which in this case is too short. Previously the types were incorrect, now they're not available at all.
The correct fix is to reverse the order of the overloads or to get rid of the overloads entirely. They are technically redundant but are probably useful as documentation.
Reacted by SlurpTheo- The playground gives the type of
which fixes types while resolving the first overload, which in this case is too short
Nathan Shively-Sanders (@sandersn) Why do you need to fix the type on the first overload? And not just choose the overload that "fits"? In my example, the second overload should clearly be used for 2 argument callbacks.
I'm pretty sure though, that previous types where not typed any, at least not on my real world application. I'm also getting the error, when defining a test with Mocha testing framework(this one I'm pretty sure it was not typed as any):
it(testName, (done) => { // error 'done' is implicitly typed as any })
Andy (Andrewkraft) (@Andy-MS) actually fixed jasmine.d.ts this morning to work with the updated compiler. The core limitation, since TypeScript 1.1, is that the compiler will only assign types to any expression once. This gives, unfortunately, extremely subtle semantics.
Normally this is fine, but overload resolution changes the contextual type as it tries each overload in order, so you can observe the effect of this limitation. I don't think there's a reasonable way to check overloads besides checking the arguments against each overload in turn. For example, these single-argument functions:
declare function f((x: number, y: number, z: number) => string): void; declare function f((x: string, y: string) => string): void; f(x => x);
Even when there's something that clearly makes an overload unsuitable for an argument, it may be nested deep in the argument or parameter type. So I think it's better to use normal type-checking machinery when possible: just ask whether all arguments are assignable to their respective parameters.
In the future, we might go back and rewrite overload resolution but right now we'd prefer to correctly report implicit-any errors from our limited algorithm instead of silently hiding them.
Reacted by Herrington Darkholme and SlurpTheoThank you for detailed comments. It's really enlightening.
I'm thinking about doing overload inference better by choosing overload by their argument length.
The reasoning behind is that at runtime we cannot tell apart two functions from their signatures other than theirlengthproperties, as pointed out here.For example
var use: Overload; use((req: string, res) => {}); interface Overload { (handler1: (req1: string) => void): void; (handler2: (req2: number, res2: number) => void): void; }
the argument function will always be contextual inferred against
handler2that has length of 2. And at runtime, anOverloadimplementation can only inspect the function by theirlengthproperty, so even if user manually annotatereqasstring, it getsnumberat runtime.Also, the paramter annotation of string is incompatible with number, it is desirable compiler can detect this and report an error.
(theoretically users can inspect function type by inspecting function's source string, Angular1 actually have done this. But that isn't common in JS)
Well, the top-level overload resolution does check function arity. But in your example, both signatures have arity=1 and the parameters themselves are functions that differ in arity. And this difference in arity could occur at any level, eg
class Foo1 { foo: Bar1; } class Foo2 { foo: Bar2; } class Bar1 { nested(req1: string) => void): void; } class Bar2 { nested(req2: number, res2: number) => void): void; } interface Overload { (handler1: Foo1): void; (handler2: Foo2): void; } let use: Overload use({ foo: { nested: (req2, res2) => { console.log("req2: string, oh no!"); } } })
An instance of
Foo2looks like it could matchhandler1until we reachBar2. That level of nesting is why the check should rely on normal assignability rules to check each overload. But checking assignability requires the compiler to know the types of each parameter and argument. And asking for the type the argument will fix the types for later overloads.So I think we need to remove the limitation that once checked, types are fixed, and let type checking operate in a non-permanent fashion.
Reacted by SlurpTheoNathan Shively-Sanders (@sandersn), why it is not merged to
(req2: string | number, res2?: number)?Artem (@ArtemAvramenko) this merging is too permissive in many cases. If the two overload signatures are
(s: string, t: string)and(n: number, m: number)then merging them to twostring | numberparameters incorrectly allows me to pass a string and a number.- locked and limited conversation to collaborators
on Jun 19, 2018
This code was working for me before but broken in the latest: