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

Generic literal marker not obeyed for unknown and {} #32169

Closed
evmar opened this issue Jun 28, 2019 · 6 comments · Fixed by #32260
Closed

Generic literal marker not obeyed for unknown and {} #32169

evmar opened this issue Jun 28, 2019 · 6 comments · Fixed by #32260
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@evmar
Copy link
Contributor

evmar commented Jun 28, 2019

TypeScript Version: 3.4 up to 3.6-dev20190628

Search Terms:
generic literal unknown

Code

This is expected:

function f<T>(x: T): T { return x; }
enum E { A, B }
let x = f(E.A);  // x has type E, not E.A

According to #24919 (comment) , you can get this to produce an E.A if you add an extends marker:

function f<T extends {}|null|undefined>(x: T): T { return x; }
enum E { A, B }
let x = f(E.A);  // x now has type E.A

Expected behavior:

I'd expect the same literal inference if you write these forms too:

function f<T extends unknown>(x: T): T { return x; }
function f<T extends {}>(x: T): T { return x; }

Actual behavior:

It has the other behavior, as if you have no extends clause

Playground Link:

http://www.typescriptlang.org/play/#code/FAUwdgrgtgBAojA3jAggGhgIRgX2MAMwjAGMAXASwHswYCBGAHgBUA+ACgA8AuGZgSl7MkMAE4gyEUbU4BuXMAA2EmJ3owAvHXrs4AOhT95MAPQnVMABYBDAM4wyATwAOIePiKlKNOgCYWMCCcZOAAJvaIOAA+kIqKUcShIAQUYCChHDx8gnwi4pLSqvJ4ymSqvpp+ugZGMKbmnFZ2Di5u+igexOTUtAQAzAFBIWDhMMQA1mBUAO5gmUI5wsj5UjLFSiqcfZX91YbGZhY29k6u8DAA-BcAhEA

Related Issues:

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jul 2, 2019
@ahejlsberg
Copy link
Member

The exact rules we use for widening inference candidates are a bit more complex (see getCovariantInference function in the checker for implementation). We widen inferred literal types if:

  • all inferences were made to top-level occurrences of the type parameter, and
  • the type parameter has no constraint or its constraint includes no primitive or literal types, and
  • the type parameter was fixed during inference or does not occur at top-level in the return type.

Primitive types are string, number, bigint, boolean, symbol, void, undefined, null, and all literal and unique symbol types. The intent of the rules is to not widen when there is some indication in the constraint that a literal type is wanted. For example, a constraint number is a pretty clear indication that we want something that is a subtype of number, i.e. a numeric literal type.

I'm not sure I see a compelling reason to change these rules, in particular since it would likely break code that depends on the current behavior.

@RyanCavanaugh
Copy link
Member

@ahejlsberg why isn't e.g. widening to number gated on the number primitive type (or a numeric literal type) appearing in the union?

At a minimum, I would expect the non-values (null, undefined, void) to not have any influence on literal widening.

@evmar
Copy link
Contributor Author

evmar commented Jul 3, 2019

The specific code this breaks for us is:

function assert<T extends {}|undefined|null>(x: T): NonNullable<T> { ... }

This function has the important property that it can be inserted around any expression to get the same result type out:

enum E { A, B }
const x = assert(E.A);  // type E.A

However, you cannot pass unknown to this function as written, and changing the param to T extends unknown or plain T means it now widens.

(You might wonder why there's any code that ever assert()s on an unknown, because the return type isn't useful. But it still is useful because it throws on null.)

This came up in particular in TS3.5, because we are seeing a lot more code that ends up passing around unknowns, due to the default constraint change (#30637).

@ahejlsberg
Copy link
Member

So, first of all, in the original example

function f<T>(x: T): T { return x; }
enum E { A, B }
let x = f(E.A);  // x has type E, not E.A

the reason for the widened type is that x is a mutable location. If you declare with const we don't widen. The reason we don't widen is because T occurs at top-level in the return type, as per the rules above.

However, when the example is changed to

declare function f<T>(x: T): NonNullable<T>;

we start widening because we don't consider the occurrence of T in the return type to be top-level. From #10676, the original PR for literal types

  • An occurrence of a type X within a type S is said to be top-level if S is X or if S is a union or intersection type and X occurs at top-level within S.

We never updated this rule to account for conditional types. I think we should augment the definition to

  • An occurrence of a type X within a type S is said to be top-level if S is X or if S is a union or intersection type and X occurs at top-level within S, or if S is a conditional type and X occurs at top-level within the true or false branch of S.

That should fix the specific code that is breaking (because the type parameter constraint doesn't even matter then).

@ahejlsberg
Copy link
Member

ahejlsberg commented Jul 4, 2019

why isn't e.g. widening to number gated on the number primitive type (or a numeric literal type) appearing in the union?

In general, if we're going to keep literal types we should keep them for all types, not just some types. And if the constraint is number it doesn't really matter if we keep literal strings because they'll fail the constraint check anyways.

You could argue that we shouldn't consider null or undefined in the constraint a reason to keep literal types (i.e. to not widen), but I'm not super eager to mess with these rules and have something break because of it, and it is unrelated to the real fix for the problem in this issue.

@ahejlsberg ahejlsberg added Bug A bug in TypeScript and removed Needs Investigation This issue needs a team member to investigate its status. labels Jul 5, 2019
@ahejlsberg
Copy link
Member

Changing this to be a bug, the bug being that the check for top-level occurrences of a type parameter should be augmented to handle conditional types as I describe above.

@ahejlsberg ahejlsberg added this to the TypeScript 3.6.0 milestone Jul 5, 2019
@ahejlsberg ahejlsberg added the Fixed A PR has been merged for this issue label Jul 9, 2019
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

Successfully merging a pull request may close this issue.

3 participants