Skip to content

Commit d18e82b

Browse files
softwareCobblersoftwareCobbler
authored
fix for 45006 (microsoft#45020)
* fix for 45006 * treat setters like getters in preceding commit; move test accordingly * fix test baselines * changes per code review in `getContainerFlags`, move cases for get/set accessors to fallthrough into the block that currently handles MethodDeclaration; so get/set accessors and method declarations all get the same container flags, such that during `bindContainer`, `startFlow.node` is assigned to getters/accessors (this changes a public api in tsserverlibrary.d.ts and typescript.d.ts by adding `GetAccessorDeclaration` and `SetAccessorDeclaration` to the type of `FlowStart.node`) consolidate predicates checking whether a node is either a get or set accessor, into `isObjectLiteralOrClassExpressionMethodOrAccessor` (formerly `isObjectLiteralOrClassExpressionMethod`) annotate updated test with `@target: es2020` * fix `isObjectLiteralOrClassExpressionMethodOrAccessor` require that Getter/Setters are parented by an ObjectLiteralExpression or ClassExpression
1 parent 65ed412 commit d18e82b

11 files changed

+144
-54
lines changed

Diff for: ‎src/compiler/binder.ts

+8-8
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ namespace ts {
163163
IsFunctionExpression = 1 << 4,
164164
HasLocals = 1 << 5,
165165
IsInterface = 1 << 6,
166-
IsObjectLiteralOrClassExpressionMethod = 1 << 7,
166+
IsObjectLiteralOrClassExpressionMethodOrAccessor = 1 << 7,
167167
}
168168

169169
function initFlowNode<T extends FlowNode>(node: T) {
@@ -663,8 +663,8 @@ namespace ts {
663663
// similarly to break statements that exit to a label just past the statement body.
664664
if (!isIIFE) {
665665
currentFlow = initFlowNode({ flags: FlowFlags.Start });
666-
if (containerFlags & (ContainerFlags.IsFunctionExpression | ContainerFlags.IsObjectLiteralOrClassExpressionMethod)) {
667-
currentFlow.node = node as FunctionExpression | ArrowFunction | MethodDeclaration;
666+
if (containerFlags & (ContainerFlags.IsFunctionExpression | ContainerFlags.IsObjectLiteralOrClassExpressionMethodOrAccessor)) {
667+
currentFlow.node = node as FunctionExpression | ArrowFunction | MethodDeclaration | GetAccessorDeclaration | SetAccessorDeclaration;
668668
}
669669
}
670670
// We create a return control flow graph for IIFEs and constructors. For constructors
@@ -1815,16 +1815,16 @@ namespace ts {
18151815
case SyntaxKind.SourceFile:
18161816
return ContainerFlags.IsContainer | ContainerFlags.IsControlFlowContainer | ContainerFlags.HasLocals;
18171817

1818+
case SyntaxKind.GetAccessor:
1819+
case SyntaxKind.SetAccessor:
18181820
case SyntaxKind.MethodDeclaration:
1819-
if (isObjectLiteralOrClassExpressionMethod(node)) {
1820-
return ContainerFlags.IsContainer | ContainerFlags.IsControlFlowContainer | ContainerFlags.HasLocals | ContainerFlags.IsFunctionLike | ContainerFlags.IsObjectLiteralOrClassExpressionMethod;
1821+
if (isObjectLiteralOrClassExpressionMethodOrAccessor(node)) {
1822+
return ContainerFlags.IsContainer | ContainerFlags.IsControlFlowContainer | ContainerFlags.HasLocals | ContainerFlags.IsFunctionLike | ContainerFlags.IsObjectLiteralOrClassExpressionMethodOrAccessor;
18211823
}
18221824
// falls through
18231825
case SyntaxKind.Constructor:
18241826
case SyntaxKind.FunctionDeclaration:
18251827
case SyntaxKind.MethodSignature:
1826-
case SyntaxKind.GetAccessor:
1827-
case SyntaxKind.SetAccessor:
18281828
case SyntaxKind.CallSignature:
18291829
case SyntaxKind.JSDocSignature:
18301830
case SyntaxKind.JSDocFunctionType:
@@ -3372,7 +3372,7 @@ namespace ts {
33723372
emitFlags |= NodeFlags.HasAsyncFunctions;
33733373
}
33743374

3375-
if (currentFlow && isObjectLiteralOrClassExpressionMethod(node)) {
3375+
if (currentFlow && isObjectLiteralOrClassExpressionMethodOrAccessor(node)) {
33763376
node.flowNode = currentFlow;
33773377
}
33783378

Diff for: ‎src/compiler/checker.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -24492,7 +24492,7 @@ namespace ts {
2449224492
// a const variable or parameter from an outer function, we extend the origin of the control flow
2449324493
// analysis to include the immediately enclosing function.
2449424494
while (flowContainer !== declarationContainer && (flowContainer.kind === SyntaxKind.FunctionExpression ||
24495-
flowContainer.kind === SyntaxKind.ArrowFunction || isObjectLiteralOrClassExpressionMethod(flowContainer)) &&
24495+
flowContainer.kind === SyntaxKind.ArrowFunction || isObjectLiteralOrClassExpressionMethodOrAccessor(flowContainer)) &&
2449624496
(isConstVariable(localOrExportSymbol) && type !== autoArrayType || isParameter && !isParameterAssigned(localOrExportSymbol))) {
2449724497
flowContainer = getControlFlowContainer(flowContainer);
2449824498
}

Diff for: ‎src/compiler/types.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -3410,7 +3410,7 @@ namespace ts {
34103410
// function, the node property references the function (which in turn has a flowNode
34113411
// property for the containing control flow).
34123412
export interface FlowStart extends FlowNodeBase {
3413-
node?: FunctionExpression | ArrowFunction | MethodDeclaration;
3413+
node?: FunctionExpression | ArrowFunction | MethodDeclaration | GetAccessorDeclaration | SetAccessorDeclaration;
34143414
}
34153415

34163416
// FlowLabel represents a junction with multiple possible preceding control flows.

Diff for: ‎src/compiler/utilities.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1526,8 +1526,8 @@ namespace ts {
15261526
return node && node.kind === SyntaxKind.MethodDeclaration && node.parent.kind === SyntaxKind.ObjectLiteralExpression;
15271527
}
15281528

1529-
export function isObjectLiteralOrClassExpressionMethod(node: Node): node is MethodDeclaration {
1530-
return node.kind === SyntaxKind.MethodDeclaration &&
1529+
export function isObjectLiteralOrClassExpressionMethodOrAccessor(node: Node): node is MethodDeclaration {
1530+
return (node.kind === SyntaxKind.MethodDeclaration || node.kind === SyntaxKind.GetAccessor || node.kind === SyntaxKind.SetAccessor) &&
15311531
(node.parent.kind === SyntaxKind.ObjectLiteralExpression ||
15321532
node.parent.kind === SyntaxKind.ClassExpression);
15331533
}

Diff for: ‎tests/baselines/reference/api/tsserverlibrary.d.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1925,7 +1925,7 @@ declare namespace ts {
19251925
id?: number;
19261926
}
19271927
export interface FlowStart extends FlowNodeBase {
1928-
node?: FunctionExpression | ArrowFunction | MethodDeclaration;
1928+
node?: FunctionExpression | ArrowFunction | MethodDeclaration | GetAccessorDeclaration | SetAccessorDeclaration;
19291929
}
19301930
export interface FlowLabel extends FlowNodeBase {
19311931
antecedents: FlowNode[] | undefined;

Diff for: ‎tests/baselines/reference/api/typescript.d.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1925,7 +1925,7 @@ declare namespace ts {
19251925
id?: number;
19261926
}
19271927
export interface FlowStart extends FlowNodeBase {
1928-
node?: FunctionExpression | ArrowFunction | MethodDeclaration;
1928+
node?: FunctionExpression | ArrowFunction | MethodDeclaration | GetAccessorDeclaration | SetAccessorDeclaration;
19291929
}
19301930
export interface FlowLabel extends FlowNodeBase {
19311931
antecedents: FlowNode[] | undefined;

Diff for: ‎tests/baselines/reference/gettersAndSetters.errors.txt

+11-19
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,19 @@
1-
tests/cases/compiler/gettersAndSetters.ts(7,16): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
2-
tests/cases/compiler/gettersAndSetters.ts(8,16): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
3-
tests/cases/compiler/gettersAndSetters.ts(10,16): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
4-
tests/cases/compiler/gettersAndSetters.ts(11,16): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
51
tests/cases/compiler/gettersAndSetters.ts(25,13): error TS2339: Property 'Baz' does not exist on type 'C'.
62
tests/cases/compiler/gettersAndSetters.ts(26,3): error TS2339: Property 'Baz' does not exist on type 'C'.
7-
tests/cases/compiler/gettersAndSetters.ts(29,30): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
8-
tests/cases/compiler/gettersAndSetters.ts(29,53): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
93

104

11-
==== tests/cases/compiler/gettersAndSetters.ts (8 errors) ====
5+
==== tests/cases/compiler/gettersAndSetters.ts (2 errors) ====
126
// classes
137
class C {
148
public fooBack = "";
159
static barBack:string = "";
1610
public bazBack = "";
1711

1812
public get Foo() { return this.fooBack;} // ok
19-
~~~
20-
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
2113
public set Foo(foo:string) {this.fooBack = foo;} // ok
22-
~~~
23-
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
2414

2515
static get Bar() {return C.barBack;} // ok
26-
~~~
27-
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
2816
static set Bar(bar:string) {C.barBack = bar;} // ok
29-
~~~
30-
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
3117

3218
public get = function() {} // ok
3319
public set = function() {} // ok
@@ -50,10 +36,6 @@ tests/cases/compiler/gettersAndSetters.ts(29,53): error TS1056: Accessors are on
5036

5137
// The Foo accessors' return and param types should be contextually typed to the Foo field
5238
var o : {Foo:number;} = {get Foo() {return 0;}, set Foo(val:number){val}}; // o
53-
~~~
54-
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
55-
~~~
56-
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
5739

5840
var ofg = o.Foo;
5941
o.Foo = 0;
@@ -64,4 +46,14 @@ tests/cases/compiler/gettersAndSetters.ts(29,53): error TS1056: Accessors are on
6446
}
6547

6648
var i:I1 = function (n) {return n;}
49+
50+
// Repro from #45006
51+
const x: string | number = Math.random() < 0.5 ? "str" : 123;
52+
if (typeof x === "string") {
53+
let obj = {
54+
set prop(_: any) { x.toUpperCase(); },
55+
get prop() { return x.toUpperCase() },
56+
method() { return x.toUpperCase() }
57+
}
58+
}
6759

Diff for: ‎tests/baselines/reference/gettersAndSetters.js

+27-21
Original file line numberDiff line numberDiff line change
@@ -38,36 +38,33 @@ interface I1 {
3838
}
3939

4040
var i:I1 = function (n) {return n;}
41+
42+
// Repro from #45006
43+
const x: string | number = Math.random() < 0.5 ? "str" : 123;
44+
if (typeof x === "string") {
45+
let obj = {
46+
set prop(_: any) { x.toUpperCase(); },
47+
get prop() { return x.toUpperCase() },
48+
method() { return x.toUpperCase() }
49+
}
50+
}
4151

4252

4353
//// [gettersAndSetters.js]
4454
// classes
45-
var C = /** @class */ (function () {
46-
function C() {
55+
class C {
56+
constructor() {
4757
this.fooBack = "";
4858
this.bazBack = "";
4959
this.get = function () { }; // ok
5060
this.set = function () { }; // ok
5161
}
52-
Object.defineProperty(C.prototype, "Foo", {
53-
get: function () { return this.fooBack; } // ok
54-
,
55-
set: function (foo) { this.fooBack = foo; } // ok
56-
,
57-
enumerable: false,
58-
configurable: true
59-
});
60-
Object.defineProperty(C, "Bar", {
61-
get: function () { return C.barBack; } // ok
62-
,
63-
set: function (bar) { C.barBack = bar; } // ok
64-
,
65-
enumerable: false,
66-
configurable: true
67-
});
68-
C.barBack = "";
69-
return C;
70-
}());
62+
get Foo() { return this.fooBack; } // ok
63+
set Foo(foo) { this.fooBack = foo; } // ok
64+
static get Bar() { return C.barBack; } // ok
65+
static set Bar(bar) { C.barBack = bar; } // ok
66+
}
67+
C.barBack = "";
7168
var c = new C();
7269
var foo = c.Foo;
7370
c.Foo = "foov";
@@ -80,3 +77,12 @@ var o = { get Foo() { return 0; }, set Foo(val) { val; } }; // o
8077
var ofg = o.Foo;
8178
o.Foo = 0;
8279
var i = function (n) { return n; };
80+
// Repro from #45006
81+
const x = Math.random() < 0.5 ? "str" : 123;
82+
if (typeof x === "string") {
83+
let obj = {
84+
set prop(_) { x.toUpperCase(); },
85+
get prop() { return x.toUpperCase(); },
86+
method() { return x.toUpperCase(); }
87+
};
88+
}

Diff for: ‎tests/baselines/reference/gettersAndSetters.symbols

+34
Original file line numberDiff line numberDiff line change
@@ -114,3 +114,37 @@ var i:I1 = function (n) {return n;}
114114
>n : Symbol(n, Decl(gettersAndSetters.ts, 38, 21))
115115
>n : Symbol(n, Decl(gettersAndSetters.ts, 38, 21))
116116

117+
// Repro from #45006
118+
const x: string | number = Math.random() < 0.5 ? "str" : 123;
119+
>x : Symbol(x, Decl(gettersAndSetters.ts, 41, 5))
120+
>Math.random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --))
121+
>Math : Symbol(Math, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.core.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --))
122+
>random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --))
123+
124+
if (typeof x === "string") {
125+
>x : Symbol(x, Decl(gettersAndSetters.ts, 41, 5))
126+
127+
let obj = {
128+
>obj : Symbol(obj, Decl(gettersAndSetters.ts, 43, 5))
129+
130+
set prop(_: any) { x.toUpperCase(); },
131+
>prop : Symbol(prop, Decl(gettersAndSetters.ts, 43, 13), Decl(gettersAndSetters.ts, 44, 42))
132+
>_ : Symbol(_, Decl(gettersAndSetters.ts, 44, 13))
133+
>x.toUpperCase : Symbol(String.toUpperCase, Decl(lib.es5.d.ts, --, --))
134+
>x : Symbol(x, Decl(gettersAndSetters.ts, 41, 5))
135+
>toUpperCase : Symbol(String.toUpperCase, Decl(lib.es5.d.ts, --, --))
136+
137+
get prop() { return x.toUpperCase() },
138+
>prop : Symbol(prop, Decl(gettersAndSetters.ts, 43, 13), Decl(gettersAndSetters.ts, 44, 42))
139+
>x.toUpperCase : Symbol(String.toUpperCase, Decl(lib.es5.d.ts, --, --))
140+
>x : Symbol(x, Decl(gettersAndSetters.ts, 41, 5))
141+
>toUpperCase : Symbol(String.toUpperCase, Decl(lib.es5.d.ts, --, --))
142+
143+
method() { return x.toUpperCase() }
144+
>method : Symbol(method, Decl(gettersAndSetters.ts, 45, 42))
145+
>x.toUpperCase : Symbol(String.toUpperCase, Decl(lib.es5.d.ts, --, --))
146+
>x : Symbol(x, Decl(gettersAndSetters.ts, 41, 5))
147+
>toUpperCase : Symbol(String.toUpperCase, Decl(lib.es5.d.ts, --, --))
148+
}
149+
}
150+

Diff for: ‎tests/baselines/reference/gettersAndSetters.types

+47
Original file line numberDiff line numberDiff line change
@@ -134,3 +134,50 @@ var i:I1 = function (n) {return n;}
134134
>n : number
135135
>n : number
136136

137+
// Repro from #45006
138+
const x: string | number = Math.random() < 0.5 ? "str" : 123;
139+
>x : string | number
140+
>Math.random() < 0.5 ? "str" : 123 : "str" | 123
141+
>Math.random() < 0.5 : boolean
142+
>Math.random() : number
143+
>Math.random : () => number
144+
>Math : Math
145+
>random : () => number
146+
>0.5 : 0.5
147+
>"str" : "str"
148+
>123 : 123
149+
150+
if (typeof x === "string") {
151+
>typeof x === "string" : boolean
152+
>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
153+
>x : string | number
154+
>"string" : "string"
155+
156+
let obj = {
157+
>obj : { prop: any; method(): string; }
158+
>{ set prop(_: any) { x.toUpperCase(); }, get prop() { return x.toUpperCase() }, method() { return x.toUpperCase() } } : { prop: any; method(): string; }
159+
160+
set prop(_: any) { x.toUpperCase(); },
161+
>prop : any
162+
>_ : any
163+
>x.toUpperCase() : string
164+
>x.toUpperCase : () => string
165+
>x : string
166+
>toUpperCase : () => string
167+
168+
get prop() { return x.toUpperCase() },
169+
>prop : any
170+
>x.toUpperCase() : string
171+
>x.toUpperCase : () => string
172+
>x : string
173+
>toUpperCase : () => string
174+
175+
method() { return x.toUpperCase() }
176+
>method : () => string
177+
>x.toUpperCase() : string
178+
>x.toUpperCase : () => string
179+
>x : string
180+
>toUpperCase : () => string
181+
}
182+
}
183+

Diff for: ‎tests/cases/compiler/gettersAndSetters.ts

+11
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// @target: es2020
12
// classes
23
class C {
34
public fooBack = "";
@@ -37,3 +38,13 @@ interface I1 {
3738
}
3839

3940
var i:I1 = function (n) {return n;}
41+
42+
// Repro from #45006
43+
const x: string | number = Math.random() < 0.5 ? "str" : 123;
44+
if (typeof x === "string") {
45+
let obj = {
46+
set prop(_: any) { x.toUpperCase(); },
47+
get prop() { return x.toUpperCase() },
48+
method() { return x.toUpperCase() }
49+
}
50+
}

0 commit comments

Comments
 (0)