Skip to content

Fix discriminant property narrowing through optional chain with null #42503

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 29, 2021
Merged

Fix discriminant property narrowing through optional chain with null #42503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
19e06f9
Fix discriminant property narrowing through optional chain with null
andrewbranch Jan 26, 2021
8ca874a
Accept baselines
andrewbranch Jan 26, 2021
5896760
Add tests from Anders
andrewbranch Jan 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22624,13 +22624,13 @@ namespace ts {
if (propName === undefined) {
return type;
}
const includesUndefined = strictNullChecks && maybeTypeOfKind(type, TypeFlags.Undefined);
const removeOptional = includesUndefined && isOptionalChain(access);
let propType = getTypeOfPropertyOfType(removeOptional ? getTypeWithFacts(type, TypeFacts.NEUndefined) : type, propName);
const includesNullable = strictNullChecks && maybeTypeOfKind(type, TypeFlags.Nullable);
const removeNullable = includesNullable && isOptionalChain(access);
let propType = getTypeOfPropertyOfType(removeNullable ? getTypeWithFacts(type, TypeFacts.NEUndefinedOrNull) : type, propName);
if (!propType) {
return type;
}
propType = removeOptional ? getOptionalType(propType) : propType;
propType = removeNullable ? getOptionalType(propType) : propType;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional chaining leverages an optionalType marker type to indicate the type of an expression came from a ?.. The optionalType marker is essentially undefined, but its presence has meaning to completions as well as within the checker itself. The code here (and from #42450), removes undefined (regardless of source), then mixes back in undefinedType. In functions like checkPropertyAccessChain, I use the function propagateOptionalTypeMarker to restore optionalType to the result if it was found in the source type to carry that marker forward.

I'm not certain yet whether this (or #42450) will cause problems as a result of not restoring the optionalType marker, as I need to consider if there is a test case that might trigger unwanted behavior.

const narrowedPropType = narrowType(propType);
return filterType(type, t => {
const discriminantType = getTypeOfPropertyOrIndexSignature(t, propName);
Expand Down
158 changes: 158 additions & 0 deletions tests/baselines/reference/controlFlowOptionalChain2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,92 @@ function funcTwo(arg: A | B | undefined) {
arg;
arg?.name;
}

function funcThree(arg: A | B | null) {
if (arg?.type === 'B') {
arg; // `B`
return;
}

arg;
arg?.name;
}

type U = { kind: undefined, u: 'u' }
type N = { kind: null, n: 'n' }
type X = { kind: 'X', x: 'x' }

function f1(x: X | U | undefined) {
if (x?.kind === undefined) {
x; // U | undefined
}
else {
x; // X
}
}

function f2(x: X | N | undefined) {
if (x?.kind === undefined) {
x; // undefined
}
else {
x; // X | N
}
}

function f3(x: X | U | null) {
if (x?.kind === undefined) {
x; // U | null
}
else {
x; // X
}
}

function f4(x: X | N | null) {
if (x?.kind === undefined) {
x; // null
}
else {
x; // X | N
}
}

function f5(x: X | U | undefined) {
if (x?.kind === null) {
x; // never
}
else {
x; // X | U | undefined
}
}

function f6(x: X | N | undefined) {
if (x?.kind === null) {
x; // N
}
else {
x; // X | undefined
}
}

function f7(x: X | U | null) {
if (x?.kind === null) {
x; // never
}
else {
x; // X | U | null
}
}

function f8(x: X | N | null) {
if (x?.kind === null) {
x; // N
}
else {
x; // X | null
}
}


//// [controlFlowOptionalChain2.js]
Expand All @@ -28,3 +114,75 @@ function funcTwo(arg) {
arg;
arg === null || arg === void 0 ? void 0 : arg.name;
}
function funcThree(arg) {
if ((arg === null || arg === void 0 ? void 0 : arg.type) === 'B') {
arg; // `B`
return;
}
arg;
arg === null || arg === void 0 ? void 0 : arg.name;
}
function f1(x) {
if ((x === null || x === void 0 ? void 0 : x.kind) === undefined) {
x; // U | undefined
}
else {
x; // X
}
}
function f2(x) {
if ((x === null || x === void 0 ? void 0 : x.kind) === undefined) {
x; // undefined
}
else {
x; // X | N
}
}
function f3(x) {
if ((x === null || x === void 0 ? void 0 : x.kind) === undefined) {
x; // U | null
}
else {
x; // X
}
}
function f4(x) {
if ((x === null || x === void 0 ? void 0 : x.kind) === undefined) {
x; // null
}
else {
x; // X | N
}
}
function f5(x) {
if ((x === null || x === void 0 ? void 0 : x.kind) === null) {
x; // never
}
else {
x; // X | U | undefined
}
}
function f6(x) {
if ((x === null || x === void 0 ? void 0 : x.kind) === null) {
x; // N
}
else {
x; // X | undefined
}
}
function f7(x) {
if ((x === null || x === void 0 ? void 0 : x.kind) === null) {
x; // never
}
else {
x; // X | U | null
}
}
function f8(x) {
if ((x === null || x === void 0 ? void 0 : x.kind) === null) {
x; // N
}
else {
x; // X | null
}
}
205 changes: 205 additions & 0 deletions tests/baselines/reference/controlFlowOptionalChain2.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,208 @@ function funcTwo(arg: A | B | undefined) {
>name : Symbol(name, Decl(controlFlowOptionalChain2.ts, 1, 12))
}

function funcThree(arg: A | B | null) {
>funcThree : Symbol(funcThree, Decl(controlFlowOptionalChain2.ts, 17, 1))
>arg : Symbol(arg, Decl(controlFlowOptionalChain2.ts, 19, 19))
>A : Symbol(A, Decl(controlFlowOptionalChain2.ts, 0, 0))
>B : Symbol(B, Decl(controlFlowOptionalChain2.ts, 3, 1))

if (arg?.type === 'B') {
>arg?.type : Symbol(type, Decl(controlFlowOptionalChain2.ts, 0, 10), Decl(controlFlowOptionalChain2.ts, 5, 10))
>arg : Symbol(arg, Decl(controlFlowOptionalChain2.ts, 19, 19))
>type : Symbol(type, Decl(controlFlowOptionalChain2.ts, 0, 10), Decl(controlFlowOptionalChain2.ts, 5, 10))

arg; // `B`
>arg : Symbol(arg, Decl(controlFlowOptionalChain2.ts, 19, 19))

return;
}

arg;
>arg : Symbol(arg, Decl(controlFlowOptionalChain2.ts, 19, 19))

arg?.name;
>arg?.name : Symbol(name, Decl(controlFlowOptionalChain2.ts, 1, 12))
>arg : Symbol(arg, Decl(controlFlowOptionalChain2.ts, 19, 19))
>name : Symbol(name, Decl(controlFlowOptionalChain2.ts, 1, 12))
}

type U = { kind: undefined, u: 'u' }
>U : Symbol(U, Decl(controlFlowOptionalChain2.ts, 27, 1))
>kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 29, 10))
>u : Symbol(u, Decl(controlFlowOptionalChain2.ts, 29, 27))

type N = { kind: null, n: 'n' }
>N : Symbol(N, Decl(controlFlowOptionalChain2.ts, 29, 36))
>kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 30, 10))
>n : Symbol(n, Decl(controlFlowOptionalChain2.ts, 30, 22))

type X = { kind: 'X', x: 'x' }
>X : Symbol(X, Decl(controlFlowOptionalChain2.ts, 30, 31))
>kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 31, 10))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 31, 21))

function f1(x: X | U | undefined) {
>f1 : Symbol(f1, Decl(controlFlowOptionalChain2.ts, 31, 30))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 33, 12))
>X : Symbol(X, Decl(controlFlowOptionalChain2.ts, 30, 31))
>U : Symbol(U, Decl(controlFlowOptionalChain2.ts, 27, 1))

if (x?.kind === undefined) {
>x?.kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 29, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 33, 12))
>kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 29, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))
>undefined : Symbol(undefined)

x; // U | undefined
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 33, 12))
}
else {
x; // X
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 33, 12))
}
}

function f2(x: X | N | undefined) {
>f2 : Symbol(f2, Decl(controlFlowOptionalChain2.ts, 40, 1))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 42, 12))
>X : Symbol(X, Decl(controlFlowOptionalChain2.ts, 30, 31))
>N : Symbol(N, Decl(controlFlowOptionalChain2.ts, 29, 36))

if (x?.kind === undefined) {
>x?.kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 30, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 42, 12))
>kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 30, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))
>undefined : Symbol(undefined)

x; // undefined
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 42, 12))
}
else {
x; // X | N
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 42, 12))
}
}

function f3(x: X | U | null) {
>f3 : Symbol(f3, Decl(controlFlowOptionalChain2.ts, 49, 1))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 51, 12))
>X : Symbol(X, Decl(controlFlowOptionalChain2.ts, 30, 31))
>U : Symbol(U, Decl(controlFlowOptionalChain2.ts, 27, 1))

if (x?.kind === undefined) {
>x?.kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 29, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 51, 12))
>kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 29, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))
>undefined : Symbol(undefined)

x; // U | null
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 51, 12))
}
else {
x; // X
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 51, 12))
}
}

function f4(x: X | N | null) {
>f4 : Symbol(f4, Decl(controlFlowOptionalChain2.ts, 58, 1))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 60, 12))
>X : Symbol(X, Decl(controlFlowOptionalChain2.ts, 30, 31))
>N : Symbol(N, Decl(controlFlowOptionalChain2.ts, 29, 36))

if (x?.kind === undefined) {
>x?.kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 30, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 60, 12))
>kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 30, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))
>undefined : Symbol(undefined)

x; // null
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 60, 12))
}
else {
x; // X | N
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 60, 12))
}
}

function f5(x: X | U | undefined) {
>f5 : Symbol(f5, Decl(controlFlowOptionalChain2.ts, 67, 1))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 69, 12))
>X : Symbol(X, Decl(controlFlowOptionalChain2.ts, 30, 31))
>U : Symbol(U, Decl(controlFlowOptionalChain2.ts, 27, 1))

if (x?.kind === null) {
>x?.kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 29, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 69, 12))
>kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 29, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))

x; // never
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 69, 12))
}
else {
x; // X | U | undefined
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 69, 12))
}
}

function f6(x: X | N | undefined) {
>f6 : Symbol(f6, Decl(controlFlowOptionalChain2.ts, 76, 1))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 78, 12))
>X : Symbol(X, Decl(controlFlowOptionalChain2.ts, 30, 31))
>N : Symbol(N, Decl(controlFlowOptionalChain2.ts, 29, 36))

if (x?.kind === null) {
>x?.kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 30, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 78, 12))
>kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 30, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))

x; // N
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 78, 12))
}
else {
x; // X | undefined
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 78, 12))
}
}

function f7(x: X | U | null) {
>f7 : Symbol(f7, Decl(controlFlowOptionalChain2.ts, 85, 1))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 87, 12))
>X : Symbol(X, Decl(controlFlowOptionalChain2.ts, 30, 31))
>U : Symbol(U, Decl(controlFlowOptionalChain2.ts, 27, 1))

if (x?.kind === null) {
>x?.kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 29, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 87, 12))
>kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 29, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))

x; // never
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 87, 12))
}
else {
x; // X | U | null
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 87, 12))
}
}

function f8(x: X | N | null) {
>f8 : Symbol(f8, Decl(controlFlowOptionalChain2.ts, 94, 1))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 96, 12))
>X : Symbol(X, Decl(controlFlowOptionalChain2.ts, 30, 31))
>N : Symbol(N, Decl(controlFlowOptionalChain2.ts, 29, 36))

if (x?.kind === null) {
>x?.kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 30, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 96, 12))
>kind : Symbol(kind, Decl(controlFlowOptionalChain2.ts, 30, 10), Decl(controlFlowOptionalChain2.ts, 31, 10))

x; // N
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 96, 12))
}
else {
x; // X | null
>x : Symbol(x, Decl(controlFlowOptionalChain2.ts, 96, 12))
}
}

Loading