Skip to content

Commit de6e6ab

Browse files
a-tarasyuka-tarasyuk
authored
fix(54465): Broken emit with private field in class decorator (microsoft#54679)
1 parent 39da6b1 commit de6e6ab

14 files changed

+361
-8
lines changed

‎src/compiler/checker.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ import {
248248
getCombinedModifierFlags,
249249
getCombinedNodeFlags,
250250
getContainingClass,
251+
getContainingClassExcludingClassDecorators,
251252
getContainingClassStaticBlock,
252253
getContainingFunction,
253254
getContainingFunctionOrClassStaticBlock,
@@ -31407,7 +31408,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
3140731408

3140831409
// Lookup the private identifier lexically.
3140931410
function lookupSymbolForPrivateIdentifierDeclaration(propName: __String, location: Node): Symbol | undefined {
31410-
for (let containingClass = getContainingClass(location); !!containingClass; containingClass = getContainingClass(containingClass)) {
31411+
for (let containingClass = getContainingClassExcludingClassDecorators(location); !!containingClass; containingClass = getContainingClass(containingClass)) {
3141131412
const { symbol } = containingClass;
3141231413
const name = getSymbolNameForPrivateIdentifier(symbol, propName);
3141331414
const prop = (symbol.members && symbol.members.get(name)) || (symbol.exports && symbol.exports.get(name));
@@ -31547,23 +31548,29 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
3154731548
if (assignmentKind && lexicallyScopedSymbol && lexicallyScopedSymbol.valueDeclaration && isMethodDeclaration(lexicallyScopedSymbol.valueDeclaration)) {
3154831549
grammarErrorOnNode(right, Diagnostics.Cannot_assign_to_private_method_0_Private_methods_are_not_writable, idText(right));
3154931550
}
31550-
3155131551
if (isAnyLike) {
3155231552
if (lexicallyScopedSymbol) {
3155331553
return isErrorType(apparentType) ? errorType : apparentType;
3155431554
}
31555-
if (!getContainingClass(right)) {
31555+
if (getContainingClassExcludingClassDecorators(right) === undefined) {
3155631556
grammarErrorOnNode(right, Diagnostics.Private_identifiers_are_not_allowed_outside_class_bodies);
3155731557
return anyType;
3155831558
}
3155931559
}
31560-
prop = lexicallyScopedSymbol ? getPrivateIdentifierPropertyOfType(leftType, lexicallyScopedSymbol) : undefined;
31561-
// Check for private-identifier-specific shadowing and lexical-scoping errors.
31562-
if (!prop && checkPrivateIdentifierPropertyAccess(leftType, right, lexicallyScopedSymbol)) {
31563-
return errorType;
31560+
31561+
prop = lexicallyScopedSymbol && getPrivateIdentifierPropertyOfType(leftType, lexicallyScopedSymbol);
31562+
if (prop === undefined) {
31563+
// Check for private-identifier-specific shadowing and lexical-scoping errors.
31564+
if (checkPrivateIdentifierPropertyAccess(leftType, right, lexicallyScopedSymbol)) {
31565+
return errorType;
31566+
}
31567+
const containingClass = getContainingClassExcludingClassDecorators(right);
31568+
if (containingClass && isPlainJsFile(getSourceFileOfNode(containingClass), compilerOptions.checkJs)) {
31569+
grammarErrorOnNode(right, Diagnostics.Private_field_0_must_be_declared_in_an_enclosing_class, idText(right));
31570+
}
3156431571
}
3156531572
else {
31566-
const isSetonlyAccessor = prop && prop.flags & SymbolFlags.SetAccessor && !(prop.flags & SymbolFlags.GetAccessor);
31573+
const isSetonlyAccessor = prop.flags & SymbolFlags.SetAccessor && !(prop.flags & SymbolFlags.GetAccessor);
3156731574
if (isSetonlyAccessor && assignmentKind !== AssignmentKind.Definite) {
3156831575
error(node, Diagnostics.Private_accessor_was_defined_without_a_getter);
3156931576
}

‎src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,10 @@
315315
"category": "Error",
316316
"code": 1110
317317
},
318+
"Private field '{0}' must be declared in an enclosing class.": {
319+
"category": "Error",
320+
"code": 1111
321+
},
318322
"A 'default' clause cannot appear more than once in a 'switch' statement.": {
319323
"category": "Error",
320324
"code": 1113

‎src/compiler/program.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,6 +1430,7 @@ export const plainJSErrors: Set<number> = new Set([
14301430
Diagnostics.Class_constructor_may_not_be_an_accessor.code,
14311431
Diagnostics.await_expressions_are_only_allowed_within_async_functions_and_at_the_top_levels_of_modules.code,
14321432
Diagnostics.await_using_statements_are_only_allowed_within_async_functions_and_at_the_top_levels_of_modules.code,
1433+
Diagnostics.Private_field_0_must_be_declared_in_an_enclosing_class.code,
14331434
// Type errors
14341435
Diagnostics.This_condition_will_always_return_0_since_JavaScript_compares_objects_by_reference_not_value.code,
14351436
]);

‎src/compiler/utilities.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2838,6 +2838,12 @@ export function getContainingFunctionOrClassStaticBlock(node: Node): SignatureDe
28382838
return findAncestor(node.parent, isFunctionLikeOrClassStaticBlockDeclaration);
28392839
}
28402840

2841+
/** @internal */
2842+
export function getContainingClassExcludingClassDecorators(node: Node): ClassLikeDeclaration | undefined {
2843+
const decorator = findAncestor(node.parent, n => isClassLike(n) ? "quit" : isDecorator(n));
2844+
return decorator && isClassLike(decorator.parent) ? getContainingClass(decorator.parent) : getContainingClass(decorator ?? node);
2845+
}
2846+
28412847
/** @internal */
28422848
export type ThisContainer =
28432849
| FunctionDeclaration

‎tests/baselines/reference/esDecorators-privateFieldAccess.errors.txt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
esDecorators-privateFieldAccess.ts(3,13): error TS18016: Private identifiers are not allowed outside class bodies.
2+
esDecorators-privateFieldAccess.ts(11,18): error TS18013: Property '#foo' is not accessible outside class 'B' because it has a private identifier.
3+
4+
5+
==== esDecorators-privateFieldAccess.ts (2 errors) ====
6+
declare let dec: any;
7+
8+
@dec(x => x.#foo) // error
9+
~~~~
10+
!!! error TS18016: Private identifiers are not allowed outside class bodies.
11+
class A {
12+
#foo = 3;
13+
14+
@dec(this, (x: A) => x.#foo) // ok
15+
m() {}
16+
}
17+
18+
@dec((x: B) => x.#foo) // error
19+
~~~~
20+
!!! error TS18013: Property '#foo' is not accessible outside class 'B' because it has a private identifier.
21+
class B {
22+
#foo = 3;
23+
}
24+
25+
class C {
26+
#foo = 2;
27+
m() {
28+
@dec(() => this.#foo) // ok
29+
class D {}
30+
return D;
31+
}
32+
}
33+

‎tests/baselines/reference/esDecorators-privateFieldAccess.js

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
//// [tests/cases/conformance/esDecorators/esDecorators-privateFieldAccess.ts] ////
2+
3+
//// [esDecorators-privateFieldAccess.ts]
4+
declare let dec: any;
5+
6+
@dec(x => x.#foo) // error
7+
class A {
8+
#foo = 3;
9+
10+
@dec(this, (x: A) => x.#foo) // ok
11+
m() {}
12+
}
13+
14+
@dec((x: B) => x.#foo) // error
15+
class B {
16+
#foo = 3;
17+
}
18+
19+
class C {
20+
#foo = 2;
21+
m() {
22+
@dec(() => this.#foo) // ok
23+
class D {}
24+
return D;
25+
}
26+
}
27+
28+
29+
//// [esDecorators-privateFieldAccess.js]
30+
@dec(x => x.#foo) // error
31+
class A {
32+
#foo = 3;
33+
@dec(this, (x) => x.#foo) // ok
34+
m() { }
35+
}
36+
@dec((x) => x.#foo) // error
37+
class B {
38+
#foo = 3;
39+
}
40+
class C {
41+
#foo = 2;
42+
m() {
43+
@dec(() => this.#foo) // ok
44+
class D {
45+
}
46+
return D;
47+
}
48+
}

‎tests/baselines/reference/esDecorators-privateFieldAccess.symbols

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
//// [tests/cases/conformance/esDecorators/esDecorators-privateFieldAccess.ts] ////
2+
3+
=== esDecorators-privateFieldAccess.ts ===
4+
declare let dec: any;
5+
>dec : Symbol(dec, Decl(esDecorators-privateFieldAccess.ts, 0, 11))
6+
7+
@dec(x => x.#foo) // error
8+
>dec : Symbol(dec, Decl(esDecorators-privateFieldAccess.ts, 0, 11))
9+
>x : Symbol(x, Decl(esDecorators-privateFieldAccess.ts, 2, 5))
10+
>x : Symbol(x, Decl(esDecorators-privateFieldAccess.ts, 2, 5))
11+
12+
class A {
13+
>A : Symbol(A, Decl(esDecorators-privateFieldAccess.ts, 0, 21))
14+
15+
#foo = 3;
16+
>#foo : Symbol(A.#foo, Decl(esDecorators-privateFieldAccess.ts, 3, 9))
17+
18+
@dec(this, (x: A) => x.#foo) // ok
19+
>dec : Symbol(dec, Decl(esDecorators-privateFieldAccess.ts, 0, 11))
20+
>this : Symbol(globalThis)
21+
>x : Symbol(x, Decl(esDecorators-privateFieldAccess.ts, 6, 16))
22+
>A : Symbol(A, Decl(esDecorators-privateFieldAccess.ts, 0, 21))
23+
>x.#foo : Symbol(A.#foo, Decl(esDecorators-privateFieldAccess.ts, 3, 9))
24+
>x : Symbol(x, Decl(esDecorators-privateFieldAccess.ts, 6, 16))
25+
26+
m() {}
27+
>m : Symbol(A.m, Decl(esDecorators-privateFieldAccess.ts, 4, 13))
28+
}
29+
30+
@dec((x: B) => x.#foo) // error
31+
>dec : Symbol(dec, Decl(esDecorators-privateFieldAccess.ts, 0, 11))
32+
>x : Symbol(x, Decl(esDecorators-privateFieldAccess.ts, 10, 6))
33+
>B : Symbol(B, Decl(esDecorators-privateFieldAccess.ts, 8, 1))
34+
>x : Symbol(x, Decl(esDecorators-privateFieldAccess.ts, 10, 6))
35+
36+
class B {
37+
>B : Symbol(B, Decl(esDecorators-privateFieldAccess.ts, 8, 1))
38+
39+
#foo = 3;
40+
>#foo : Symbol(B.#foo, Decl(esDecorators-privateFieldAccess.ts, 11, 9))
41+
}
42+
43+
class C {
44+
>C : Symbol(C, Decl(esDecorators-privateFieldAccess.ts, 13, 1))
45+
46+
#foo = 2;
47+
>#foo : Symbol(C.#foo, Decl(esDecorators-privateFieldAccess.ts, 15, 9))
48+
49+
m() {
50+
>m : Symbol(C.m, Decl(esDecorators-privateFieldAccess.ts, 16, 13))
51+
52+
@dec(() => this.#foo) // ok
53+
>dec : Symbol(dec, Decl(esDecorators-privateFieldAccess.ts, 0, 11))
54+
>this.#foo : Symbol(C.#foo, Decl(esDecorators-privateFieldAccess.ts, 15, 9))
55+
>this : Symbol(C, Decl(esDecorators-privateFieldAccess.ts, 13, 1))
56+
57+
class D {}
58+
>D : Symbol(D, Decl(esDecorators-privateFieldAccess.ts, 17, 9))
59+
60+
return D;
61+
>D : Symbol(D, Decl(esDecorators-privateFieldAccess.ts, 17, 9))
62+
}
63+
}
64+

‎tests/baselines/reference/esDecorators-privateFieldAccess.types

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
//// [tests/cases/conformance/esDecorators/esDecorators-privateFieldAccess.ts] ////
2+
3+
=== esDecorators-privateFieldAccess.ts ===
4+
declare let dec: any;
5+
>dec : any
6+
7+
@dec(x => x.#foo) // error
8+
>dec(x => x.#foo) : any
9+
>dec : any
10+
>x => x.#foo : (x: any) => any
11+
>x : any
12+
>x.#foo : any
13+
>x : any
14+
15+
class A {
16+
>A : A
17+
18+
#foo = 3;
19+
>#foo : number
20+
>3 : 3
21+
22+
@dec(this, (x: A) => x.#foo) // ok
23+
>dec(this, (x: A) => x.#foo) : any
24+
>dec : any
25+
>this : typeof globalThis
26+
>(x: A) => x.#foo : (x: A) => number
27+
>x : A
28+
>x.#foo : number
29+
>x : A
30+
31+
m() {}
32+
>m : () => void
33+
}
34+
35+
@dec((x: B) => x.#foo) // error
36+
>dec((x: B) => x.#foo) : any
37+
>dec : any
38+
>(x: B) => x.#foo : (x: B) => any
39+
>x : B
40+
>x.#foo : any
41+
>x : B
42+
43+
class B {
44+
>B : B
45+
46+
#foo = 3;
47+
>#foo : number
48+
>3 : 3
49+
}
50+
51+
class C {
52+
>C : C
53+
54+
#foo = 2;
55+
>#foo : number
56+
>2 : 2
57+
58+
m() {
59+
>m : () => typeof D
60+
61+
@dec(() => this.#foo) // ok
62+
>dec(() => this.#foo) : any
63+
>dec : any
64+
>() => this.#foo : () => number
65+
>this.#foo : number
66+
>this : this
67+
68+
class D {}
69+
>D : D
70+
71+
return D;
72+
>D : typeof D
73+
}
74+
}
75+

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
plainJSGrammarErrors4.js(5,14): error TS1111: Private field '#b' must be declared in an enclosing class.
2+
3+
4+
==== plainJSGrammarErrors4.js (1 errors) ====
5+
class A {
6+
#a;
7+
m() {
8+
this.#a; // ok
9+
this.#b; // error
10+
~~
11+
!!! error TS1111: Private field '#b' must be declared in an enclosing class.
12+
}
13+
}
14+

‎tests/baselines/reference/plainJSGrammarErrors4.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
//// [tests/cases/conformance/salsa/plainJSGrammarErrors4.ts] ////
2+
3+
//// [plainJSGrammarErrors4.js]
4+
class A {
5+
#a;
6+
m() {
7+
this.#a; // ok
8+
this.#b; // error
9+
}
10+
}
11+
12+
13+
//// [plainJSGrammarErrors4.js]
14+
class A {
15+
#a;
16+
m() {
17+
this.#a; // ok
18+
this.#b; // error
19+
}
20+
}

‎tests/baselines/reference/plainJSGrammarErrors4.symbols

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
//// [tests/cases/conformance/salsa/plainJSGrammarErrors4.ts] ////
2+
3+
=== plainJSGrammarErrors4.js ===
4+
class A {
5+
>A : Symbol(A, Decl(plainJSGrammarErrors4.js, 0, 0))
6+
7+
#a;
8+
>#a : Symbol(A.#a, Decl(plainJSGrammarErrors4.js, 0, 9))
9+
10+
m() {
11+
>m : Symbol(A.m, Decl(plainJSGrammarErrors4.js, 1, 7))
12+
13+
this.#a; // ok
14+
>this.#a : Symbol(A.#a, Decl(plainJSGrammarErrors4.js, 0, 9))
15+
>this : Symbol(A, Decl(plainJSGrammarErrors4.js, 0, 0))
16+
17+
this.#b; // error
18+
>this : Symbol(A, Decl(plainJSGrammarErrors4.js, 0, 0))
19+
}
20+
}
21+

‎tests/baselines/reference/plainJSGrammarErrors4.types

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
//// [tests/cases/conformance/salsa/plainJSGrammarErrors4.ts] ////
2+
3+
=== plainJSGrammarErrors4.js ===
4+
class A {
5+
>A : A
6+
7+
#a;
8+
>#a : any
9+
10+
m() {
11+
>m : () => void
12+
13+
this.#a; // ok
14+
>this.#a : any
15+
>this : this
16+
17+
this.#b; // error
18+
>this.#b : any
19+
>this : this
20+
}
21+
}
22+

0 commit comments

Comments
 (0)