Skip to content

Fix handling of prologue statements when there are parameter property declarations #48775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 16 additions & 14 deletions src/compiler/transformers/classFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1294,7 +1294,7 @@ namespace ts {
resumeLexicalEnvironment();

const needsSyntheticConstructor = !constructor && isDerivedClass;
let indexOfFirstStatementAfterSuper = 0;
let indexOfFirstStatementAfterSuperAndPrologue = 0;
let prologueStatementCount = 0;
let superStatementIndex = -1;
let statements: Statement[] = [];
Expand All @@ -1305,13 +1305,16 @@ namespace ts {

// If there was a super call, visit existing statements up to and including it
if (superStatementIndex >= 0) {
indexOfFirstStatementAfterSuper = superStatementIndex + 1;
indexOfFirstStatementAfterSuperAndPrologue = superStatementIndex + 1;
statements = [
...statements.slice(0, prologueStatementCount),
...visitNodes(constructor.body.statements, visitor, isStatement, prologueStatementCount, indexOfFirstStatementAfterSuper - prologueStatementCount),
...visitNodes(constructor.body.statements, visitor, isStatement, prologueStatementCount, indexOfFirstStatementAfterSuperAndPrologue - prologueStatementCount),
...statements.slice(prologueStatementCount),
];
}
else if (prologueStatementCount >= 0) {
indexOfFirstStatementAfterSuperAndPrologue = prologueStatementCount;
}
}

if (needsSyntheticConstructor) {
Expand Down Expand Up @@ -1354,26 +1357,25 @@ namespace ts {
}
}
if (parameterPropertyDeclarationCount > 0) {
const parameterProperties = visitNodes(constructor.body.statements, visitor, isStatement, indexOfFirstStatementAfterSuper, parameterPropertyDeclarationCount);
const parameterProperties = visitNodes(constructor.body.statements, visitor, isStatement, indexOfFirstStatementAfterSuperAndPrologue, parameterPropertyDeclarationCount);

// If there was a super() call found, add parameter properties immediately after it
if (superStatementIndex >= 0) {
addRange(statements, parameterProperties);
}
// If a synthetic super() call was added, add them just after it
else if (needsSyntheticConstructor) {
else {
// Add add parameter properties to the top of the constructor after the prologue
let superAndPrologueStatementCount = prologueStatementCount;
// If a synthetic super() call was added, need to account for that
if (needsSyntheticConstructor) superAndPrologueStatementCount++;
statements = [
statements[0],
...statements.slice(0, superAndPrologueStatementCount),
Comment on lines -1366 to +1372
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated so we include parameter properties after any prologues - I believe this is the correct way of doing it, since that's what the existing TS version does

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, see also #48687 and #48765, which fixed this for other transforms.

...parameterProperties,
...statements.slice(1),
...statements.slice(superAndPrologueStatementCount),
];
}
// Since there wasn't a super() call, add them to the top of the constructor
else {
statements = [...parameterProperties, ...statements];
}

indexOfFirstStatementAfterSuper += parameterPropertyDeclarationCount;
indexOfFirstStatementAfterSuperAndPrologue += parameterPropertyDeclarationCount;
}
}
}
Expand All @@ -1385,7 +1387,7 @@ namespace ts {

// Add existing statements after the initial prologues and super call
if (constructor) {
addRange(statements, visitNodes(constructor.body!.statements, visitBodyStatement, isStatement, indexOfFirstStatementAfterSuper + prologueStatementCount));
addRange(statements, visitNodes(constructor.body!.statements, visitBodyStatement, isStatement, indexOfFirstStatementAfterSuperAndPrologue));
}

statements = factory.mergeLexicalEnvironment(statements, endLexicalEnvironment());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
//// [constructorWithParameterPropertiesAndPrivateFields.es2015.ts]
// https://github.com/microsoft/TypeScript/issues/48771

class A {
readonly #privateField: string;

constructor(arg: { key: string }, public exposedField: number) {
({ key: this.#privateField } = arg);
}

log() {
console.log(this.#privateField);
console.log(this.exposedField);
}
}

class B {
readonly #privateField: string;

constructor(arg: { key: string }, public exposedField: number) {
"prologue";
({ key: this.#privateField } = arg);
}

log() {
console.log(this.#privateField);
console.log(this.exposedField);
}
}


//// [constructorWithParameterPropertiesAndPrivateFields.es2015.js]
// https://github.com/microsoft/TypeScript/issues/48771
var __classPrivateFieldSet = (this && this.__classPrivateFieldSet) || function (receiver, state, value, kind, f) {
if (kind === "m") throw new TypeError("Private method is not writable");
if (kind === "a" && !f) throw new TypeError("Private accessor was defined without a setter");
if (typeof state === "function" ? receiver !== state || !f : !state.has(receiver)) throw new TypeError("Cannot write private member to an object whose class did not declare it");
return (kind === "a" ? f.call(receiver, value) : f ? f.value = value : state.set(receiver, value)), value;
};
var __classPrivateFieldGet = (this && this.__classPrivateFieldGet) || function (receiver, state, kind, f) {
if (kind === "a" && !f) throw new TypeError("Private accessor was defined without a getter");
if (typeof state === "function" ? receiver !== state || !f : !state.has(receiver)) throw new TypeError("Cannot read private member from an object whose class did not declare it");
return kind === "m" ? f : kind === "a" ? f.call(receiver) : f ? f.value : state.get(receiver);
};
var _A_privateField, _B_privateField;
class A {
constructor(arg, exposedField) {
var _a;
this.exposedField = exposedField;
_A_privateField.set(this, void 0);
(_a = this, { key: ({ set value(_b) { __classPrivateFieldSet(_a, _A_privateField, _b, "f"); } }).value } = arg);
}
log() {
console.log(__classPrivateFieldGet(this, _A_privateField, "f"));
console.log(this.exposedField);
}
}
_A_privateField = new WeakMap();
class B {
constructor(arg, exposedField) {
"prologue";
var _a;
this.exposedField = exposedField;
_B_privateField.set(this, void 0);
(_a = this, { key: ({ set value(_b) { __classPrivateFieldSet(_a, _B_privateField, _b, "f"); } }).value } = arg);
}
log() {
console.log(__classPrivateFieldGet(this, _B_privateField, "f"));
console.log(this.exposedField);
}
}
_B_privateField = new WeakMap();
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
=== tests/cases/compiler/constructorWithParameterPropertiesAndPrivateFields.es2015.ts ===
// https://github.com/microsoft/TypeScript/issues/48771

class A {
>A : Symbol(A, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 0, 0))

readonly #privateField: string;
>#privateField : Symbol(A.#privateField, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 2, 9))

constructor(arg: { key: string }, public exposedField: number) {
>arg : Symbol(arg, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 5, 14))
>key : Symbol(key, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 5, 20))
>exposedField : Symbol(A.exposedField, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 5, 35))

({ key: this.#privateField } = arg);
>key : Symbol(key, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 6, 6))
>this.#privateField : Symbol(A.#privateField, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 2, 9))
>this : Symbol(A, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 0, 0))
>arg : Symbol(arg, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 5, 14))
}

log() {
>log : Symbol(A.log, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 7, 3))

console.log(this.#privateField);
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>this.#privateField : Symbol(A.#privateField, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 2, 9))
>this : Symbol(A, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 0, 0))

console.log(this.exposedField);
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>this.exposedField : Symbol(A.exposedField, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 5, 35))
>this : Symbol(A, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 0, 0))
>exposedField : Symbol(A.exposedField, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 5, 35))
}
}

