Skip to content

Commit 5f8a9e5

Browse files
Fix constant reference check in CFA (microsoft#44762)
* Check entire access path is constant when narrowing by inlining * Add tests * Accept new baselines * Added test cases for parameter properties. * Accepted baselines. Co-authored-by: Daniel Rosenwasser <[email protected]>
1 parent c0d5c29 commit 5f8a9e5

File tree

6 files changed

+969
-290
lines changed

6 files changed

+969
-290
lines changed

src/compiler/checker.ts

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23018,7 +23018,20 @@ namespace ts {
2301823018
}
2301923019
}
2302023020

23021-
function getFlowTypeOfReference(reference: Node, declaredType: Type, initialType = declaredType, isConstant?: boolean, flowContainer?: Node) {
23021+
function isConstantReference(node: Node): boolean {
23022+
switch (node.kind) {
23023+
case SyntaxKind.Identifier:
23024+
const symbol = getResolvedSymbol(node as Identifier);
23025+
return isConstVariable(symbol) || !!symbol.valueDeclaration && getRootDeclaration(symbol.valueDeclaration).kind === SyntaxKind.Parameter && !isParameterAssigned(symbol);
23026+
case SyntaxKind.PropertyAccessExpression:
23027+
case SyntaxKind.ElementAccessExpression:
23028+
// The resolvedSymbol property is initialized by checkPropertyAccess or checkElementAccess before we get here.
23029+
return isConstantReference((node as AccessExpression).expression) && isReadonlySymbol(getNodeLinks(node).resolvedSymbol || unknownSymbol);
23030+
}
23031+
return false;
23032+
}
23033+
23034+
function getFlowTypeOfReference(reference: Node, declaredType: Type, initialType = declaredType, flowContainer?: Node) {
2302223035
let key: string | undefined;
2302323036
let isKeySet = false;
2302423037
let flowDepth = 0;
@@ -24063,11 +24076,11 @@ namespace ts {
2406324076
case SyntaxKind.Identifier:
2406424077
// When narrowing a reference to a const variable, non-assigned parameter, or readonly property, we inline
2406524078
// up to five levels of aliased conditional expressions that are themselves declared as const variables.
24066-
if (isConstant && !isMatchingReference(reference, expr) && inlineLevel < 5) {
24079+
if (!isMatchingReference(reference, expr) && inlineLevel < 5) {
2406724080
const symbol = getResolvedSymbol(expr as Identifier);
2406824081
if (isConstVariable(symbol)) {
2406924082
const declaration = symbol.valueDeclaration;
24070-
if (declaration && isVariableDeclaration(declaration) && !declaration.type && declaration.initializer) {
24083+
if (declaration && isVariableDeclaration(declaration) && !declaration.type && declaration.initializer && isConstantReference(reference)) {
2407124084
inlineLevel++;
2407224085
const result = narrowType(type, declaration.initializer, assumeTrue);
2407324086
inlineLevel--;
@@ -24413,12 +24426,12 @@ namespace ts {
2441324426
const isOuterVariable = flowContainer !== declarationContainer;
2441424427
const isSpreadDestructuringAssignmentTarget = node.parent && node.parent.parent && isSpreadAssignment(node.parent) && isDestructuringAssignmentTarget(node.parent.parent);
2441524428
const isModuleExports = symbol.flags & SymbolFlags.ModuleExports;
24416-
const isConstant = isConstVariable(localOrExportSymbol) && getTypeOfSymbol(localOrExportSymbol) !== autoArrayType || isParameter && !isParameterAssigned(localOrExportSymbol);
2441724429
// When the control flow originates in a function expression or arrow function and we are referencing
2441824430
// a const variable or parameter from an outer function, we extend the origin of the control flow
2441924431
// analysis to include the immediately enclosing function.
24420-
while (isConstant && flowContainer !== declarationContainer && (flowContainer.kind === SyntaxKind.FunctionExpression ||
24421-
flowContainer.kind === SyntaxKind.ArrowFunction || isObjectLiteralOrClassExpressionMethod(flowContainer))) {
24432+
while (flowContainer !== declarationContainer && (flowContainer.kind === SyntaxKind.FunctionExpression ||
24433+
flowContainer.kind === SyntaxKind.ArrowFunction || isObjectLiteralOrClassExpressionMethod(flowContainer)) &&
24434+
(isConstVariable(localOrExportSymbol) && type !== autoArrayType || isParameter && !isParameterAssigned(localOrExportSymbol))) {
2442224435
flowContainer = getControlFlowContainer(flowContainer);
2442324436
}
2442424437
// We only look for uninitialized variables in strict null checking mode, and only when we can analyze
@@ -24433,7 +24446,7 @@ namespace ts {
2443324446
const initialType = assumeInitialized ? (isParameter ? removeOptionalityFromDeclaredType(type, declaration as VariableLikeDeclaration) : type) :
2443424447
type === autoType || type === autoArrayType ? undefinedType :
2443524448
getOptionalType(type);
24436-
const flowType = getFlowTypeOfReference(node, type, initialType, isConstant, flowContainer);
24449+
const flowType = getFlowTypeOfReference(node, type, initialType, flowContainer);
2443724450
// A variable is considered uninitialized when it is possible to analyze the entire control flow graph
2443824451
// from declaration to use, and when the variable's declared type doesn't include undefined but the
2443924452
// control flow based type does include undefined.
@@ -27652,7 +27665,7 @@ namespace ts {
2765227665
getControlFlowContainer(node) === getControlFlowContainer(prop.valueDeclaration)) {
2765327666
assumeUninitialized = true;
2765427667
}
27655-
const flowType = getFlowTypeOfReference(node, propType, assumeUninitialized ? getOptionalType(propType) : propType, prop && isReadonlySymbol(prop));
27668+
const flowType = getFlowTypeOfReference(node, propType, assumeUninitialized ? getOptionalType(propType) : propType);
2765627669
if (assumeUninitialized && !(getFalsyFlags(propType) & TypeFlags.Undefined) && getFalsyFlags(flowType) & TypeFlags.Undefined) {
2765727670
error(errorNode, Diagnostics.Property_0_is_used_before_being_assigned, symbolToString(prop!)); // TODO: GH#18217
2765827671
// Return the declared type to reduce follow-on errors

tests/baselines/reference/controlFlowAliasing.errors.txt

Lines changed: 98 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,40 @@
1-
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(61,13): error TS2339: Property 'foo' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
1+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(59,13): error TS2322: Type 'string | number' is not assignable to type 'string'.
2+
Type 'number' is not assignable to type 'string'.
3+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(74,13): error TS2322: Type 'string | number' is not assignable to type 'string'.
4+
Type 'number' is not assignable to type 'string'.
5+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(91,13): error TS2339: Property 'foo' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
26
Property 'foo' does not exist on type '{ kind: "bar"; bar: number; }'.
3-
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(64,13): error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
7+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(94,13): error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
48
Property 'bar' does not exist on type '{ kind: "foo"; foo: string; }'.
5-
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(71,13): error TS2339: Property 'foo' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
9+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(101,13): error TS2339: Property 'foo' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
610
Property 'foo' does not exist on type '{ kind: "bar"; bar: number; }'.
7-
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(74,13): error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
11+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(104,13): error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
812
Property 'bar' does not exist on type '{ kind: "foo"; foo: string; }'.
9-
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(82,13): error TS2339: Property 'foo' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
13+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(112,13): error TS2339: Property 'foo' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
1014
Property 'foo' does not exist on type '{ kind: "bar"; bar: number; }'.
11-
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(85,13): error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
15+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(115,13): error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
1216
Property 'bar' does not exist on type '{ kind: "foo"; foo: string; }'.
13-
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(104,13): error TS2339: Property 'foo' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
17+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(134,13): error TS2339: Property 'foo' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
1418
Property 'foo' does not exist on type '{ kind: "bar"; bar: number; }'.
15-
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(107,13): error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
19+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(137,13): error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
1620
Property 'bar' does not exist on type '{ kind: "foo"; foo: string; }'.
17-
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(124,19): error TS2339: Property 'foo' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
21+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(154,19): error TS2339: Property 'foo' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
1822
Property 'foo' does not exist on type '{ kind: "bar"; bar: number; }'.
19-
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(127,19): error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
23+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(157,19): error TS2339: Property 'bar' does not exist on type '{ kind: "foo"; foo: string; } | { kind: "bar"; bar: number; }'.
2024
Property 'bar' does not exist on type '{ kind: "foo"; foo: string; }'.
21-
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(207,13): error TS2322: Type 'string | number' is not assignable to type 'string'.
25+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(219,13): error TS2322: Type 'string | number' is not assignable to type 'string'.
26+
Type 'number' is not assignable to type 'string'.
27+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(232,13): error TS2322: Type 'string | number' is not assignable to type 'string'.
2228
Type 'number' is not assignable to type 'string'.
23-
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(210,13): error TS2322: Type 'string | number' is not assignable to type 'number'.
29+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(233,13): error TS2322: Type 'string | number' is not assignable to type 'string'.
30+
Type 'number' is not assignable to type 'string'.
31+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(267,13): error TS2322: Type 'string | number' is not assignable to type 'string'.
32+
Type 'number' is not assignable to type 'string'.
33+
tests/cases/conformance/controlFlow/controlFlowAliasing.ts(270,13): error TS2322: Type 'string | number' is not assignable to type 'number'.
2434
Type 'string' is not assignable to type 'number'.
2535

2636

27-
==== tests/cases/conformance/controlFlow/controlFlowAliasing.ts (12 errors) ====
37+
==== tests/cases/conformance/controlFlow/controlFlowAliasing.ts (17 errors) ====
2838
// Narrowing by aliased conditional expressions
2939

3040
function f10(x: string | number) {
@@ -72,6 +82,42 @@ tests/cases/conformance/controlFlow/controlFlowAliasing.ts(210,13): error TS2322
7282
return notUndefined ? x : 0;
7383
}
7484

85+
function f15(obj: { readonly x: string | number }) {
86+
const isString = typeof obj.x === 'string';
87+
if (isString) {
88+
let s: string = obj.x;
89+
}
90+
}
91+
92+
function f16(obj: { readonly x: string | number }) {
93+
const isString = typeof obj.x === 'string';
94+
obj = { x: 42 };
95+
if (isString) {
96+
let s: string = obj.x; // Not narrowed because of is assigned in function body
97+
~
98+
!!! error TS2322: Type 'string | number' is not assignable to type 'string'.
99+
!!! error TS2322: Type 'number' is not assignable to type 'string'.
100+
}
101+
}
102+
103+
function f17(obj: readonly [string | number]) {
104+
const isString = typeof obj[0] === 'string';
105+
if (isString) {
106+
let s: string = obj[0];
107+
}
108+
}
109+
110+
function f18(obj: readonly [string | number]) {
111+
const isString = typeof obj[0] === 'string';
112+
obj = [42];
113+
if (isString) {
114+
let s: string = obj[0]; // Not narrowed because of is assigned in function body
115+
~
116+
!!! error TS2322: Type 'string | number' is not assignable to type 'string'.
117+
!!! error TS2322: Type 'number' is not assignable to type 'string'.
118+
}
119+
}
120+
75121
function f20(obj: { kind: 'foo', foo: string } | { kind: 'bar', bar: number }) {
76122
const isFoo = obj.kind === 'foo';
77123
if (isFoo) {
@@ -236,6 +282,45 @@ tests/cases/conformance/controlFlow/controlFlowAliasing.ts(210,13): error TS2322
236282
}
237283
}
238284

285+
286+
class C10 {
287+
constructor(readonly x: string | number) {
288+
const thisX_isString = typeof this.x === 'string';
289+
const xIsString = typeof x === 'string';
290+
if (thisX_isString && xIsString) {
291+
let s: string;
292+
s = this.x;
293+
~
294+
!!! error TS2322: Type 'string | number' is not assignable to type 'string'.
295+
!!! error TS2322: Type 'number' is not assignable to type 'string'.
296+
s = x;
297+
}
298+
}
299+
}
300+
301+
class C11 {
302+
constructor(readonly x: string | number) {
303+
const thisX_isString = typeof this.x === 'string';
304+
const xIsString = typeof x === 'string';
305+
if (thisX_isString && xIsString) {
306+
// Some narrowings may be invalidated due to later assignments.
307+
let s: string;
308+
s = this.x;
309+
~
310+
!!! error TS2322: Type 'string | number' is not assignable to type 'string'.
311+
!!! error TS2322: Type 'number' is not assignable to type 'string'.
312+
s = x;
313+
~
314+
!!! error TS2322: Type 'string | number' is not assignable to type 'string'.
315+
!!! error TS2322: Type 'number' is not assignable to type 'string'.
316+
}
317+
else {
318+
this.x = 10;
319+
x = 10;
320+
}
321+
}
322+
}
323+
239324
// Mixing of aliased discriminants and conditionals
240325

241326
function f40(obj: { kind: 'foo', foo?: string } | { kind: 'bar', bar?: number }) {

0 commit comments

Comments
 (0)