Skip to content

Generic lookup type wrongly narrowed to never with strictNullChecks #26130

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

Closed
mattmccutchen opened this issue Aug 1, 2018 · 5 comments
Closed
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@mattmccutchen
Copy link
Contributor

I ran across this while migrating my project to strictNullChecks and it's positively bizarre.

TypeScript Version: master (50f442f)

Search Terms: mapped type narrow narrowing null strictNullChecks never

Code (with strictNullChecks)

let mappedObject: {[K in "foo"]: null | {x: string}} = {foo: {x: "hello"}};
declare function foo<T>(x: T): null | T;

function bar<K extends "foo">(key: K) {
  const element = foo(mappedObject[key]);
  if (element == null)
    return;
  const x = element.x;
}

Expected behavior: Compile without error.

Actual behavior: Error: Property 'x' does not exist on type 'never'.

Playground Link: link (remember to enable strictNullChecks)

Related Issues: Didn't find any.

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Aug 1, 2018
@RyanCavanaugh
Copy link
Member

This is just weird. Writing

const x = element!.x;

eliminates the error, but there shouldn't be anything for ! to take away

@RyanCavanaugh
Copy link
Member

Not a regression FWIW

@weswigham
Copy link
Member

weswigham commented Aug 2, 2018

This is just weird. Writing
const x = element!.x;
eliminates the error, but there shouldn't be anything for ! to take away

FWIW !, as a cast, also resets a control flow-narrowed type to its declared type. We have an open issue to make it respect narrowing: #16945 (I was looking at it recently)

@mattmccutchen
Copy link
Contributor Author

I found the problem: in getAssignmentReducedType, we have a declared type of {x: string} | null and an assigned type of {foo: {x: string} | null}[K] | null. When we ask if the assigned type is maybe assignable to {x: string}, we decompose the first level of union and check if {foo: {x: string} | null}[K] is assignable to {x: string}, but it isn't because its constraint is {x: string} | null. So we throw {x: string} out of the declared type and are left with null, and after the null check, we are left with never.

Using the comparable relation in place of typeMaybeAssignableTo fixes this issue. I'm going to see what else breaks if I do that.

mattmccutchen added a commit to mattmccutchen/TypeScript that referenced this issue Aug 2, 2018
typeMaybeAssignableTo.

typeMaybeAssignableTo decomposed unions at the top level of the assigned
type but didn't properly handle other unions that arose during
assignability checking, e.g., in the constraint of a generic lookup
type.

Fixes microsoft#26130.
@mattmccutchen
Copy link
Contributor Author

Proposed fix is in #26143.

@ahejlsberg ahejlsberg added the Fixed A PR has been merged for this issue label Aug 9, 2018
mattmccutchen added a commit to mattmccutchen/TypeScript that referenced this issue Aug 13, 2018
types.

Fixes microsoft#26405.

Also add a debug assert to catch any future cases like microsoft#26130 in which
the target type of a valid assignment is narrowed to something to which
the source type is no longer assignable.
mattmccutchen added a commit to mattmccutchen/TypeScript that referenced this issue Aug 18, 2018
…microsoft#26130 by

skipping narrowing if the old algorithm produces a type to which the
assigned type is not assignable.

This also means we'll no longer narrow for erroneous assignments where
the assigned type is not assignable to the declared type.  This is the
reason for the numericLiteralTypes3 baseline change.

Fixes microsoft#26405.
ahejlsberg added a commit that referenced this issue Aug 20, 2018
Go back to old narrowing algorithm but don't narrow in cases like #26130
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

No branches or pull requests

4 participants