class B {
>B : Symbol(B, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 13, 1))

readonly #privateField: string;
>#privateField : Symbol(B.#privateField, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 15, 9))

constructor(arg: { key: string }, public exposedField: number) {
>arg : Symbol(arg, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 18, 14))
>key : Symbol(key, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 18, 20))
>exposedField : Symbol(B.exposedField, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 18, 35))

"prologue";
({ key: this.#privateField } = arg);
>key : Symbol(key, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 20, 6))
>this.#privateField : Symbol(B.#privateField, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 15, 9))
>this : Symbol(B, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 13, 1))
>arg : Symbol(arg, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 18, 14))
}

log() {
>log : Symbol(B.log, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 21, 3))

console.log(this.#privateField);
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>this.#privateField : Symbol(B.#privateField, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 15, 9))
>this : Symbol(B, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 13, 1))

console.log(this.exposedField);
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>this.exposedField : Symbol(B.exposedField, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 18, 35))
>this : Symbol(B, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 13, 1))
>exposedField : Symbol(B.exposedField, Decl(constructorWithParameterPropertiesAndPrivateFields.es2015.ts, 18, 35))
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
=== tests/cases/compiler/constructorWithParameterPropertiesAndPrivateFields.es2015.ts ===
// https://github.com/microsoft/TypeScript/issues/48771

class A {
>A : A

readonly #privateField: string;
>#privateField : string

constructor(arg: { key: string }, public exposedField: number) {
>arg : { key: string; }
>key : string
>exposedField : number

({ key: this.#privateField } = arg);
>({ key: this.#privateField } = arg) : { key: string; }
>{ key: this.#privateField } = arg : { key: string; }
>{ key: this.#privateField } : { key: string; }
>key : string
>this.#privateField : string
>this : this
>arg : { key: string; }
}

log() {
>log : () => void

console.log(this.#privateField);
>console.log(this.#privateField) : void
>console.log : (...data: any[]) => void
>console : Console
>log : (...data: any[]) => void
>this.#privateField : string
>this : this

console.log(this.exposedField);
>console.log(this.exposedField) : void
>console.log : (...data: any[]) => void
>console : Console
>log : (...data: any[]) => void
>this.exposedField : number
>this : this
>exposedField : number
}
}

class B {
>B : B

readonly #privateField: string;
>#privateField : string

constructor(arg: { key: string }, public exposedField: number) {
>arg : { key: string; }
>key : string
>exposedField : number

"prologue";
>"prologue" : "prologue"

({ key: this.#privateField } = arg);
>({ key: this.#privateField } = arg) : { key: string; }
>{ key: this.#privateField } = arg : { key: string; }
>{ key: this.#privateField } : { key: string; }
>key : string
>this.#privateField : string
>this : this
>arg : { key: string; }
}

log() {
>log : () => void

console.log(this.#privateField);
>console.log(this.#privateField) : void
>console.log : (...data: any[]) => void
>console : Console
>log : (...data: any[]) => void
>this.#privateField : string
>this : this

console.log(this.exposedField);
>console.log(this.exposedField) : void
>console.log : (...data: any[]) => void
>console : Console
>log : (...data: any[]) => void
>this.exposedField : number
>this : this
>exposedField : number
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// @target: es2015
// https://github.com/microsoft/TypeScript/issues/48771

class A {
readonly #privateField: string;

constructor(arg: { key: string }, public exposedField: number) {
({ key: this.#privateField } = arg);
}

log() {
console.log(this.#privateField);
console.log(this.exposedField);
}
}

class B {
readonly #privateField: string;

constructor(arg: { key: string }, public exposedField: number) {
"prologue";
({ key: this.#privateField } = arg);
}

log() {
console.log(this.#privateField);
console.log(this.exposedField);
}
}