Skip to content

Commit 32934a9

Browse files
rbucktonrbuckton
authored
Merge pull request #39824 from microsoft/fix35484
Allow assignments to a narrowable reference to be considered narrowable
2 parents 7119e2b + 315b5f4 commit 32934a9

File tree

7 files changed

+121
-24
lines changed

7 files changed

+121
-24
lines changed

‎src/compiler/binder.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -837,7 +837,8 @@ namespace ts {
837837
function isNarrowableReference(expr: Expression): boolean {
838838
return expr.kind === SyntaxKind.Identifier || expr.kind === SyntaxKind.ThisKeyword || expr.kind === SyntaxKind.SuperKeyword ||
839839
(isPropertyAccessExpression(expr) || isNonNullExpression(expr) || isParenthesizedExpression(expr)) && isNarrowableReference(expr.expression) ||
840-
isElementAccessExpression(expr) && isStringOrNumericLiteralLike(expr.argumentExpression) && isNarrowableReference(expr.expression);
840+
isElementAccessExpression(expr) && isStringOrNumericLiteralLike(expr.argumentExpression) && isNarrowableReference(expr.expression) ||
841+
isAssignmentExpression(expr) && isNarrowableReference(expr.left);
841842
}
842843

843844
function containsNarrowableReference(expr: Expression): boolean {

‎src/compiler/checker.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20141,6 +20141,8 @@ namespace ts {
2014120141
case SyntaxKind.ParenthesizedExpression:
2014220142
case SyntaxKind.NonNullExpression:
2014320143
return isMatchingReference(source, (target as NonNullExpression | ParenthesizedExpression).expression);
20144+
case SyntaxKind.BinaryExpression:
20145+
return isAssignmentExpression(target) && isMatchingReference(source, target.left);
2014420146
}
2014520147
switch (source.kind) {
2014620148
case SyntaxKind.Identifier:

‎src/debug/dbg.ts

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ namespace Debug {
1111
type MethodDeclaration = Node;
1212
type Expression = Node;
1313
type SourceFile = Node;
14+
type VariableDeclaration = Node;
15+
type BindingElement = Node;
16+
type CallExpression = Node;
17+
type BinaryExpression = Node;
1418

1519
interface SwitchStatement extends Node {
1620
caseBlock: CaseBlock;
@@ -59,8 +63,6 @@ namespace Debug {
5963
}
6064

6165
type FlowNode =
62-
| AfterFinallyFlow
63-
| PreFinallyFlow
6466
| FlowStart
6567
| FlowLabel
6668
| FlowAssignment
@@ -76,14 +78,6 @@ namespace Debug {
7678
id?: number;
7779
}
7880

79-
interface AfterFinallyFlow extends FlowNodeBase {
80-
antecedent: FlowNode;
81-
}
82-
83-
interface PreFinallyFlow extends FlowNodeBase {
84-
antecedent: FlowNode;
85-
}
86-
8781
interface FlowStart extends FlowNodeBase {
8882
node?: FunctionExpression | ArrowFunction | MethodDeclaration;
8983
}
@@ -93,12 +87,12 @@ namespace Debug {
9387
}
9488

9589
interface FlowAssignment extends FlowNodeBase {
96-
node: Expression;
90+
node: Expression | VariableDeclaration | BindingElement;
9791
antecedent: FlowNode;
9892
}
9993

10094
interface FlowCall extends FlowNodeBase {
101-
node: Expression;
95+
node: CallExpression;
10296
antecedent: FlowNode;
10397
}
10498

@@ -115,7 +109,7 @@ namespace Debug {
115109
}
116110

117111
interface FlowArrayMutation extends FlowNodeBase {
118-
node: Expression;
112+
node: CallExpression | BinaryExpression;
119113
antecedent: FlowNode;
120114
}
121115

@@ -192,6 +186,7 @@ namespace Debug {
192186
lane: number;
193187
endLane: number;
194188
level: number;
189+
circular: boolean | "circularity";
195190
}
196191

197192
interface FlowGraphEdge {
@@ -217,8 +212,9 @@ namespace Debug {
217212
const links: Record<number, FlowGraphNode> = Object.create(/*o*/ null); // eslint-disable-line no-null/no-null
218213
const nodes: FlowGraphNode[] = [];
219214
const edges: FlowGraphEdge[] = [];
220-
const root = buildGraphNode(flowNode);
215+
const root = buildGraphNode(flowNode, new Set());
221216
for (const node of nodes) {
217+
node.text = renderFlowNode(node.flowNode, node.circular);
222218
computeLevel(node);
223219
}
224220

@@ -263,26 +259,43 @@ namespace Debug {
263259
return parents;
264260
}
265261

266-
function buildGraphNode(flowNode: FlowNode) {
262+
function buildGraphNode(flowNode: FlowNode, seen: Set<FlowNode>): FlowGraphNode {
267263
const id = getDebugFlowNodeId(flowNode);
268264
let graphNode = links[id];
265+
if (graphNode && seen.has(flowNode)) {
266+
graphNode.circular = true;
267+
graphNode = {
268+
id: -1,
269+
flowNode,
270+
edges: [],
271+
text: "",
272+
lane: -1,
273+
endLane: -1,
274+
level: -1,
275+
circular: "circularity"
276+
};
277+
nodes.push(graphNode);
278+
return graphNode;
279+
}
280+
seen.add(flowNode);
269281
if (!graphNode) {
270-
links[id] = graphNode = { id, flowNode, edges: [], text: renderFlowNode(flowNode), lane: -1, endLane: -1, level: -1 };
282+
links[id] = graphNode = { id, flowNode, edges: [], text: "", lane: -1, endLane: -1, level: -1, circular: false };
271283
nodes.push(graphNode);
272284
if (hasAntecedents(flowNode)) {
273285
for (const antecedent of flowNode.antecedents) {
274-
buildGraphEdge(graphNode, antecedent);
286+
buildGraphEdge(graphNode, antecedent, seen);
275287
}
276288
}
277289
else if (hasAntecedent(flowNode)) {
278-
buildGraphEdge(graphNode, flowNode.antecedent);
290+
buildGraphEdge(graphNode, flowNode.antecedent, seen);
279291
}
280292
}
293+
seen.delete(flowNode);
281294
return graphNode;
282295
}
283296

284-
function buildGraphEdge(source: FlowGraphNode, antecedent: FlowNode) {
285-
const target = buildGraphNode(antecedent);
297+
function buildGraphEdge(source: FlowGraphNode, antecedent: FlowNode, seen: Set<FlowNode>) {
298+
const target = buildGraphNode(antecedent, seen);
286299
const edge: FlowGraphEdge = { source, target };
287300
edges.push(edge);
288301
source.edges.push(edge);
@@ -353,8 +366,11 @@ namespace Debug {
353366
return getSourceTextOfNodeFromSourceFile(sourceFile, node, /*includeTrivia*/ false);
354367
}
355368

356-
function renderFlowNode(flowNode: FlowNode) {
369+
function renderFlowNode(flowNode: FlowNode, circular: boolean | "circularity") {
357370
let text = getHeader(flowNode.flags);
371+
if (circular) {
372+
text = `${text}#${getDebugFlowNodeId(flowNode)}`;
373+
}
358374
if (hasNode(flowNode)) {
359375
if (flowNode.node) {
360376
text += ` (${getNodeText(flowNode.node)})`;
@@ -373,7 +389,7 @@ namespace Debug {
373389
}
374390
text += ` (${clauses.join(", ")})`;
375391
}
376-
return text;
392+
return circular === "circularity" ? `Circular(${text})` : text;
377393
}
378394

379395
function renderGraph() {

‎tests/baselines/reference/controlFlowAssignmentExpression.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,14 @@ x; // number
99
x = true;
1010
(x = "", obj).foo = (x = x.length);
1111
x; // number
12-
12+
13+
// https://github.com/microsoft/TypeScript/issues/35484
14+
type D = { done: true, value: 1 } | { done: false, value: 2 };
15+
declare function fn(): D;
16+
let o: D;
17+
if ((o = fn()).done) {
18+
const y: 1 = o.value;
19+
}
1320

1421
//// [controlFlowAssignmentExpression.js]
1522
var x;
@@ -20,3 +27,7 @@ x; // number
2027
x = true;
2128
(x = "", obj).foo = (x = x.length);
2229
x; // number
30+
var o;
31+
if ((o = fn()).done) {
32+
var y = o.value;
33+
}

‎tests/baselines/reference/controlFlowAssignmentExpression.symbols

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,31 @@ x = true;
3131
x; // number
3232
>x : Symbol(x, Decl(controlFlowAssignmentExpression.ts, 0, 3))
3333

34+
// https://github.com/microsoft/TypeScript/issues/35484
35+
type D = { done: true, value: 1 } | { done: false, value: 2 };
36+
>D : Symbol(D, Decl(controlFlowAssignmentExpression.ts, 9, 2))
37+
>done : Symbol(done, Decl(controlFlowAssignmentExpression.ts, 12, 10))
38+
>value : Symbol(value, Decl(controlFlowAssignmentExpression.ts, 12, 22))
39+
>done : Symbol(done, Decl(controlFlowAssignmentExpression.ts, 12, 37))
40+
>value : Symbol(value, Decl(controlFlowAssignmentExpression.ts, 12, 50))
41+
42+
declare function fn(): D;
43+
>fn : Symbol(fn, Decl(controlFlowAssignmentExpression.ts, 12, 62))
44+
>D : Symbol(D, Decl(controlFlowAssignmentExpression.ts, 9, 2))
45+
46+
let o: D;
47+
>o : Symbol(o, Decl(controlFlowAssignmentExpression.ts, 14, 3))
48+
>D : Symbol(D, Decl(controlFlowAssignmentExpression.ts, 9, 2))
49+
50+
if ((o = fn()).done) {
51+
>(o = fn()).done : Symbol(done, Decl(controlFlowAssignmentExpression.ts, 12, 10), Decl(controlFlowAssignmentExpression.ts, 12, 37))
52+
>o : Symbol(o, Decl(controlFlowAssignmentExpression.ts, 14, 3))
53+
>fn : Symbol(fn, Decl(controlFlowAssignmentExpression.ts, 12, 62))
54+
>done : Symbol(done, Decl(controlFlowAssignmentExpression.ts, 12, 10), Decl(controlFlowAssignmentExpression.ts, 12, 37))
55+
56+
const y: 1 = o.value;
57+
>y : Symbol(y, Decl(controlFlowAssignmentExpression.ts, 16, 9))
58+
>o.value : Symbol(value, Decl(controlFlowAssignmentExpression.ts, 12, 22))
59+
>o : Symbol(o, Decl(controlFlowAssignmentExpression.ts, 14, 3))
60+
>value : Symbol(value, Decl(controlFlowAssignmentExpression.ts, 12, 22))
61+
}

‎tests/baselines/reference/controlFlowAssignmentExpression.types

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,34 @@ x = true;
4545
x; // number
4646
>x : number
4747

48+
// https://github.com/microsoft/TypeScript/issues/35484
49+
type D = { done: true, value: 1 } | { done: false, value: 2 };
50+
>D : D
51+
>done : true
52+
>true : true
53+
>value : 1
54+
>done : false
55+
>false : false
56+
>value : 2
57+
58+
declare function fn(): D;
59+
>fn : () => D
60+
61+
let o: D;
62+
>o : D
63+
64+
if ((o = fn()).done) {
65+
>(o = fn()).done : boolean
66+
>(o = fn()) : D
67+
>o = fn() : D
68+
>o : D
69+
>fn() : D
70+
>fn : () => D
71+
>done : boolean
72+
73+
const y: 1 = o.value;
74+
>y : 1
75+
>o.value : 1
76+
>o : { done: true; value: 1; }
77+
>value : 1
78+
}

‎tests/cases/conformance/controlFlow/controlFlowAssignmentExpression.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,11 @@ x; // number
88
x = true;
99
(x = "", obj).foo = (x = x.length);
1010
x; // number
11+
12+
// https://github.com/microsoft/TypeScript/issues/35484
13+
type D = { done: true, value: 1 } | { done: false, value: 2 };
14+
declare function fn(): D;
15+
let o: D;
16+
if ((o = fn()).done) {
17+
const y: 1 = o.value;
18+
}

0 commit comments

Comments
 (0)