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

Stricter generic signature checks #16368

Merged
merged 15 commits into from
Jun 12, 2017
Merged

Stricter generic signature checks #16368

merged 15 commits into from
Jun 12, 2017

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Jun 8, 2017

This PR complements #16305 by implementing stricter checking of type relationships for generic signatures. Previously, when checking type relationships for generic signatures we would always first erase type parameters (by substituting type any for the type parameters). Now, we properly unify the type parameters by first inferring from the target signature to the source signature and then instantiating the source signature with the inferences. For example:

type A = <T, U>(x: T, y: U) => [T, U];
type B = <S>(x: S, y: S) => [S, S];

function f(a: A, b: B) {
    a = b;  // Error
    b = a;  // Ok
}

Previously, no errors were reported above because S, T, and U were replaced by any, causing the signatures to become identical. Now, once we unify the type parameters we can correctly determine that A is assignable to B, but not vice versa. Specifically, in the first assignment we infer T | U for S which after instantiation yields a signature that isn't assignable to A, whereas in the second assignment we infer S for each of T and U which after instantiation yields the same signature as B.

Note that we perform unification only when the source and target types each have a single signature. When either the source or target has multiple signatures (i.e. overloads) we still resort to type parameter erasure as it otherwise becomes prohibitively expensive to determine the relationships.

This PR is technically a breaking change as we now uncover errors we previously wouldn't catch. For this reason we have included a --noStrictGenericChecks compiler option to disable the stricter generic signature checks. Existing projects can use this as a stopgap solution until errors resulting from the stricter checking are corrected.

Fixes #138, #3410, #5616.

@ahejlsberg ahejlsberg changed the title Stricter generic checks Stricter generic signature checks Jun 8, 2017
@ahejlsberg
Copy link
Member Author

@Aleksey-Bykov Now you can retire the shirt!

@ahejlsberg ahejlsberg requested a review from mhegazy June 8, 2017 19:49
@ahejlsberg ahejlsberg added the Breaking Change Would introduce errors in existing code label Jun 8, 2017
@zpdDG4gta8XKpMCd
Copy link

@ahejlsberg this is one of all time grand features of TS since unions, type guards and exhaustive switches, it brings point free programming style like what you can find in big boy's FP languages, thank you for your hard work and consideration

@gcnew
Copy link
Contributor

gcnew commented Jun 9, 2017

Great work!

One issue that still stands is #16107. As #16368 is already a breaking change, is it now the time to correct the union inference wart as well? I hope it is.

if (!related) {
return Ternary.False;
}
result &= related;
}
}
else if (sourceSignatures.length === 1 && targetSignatures.length === 1) {
// For pure functions (functions with a single signature) we only erase type parameters for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe simple/just/only functions would be a better name than pure, as pure brings a different connotation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Will fix.

@ahejlsberg
Copy link
Member Author

Latest commits add a --noStrictGenericChecks compiler option to disable the stricter generic signature checks. Existing projects can use this as a stopgap solution until errors resulting from the stricter checking are corrected.

@ahejlsberg ahejlsberg added this to the TypeScript 2.4 milestone Jun 12, 2017
@ahejlsberg ahejlsberg merged commit dde60bb into master Jun 12, 2017
@ghost ghost mentioned this pull request Oct 4, 2017
rkirov added a commit to rkirov/zone.js that referenced this pull request Oct 11, 2017
Due to stricter generic type checks, signatures for ZoneAwarePromise
need to be changed. See
microsoft/TypeScript#16368

The signatures from lib.es5.d.ts were copied over for .then and .catch.
rkirov added a commit to rkirov/zone.js that referenced this pull request Oct 11, 2017
Due to stricter generic type checks, signatures for ZoneAwarePromise
need to be changed. See
microsoft/TypeScript#16368

The signatures from lib.es5.d.ts were copied over for .then and .catch.
rkirov added a commit to rkirov/zone.js that referenced this pull request Oct 12, 2017
Due to stricter generic type checks, signatures for ZoneAwarePromise
need to be changed. See
microsoft/TypeScript#16368

The signatures from lib.es5.d.ts were copied over for .then and .catch.
mhevery pushed a commit to angular/zone.js that referenced this pull request Dec 27, 2017
Due to stricter generic type checks, signatures for ZoneAwarePromise
need to be changed. See
microsoft/TypeScript#16368

The signatures from lib.es5.d.ts were copied over for .then and .catch.
@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
Breaking Change Would introduce errors in existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants