Skip to content

Commit 8517df6

Browse files
authored
Fix erroneous optional chain narrowing (#36145)
* Not all optional chains are narrowable references * Add regression test
1 parent b2a7d42 commit 8517df6

File tree

6 files changed

+128
-8
lines changed

6 files changed

+128
-8
lines changed

src/compiler/binder.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -861,7 +861,7 @@ namespace ts {
861861
case SyntaxKind.ThisKeyword:
862862
case SyntaxKind.PropertyAccessExpression:
863863
case SyntaxKind.ElementAccessExpression:
864-
return isNarrowableReference(expr);
864+
return containsNarrowableReference(expr);
865865
case SyntaxKind.CallExpression:
866866
return hasNarrowableArgument(<CallExpression>expr);
867867
case SyntaxKind.ParenthesizedExpression:
@@ -879,20 +879,23 @@ namespace ts {
879879
function isNarrowableReference(expr: Expression): boolean {
880880
return expr.kind === SyntaxKind.Identifier || expr.kind === SyntaxKind.ThisKeyword || expr.kind === SyntaxKind.SuperKeyword ||
881881
(isPropertyAccessExpression(expr) || isNonNullExpression(expr) || isParenthesizedExpression(expr)) && isNarrowableReference(expr.expression) ||
882-
isElementAccessExpression(expr) && isStringOrNumericLiteralLike(expr.argumentExpression) && isNarrowableReference(expr.expression) ||
883-
isOptionalChain(expr);
882+
isElementAccessExpression(expr) && isStringOrNumericLiteralLike(expr.argumentExpression) && isNarrowableReference(expr.expression);
883+
}
884+
885+
function containsNarrowableReference(expr: Expression): boolean {
886+
return isNarrowableReference(expr) || isOptionalChain(expr) && containsNarrowableReference(expr.expression);
884887
}
885888

886889
function hasNarrowableArgument(expr: CallExpression) {
887890
if (expr.arguments) {
888891
for (const argument of expr.arguments) {
889-
if (isNarrowableReference(argument)) {
892+
if (containsNarrowableReference(argument)) {
890893
return true;
891894
}
892895
}
893896
}
894897
if (expr.expression.kind === SyntaxKind.PropertyAccessExpression &&
895-
isNarrowableReference((<PropertyAccessExpression>expr.expression).expression)) {
898+
containsNarrowableReference((<PropertyAccessExpression>expr.expression).expression)) {
896899
return true;
897900
}
898901
return false;
@@ -909,7 +912,7 @@ namespace ts {
909912
function isNarrowingBinaryExpression(expr: BinaryExpression) {
910913
switch (expr.operatorToken.kind) {
911914
case SyntaxKind.EqualsToken:
912-
return isNarrowableReference(expr.left);
915+
return containsNarrowableReference(expr.left);
913916
case SyntaxKind.EqualsEqualsToken:
914917
case SyntaxKind.ExclamationEqualsToken:
915918
case SyntaxKind.EqualsEqualsEqualsToken:
@@ -938,7 +941,7 @@ namespace ts {
938941
return isNarrowableOperand((<BinaryExpression>expr).right);
939942
}
940943
}
941-
return isNarrowableReference(expr);
944+
return containsNarrowableReference(expr);
942945
}
943946

944947
function createBranchLabel(): FlowLabel {

tests/baselines/reference/controlFlowOptionalChain.errors.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -764,4 +764,16 @@ tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts(567,21): error T
764764

765765
someFunction(someObject);
766766
someFunction(undefined);
767+
768+
// Repro from #35970
769+
770+
let i = 0;
771+
declare const arr: { tag: ("left" | "right") }[];
772+
773+
while (arr[i]?.tag === "left") {
774+
i += 1;
775+
if (arr[i]?.tag === "right") {
776+
console.log("I should ALSO be reachable");
777+
}
778+
}
767779

tests/baselines/reference/controlFlowOptionalChain.js

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -576,11 +576,23 @@ const someObject: SomeObject = {
576576

577577
someFunction(someObject);
578578
someFunction(undefined);
579+
580+
// Repro from #35970
581+
582+
let i = 0;
583+
declare const arr: { tag: ("left" | "right") }[];
584+
585+
while (arr[i]?.tag === "left") {
586+
i += 1;
587+
if (arr[i]?.tag === "right") {
588+
console.log("I should ALSO be reachable");
589+
}
590+
}
579591

580592

581593
//// [controlFlowOptionalChain.js]
582594
"use strict";
583-
var _a, _b, _c, _d, _e, _f, _g, _h, _j, _k, _l, _m, _o, _p, _q, _r, _s, _t;
595+
var _a, _b, _c, _d, _e, _f, _g, _h, _j, _k, _l, _m, _o, _p, _q, _r, _s, _t, _u, _v;
584596
var a;
585597
o === null || o === void 0 ? void 0 : o[a = 1];
586598
a.toString();
@@ -1071,3 +1083,11 @@ var someObject = {
10711083
};
10721084
someFunction(someObject);
10731085
someFunction(undefined);
1086+
// Repro from #35970
1087+
var i = 0;
1088+
while (((_u = arr[i]) === null || _u === void 0 ? void 0 : _u.tag) === "left") {
1089+
i += 1;
1090+
if (((_v = arr[i]) === null || _v === void 0 ? void 0 : _v.tag) === "right") {
1091+
console.log("I should ALSO be reachable");
1092+
}
1093+
}

tests/baselines/reference/controlFlowOptionalChain.symbols

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1801,3 +1801,34 @@ someFunction(undefined);
18011801
>someFunction : Symbol(someFunction, Decl(controlFlowOptionalChain.ts, 561, 42))
18021802
>undefined : Symbol(undefined)
18031803

1804+
// Repro from #35970
1805+
1806+
let i = 0;
1807+
>i : Symbol(i, Decl(controlFlowOptionalChain.ts, 580, 3))
1808+
1809+
declare const arr: { tag: ("left" | "right") }[];
1810+
>arr : Symbol(arr, Decl(controlFlowOptionalChain.ts, 581, 13))
1811+
>tag : Symbol(tag, Decl(controlFlowOptionalChain.ts, 581, 20))
1812+
1813+
while (arr[i]?.tag === "left") {
1814+
>arr[i]?.tag : Symbol(tag, Decl(controlFlowOptionalChain.ts, 581, 20))
1815+
>arr : Symbol(arr, Decl(controlFlowOptionalChain.ts, 581, 13))
1816+
>i : Symbol(i, Decl(controlFlowOptionalChain.ts, 580, 3))
1817+
>tag : Symbol(tag, Decl(controlFlowOptionalChain.ts, 581, 20))
1818+
1819+
i += 1;
1820+
>i : Symbol(i, Decl(controlFlowOptionalChain.ts, 580, 3))
1821+
1822+
if (arr[i]?.tag === "right") {
1823+
>arr[i]?.tag : Symbol(tag, Decl(controlFlowOptionalChain.ts, 581, 20))
1824+
>arr : Symbol(arr, Decl(controlFlowOptionalChain.ts, 581, 13))
1825+
>i : Symbol(i, Decl(controlFlowOptionalChain.ts, 580, 3))
1826+
>tag : Symbol(tag, Decl(controlFlowOptionalChain.ts, 581, 20))
1827+
1828+
console.log("I should ALSO be reachable");
1829+
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
1830+
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
1831+
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
1832+
}
1833+
}
1834+

tests/baselines/reference/controlFlowOptionalChain.types

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2031,3 +2031,45 @@ someFunction(undefined);
20312031
>someFunction : (someOptionalObject: SomeObject | undefined) => void
20322032
>undefined : undefined
20332033

2034+
// Repro from #35970
2035+
2036+
let i = 0;
2037+
>i : number
2038+
>0 : 0
2039+
2040+
declare const arr: { tag: ("left" | "right") }[];
2041+
>arr : { tag: "left" | "right"; }[]
2042+
>tag : "left" | "right"
2043+
2044+
while (arr[i]?.tag === "left") {
2045+
>arr[i]?.tag === "left" : boolean
2046+
>arr[i]?.tag : "left" | "right"
2047+
>arr[i] : { tag: "left" | "right"; }
2048+
>arr : { tag: "left" | "right"; }[]
2049+
>i : number
2050+
>tag : "left" | "right"
2051+
>"left" : "left"
2052+
2053+
i += 1;
2054+
>i += 1 : number
2055+
>i : number
2056+
>1 : 1
2057+
2058+
if (arr[i]?.tag === "right") {
2059+
>arr[i]?.tag === "right" : boolean
2060+
>arr[i]?.tag : "left" | "right"
2061+
>arr[i] : { tag: "left" | "right"; }
2062+
>arr : { tag: "left" | "right"; }[]
2063+
>i : number
2064+
>tag : "left" | "right"
2065+
>"right" : "right"
2066+
2067+
console.log("I should ALSO be reachable");
2068+
>console.log("I should ALSO be reachable") : void
2069+
>console.log : (message?: any, ...optionalParams: any[]) => void
2070+
>console : Console
2071+
>log : (message?: any, ...optionalParams: any[]) => void
2072+
>"I should ALSO be reachable" : "I should ALSO be reachable"
2073+
}
2074+
}
2075+

tests/cases/conformance/controlFlow/controlFlowOptionalChain.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -578,3 +578,15 @@ const someObject: SomeObject = {
578578

579579
someFunction(someObject);
580580
someFunction(undefined);
581+
582+
// Repro from #35970
583+
584+
let i = 0;
585+
declare const arr: { tag: ("left" | "right") }[];
586+
587+
while (arr[i]?.tag === "left") {
588+
i += 1;
589+
if (arr[i]?.tag === "right") {
590+
console.log("I should ALSO be reachable");
591+
}
592+
}

0 commit comments

Comments
 (0)