Skip to content

Commit e5f8968

Browse files
authored
Port CFA for expando property access expressions (#4177)
1 parent 306f69d commit e5f8968

9 files changed

Lines changed: 131 additions & 139 deletions

internal/checker/checker.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11362,18 +11362,21 @@ func (c *Checker) getFlowTypeOfAccessExpression(node *ast.Node, prop *ast.Symbol
1136211362
// and if we are in a constructor of the same class as the property declaration, assume that
1136311363
// the property is uninitialized at the top of the control flow.
1136411364
assumeUninitialized := false
11365-
initialType := propType
1136611365
if c.strictNullChecks && prop != nil {
11367-
declaration := prop.ValueDeclaration
11368-
if declaration != nil && c.strictPropertyInitialization && ast.IsAccessExpression(node) && node.Expression().Kind == ast.KindThisKeyword && c.isPropertyWithoutInitializer(declaration) && !ast.IsStatic(declaration) {
11369-
flowContainer := c.getControlFlowContainer(node)
11370-
if ast.IsConstructorDeclaration(flowContainer) && flowContainer.Parent == declaration.Parent && declaration.Flags&ast.NodeFlagsAmbient == 0 {
11366+
if declaration := prop.ValueDeclaration; declaration != nil {
11367+
if c.strictPropertyInitialization && ast.IsAccessExpression(node) && node.Expression().Kind == ast.KindThisKeyword &&
11368+
c.isPropertyWithoutInitializer(declaration) && !ast.IsStatic(declaration) {
11369+
flowContainer := c.getControlFlowContainer(node)
11370+
if ast.IsConstructorDeclaration(flowContainer) && flowContainer.Parent == declaration.Parent && declaration.Flags&ast.NodeFlagsAmbient == 0 {
11371+
assumeUninitialized = true
11372+
}
11373+
} else if ast.IsBinaryExpression(declaration) && ast.IsPropertyAccessExpression(declaration.AsBinaryExpression().Left) &&
11374+
c.getControlFlowContainer(node) == c.getControlFlowContainer(declaration) {
1137111375
assumeUninitialized = true
11372-
initialType = c.getOptionalType(propType, false /*isProperty*/)
1137311376
}
1137411377
}
1137511378
}
11376-
flowType := c.getFlowTypeOfReferenceEx(node, propType, initialType, nil, nil)
11379+
flowType := c.getFlowTypeOfReferenceEx(node, propType, c.addOptionalityEx(propType, false /*isProperty*/, assumeUninitialized), nil, nil)
1137711380
if assumeUninitialized && !c.containsUndefinedType(propType) && c.containsUndefinedType(flowType) {
1137811381
c.error(errorNode, diagnostics.Property_0_is_used_before_being_assigned, c.symbolToString(prop))
1137911382
// Return the declared type to reduce follow-on errors

internal/checker/flow.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,13 @@ func (c *Checker) getTypeAtFlowAssignment(f *FlowState, flow *ast.FlowNode) Flow
256256
if !c.isReachableFlowNode(flow) {
257257
return FlowType{t: c.unreachableNeverType}
258258
}
259+
// A matching dotted name might also be an expando property on a function *expression*,
260+
// in which case we continue control flow analysis back to the function's declaration
261+
if ast.IsVariableDeclaration(node) && (ast.IsInJSFile(node) || ast.IsVarConstLike(node)) {
262+
if init := node.Initializer(); init != nil && ast.IsFunctionExpressionOrArrowFunction(init) {
263+
return c.getTypeAtFlowNode(f, flow.Antecedent)
264+
}
265+
}
259266
return FlowType{t: f.declaredType}
260267
}
261268
// for (const _ in ref) acts as a nonnull on ref
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
index.js(9,23): error TS2565: Property 'blah2' is used before being assigned.
2+
3+
4+
==== index.js (1 errors) ====
5+
class C {
6+
static blah1 = 123;
7+
}
8+
C.blah2 = 456;
9+
10+
class D extends C {
11+
static {
12+
console.log(super.blah1);
13+
console.log(super.blah2);
14+
~~~~~
15+
!!! error TS2565: Property 'blah2' is used before being assigned.
16+
}
17+
}
18+

testdata/baselines/reference/submodule/conformance/jsDeclarationsFunctionsCjs.errors.txt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
index.js(4,18): error TS2339: Property 'cat' does not exist on type '() => void'.
22
index.js(7,18): error TS2339: Property 'Cls' does not exist on type '() => void'.
33
index.js(31,18): error TS2339: Property 'self' does not exist on type '<T>(a: T) => T'.
4+
index.js(57,36): error TS2565: Property 'j' is used before being assigned.
45

56

6-
==== index.js (3 errors) ====
7+
==== index.js (4 errors) ====
78
module.exports.a = function a() {}
89

910
module.exports.b = function b() {}
@@ -67,5 +68,7 @@ index.js(31,18): error TS2339: Property 'self' does not exist on type '<T>(a: T)
6768

6869
// note that this last one doesn't make much sense in cjs, since exports aren't hoisted bindings
6970
module.exports.jj = module.exports.j;
71+
~
72+
!!! error TS2565: Property 'j' is used before being assigned.
7073
module.exports.j = function j() {}
7174

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
11
--- old.jsDeclarationsFunctionsCjs.errors.txt
22
+++ new.jsDeclarationsFunctionsCjs.errors.txt
33
@@= skipped -0, +0 lines =@@
4-
-index.js(57,36): error TS2565: Property 'j' is used before being assigned.
5-
-
6-
-
7-
-==== index.js (1 errors) ====
84
+index.js(4,18): error TS2339: Property 'cat' does not exist on type '() => void'.
95
+index.js(7,18): error TS2339: Property 'Cls' does not exist on type '() => void'.
106
+index.js(31,18): error TS2339: Property 'self' does not exist on type '<T>(a: T) => T'.
11-
+
12-
+
13-
+==== index.js (3 errors) ====
7+
index.js(57,36): error TS2565: Property 'j' is used before being assigned.
8+
9+
10+
-==== index.js (1 errors) ====
11+
+==== index.js (4 errors) ====
1412
module.exports.a = function a() {}
1513

1614
module.exports.b = function b() {}
@@ -25,20 +23,12 @@
2523

2624
/**
2725
* @param {number} a
28-
@@= skipped -32, +38 lines =@@
26+
@@= skipped -32, +39 lines =@@
2927
return a;
3028
}
3129
module.exports.f.self = module.exports.f;
3230
+ ~~~~
3331
+!!! error TS2339: Property 'self' does not exist on type '<T>(a: T) => T'.
3432

3533
/**
36-
* @param {{x: string}} a
37-
@@= skipped -26, +28 lines =@@
38-
39-
// note that this last one doesn't make much sense in cjs, since exports aren't hoisted bindings
40-
module.exports.jj = module.exports.j;
41-
- ~
42-
-!!! error TS2565: Property 'j' is used before being assigned.
43-
module.exports.j = function j() {}
44-
34+
* @param {{x: string}} a
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
typeFromPropertyAssignment36.ts(11,7): error TS2565: Property 'q' is used before being assigned.
2+
typeFromPropertyAssignment36.ts(42,3): error TS2565: Property 'q' is used before being assigned.
3+
typeFromPropertyAssignment36.ts(64,3): error TS2565: Property 'expando' is used before being assigned.
4+
5+
6+
==== typeFromPropertyAssignment36.ts (3 errors) ====
7+
function f(b: boolean) {
8+
function d() {
9+
}
10+
d.e = 12
11+
d.e
12+
13+
if (b) {
14+
d.q = false
15+
}
16+
// error d.q might not be assigned
17+
d.q
18+
~
19+
!!! error TS2565: Property 'q' is used before being assigned.
20+
if (b) {
21+
d.q = false
22+
}
23+
else {
24+
d.q = true
25+
}
26+
d.q
27+
if (b) {
28+
d.r = 1
29+
}
30+
else {
31+
d.r = 2
32+
}
33+
d.r
34+
if (b) {
35+
d.s = 'hi'
36+
}
37+
return d
38+
}
39+
// OK to access possibly-unassigned properties outside the initialising scope
40+
var test = f(true).s
41+
42+
function d() {
43+
}
44+
d.e = 12
45+
d.e
46+
47+
if (!!false) {
48+
d.q = false
49+
}
50+
d.q
51+
~
52+
!!! error TS2565: Property 'q' is used before being assigned.
53+
if (!!false) {
54+
d.q = false
55+
}
56+
else {
57+
d.q = true
58+
}
59+
d.q
60+
if (!!false) {
61+
d.r = 1
62+
}
63+
else {
64+
d.r = 2
65+
}
66+
d.r
67+
68+
// test function expressions too
69+
const g = function() {
70+
}
71+
if (!!false) {
72+
g.expando = 1
73+
}
74+
g.expando // error
75+
~~~~~~~
76+
!!! error TS2565: Property 'expando' is used before being assigned.
77+
78+
if (!!false) {
79+
g.both = 'hi'
80+
}
81+
else {
82+
g.both = 0
83+
}
84+
g.both
85+

testdata/baselines/reference/submodule/conformance/typeFromPropertyAssignment36.errors.txt.diff

Lines changed: 0 additions & 89 deletions
This file was deleted.

testdata/baselines/reference/submoduleAccepted/compiler/classFieldSuperAccessibleJs1.errors.txt.diff

Lines changed: 0 additions & 22 deletions
This file was deleted.

testdata/submoduleAccepted.txt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -463,9 +463,6 @@ compiler/invalidUnicodeEscapeSequance2.errors.txt.diff
463463
compiler/invalidUnicodeEscapeSequance3.errors.txt.diff
464464
compiler/invalidUnicodeEscapeSequance4.errors.txt.diff
465465

466-
## wrong errors ##
467-
compiler/classFieldSuperAccessibleJs1.errors.txt.diff
468-
469466
## deprecated errors ##
470467
compiler/declarationFileNoCrashOnExtraExportModifier.errors.txt.diff
471468
compiler/declarationFileNoCrashOnExtraExportModifier.symbols.diff

0 commit comments

Comments
 (0)