From de6e6abbac4da0d53b8a9d6f4d2a02d0a37631db Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Sat, 24 Jun 2023 00:09:48 +0300 Subject: [PATCH] fix(54465): Broken emit with private field in class decorator (#54679) --- src/compiler/checker.ts | 23 ++++-- src/compiler/diagnosticMessages.json | 4 + src/compiler/program.ts | 1 + src/compiler/utilities.ts | 6 ++ ...esDecorators-privateFieldAccess.errors.txt | 33 ++++++++ .../esDecorators-privateFieldAccess.js | 48 ++++++++++++ .../esDecorators-privateFieldAccess.symbols | 64 ++++++++++++++++ .../esDecorators-privateFieldAccess.types | 75 +++++++++++++++++++ .../plainJSGrammarErrors4.errors.txt | 14 ++++ .../reference/plainJSGrammarErrors4.js | 20 +++++ .../reference/plainJSGrammarErrors4.symbols | 21 ++++++ .../reference/plainJSGrammarErrors4.types | 22 ++++++ .../esDecorators-privateFieldAccess.ts | 26 +++++++ .../salsa/plainJSGrammarErrors4.ts | 12 +++ 14 files changed, 361 insertions(+), 8 deletions(-) create mode 100644 tests/baselines/reference/esDecorators-privateFieldAccess.errors.txt create mode 100644 tests/baselines/reference/esDecorators-privateFieldAccess.js create mode 100644 tests/baselines/reference/esDecorators-privateFieldAccess.symbols create mode 100644 tests/baselines/reference/esDecorators-privateFieldAccess.types create mode 100644 tests/baselines/reference/plainJSGrammarErrors4.errors.txt create mode 100644 tests/baselines/reference/plainJSGrammarErrors4.js create mode 100644 tests/baselines/reference/plainJSGrammarErrors4.symbols create mode 100644 tests/baselines/reference/plainJSGrammarErrors4.types create mode 100644 tests/cases/conformance/esDecorators/esDecorators-privateFieldAccess.ts create mode 100644 tests/cases/conformance/salsa/plainJSGrammarErrors4.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 65282f83a11..ee86ae50306 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -248,6 +248,7 @@ import { getCombinedModifierFlags, getCombinedNodeFlags, getContainingClass, + getContainingClassExcludingClassDecorators, getContainingClassStaticBlock, getContainingFunction, getContainingFunctionOrClassStaticBlock, @@ -31407,7 +31408,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { // Lookup the private identifier lexically. function lookupSymbolForPrivateIdentifierDeclaration(propName: __String, location: Node): Symbol | undefined { - for (let containingClass = getContainingClass(location); !!containingClass; containingClass = getContainingClass(containingClass)) { + for (let containingClass = getContainingClassExcludingClassDecorators(location); !!containingClass; containingClass = getContainingClass(containingClass)) { const { symbol } = containingClass; const name = getSymbolNameForPrivateIdentifier(symbol, propName); const prop = (symbol.members && symbol.members.get(name)) || (symbol.exports && symbol.exports.get(name)); @@ -31547,23 +31548,29 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { if (assignmentKind && lexicallyScopedSymbol && lexicallyScopedSymbol.valueDeclaration && isMethodDeclaration(lexicallyScopedSymbol.valueDeclaration)) { grammarErrorOnNode(right, Diagnostics.Cannot_assign_to_private_method_0_Private_methods_are_not_writable, idText(right)); } - if (isAnyLike) { if (lexicallyScopedSymbol) { return isErrorType(apparentType) ? errorType : apparentType; } - if (!getContainingClass(right)) { + if (getContainingClassExcludingClassDecorators(right) === undefined) { grammarErrorOnNode(right, Diagnostics.Private_identifiers_are_not_allowed_outside_class_bodies); return anyType; } } - prop = lexicallyScopedSymbol ? getPrivateIdentifierPropertyOfType(leftType, lexicallyScopedSymbol) : undefined; - // Check for private-identifier-specific shadowing and lexical-scoping errors. - if (!prop && checkPrivateIdentifierPropertyAccess(leftType, right, lexicallyScopedSymbol)) { - return errorType; + + prop = lexicallyScopedSymbol && getPrivateIdentifierPropertyOfType(leftType, lexicallyScopedSymbol); + if (prop === undefined) { + // Check for private-identifier-specific shadowing and lexical-scoping errors. + if (checkPrivateIdentifierPropertyAccess(leftType, right, lexicallyScopedSymbol)) { + return errorType; + } + const containingClass = getContainingClassExcludingClassDecorators(right); + if (containingClass && isPlainJsFile(getSourceFileOfNode(containingClass), compilerOptions.checkJs)) { + grammarErrorOnNode(right, Diagnostics.Private_field_0_must_be_declared_in_an_enclosing_class, idText(right)); + } } else { - const isSetonlyAccessor = prop && prop.flags & SymbolFlags.SetAccessor && !(prop.flags & SymbolFlags.GetAccessor); + const isSetonlyAccessor = prop.flags & SymbolFlags.SetAccessor && !(prop.flags & SymbolFlags.GetAccessor); if (isSetonlyAccessor && assignmentKind !== AssignmentKind.Definite) { error(node, Diagnostics.Private_accessor_was_defined_without_a_getter); } diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index bda674e9666..0274bb7ece2 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -315,6 +315,10 @@ "category": "Error", "code": 1110 }, + "Private field '{0}' must be declared in an enclosing class.": { + "category": "Error", + "code": 1111 + }, "A 'default' clause cannot appear more than once in a 'switch' statement.": { "category": "Error", "code": 1113 diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 36571405cbe..a61e46e3565 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -1430,6 +1430,7 @@ export const plainJSErrors: Set = new Set([ Diagnostics.Class_constructor_may_not_be_an_accessor.code, Diagnostics.await_expressions_are_only_allowed_within_async_functions_and_at_the_top_levels_of_modules.code, Diagnostics.await_using_statements_are_only_allowed_within_async_functions_and_at_the_top_levels_of_modules.code, + Diagnostics.Private_field_0_must_be_declared_in_an_enclosing_class.code, // Type errors Diagnostics.This_condition_will_always_return_0_since_JavaScript_compares_objects_by_reference_not_value.code, ]); diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 44a62be7e3a..b8504830383 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -2838,6 +2838,12 @@ export function getContainingFunctionOrClassStaticBlock(node: Node): SignatureDe return findAncestor(node.parent, isFunctionLikeOrClassStaticBlockDeclaration); } +/** @internal */ +export function getContainingClassExcludingClassDecorators(node: Node): ClassLikeDeclaration | undefined { + const decorator = findAncestor(node.parent, n => isClassLike(n) ? "quit" : isDecorator(n)); + return decorator && isClassLike(decorator.parent) ? getContainingClass(decorator.parent) : getContainingClass(decorator ?? node); +} + /** @internal */ export type ThisContainer = | FunctionDeclaration diff --git a/tests/baselines/reference/esDecorators-privateFieldAccess.errors.txt b/tests/baselines/reference/esDecorators-privateFieldAccess.errors.txt new file mode 100644 index 00000000000..e999cde746c --- /dev/null +++ b/tests/baselines/reference/esDecorators-privateFieldAccess.errors.txt @@ -0,0 +1,33 @@ +esDecorators-privateFieldAccess.ts(3,13): error TS18016: Private identifiers are not allowed outside class bodies. +esDecorators-privateFieldAccess.ts(11,18): error TS18013: Property '#foo' is not accessible outside class 'B' because it has a private identifier. + + +==== esDecorators-privateFieldAccess.ts (2 errors) ==== + declare let dec: any; + + @dec(x => x.#foo) // error + ~~~~ +!!! error TS18016: Private identifiers are not allowed outside class bodies. + class A { + #foo = 3; + + @dec(this, (x: A) => x.#foo) // ok + m() {} + } + + @dec((x: B) => x.#foo) // error + ~~~~ +!!! error TS18013: Property '#foo' is not accessible outside class 'B' because it has a private identifier. + class B { + #foo = 3; + } + + class C { + #foo = 2; + m() { + @dec(() => this.#foo) // ok + class D {} + return D; + } + } + \ No newline at end of file diff --git a/tests/baselines/reference/esDecorators-privateFieldAccess.js b/tests/baselines/reference/esDecorators-privateFieldAccess.js new file mode 100644 index 00000000000..d8cf63be70e --- /dev/null +++ b/tests/baselines/reference/esDecorators-privateFieldAccess.js @@ -0,0 +1,48 @@ +//// [tests/cases/conformance/esDecorators/esDecorators-privateFieldAccess.ts] //// + +//// [esDecorators-privateFieldAccess.ts] +declare let dec: any; + +@dec(x => x.#foo) // error +class A { + #foo = 3; + + @dec(this, (x: A) => x.#foo) // ok + m() {} +} + +@dec((x: B) => x.#foo) // error +class B { + #foo = 3; +} + +class C { + #foo = 2; + m() { + @dec(() => this.#foo) // ok + class D {} + return D; + } +} + + +//// [esDecorators-privateFieldAccess.js] +@dec(x => x.#foo) // error +class A { + #foo = 3; + @dec(this, (x) => x.#foo) // ok + m() { } +} +@dec((x) => x.#foo) // error +class B { + #foo = 3; +} +class C { + #foo = 2; + m() { + @dec(() => this.#foo) // ok + class D { + } + return D; + } +} diff --git a/tests/baselines/reference/esDecorators-privateFieldAccess.symbols b/tests/baselines/reference/esDecorators-privateFieldAccess.symbols new file mode 100644 index 00000000000..7487320eb30 --- /dev/null +++ b/tests/baselines/reference/esDecorators-privateFieldAccess.symbols @@ -0,0 +1,64 @@ +//// [tests/cases/conformance/esDecorators/esDecorators-privateFieldAccess.ts] //// + +=== esDecorators-privateFieldAccess.ts === +declare let dec: any; +>dec : Symbol(dec, Decl(esDecorators-privateFieldAccess.ts, 0, 11)) + +@dec(x => x.#foo) // error +>dec : Symbol(dec, Decl(esDecorators-privateFieldAccess.ts, 0, 11)) +>x : Symbol(x, Decl(esDecorators-privateFieldAccess.ts, 2, 5)) +>x : Symbol(x, Decl(esDecorators-privateFieldAccess.ts, 2, 5)) + +class A { +>A : Symbol(A, Decl(esDecorators-privateFieldAccess.ts, 0, 21)) + + #foo = 3; +>#foo : Symbol(A.#foo, Decl(esDecorators-privateFieldAccess.ts, 3, 9)) + + @dec(this, (x: A) => x.#foo) // ok +>dec : Symbol(dec, Decl(esDecorators-privateFieldAccess.ts, 0, 11)) +>this : Symbol(globalThis) +>x : Symbol(x, Decl(esDecorators-privateFieldAccess.ts, 6, 16)) +>A : Symbol(A, Decl(esDecorators-privateFieldAccess.ts, 0, 21)) +>x.#foo : Symbol(A.#foo, Decl(esDecorators-privateFieldAccess.ts, 3, 9)) +>x : Symbol(x, Decl(esDecorators-privateFieldAccess.ts, 6, 16)) + + m() {} +>m : Symbol(A.m, Decl(esDecorators-privateFieldAccess.ts, 4, 13)) +} + +@dec((x: B) => x.#foo) // error +>dec : Symbol(dec, Decl(esDecorators-privateFieldAccess.ts, 0, 11)) +>x : Symbol(x, Decl(esDecorators-privateFieldAccess.ts, 10, 6)) +>B : Symbol(B, Decl(esDecorators-privateFieldAccess.ts, 8, 1)) +>x : Symbol(x, Decl(esDecorators-privateFieldAccess.ts, 10, 6)) + +class B { +>B : Symbol(B, Decl(esDecorators-privateFieldAccess.ts, 8, 1)) + + #foo = 3; +>#foo : Symbol(B.#foo, Decl(esDecorators-privateFieldAccess.ts, 11, 9)) +} + +class C { +>C : Symbol(C, Decl(esDecorators-privateFieldAccess.ts, 13, 1)) + + #foo = 2; +>#foo : Symbol(C.#foo, Decl(esDecorators-privateFieldAccess.ts, 15, 9)) + + m() { +>m : Symbol(C.m, Decl(esDecorators-privateFieldAccess.ts, 16, 13)) + + @dec(() => this.#foo) // ok +>dec : Symbol(dec, Decl(esDecorators-privateFieldAccess.ts, 0, 11)) +>this.#foo : Symbol(C.#foo, Decl(esDecorators-privateFieldAccess.ts, 15, 9)) +>this : Symbol(C, Decl(esDecorators-privateFieldAccess.ts, 13, 1)) + + class D {} +>D : Symbol(D, Decl(esDecorators-privateFieldAccess.ts, 17, 9)) + + return D; +>D : Symbol(D, Decl(esDecorators-privateFieldAccess.ts, 17, 9)) + } +} + diff --git a/tests/baselines/reference/esDecorators-privateFieldAccess.types b/tests/baselines/reference/esDecorators-privateFieldAccess.types new file mode 100644 index 00000000000..4cda3959729 --- /dev/null +++ b/tests/baselines/reference/esDecorators-privateFieldAccess.types @@ -0,0 +1,75 @@ +//// [tests/cases/conformance/esDecorators/esDecorators-privateFieldAccess.ts] //// + +=== esDecorators-privateFieldAccess.ts === +declare let dec: any; +>dec : any + +@dec(x => x.#foo) // error +>dec(x => x.#foo) : any +>dec : any +>x => x.#foo : (x: any) => any +>x : any +>x.#foo : any +>x : any + +class A { +>A : A + + #foo = 3; +>#foo : number +>3 : 3 + + @dec(this, (x: A) => x.#foo) // ok +>dec(this, (x: A) => x.#foo) : any +>dec : any +>this : typeof globalThis +>(x: A) => x.#foo : (x: A) => number +>x : A +>x.#foo : number +>x : A + + m() {} +>m : () => void +} + +@dec((x: B) => x.#foo) // error +>dec((x: B) => x.#foo) : any +>dec : any +>(x: B) => x.#foo : (x: B) => any +>x : B +>x.#foo : any +>x : B + +class B { +>B : B + + #foo = 3; +>#foo : number +>3 : 3 +} + +class C { +>C : C + + #foo = 2; +>#foo : number +>2 : 2 + + m() { +>m : () => typeof D + + @dec(() => this.#foo) // ok +>dec(() => this.#foo) : any +>dec : any +>() => this.#foo : () => number +>this.#foo : number +>this : this + + class D {} +>D : D + + return D; +>D : typeof D + } +} + diff --git a/tests/baselines/reference/plainJSGrammarErrors4.errors.txt b/tests/baselines/reference/plainJSGrammarErrors4.errors.txt new file mode 100644 index 00000000000..a5d01e6f550 --- /dev/null +++ b/tests/baselines/reference/plainJSGrammarErrors4.errors.txt @@ -0,0 +1,14 @@ +plainJSGrammarErrors4.js(5,14): error TS1111: Private field '#b' must be declared in an enclosing class. + + +==== plainJSGrammarErrors4.js (1 errors) ==== + class A { + #a; + m() { + this.#a; // ok + this.#b; // error + ~~ +!!! error TS1111: Private field '#b' must be declared in an enclosing class. + } + } + \ No newline at end of file diff --git a/tests/baselines/reference/plainJSGrammarErrors4.js b/tests/baselines/reference/plainJSGrammarErrors4.js new file mode 100644 index 00000000000..bced1d0c7f4 --- /dev/null +++ b/tests/baselines/reference/plainJSGrammarErrors4.js @@ -0,0 +1,20 @@ +//// [tests/cases/conformance/salsa/plainJSGrammarErrors4.ts] //// + +//// [plainJSGrammarErrors4.js] +class A { + #a; + m() { + this.#a; // ok + this.#b; // error + } +} + + +//// [plainJSGrammarErrors4.js] +class A { + #a; + m() { + this.#a; // ok + this.#b; // error + } +} diff --git a/tests/baselines/reference/plainJSGrammarErrors4.symbols b/tests/baselines/reference/plainJSGrammarErrors4.symbols new file mode 100644 index 00000000000..4f91c57af3b --- /dev/null +++ b/tests/baselines/reference/plainJSGrammarErrors4.symbols @@ -0,0 +1,21 @@ +//// [tests/cases/conformance/salsa/plainJSGrammarErrors4.ts] //// + +=== plainJSGrammarErrors4.js === +class A { +>A : Symbol(A, Decl(plainJSGrammarErrors4.js, 0, 0)) + + #a; +>#a : Symbol(A.#a, Decl(plainJSGrammarErrors4.js, 0, 9)) + + m() { +>m : Symbol(A.m, Decl(plainJSGrammarErrors4.js, 1, 7)) + + this.#a; // ok +>this.#a : Symbol(A.#a, Decl(plainJSGrammarErrors4.js, 0, 9)) +>this : Symbol(A, Decl(plainJSGrammarErrors4.js, 0, 0)) + + this.#b; // error +>this : Symbol(A, Decl(plainJSGrammarErrors4.js, 0, 0)) + } +} + diff --git a/tests/baselines/reference/plainJSGrammarErrors4.types b/tests/baselines/reference/plainJSGrammarErrors4.types new file mode 100644 index 00000000000..bed42d1886b --- /dev/null +++ b/tests/baselines/reference/plainJSGrammarErrors4.types @@ -0,0 +1,22 @@ +//// [tests/cases/conformance/salsa/plainJSGrammarErrors4.ts] //// + +=== plainJSGrammarErrors4.js === +class A { +>A : A + + #a; +>#a : any + + m() { +>m : () => void + + this.#a; // ok +>this.#a : any +>this : this + + this.#b; // error +>this.#b : any +>this : this + } +} + diff --git a/tests/cases/conformance/esDecorators/esDecorators-privateFieldAccess.ts b/tests/cases/conformance/esDecorators/esDecorators-privateFieldAccess.ts new file mode 100644 index 00000000000..fe3085e8e98 --- /dev/null +++ b/tests/cases/conformance/esDecorators/esDecorators-privateFieldAccess.ts @@ -0,0 +1,26 @@ +// @target: esnext +// @noEmitHelpers: true + +declare let dec: any; + +@dec(x => x.#foo) // error +class A { + #foo = 3; + + @dec(this, (x: A) => x.#foo) // ok + m() {} +} + +@dec((x: B) => x.#foo) // error +class B { + #foo = 3; +} + +class C { + #foo = 2; + m() { + @dec(() => this.#foo) // ok + class D {} + return D; + } +} diff --git a/tests/cases/conformance/salsa/plainJSGrammarErrors4.ts b/tests/cases/conformance/salsa/plainJSGrammarErrors4.ts new file mode 100644 index 00000000000..64478311a86 --- /dev/null +++ b/tests/cases/conformance/salsa/plainJSGrammarErrors4.ts @@ -0,0 +1,12 @@ +// @outdir: out/ +// @target: esnext +// @module: esnext +// @allowJs: true +// @filename: plainJSGrammarErrors4.js +class A { + #a; + m() { + this.#a; // ok + this.#b; // error + } +}