Skip to content

Type parameter leaks #19854

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

Closed
ghost opened this issue Nov 9, 2017 · 11 comments
Closed

Type parameter leaks #19854

ghost opened this issue Nov 9, 2017 · 11 comments
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@ghost
Copy link

ghost commented Nov 9, 2017

TypeScript Version: 2.7.0-dev.20171108

Code

declare function first<T>(array: T[]): T | undefined;
function f(x?: string[]) {
	return first(x);
}
const x = f([]);

Expected behavior:

Type of x is {} | undefined.

Actual behavior:

Type of x is T | undefined.

@ghost ghost added the Bug A bug in TypeScript label Nov 9, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Nov 9, 2017

There is an error resolving the call for first. x of type string[] | undefined is not assignable to T[]. this goes through the resolveErrorCall..

@mhegazy mhegazy added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Bug A bug in TypeScript labels Nov 9, 2017
@ghost
Copy link
Author

ghost commented Nov 9, 2017

Shouldn't we fill in type parameters in resolveErrorCall?

@mhegazy
Copy link
Contributor

mhegazy commented Nov 9, 2017

it is an error.. what difference does it make really..

@ghost
Copy link
Author

ghost commented Nov 9, 2017

Due to return type inference, the erroneous type can occur arbitrarily far from the erroneous call site. A user won't know where to look first -- they'll just see T come out of nowhere, and since this is a common name they might think it refers to a different T. (This actually happened to me...)

@gcnew
Copy link
Contributor

gcnew commented Nov 10, 2017

Considering @ts-ignore, this might lead to surprising results. Checking of the leaked type parameter is off.

declare function first<T>(array: T[]): T;
function mkSkolem(x?: string[]) {
    // @ts-ignore
    return undefined! && first(x);
}

const skolemWitness = mkSkolem(undefined);
const val: typeof skolemWitness = 5; // should be an error

@mhegazy
Copy link
Contributor

mhegazy commented Nov 10, 2017

@ts-ignore leads to surprising results, and should not be used IMHO.

@gcnew
Copy link
Contributor

gcnew commented Nov 11, 2017

Sure, @ts-ignore is obviously a red mark. I'm still concerned with the leaked parameter, though. It seems to behave like an implicit any that's not being flagged. If it were a type not assignable to/from anything, the leak would have been less bad.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 13, 2017

that's not being flagged

It is.. there is an error!

@typescript-bot
Copy link
Collaborator

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

@ghost ghost added Bug A bug in TypeScript Fixed A PR has been merged for this issue and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Dec 6, 2017
@ghost ghost reopened this Dec 6, 2017
@ghost
Copy link
Author

ghost commented Dec 6, 2017

#19620 will fix this. This was only a problem in services because we forgot to always fill in all the type arguments in the error case pickLongestCandidateSignature that only runs if we're in services.

@mhegazy mhegazy added this to the TypeScript 2.9 milestone Apr 12, 2018
@mhegazy mhegazy assigned ghost Apr 12, 2018
@ghost ghost added Fixed A PR has been merged for this issue and removed Fixed A PR has been merged for this issue labels Jun 20, 2018
@mhegazy mhegazy modified the milestones: TypeScript 3.0, TypeScript 3.1 Jul 2, 2018
@ghost
Copy link
Author

ghost commented Jul 11, 2018

Fixed by #25100

@ghost ghost closed this as completed Jul 11, 2018
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

3 participants