Infer contextual types from generic return types

[ahejlsberg]

When a generic method has one or more type parameters that appear in both parameter and return types and that method is called in a context where a contextual type exists, we now use inferences from the contextual type to the return type to produce contextual types for the argument expressions. Some examples:

type Box<T> = { value: T };

function box<T>(value: T): Box<T> {
  return { value }
}

let boxed: Box<'win' | 'draw'> = box('draw');  // Ok

let kinds: { kind: 'odd' | 'even' }[];
kinds = [0, 1, 2, 3].map(x => ({ kind: x % 2 ? 'odd' : 'even' }));  // Ok

let p: Promise<{ kind: 'odd' | 'even' }[]>;
p = Promise.all([{ kind: 'even' }, { kind: 'odd' }]);  // Ok

Previously, each of the examples above would error because, lacking a contextual type, the literals were widened to type string. With this PR, contextual types for the literals in the argument expressions are properly inferred from the intended return types.

Fixes #5487 and #11152 (and numerous duplicate issues).

[weswigham]

This mapType call should probably have it's third argument set since we're in contextual typing and preserving string | "x"'s literaliness is desirable.

[weswigham]

Also: I'm wondering why bother with the map type/instantiate flag check at all - instantiateType is only going to instantiate instantiable types anyway.

[jack-williams]

If you change the mapType I think you can go ahead and close #29174.

(or ping me and I’ll close it)

[ahejlsberg]

@weswigham Agreed, we should make sure we use UnionReduction.None. The reason we don't just call instantiateType is that getContextualMapper isn't a super cheap operation and also that we don't want to instantiate contained object types as it is unnecessary and can be expensive.

@jack-williams I'll fix it so we don't need #29174.

[jack-williams]

@ahejlsberg Grand, closing the PR. Might be worth adding the example from #29168 to this PR, just to make sure it’s fixed.

[weswigham]

Why do you need to strip literal freshness here? Minimally that'd deserve a comment, IMO.

[cyrilletuzi]

Thanks for this PR. 👍 Do we agree it fixes #29168?

As a reminder, #29168 is a regression, causing existing valid code to break at compilation. It's already quite difficult for me to understand that regressions are not fixed in the current branch in a project like TypeScript, so I would greatly appreciate that things are managed in a way so it's fixed in TS 3.3. I'm quite worried to see 3.3 planned for January. Time is needed after the PR is merged to check if the issue is solved.

[jack-williams]

constains -> contains

[ahejlsberg]

Do we agree it fixes #29168?

Yes, with the latest commits this now fixes #29168.

[RyanCavanaugh]

@ahejlsberg is this practical to port to the 3.3 branch?

[RyanCavanaugh]

Merge Friday pending any last concerns

[RyanCavanaugh]

@typescript-bot test this

[typescript-bot]

Heya @RyanCavanaugh, I've started to run the extended test suite on this PR at 2ea0251. You can monitor the build here. It should now contribute to this PR's status checks.

[ahejlsberg]

@typescript-bot test this again

[typescript-bot]

Heya @ahejlsberg, I've started to run the extended test suite on this PR at 7c1bb14. You can monitor the build here. It should now contribute to this PR's status checks.

[ahejlsberg]

@RyanCavanaugh There are two new errors in the RWC baselines related to this PR. I have condensed them down to simple repros below.

First error:

declare function foldLeft<U>(z: U, f: (acc: U, t: boolean) => U): U;
let result: boolean = foldLeft(true, (acc, t) => acc && t);  // Error

This errors with Type 'boolean' is not assignable to type 'true'. Previously we would infer boolean for U because there was no contextual type for z. We now infer true (because the literal is contextually typed by the return type) which then gets fixed as we contextually type the acc parameter.

This issue is basically the opposite of the issue we currently have with foldLeft-like functions where the seed gets widened because there is no contextual type.

Second error:

enum State { A, B }
type Foo = { state: State }
declare function bar<T>(f: () => T[]): T[];
let x: Foo[] = bar(() => !!true ? [{ state: State.A }] : [{ state: State.B }]);  // Error

This errors with Type '{ state: State.A; }[] | { state: State.B; }[]' is not assignable to type '{ state: State.A; }[]'. Previously we'd infer State for state in each of the object literals. We now infer literal types and because the two array literals have no best common supertype we end up picking the first one.

I'm not sure there's much we can do in the PR to avoid the errors. Any further tweaks to inference are likely just going to produce more noise. I think it comes down to deciding whether we think they're acceptable breaks. I personally think the fixed scenarios outweigh the two new errors. That said, we may want to wait for 3.4, in which case we should revive the fix for #29168.

[RyanCavanaugh]

I agree the RWC breaks are acceptable

[RyanCavanaugh]

LGTM - would be best to add the RWC reductions as testcases too

[weswigham]

I agree the RWC breaks are acceptable

We should have a Breaking Changes entry for this ready and have a (general) workaround at the ready to tell people, since I'm sure we'll receive issue reports about this.