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

Incorrect inference of arrow function with intersected mapped type argument under strictFunctionTypes #32664

Open
smoogly opened this issue Aug 1, 2019 · 17 comments
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@smoogly
Copy link

smoogly commented Aug 1, 2019

TypeScript Version: 3.6.0-dev.20190801

Search Terms: inference intersection type strictFunctionTypes

Code

type MappedIntersection<T extends {}> = { [P in keyof T]: string } & {};
type MappedRaw<T extends {}> = { [P in keyof T]: string };

// Case 1: Intersection + no arrow = succeed
type ToInferNoArrowIntersection<T extends {}> = {
    fn(params: MappedIntersection<T>): void;
};

type NoArrowIntersection<T extends ToInferNoArrowIntersection<any>> = void;
declare const case1: NoArrowIntersection<{
    fn: (params: MappedIntersection<{a: 1}>) => string,
}>;


// Case 2: No intersection + arrow = succeed
type ToInferArrowNoIntersection<T extends {}> = {
    fn: (params: MappedRaw<T>) => void;
};

type ArrowNoIntersection<T extends ToInferArrowNoIntersection<any>> = void;
declare const case2: ArrowNoIntersection<{
    fn: (params: MappedRaw<{a: 1}>) => void,
}>;


// Case 3: Intersection + arrow = fail
type ToInferArrowIntersection<T extends {}> = {
    fn: (params: MappedIntersection<T>) => void;
};

type ArrowIntersection<T extends ToInferArrowIntersection<any>> = void;
declare const case3: ArrowIntersection<{ // Fails here
    fn: (params: MappedIntersection<{a: 1}>) => void,
}>;

Expected behavior:
Expected case 3 to succeed.

Actual behavior:
Case 3 produces:

 Type '{ fn: (params: { a: string; }) => void; }' does not satisfy the constraint 'ToInferArrowIntersection<any>'.
  Types of property 'fn' are incompatible.
    Type '(params: { a: string; }) => void' is not assignable to type '(params: { [x: string]: string; }) => void'.
      Types of parameters 'params' and 'params' are incompatible.
        Property 'a' is missing in type '{ [x: string]: string; }' but required in type '{ a: string; }'.

Seems relevant that compilation succeeds without strictFunctionTypes

Playground Link: Playground

Related Issues:
#29123 (comment)
#31081

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Aug 1, 2019
@jack-williams
Copy link
Collaborator

Case 1 correctly produces no error because fn is a method and therefore related by bivariantly.

Case 3 should produce an error because under strict function types the function inputs are related in reverse, and a string indexer is not related to the object with property a.

declare const x: { [x: string]: string; };
const b: { a: string } = x; // error

It appears to be Case 2 that is questionable, and is most likely due to variance measurements. I can correctly synthesise the error by using identical but distinct type aliases.

type MappedIntersection<T extends {}> = { [P in keyof T]: string } & {};
type MappedRaw1<T extends {}> = { [P in keyof T]: string };
type MappedRaw2<T extends {}> = { [P in keyof T]: string };

// Case 2: No intersection + arrow = succeed
type ToInferArrowNoIntersection<T extends {}> = {
    fn: (params: MappedRaw1<T>) => void;
};

type ArrowNoIntersection<T extends ToInferArrowNoIntersection<any>> = void;
declare const case2: ArrowNoIntersection<{
    fn: (params: MappedRaw2<{a: 1}>) => void, // error
}>;

So what seems to be going on is that in Case 2 the params arguments are being related by their type parameters: this succeeds because any is related to everything. In Case 3 it appears that the intersection causes variance checking to be skipped and a structural check is used instead, which correctly detects the error.

@jack-williams
Copy link
Collaborator

Minimal repro:

declare const a: MappedIntersection<any>;
declare const b: MappedRaw1<any>;
const zMeasure: MappedRaw1<{x: string}> = b; // no error
const zStructural: MappedRaw2<{x: string}> = b; // error
const zDifferent: MappedRaw2<{x: string}> = a; // error

@weswigham
Copy link
Member

Does #32225 fix the issue, I wonder?

@jack-williams
Copy link
Collaborator

Yeah I wasn't sure if that was merged, but it seems to target the problem!

@smoogly
Copy link
Author

smoogly commented Aug 1, 2019

I might be wrong somewhere, but aren't Case 1 and Case 3 supposed to be identical?

I was under impression that fn(): void and fn: () => void describe the same thing.

