Skip to content

Commit 40fa0c9

Browse files
a-tarasyuka-tarasyuk
authored
fix(44880): allow narrowing aliased conditions for catch variables (microsoft#45797)
1 parent 4f8aa52 commit 40fa0c9

11 files changed

Lines changed: 453 additions & 14 deletions

‎src/compiler/checker.ts‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23240,9 +23240,10 @@ namespace ts {
2324023240

2324123241
function isConstantReference(node: Node): boolean {
2324223242
switch (node.kind) {
23243-
case SyntaxKind.Identifier:
23243+
case SyntaxKind.Identifier: {
2324423244
const symbol = getResolvedSymbol(node as Identifier);
23245-
return isConstVariable(symbol) || !!symbol.valueDeclaration && getRootDeclaration(symbol.valueDeclaration).kind === SyntaxKind.Parameter && !isParameterAssigned(symbol);
23245+
return isConstVariable(symbol) || isParameterOrCatchClauseVariable(symbol) && !isSymbolAssigned(symbol);
23246+
}
2324623247
case SyntaxKind.PropertyAccessExpression:
2324723248
case SyntaxKind.ElementAccessExpression:
2324823249
// The resolvedSymbol property is initialized by checkPropertyAccess or checkElementAccess before we get here.
@@ -24376,37 +24377,38 @@ namespace ts {
2437624377
node.kind === SyntaxKind.PropertyDeclaration)!;
2437724378
}
2437824379

24379-
// Check if a parameter is assigned anywhere within its declaring function.
24380-
function isParameterAssigned(symbol: Symbol) {
24380+
// Check if a parameter or catch variable is assigned anywhere
24381+
function isSymbolAssigned(symbol: Symbol) {
2438124382
if (!symbol.valueDeclaration) {
2438224383
return false;
2438324384
}
24384-
const func = getRootDeclaration(symbol.valueDeclaration).parent as FunctionLikeDeclaration;
24385-
const links = getNodeLinks(func);
24385+
const parent = getRootDeclaration(symbol.valueDeclaration).parent;
24386+
const links = getNodeLinks(parent);
2438624387
if (!(links.flags & NodeCheckFlags.AssignmentsMarked)) {
2438724388
links.flags |= NodeCheckFlags.AssignmentsMarked;
24388-
if (!hasParentWithAssignmentsMarked(func)) {
24389-
markParameterAssignments(func);
24389+
if (!hasParentWithAssignmentsMarked(parent)) {
24390+
markNodeAssignments(parent);
2439024391
}
2439124392
}
2439224393
return symbol.isAssigned || false;
2439324394
}
2439424395

2439524396
function hasParentWithAssignmentsMarked(node: Node) {
24396-
return !!findAncestor(node.parent, node => isFunctionLike(node) && !!(getNodeLinks(node).flags & NodeCheckFlags.AssignmentsMarked));
24397+
return !!findAncestor(node.parent, node =>
24398+
(isFunctionLike(node) || isCatchClause(node)) && !!(getNodeLinks(node).flags & NodeCheckFlags.AssignmentsMarked));
2439724399
}
2439824400

24399-
function markParameterAssignments(node: Node) {
24401+
function markNodeAssignments(node: Node) {
2440024402
if (node.kind === SyntaxKind.Identifier) {
2440124403
if (isAssignmentTarget(node)) {
2440224404
const symbol = getResolvedSymbol(node as Identifier);
24403-
if (symbol.valueDeclaration && getRootDeclaration(symbol.valueDeclaration).kind === SyntaxKind.Parameter) {
24405+
if (isParameterOrCatchClauseVariable(symbol)) {
2440424406
symbol.isAssigned = true;
2440524407
}
2440624408
}
2440724409
}
2440824410
else {
24409-
forEachChild(node, markParameterAssignments);
24411+
forEachChild(node, markNodeAssignments);
2441024412
}
2441124413
}
2441224414

@@ -24644,7 +24646,7 @@ namespace ts {
2464424646
// analysis to include the immediately enclosing function.
2464524647
while (flowContainer !== declarationContainer && (flowContainer.kind === SyntaxKind.FunctionExpression ||
2464624648
flowContainer.kind === SyntaxKind.ArrowFunction || isObjectLiteralOrClassExpressionMethodOrAccessor(flowContainer)) &&
24647-
(isConstVariable(localOrExportSymbol) && type !== autoArrayType || isParameter && !isParameterAssigned(localOrExportSymbol))) {
24649+
(isConstVariable(localOrExportSymbol) && type !== autoArrayType || isParameter && !isSymbolAssigned(localOrExportSymbol))) {
2464824650
flowContainer = getControlFlowContainer(flowContainer);
2464924651
}
2465024652
// We only look for uninitialized variables in strict null checking mode, and only when we can analyze

‎src/compiler/utilities.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7409,4 +7409,13 @@ namespace ts {
74097409
export function isInfinityOrNaNString(name: string | __String): boolean {
74107410
return name === "Infinity" || name === "-Infinity" || name === "NaN";
74117411
}
7412+
7413+
export function isCatchClauseVariableDeclaration(node: Node) {
7414+
return node.kind === SyntaxKind.VariableDeclaration && node.parent.kind === SyntaxKind.CatchClause;
7415+
}
7416+
7417+
export function isParameterOrCatchClauseVariable(symbol: Symbol) {
7418+
const declaration = symbol.valueDeclaration && getRootDeclaration(symbol.valueDeclaration);
7419+
return !!declaration && (isParameter(declaration) || isCatchClauseVariableDeclaration(declaration));
7420+
}
74127421
}

‎src/testRunner/compilerRunner.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,8 @@ namespace Harness {
138138
"skipDefaultLibCheck",
139139
"preserveConstEnums",
140140
"skipLibCheck",
141-
"exactOptionalPropertyTypes"
141+
"exactOptionalPropertyTypes",
142+
"useUnknownInCatchVariables"
142143
];
143144
private fileName: string;
144145
private justName: string;

‎tests/baselines/reference/controlFlowAliasingCatchVariables(useunknownincatchvariables=false).js‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
//// [controlFlowAliasingCatchVariables.ts]
2+
try {}
3+
catch (e) {
4+
const isString = typeof e === 'string';
5+
if (isString) {
6+
e.toUpperCase(); // e string
7+
}
8+
9+
if (typeof e === 'string') {
10+
e.toUpperCase(); // e string
11+
}
12+
}
13+
14+
try {}
15+
catch (e) {
16+
const isString = typeof e === 'string';
17+
18+
e = 1;
19+
20+
if (isString) {
21+
e.toUpperCase(); // e any/unknown
22+
}
23+
24+
if (typeof e === 'string') {
25+
e.toUpperCase(); // e string
26+
}
27+
}
28+
29+
30+
//// [controlFlowAliasingCatchVariables.js]
31+
try { }
32+
catch (e) {
33+
var isString = typeof e === 'string';
34+
if (isString) {
35+
e.toUpperCase(); // e string
36+
}
37+
if (typeof e === 'string') {
38+
e.toUpperCase(); // e string
39+
}
40+
}
41+
try { }
42+
catch (e) {
43+
var isString = typeof e === 'string';
44+
e = 1;
45+
if (isString) {
46+
e.toUpperCase(); // e any/unknown
47+
}
48+
if (typeof e === 'string') {
49+
e.toUpperCase(); // e string
50+
}
51+
}

‎tests/baselines/reference/controlFlowAliasingCatchVariables(useunknownincatchvariables=false).symbols‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
=== tests/cases/conformance/controlFlow/controlFlowAliasingCatchVariables.ts ===
2+
try {}
3+
catch (e) {
4+
>e : Symbol(e, Decl(controlFlowAliasingCatchVariables.ts, 1, 7))
5+
6+
const isString = typeof e === 'string';
7+
>isString : Symbol(isString, Decl(controlFlowAliasingCatchVariables.ts, 2, 9))
8+
>e : Symbol(e, Decl(controlFlowAliasingCatchVariables.ts, 1, 7))
9+
10+
if (isString) {
11+
>isString : Symbol(isString, Decl(controlFlowAliasingCatchVariables.ts, 2, 9))
12+
13+
e.toUpperCase(); // e string
14+
>e.toUpperCase : Symbol(String.toUpperCase, Decl(lib.es5.d.ts, --, --))
15+
>e : Symbol(e, Decl(controlFlowAliasingCatchVariables.ts, 1, 7))
16+
>toUpperCase : Symbol(String.toUpperCase, Decl(lib.es5.d.ts, --, --))
17+
}
18+
19+
if (typeof e === 'string') {
20+
>e : Symbol(e, Decl(controlFlowAliasingCatchVariables.ts, 1, 7))
21+
22+
e.toUpperCase(); // e string
23+
>e.toUpperCase : Symbol(String.toUpperCase, Decl(lib.es5.d.ts, --, --))
24+
>e : Symbol(e, Decl(controlFlowAliasingCatchVariables.ts, 1, 7))
25+
>toUpperCase : Symbol(String.toUpperCase, Decl(lib.es5.d.ts, --, --))
26+
}
27+
}
28+
29+
try {}
30+
catch (e) {
31+
>e : Symbol(e, Decl(controlFlowAliasingCatchVariables.ts, 13, 7))
32+
33+
const isString = typeof e === 'string';
34+
>isString : Symbol(isString, Decl(controlFlowAliasingCatchVariables.ts, 14, 9))
35+
>e : Symbol(e, Decl(controlFlowAliasingCatchVariables.ts, 13, 7))
36+
37+
e = 1;
38+
>e : Symbol(e, Decl(controlFlowAliasingCatchVariables.ts, 13, 7))
39+
40+
if (isString) {
41+
>isString : Symbol(isString, Decl(controlFlowAliasingCatchVariables.ts, 14, 9))
42+
43+
e.toUpperCase(); // e any/unknown
44+
>e : Symbol(e, Decl(controlFlowAliasingCatchVariables.ts, 13, 7))
45+
}
46+
47+
if (typeof e === 'string') {
48+
>e : Symbol(e, Decl(controlFlowAliasingCatchVariables.ts, 13, 7))
49+
50+
e.toUpperCase(); // e string
51+
>e.toUpperCase : Symbol(String.toUpperCase, Decl(lib.es5.d.ts, --, --))
52+
>e : Symbol(e, Decl(controlFlowAliasingCatchVariables.ts, 13, 7))
53+
>toUpperCase : Symbol(String.toUpperCase, Decl(lib.es5.d.ts, --, --))
54+
}
55+
}
56+

‎tests/baselines/reference/controlFlowAliasingCatchVariables(useunknownincatchvariables=false).types‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
=== tests/cases/conformance/controlFlow/controlFlowAliasingCatchVariables.ts ===
2+
try {}
3+
catch (e) {
4+
>e : any
5+
6+
const isString = typeof e === 'string';
7+
>isString : boolean
8+
>typeof e === 'string' : boolean
9+
>typeof e : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
10+
>e : any
11+
>'string' : "string"
12+
13+
if (isString) {
14+
>isString : boolean
15+
16+
e.toUpperCase(); // e string
17+
>e.toUpperCase() : string
18+
>e.toUpperCase : () => string
19+
>e : string
20+
>toUpperCase : () => string
21+
}
22+
23+
if (typeof e === 'string') {
24+
>typeof e === 'string' : boolean
25+
>typeof e : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
26+
>e : any
27+
>'string' : "string"
28+
29+
e.toUpperCase(); // e string
30+
>e.toUpperCase() : string
31+
>e.toUpperCase : () => string
32+
>e : string
33+
>toUpperCase : () => string
34+
}
35+
}
36+
37+
try {}
38+
catch (e) {
39+
>e : any
40+
41+
const isString = typeof e === 'string';
42+
>isString : boolean
43+
>typeof e === 'string' : boolean
44+
>typeof e : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
45+
>e : any
46+
>'string' : "string"
47+
48+
e = 1;
49+
>e = 1 : 1
50+
>e : any
51+
>1 : 1
52+
53+
if (isString) {
54+
>isString : boolean
55+
56+
e.toUpperCase(); // e any/unknown
57+
>e.toUpperCase() : any
58+
>e.toUpperCase : any
59+
>e : any
60+
>toUpperCase : any
61+
}
62+
63+
if (typeof e === 'string') {
64+
>typeof e === 'string' : boolean
65+
>typeof e : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
66+
>e : any
67+
>'string' : "string"
68+
69+
e.toUpperCase(); // e string
70+
>e.toUpperCase() : string
71+
>e.toUpperCase : () => string
72+
>e : string
73+
>toUpperCase : () => string
74+
}
75+
}
76+

‎tests/baselines/reference/controlFlowAliasingCatchVariables(useunknownincatchvariables=true).errors.txt‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
tests/cases/conformance/controlFlow/controlFlowAliasingCatchVariables.ts(20,11): error TS2339: Property 'toUpperCase' does not exist on type 'unknown'.
2+
3+
4+
==== tests/cases/conformance/controlFlow/controlFlowAliasingCatchVariables.ts (1 errors) ====
5+
try {}
6+
catch (e) {
7+
const isString = typeof e === 'string';
8+
if (isString) {
9+
e.toUpperCase(); // e string
10+
}
11+
12+
if (typeof e === 'string') {
13+
e.toUpperCase(); // e string
14+
}
15+
}
16+
17+
try {}
18+
catch (e) {
19+
const isString = typeof e === 'string';
20+
21+
e = 1;
22+
23+
if (isString) {
24+
e.toUpperCase(); // e any/unknown
25+
~~~~~~~~~~~
26+
!!! error TS2339: Property 'toUpperCase' does not exist on type 'unknown'.
27+
}
28+
29+
if (typeof e === 'string') {
30+
e.toUpperCase(); // e string
31+
}
32+
}
33+

‎tests/baselines/reference/controlFlowAliasingCatchVariables(useunknownincatchvariables=true).js‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
//// [controlFlowAliasingCatchVariables.ts]
2+
try {}
3+
catch (e) {
4+
const isString = typeof e === 'string';
5+
if (isString) {
6+
e.toUpperCase(); // e string
7+
}
8+
9+
if (typeof e === 'string') {
10+
e.toUpperCase(); // e string
11+
}
12+
}
13+
14+
try {}
15+
catch (e) {
16+
const isString = typeof e === 'string';
17+
18+
e = 1;
19+
20+
if (isString) {
21+
e.toUpperCase(); // e any/unknown
22+
}
23+
24+
if (typeof e === 'string') {
25+
e.toUpperCase(); // e string
26+
}
27+
}
28+
29+
30+
//// [controlFlowAliasingCatchVariables.js]
31+
try { }
32+
catch (e) {
33+
var isString = typeof e === 'string';
34+
if (isString) {
35+
e.toUpperCase(); // e string
36+
}
37+
if (typeof e === 'string') {
38+
e.toUpperCase(); // e string
39+
}
40+
}
41+
try { }
42+
catch (e) {
43+
var isString = typeof e === 'string';
44+
e = 1;
45+
if (isString) {
46+
e.toUpperCase(); // e any/unknown
47+
}
48+
if (typeof e === 'string') {
49+
e.toUpperCase(); // e string
50+
}
51+
}

0 commit comments

Comments
 (0)