Skip to content
New issue

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

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

Already on GitHub? # to your account

Incompatible code unexpectedly compiles due to intersection with empty mapped type #19869

Closed
kpdonn opened this issue Nov 9, 2017 · 4 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@kpdonn
Copy link
Contributor

kpdonn commented Nov 9, 2017

TypeScript Version: Version 2.7.0-dev.20171109

Code

type Args<S extends string, N extends string> = Record<S, string> & Record<N, number>

declare function f1<S extends string, N extends string>
    (strs: S[], nums: N[], fun: (args: Args<S, N>) => void): void

declare function takesNum(args: {a: number}): void
declare function takesOptionalNum(args: {a?: number}): void

f1(["a"], [], takesNum) // *** unexpectedly compiles despite incompatibility of "a" ***
f1(["a"], ["b"], takesNum) // Now correctly errors because of incompatibility of "a"

f1(["a"], [], takesOptionalNum) // *** unexpectedly compiles despite incompatibility of "a" ***
f1(["a"], ["b"], takesOptionalNum) // Now correctly errors because of incompatibility of "a"

Expected behavior:
All calls to f1 should error because takesNum and takesOptionalNum expect a to be a number but it will actually be a string.

Actual behavior:
It only errors if the second argument to f1 is not empty.

Also the first case shown above: f1(["a"], [], takesNum) actually errored as expected as recently as last week(I believe I was on 2.7.0-dev.20171031), it was only the optional case that unexpectedly compiled. I was about to open this for just the optional case when I checked the latest typescript version and noticed the first case started being affected also.

@kpdonn
Copy link
Contributor Author

kpdonn commented Nov 9, 2017

Also interesting is if you change takesNum:

declare function takesNum(args: number): void

f1(["a"], [], takesNum) // now errors of course but with the totally incorrect inferred type of Args:

// Error:(20, 15) TS2345: Argument of type '(args: number) => void' is not assignable to parameter of type 
// '(args: Args<"a", "toString" | "toFixed" | "toExponential" | "toPrecision" | "valueOf" | "toLocaleString">) => void'.

@kpdonn
Copy link
Contributor Author

kpdonn commented Nov 9, 2017

FYI was looking through changes recently to guess what caused the behavior change and it seems almost certain to be #19745 so maybe this behavior is expected? Looking at the original bug(#19576) that was addressing I can see it'd be a very fine needle to thread to somehow both fail in my example and compile in the #19576 example.

I did discover that I can work around half of my problem by doing:

type Args<S extends string, N extends string> = Record<S, string> & Record<N, number>

declare function f2<S extends string, N extends string, F extends (args: Args<S, N>) => void>
(strs: S[], nums: N[], fun: F): void

declare function takesNum(args: {a: number}): void
declare function takesOptionalNum(args: {a?: number}): void

f2(["a"], [], takesNum) // Now correctly errors because of incompatibility of "a"

f2(["a"], [], takesOptionalNum) // *** Still unexpectedly compiles despite incompatibility of "a" ***
f2(["a"], ["b"], takesOptionalNum) // Now correctly errors because of incompatibility of "a"

But as the comments show, the optional case still works incorrectly even after that.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 10, 2017

Should be addressed by #19912

@mhegazy mhegazy added Bug A bug in TypeScript Fixed A PR has been merged for this issue labels Nov 10, 2017
@mhegazy mhegazy added this to the TypeScript 2.7 milestone Nov 10, 2017
@ahejlsberg
Copy link
Member

I think this is working as intended and it isn't affected by #19912. In this call

f1(["a"], [], takesNum);

we make the inference "a" for S from the first argument and no inferences from the second argument (or, following #19912, a low priority inference of never for N). Then, from the third argument, we make the inference "a" for both S and N. All up this means we infer "a" for both type arguments. Once we instantiate with those inferences we get

f1(strs: "a"[], nums: "a"[], fun: (args: Args<"a", "a">) => void): void;

which is equivalent to

f1(strs: "a"[], nums: "a"[], fun: (args: { a: string } & { a: number }) => void): void;

which is a signature for which all of the arguments are compatible.

@ahejlsberg ahejlsberg added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Bug A bug in TypeScript Fixed A PR has been merged for this issue labels Nov 10, 2017
@ahejlsberg ahejlsberg removed their assignment Nov 10, 2017
@ahejlsberg ahejlsberg removed this from the TypeScript 2.7 milestone Nov 10, 2017
@kpdonn kpdonn closed this as completed Nov 10, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants