Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TS doesn't check the implementation of the overloaded function whether match its signature. #18533

Closed
zheeeng opened this issue Sep 17, 2017 · 5 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@zheeeng
Copy link

zheeeng commented Sep 17, 2017

TypeScript Version: 2.5.2

Code

TS only check the function signature whether is compatible with that overloaded function's signature. If not, TS report Overload signature is not compatible with function implementation.. It actually doesn't check the function's implementation, only the signature's compatibility. In the example below:

class Animal { }
class Dog extends Animal { }

function test(d: Dog): string
function test(d: Animal): number
function test(d: Animal | Dog): number | string {
    if (d instanceof Dog) {
        return 42
    } else {
        return '苟'
    }
}

If we call test as:

const o = test(new Dog())

o is inferred as the string type, but actually, it's number type, eventually, cause the runtime error.

Expected behavior:

Report the first and second test function doesn't be implemented. The third test function signature pass. TS should be able to check the implementation with these signatures which are overloaded, we have done the type narrowing.

Actual behavior:

no errors

@DanielRosenwasser DanielRosenwasser added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Sep 17, 2017
@DanielRosenwasser
Copy link
Member

The check is only to ensure that the signatures are sufficiently compatible. The overloads are a promise you're making to API consumers, while TypeScript allows you to be as flexible as possible to fulfill each of those signatures in the implementation.

Unfortunately we don't really have any simple ideas about how to perform this check efficiently.

@HerringtonDarkholme
Copy link
Contributor

how to perform this check efficiently

In case OP wonder why the check is prohibitively expensive:

It's TypeScript's design limitation. TS has a structural typing system, that is, the assignability between two types relies solely on their property types. (e.g. your example function test isn't "correct" because Dog and Animal has the same shape). No shortcut can be used during checking.

In order to check if a branch return value is compatible to the signature, the compiler will have to check all signatures from top to bottom until one signature matches. The complexity is about O(mn) where m is the number of branches and n is the number of signatures. For nontrivial overload functions, this complexity will easily trigger out of memory error or too time consuming.

@zheeeng
Copy link
Author

zheeeng commented Sep 18, 2017

What if adding some annotations which are required on branches? We manually tell TS this branch match which signature. Inside the function, TS just check the narrowed type and the return type. Outside, TS check if all the signature functions have their annotations in the function implementation? Just need a Map with signature and annotation pairs.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 9, 2017

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

@mhegazy mhegazy closed this as completed Oct 9, 2017
@zheeeng
Copy link
Author

zheeeng commented Oct 10, 2017

Is there any plan for breaking this limitation?

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

4 participants