Skip to content

Commit 8ff77fb

Browse files
ahejlsbergahejlsberg
authored
Preserve type refinements in closures created past last assignment (#56908)
1 parent f9cb96c commit 8ff77fb

20 files changed

+1485
-68
lines changed

‎src/compiler/checker.ts

+102-20
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,6 @@ import {
668668
isOutermostOptionalChain,
669669
isParameter,
670670
isParameterDeclaration,
671-
isParameterOrCatchClauseVariable,
672671
isParameterPropertyDeclaration,
673672
isParenthesizedExpression,
674673
isParenthesizedTypeNode,
@@ -27481,7 +27480,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
2748127480
case SyntaxKind.Identifier:
2748227481
if (!isThisInTypeQuery(node)) {
2748327482
const symbol = getResolvedSymbol(node as Identifier);
27484-
return isConstantVariable(symbol) || isParameterOrCatchClauseVariable(symbol) && !isSymbolAssigned(symbol);
27483+
return isConstantVariable(symbol) || isParameterOrMutableLocalVariable(symbol) && !isSymbolAssigned(symbol);
2748527484
}
2748627485
break;
2748727486
case SyntaxKind.PropertyAccessExpression:
@@ -28760,18 +28759,24 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
2876028759

2876128760
// Check if a parameter or catch variable is assigned anywhere
2876228761
function isSymbolAssigned(symbol: Symbol) {
28763-
if (!symbol.valueDeclaration) {
28762+
return !isPastLastAssignment(symbol, /*location*/ undefined);
28763+
}
28764+
28765+
// Return true if there are no assignments to the given symbol or if the given location
28766+
// is past the last assignment to the symbol.
28767+
function isPastLastAssignment(symbol: Symbol, location: Node | undefined) {
28768+
const parent = findAncestor(symbol.valueDeclaration, isFunctionOrSourceFile);
28769+
if (!parent) {
2876428770
return false;
2876528771
}
28766-
const parent = getRootDeclaration(symbol.valueDeclaration).parent;
2876728772
const links = getNodeLinks(parent);
2876828773
if (!(links.flags & NodeCheckFlags.AssignmentsMarked)) {
2876928774
links.flags |= NodeCheckFlags.AssignmentsMarked;
2877028775
if (!hasParentWithAssignmentsMarked(parent)) {
2877128776
markNodeAssignments(parent);
2877228777
}
2877328778
}
28774-
return symbol.isAssigned || false;
28779+
return !symbol.lastAssignmentPos || location && symbol.lastAssignmentPos < location.pos;
2877528780
}
2877628781

2877728782
// Check if a parameter or catch variable (or their bindings elements) is assigned anywhere
@@ -28789,27 +28794,98 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
2878928794
}
2879028795

2879128796
function hasParentWithAssignmentsMarked(node: Node) {
28792-
return !!findAncestor(node.parent, node => (isFunctionLike(node) || isCatchClause(node)) && !!(getNodeLinks(node).flags & NodeCheckFlags.AssignmentsMarked));
28797+
return !!findAncestor(node.parent, node => isFunctionOrSourceFile(node) && !!(getNodeLinks(node).flags & NodeCheckFlags.AssignmentsMarked));
2879328798
}
2879428799

28800+
function isFunctionOrSourceFile(node: Node) {
28801+
return isFunctionLikeDeclaration(node) || isSourceFile(node);
28802+
}
28803+
28804+
// For all assignments within the given root node, record the last assignment source position for all
28805+
// referenced parameters and mutable local variables. When assignments occur in nested functions or
28806+
// references occur in export specifiers, record Number.MAX_VALUE as the assignment position. When
28807+
// assignments occur in compound statements, record the ending source position of the compound statement
28808+
// as the assignment position (this is more conservative than full control flow analysis, but requires
28809+
// only a single walk over the AST).
2879528810
function markNodeAssignments(node: Node) {
28796-
if (node.kind === SyntaxKind.Identifier) {
28797-
if (isAssignmentTarget(node)) {
28798-
const symbol = getResolvedSymbol(node as Identifier);
28799-
if (isParameterOrCatchClauseVariable(symbol)) {
28800-
symbol.isAssigned = true;
28811+
switch (node.kind) {
28812+
case SyntaxKind.Identifier:
28813+
if (isAssignmentTarget(node)) {
28814+
const symbol = getResolvedSymbol(node as Identifier);
28815+
if (isParameterOrMutableLocalVariable(symbol) && symbol.lastAssignmentPos !== Number.MAX_VALUE) {
28816+
const referencingFunction = findAncestor(node, isFunctionOrSourceFile);
28817+
const declaringFunction = findAncestor(symbol.valueDeclaration, isFunctionOrSourceFile);
28818+
symbol.lastAssignmentPos = referencingFunction === declaringFunction ? extendAssignmentPosition(node, symbol.valueDeclaration!) : Number.MAX_VALUE;
28819+
}
2880128820
}
28802-
}
28821+
return;
28822+
case SyntaxKind.ExportSpecifier:
28823+
const exportDeclaration = (node as ExportSpecifier).parent.parent;
28824+
if (!(node as ExportSpecifier).isTypeOnly && !exportDeclaration.isTypeOnly && !exportDeclaration.moduleSpecifier) {
28825+
const symbol = resolveEntityName((node as ExportSpecifier).propertyName || (node as ExportSpecifier).name, SymbolFlags.Value, /*ignoreErrors*/ true, /*dontResolveAlias*/ true);
28826+
if (symbol && isParameterOrMutableLocalVariable(symbol)) {
28827+
symbol.lastAssignmentPos = Number.MAX_VALUE;
28828+
}
28829+
}
28830+
return;
28831+
case SyntaxKind.InterfaceDeclaration:
28832+
case SyntaxKind.TypeAliasDeclaration:
28833+
case SyntaxKind.EnumDeclaration:
28834+
return;
2880328835
}
28804-
else {
28805-
forEachChild(node, markNodeAssignments);
28836+
if (isTypeNode(node)) {
28837+
return;
2880628838
}
28839+
forEachChild(node, markNodeAssignments);
28840+
}
28841+
28842+
// Extend the position of the given assignment target node to the end of any intervening variable statement,
28843+
// expression statement, compound statement, or class declaration occurring between the node and the given
28844+
// declaration node.
28845+
function extendAssignmentPosition(node: Node, declaration: Declaration) {
28846+
let pos = node.pos;
28847+
while (node && node.pos > declaration.pos) {
28848+
switch (node.kind) {
28849+
case SyntaxKind.VariableStatement:
28850+
case SyntaxKind.ExpressionStatement:
28851+
case SyntaxKind.IfStatement:
28852+
case SyntaxKind.DoStatement:
28853+
case SyntaxKind.WhileStatement:
28854+
case SyntaxKind.ForStatement:
28855+
case SyntaxKind.ForInStatement:
28856+
case SyntaxKind.ForOfStatement:
28857+
case SyntaxKind.WithStatement:
28858+
case SyntaxKind.SwitchStatement:
28859+
case SyntaxKind.TryStatement:
28860+
case SyntaxKind.ClassDeclaration:
28861+
pos = node.end;
28862+
}
28863+
node = node.parent;
28864+
}
28865+
return pos;
2880728866
}
2880828867

2880928868
function isConstantVariable(symbol: Symbol) {
2881028869
return symbol.flags & SymbolFlags.Variable && (getDeclarationNodeFlagsFromSymbol(symbol) & NodeFlags.Constant) !== 0;
2881128870
}
2881228871

28872+
function isParameterOrMutableLocalVariable(symbol: Symbol) {
28873+
// Return true if symbol is a parameter, a catch clause variable, or a mutable local variable
28874+
const declaration = symbol.valueDeclaration && getRootDeclaration(symbol.valueDeclaration);
28875+
return !!declaration && (
28876+
isParameter(declaration) ||
28877+
isVariableDeclaration(declaration) && (isCatchClause(declaration.parent) || isMutableLocalVariableDeclaration(declaration))
28878+
);
28879+
}
28880+
28881+
function isMutableLocalVariableDeclaration(declaration: VariableDeclaration) {
28882+
// Return true if symbol is a non-exported and non-global `let` variable
28883+
return !!(declaration.parent.flags & NodeFlags.Let) && !(
28884+
getCombinedModifierFlags(declaration) & ModifierFlags.Export ||
28885+
declaration.parent.parent.kind === SyntaxKind.VariableStatement && isGlobalSourceFile(declaration.parent.parent.parent)
28886+
);
28887+
}
28888+
2881328889
function parameterInitializerContainsUndefined(declaration: ParameterDeclaration): boolean {
2881428890
const links = getNodeLinks(declaration);
2881528891

@@ -29160,13 +29236,19 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
2916029236
const isModuleExports = symbol.flags & SymbolFlags.ModuleExports;
2916129237
const typeIsAutomatic = type === autoType || type === autoArrayType;
2916229238
const isAutomaticTypeInNonNull = typeIsAutomatic && node.parent.kind === SyntaxKind.NonNullExpression;
29163-
// When the control flow originates in a function expression or arrow function and we are referencing
29164-
// a const variable or parameter from an outer function, we extend the origin of the control flow
29165-
// analysis to include the immediately enclosing function.
29239+
// When the control flow originates in a function expression, arrow function, method, or accessor, and
29240+
// we are referencing a closed-over const variable or parameter or mutable local variable past its last
29241+
// assignment, we extend the origin of the control flow analysis to include the immediately enclosing
29242+
// control flow container.
2916629243
while (
29167-
flowContainer !== declarationContainer && (flowContainer.kind === SyntaxKind.FunctionExpression ||
29168-
flowContainer.kind === SyntaxKind.ArrowFunction || isObjectLiteralOrClassExpressionMethodOrAccessor(flowContainer)) &&
29169-
(isConstantVariable(localOrExportSymbol) && type !== autoArrayType || isParameter && !isSymbolAssigned(localOrExportSymbol))
29244+
flowContainer !== declarationContainer && (
29245+
flowContainer.kind === SyntaxKind.FunctionExpression ||
29246+
flowContainer.kind === SyntaxKind.ArrowFunction ||
29247+
isObjectLiteralOrClassExpressionMethodOrAccessor(flowContainer)
29248+
) && (
29249+
isConstantVariable(localOrExportSymbol) && type !== autoArrayType ||
29250+
isParameterOrMutableLocalVariable(localOrExportSymbol) && isPastLastAssignment(localOrExportSymbol, node)
29251+
)
2917029252
) {
2917129253
flowContainer = getControlFlowContainer(flowContainer);
2917229254
}

‎src/compiler/types.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -5825,8 +5825,8 @@ export interface Symbol {
58255825
/** @internal */ exportSymbol?: Symbol; // Exported symbol associated with this symbol
58265826
/** @internal */ constEnumOnlyModule: boolean | undefined; // True if module contains only const enums or other modules with only const enums
58275827
/** @internal */ isReferenced?: SymbolFlags; // True if the symbol is referenced elsewhere. Keeps track of the meaning of a reference in case a symbol is both a type parameter and parameter.
5828+
/** @internal */ lastAssignmentPos?: number; // Source position of last node that assigns value to symbol
58285829
/** @internal */ isReplaceableByMethod?: boolean; // Can this Javascript class property be replaced by a method symbol?
5829-
/** @internal */ isAssigned?: boolean; // True if the symbol is a parameter with assignments
58305830
/** @internal */ assignmentDeclarationMembers?: Map<number, Declaration>; // detected late-bound assignment declarations associated with the symbol
58315831
}
58325832

‎src/compiler/utilities.ts

+1-7
Original file line numberDiff line numberDiff line change
@@ -8178,7 +8178,7 @@ function Symbol(this: Symbol, flags: SymbolFlags, name: __String) {
81788178
this.exportSymbol = undefined;
81798179
this.constEnumOnlyModule = undefined;
81808180
this.isReferenced = undefined;
8181-
this.isAssigned = undefined;
8181+
this.lastAssignmentPos = undefined;
81828182
(this as any).links = undefined; // used by TransientSymbol
81838183
}
81848184

@@ -10351,12 +10351,6 @@ export function isCatchClauseVariableDeclaration(node: Node) {
1035110351
return node.kind === SyntaxKind.VariableDeclaration && node.parent.kind === SyntaxKind.CatchClause;
1035210352
}
1035310353

10354-
/** @internal */
10355-
export function isParameterOrCatchClauseVariable(symbol: Symbol) {
10356-
const declaration = symbol.valueDeclaration && getRootDeclaration(symbol.valueDeclaration);
10357-
return !!declaration && (isParameter(declaration) || isCatchClauseVariableDeclaration(declaration));
10358-
}
10359-
1036010354
/** @internal */
1036110355
export function isFunctionExpressionOrArrowFunction(node: Node): node is FunctionExpression | ArrowFunction {
1036210356
return node.kind === SyntaxKind.FunctionExpression || node.kind === SyntaxKind.ArrowFunction;

‎tests/baselines/reference/controlFlowAliasing.errors.txt

+3-13
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,6 @@ controlFlowAliasing.ts(112,13): error TS2339: Property 'foo' does not exist on t
1414
Property 'foo' does not exist on type '{ kind: "bar"; bar: number; }'.
1515
controlFlowAliasing.ts(115,13): error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
1616
Property 'bar' does not exist on type '{ kind: "foo"; foo: string; }'.
17-
controlFlowAliasing.ts(134,13): error TS2339: Property 'foo' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
18-
Property 'foo' does not exist on type '{ kind: "bar"; bar: number; }'.
19-
controlFlowAliasing.ts(137,13): error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
20-
Property 'bar' does not exist on type '{ kind: "foo"; foo: string; }'.
2117
controlFlowAliasing.ts(154,19): error TS2339: Property 'foo' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
2218
Property 'foo' does not exist on type '{ kind: "bar"; bar: number; }'.
2319
controlFlowAliasing.ts(157,19): error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
@@ -28,7 +24,7 @@ controlFlowAliasing.ts(280,5): error TS2448: Block-scoped variable 'a' used befo
2824
controlFlowAliasing.ts(280,5): error TS2454: Variable 'a' is used before being assigned.
2925

3026

31-
==== controlFlowAliasing.ts (15 errors) ====
27+
==== controlFlowAliasing.ts (13 errors) ====
3228
// Narrowing by aliased conditional expressions
3329

3430
function f10(x: string | number) {
@@ -186,16 +182,10 @@ controlFlowAliasing.ts(280,5): error TS2454: Variable 'a' is used before being a
186182
let obj = arg;
187183
const isFoo = obj.kind === 'foo';
188184
if (isFoo) {
189-
obj.foo; // Not narrowed because obj is mutable
190-
~~~
191-
!!! error TS2339: Property 'foo' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
192-
!!! error TS2339: Property 'foo' does not exist on type '{ kind: "bar"; bar: number; }'.
185+
obj.foo;
193186
}
194187
else {
195-
obj.bar; // Not narrowed because obj is mutable
196-
~~~
197-
!!! error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
198-
!!! error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; }'.
188+
obj.bar;
199189
}
200190
}
201191

‎tests/baselines/reference/controlFlowAliasing.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,10 @@ function f25(arg: { kind: 'foo', foo: string } | { kind: 'bar', bar: number }) {
134134
let obj = arg;
135135
const isFoo = obj.kind === 'foo';
136136
if (isFoo) {
137-
obj.foo; // Not narrowed because obj is mutable
137+
obj.foo;
138138
}
139139
else {
140-
obj.bar; // Not narrowed because obj is mutable
140+
obj.bar;
141141
}
142142
}
143143

@@ -423,10 +423,10 @@ function f25(arg) {
423423
var obj = arg;
424424
var isFoo = obj.kind === 'foo';
425425
if (isFoo) {
426-
obj.foo; // Not narrowed because obj is mutable
426+
obj.foo;
427427
}
428428
else {
429-
obj.bar; // Not narrowed because obj is mutable
429+
obj.bar;
430430
}
431431
}
432432
function f26(outer) {

‎tests/baselines/reference/controlFlowAliasing.symbols

+6-2
Original file line numberDiff line numberDiff line change
@@ -370,12 +370,16 @@ function f25(arg: { kind: 'foo', foo: string } | { kind: 'bar', bar: number }) {
370370
if (isFoo) {
371371
>isFoo : Symbol(isFoo, Decl(controlFlowAliasing.ts, 131, 9))
372372

373-
obj.foo; // Not narrowed because obj is mutable
373+
obj.foo;
374+
>obj.foo : Symbol(foo, Decl(controlFlowAliasing.ts, 129, 32))
374375
>obj : Symbol(obj, Decl(controlFlowAliasing.ts, 130, 7))
376+
>foo : Symbol(foo, Decl(controlFlowAliasing.ts, 129, 32))
375377
}
376378
else {
377-
obj.bar; // Not narrowed because obj is mutable
379+
obj.bar;
380+
>obj.bar : Symbol(bar, Decl(controlFlowAliasing.ts, 129, 63))
378381
>obj : Symbol(obj, Decl(controlFlowAliasing.ts, 130, 7))
382+
>bar : Symbol(bar, Decl(controlFlowAliasing.ts, 129, 63))
379383
}
380384
}
381385

‎tests/baselines/reference/controlFlowAliasing.types

+8-8
Original file line numberDiff line numberDiff line change
@@ -440,16 +440,16 @@ function f25(arg: { kind: 'foo', foo: string } | { kind: 'bar', bar: number }) {
440440
if (isFoo) {
441441
>isFoo : boolean
442442

443-
obj.foo; // Not narrowed because obj is mutable
444-
>obj.foo : any
445-
>obj : { kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }
446-
>foo : any
443+
obj.foo;
444+
>obj.foo : string
445+
>obj : { kind: "foo"; foo: string; }
446+
>foo : string
447447
}
448448
else {
449-
obj.bar; // Not narrowed because obj is mutable
450-
>obj.bar : any
451-
>obj : { kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }
452-
>bar : any
449+
obj.bar;
450+
>obj.bar : number
451+
>obj : { kind: "bar"; bar: number; }
452+
>bar : number
453453
}
454454
}
455455

‎tests/baselines/reference/implicitConstParameters.errors.txt

+2-5
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
1-
implicitConstParameters.ts(38,27): error TS18048: 'x' is possibly 'undefined'.
21
implicitConstParameters.ts(44,27): error TS18048: 'x' is possibly 'undefined'.
32

43

5-
==== implicitConstParameters.ts (2 errors) ====
4+
==== implicitConstParameters.ts (1 errors) ====
65
function doSomething(cb: () => void) {
76
cb();
87
}
@@ -38,11 +37,9 @@ implicitConstParameters.ts(44,27): error TS18048: 'x' is possibly 'undefined'.
3837
}
3938

4039
function f4(x: string | undefined) {
41-
x = "abc"; // causes x to be considered non-const
40+
x = "abc";
4241
if (x) {
4342
doSomething(() => x.length);
44-
~
45-
!!! error TS18048: 'x' is possibly 'undefined'.
4643
}
4744
}
4845

‎tests/baselines/reference/implicitConstParameters.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ function f3(x: string | undefined) {
3636
}
3737

3838
function f4(x: string | undefined) {
39-
x = "abc"; // causes x to be considered non-const
39+
x = "abc";
4040
if (x) {
4141
doSomething(() => x.length);
4242
}
@@ -88,7 +88,7 @@ function f3(x) {
8888
}
8989
}
9090
function f4(x) {
91-
x = "abc"; // causes x to be considered non-const
91+
x = "abc";
9292
if (x) {
9393
doSomething(function () { return x.length; });
9494
}

‎tests/baselines/reference/implicitConstParameters.symbols

+1-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ function f4(x: string | undefined) {
8686
>f4 : Symbol(f4, Decl(implicitConstParameters.ts, 32, 1))
8787
>x : Symbol(x, Decl(implicitConstParameters.ts, 34, 12))
8888

89-
x = "abc"; // causes x to be considered non-const
89+
x = "abc";
9090
>x : Symbol(x, Decl(implicitConstParameters.ts, 34, 12))
9191

9292
if (x) {

‎tests/baselines/reference/implicitConstParameters.types

+2-2
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ function f4(x: string | undefined) {
103103
>f4 : (x: string | undefined) => void
104104
>x : string | undefined
105105

106-
x = "abc"; // causes x to be considered non-const
106+
x = "abc";
107107
>x = "abc" : "abc"
108108
>x : string | undefined
109109
>"abc" : "abc"
@@ -116,7 +116,7 @@ function f4(x: string | undefined) {
116116
>doSomething : (cb: () => void) => void
117117
>() => x.length : () => number
118118
>x.length : number
119-
>x : string | undefined
119+
>x : string
120120
>length : number
121121
}
122122
}

0 commit comments

Comments
 (0)