@weswigham
Copy link
Member

Sorta. Outside strict, yes, but within strict mode the method syntax is used as a hint for looser typechecking.

@smoogly
Copy link
Author

smoogly commented Aug 1, 2019

Does it means that by design third case will always fail?

Does that also mean that sometime in the future first case will also be failing?

I have trouble understanding why describing same underlying JS might fail or succeed based on a choice of syntax.

@jack-williams
Copy link
Collaborator

@weswigham Seems to be an issue with unreliable markers rather than unmeasurable, if my understanding is correct. Optionality modifiers work ok, but readonly (or none) do not.

type MappedRaw1RO<T extends {}> = { readonly [P in keyof T]: string };
type MappedRaw2RO<T extends {}> = { readonly [P in keyof T]: string };

declare const b: MappedRaw1RO<any>;
const zMeasure: MappedRaw1RO<{x: string}> = b; // ***no error***
const zStructural: MappedRaw2RO<{x: string}> = b; // ***error***

type MappedRaw1OPT<T extends {}> = { [P in keyof T]-?: string };
type MappedRaw2OPT<T extends {}> = { [P in keyof T]-?: string };

declare const b2: MappedRaw1OPT<any>;
const zMeasure2: MappedRaw1OPT<{x: string}> = b; // ***error***
const zStructural2: MappedRaw2OPT<{x: string}> = b; // ***error***

@smoogly
Copy link
Author

smoogly commented Aug 1, 2019

@jack-williams, if Case 3 indeed should produce an error, check out the case where type MappedIntersection<T extends {}> = MappedRaw<T> & {}; in initial submission. Because this gets Case 3 to compile successfully.

@fatcerberus
Copy link

fatcerberus commented Aug 1, 2019

@smoogly The only fully typesafe way to relate two functions is to relate their parameters contravariantly, i.e. function A is assignable to function type B only if all of B's parameters are assignable to A's. This is because the parameters represent data that flows in the opposite direction over the =: assignment typically moves data right to left, but calling the aliased function will send the arguments left to right over that same assignment. --strictFunctionTypes enforces this for functions and lambdas, but not methods.

Methods are related bivariantly as a tradeoff, because much of the public JS API (especially the DOM API) have fundamentally unsound designs that would become very difficult to use if the "correct" variance were enforced for methods. For example, Array should be invariant to be fully typesafe, but in practice is covariant because that reflects the way idiomatic JS code is written.

@jack-williams
Copy link
Collaborator

@smoogly That compiles for the same reason case two compiles: the use of the alias enables variance measurements. The type MappedRaw is never expanded and the checker just ensures that the type arguments are related.

@smoogly
Copy link
Author

smoogly commented Aug 2, 2019

Gentlemen, thank you for setting my mind straight about assignability.
My expectation was wrong about Case 3 based on habit of use in ts@2.8.4.

@weswigham
Copy link
Member

#32225 doesn't fix this :(

@jack-williams
Copy link
Collaborator

@weswigham What do unreliable markers do? From a quick glance it seems that they have no affect and only unmeasurable is guaranteed to prevent variance measurements being used.

In the most conservative case any use of keyof should discard variance measurements because it's not an order-preserving function, but that is a drastic action to avoid a few pathological cases (usually involving any or something with a string indexer).

interface KeyOf<T> {
  key_of: keyof T;
}

const x: KeyOf<{ [x: string]: number }> = { key_of: "random" } // also works with KeyOf<any>
const y: KeyOf<{ x: number }> = x;
const xKey: "x" = y.key_of;

Aside from ignoring variance when the type parameter is instantiated with a degenerate case like any is there anything that can reasonbly be done here?

@weswigham
Copy link
Member

Unreliable is supposed to be "fallback to structural if varianced-based check fails" while Unmeasurable is "don't use the variance result at all". It was a bit of a compromise to continue to allow some things that checked with variance but not by structure, so the change would break less, iirc. In theory the only thing that would change by converting all Unreliable into Unmeasurable is performance, in practice I'm not sure that's the case.

@fatcerberus
Copy link

So Unreliable can allow things to typecheck successfully that would fail a structural check? That seems like a footgun: I would expect the structural result to be the canonical source of truth.

@weswigham
Copy link
Member

I would, too, but as it turns out a Readonly<any> isn't structurally compatible with a Readonly<{x: number}> because of how any feeds into mapped types...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

5 participants