From 4c01b2f9ee6e65ff9a39e836a99b0b4959bce5fb Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 31 May 2023 18:17:43 -0400 Subject: [PATCH] Fix incorrect 'this' substitution in decorated static 'accessor' fields (#54474) --- src/compiler/transformers/classFields.ts | 125 +++++++++++++----- ...ation-classThisReference(target=es2015).js | 4 +- ...on-fields-staticAccessor(target=es2015).js | 16 +-- 3 files changed, 105 insertions(+), 40 deletions(-) diff --git a/src/compiler/transformers/classFields.ts b/src/compiler/transformers/classFields.ts index 0faf191377a..f038aa3d37a 100644 --- a/src/compiler/transformers/classFields.ts +++ b/src/compiler/transformers/classFields.ts @@ -73,6 +73,7 @@ import { Identifier, InKeyword, InternalEmitFlags, + isAccessor, isAccessorModifier, isArrayBindingOrAssignmentElement, isArrayLiteralExpression, @@ -414,8 +415,11 @@ export function transformClassFields(context: TransformationContext): (x: Source let lexicalEnvironment: LexicalEnv | undefined; const lexicalEnvironmentMap = new Map(); + // Nodes that should not be replaced during emit substitution. + const noSubstitution = new Set(); + let currentClassContainer: ClassLikeDeclaration | undefined; - let currentStaticPropertyDeclarationOrStaticBlock: PropertyDeclaration | ClassStaticBlockDeclaration | undefined; + let currentClassElement: ClassElement | undefined; let shouldSubstituteThisWithClassThis = false; let previousShouldSubstituteThisWithClassThis = false; @@ -497,13 +501,19 @@ export function transformClassFields(context: TransformationContext): (x: Source return visitForStatement(node as ForStatement); case SyntaxKind.FunctionDeclaration: case SyntaxKind.FunctionExpression: + // If we are descending into a new scope, clear the current class element + return setCurrentClassElementAnd( + /*classElement*/ undefined, + fallbackVisitor, + node + ); case SyntaxKind.Constructor: case SyntaxKind.MethodDeclaration: case SyntaxKind.GetAccessor: case SyntaxKind.SetAccessor: { - // If we are descending into a new scope, clear the current static property or block - return setCurrentStaticPropertyDeclarationOrStaticBlockAnd( - /*current*/ undefined, + // If we are descending into a class element, set the class element + return setCurrentClassElementAnd( + node as ConstructorDeclaration | MethodDeclaration | GetAccessorDeclaration | SetAccessorDeclaration, fallbackVisitor, node ); @@ -582,21 +592,27 @@ export function transformClassFields(context: TransformationContext): (x: Source function classElementVisitor(node: Node): VisitResult { switch (node.kind) { case SyntaxKind.Constructor: - return visitConstructorDeclaration(node as ConstructorDeclaration); + return setCurrentClassElementAnd( + node as ConstructorDeclaration, + visitConstructorDeclaration, + node as ConstructorDeclaration); case SyntaxKind.GetAccessor: case SyntaxKind.SetAccessor: case SyntaxKind.MethodDeclaration: - return setCurrentStaticPropertyDeclarationOrStaticBlockAnd( - /*current*/ undefined, + return setCurrentClassElementAnd( + node as MethodDeclaration | AccessorDeclaration, visitMethodOrAccessorDeclaration, node as MethodDeclaration | AccessorDeclaration); case SyntaxKind.PropertyDeclaration: - return setCurrentStaticPropertyDeclarationOrStaticBlockAnd( - /*current*/ undefined, + return setCurrentClassElementAnd( + node as PropertyDeclaration, visitPropertyDeclaration, node as PropertyDeclaration); case SyntaxKind.ClassStaticBlockDeclaration: - return visitClassStaticBlockDeclaration(node as ClassStaticBlockDeclaration); + return setCurrentClassElementAnd( + node as ClassStaticBlockDeclaration, + visitClassStaticBlockDeclaration, + node as ClassStaticBlockDeclaration); case SyntaxKind.ComputedPropertyName: return visitComputedPropertyName(node as ComputedPropertyName); case SyntaxKind.SemicolonClassElement: @@ -881,16 +897,19 @@ export function transformClassFields(context: TransformationContext): (x: Source return undefined; } - function setCurrentStaticPropertyDeclarationOrStaticBlockAnd( - current: ClassStaticBlockDeclaration | PropertyDeclaration | undefined, + function setCurrentClassElementAnd( + classElement: ClassElement | undefined, visitor: (arg: T) => U, arg: T, ) { - const savedCurrentStaticPropertyDeclarationOrStaticBlock = currentStaticPropertyDeclarationOrStaticBlock; - currentStaticPropertyDeclarationOrStaticBlock = current; - const result = visitor(arg); - currentStaticPropertyDeclarationOrStaticBlock = savedCurrentStaticPropertyDeclarationOrStaticBlock; - return result; + if (classElement !== currentClassElement) { + const savedCurrentClassElement = currentClassElement; + currentClassElement = classElement; + const result = visitor(arg); + currentClassElement = savedCurrentClassElement; + return result; + } + return visitor(arg); } function getHoistedFunctionName(node: MethodDeclaration | AccessorDeclaration) { @@ -1095,8 +1114,31 @@ export function transformClassFields(context: TransformationContext): (x: Source return transformFieldInitializer(node); } + function shouldForceDynamicThis() { + return !!currentClassElement && + hasStaticModifier(currentClassElement) && + isAccessor(currentClassElement) && + isAutoAccessorPropertyDeclaration(getOriginalNode(currentClassElement)); + } + + /** + * Prevent substitution of `this` to `_classThis` in static getters and setters that wrap `accessor` fields. + */ + function ensureDynamicThisIfNeeded(node: Expression) { + if (shouldForceDynamicThis()) { + // do not substitute `this` with `_classThis` when `this` + // should be bound dynamically. + const innerExpression = skipOuterExpressions(node); + if (innerExpression.kind === SyntaxKind.ThisKeyword) { + noSubstitution.add(innerExpression); + } + } + } + function createPrivateIdentifierAccess(info: PrivateIdentifierInfo, receiver: Expression): Expression { - return createPrivateIdentifierAccessHelper(info, visitNode(receiver, visitor, isExpression)); + receiver = visitNode(receiver, visitor, isExpression); + ensureDynamicThisIfNeeded(receiver); + return createPrivateIdentifierAccessHelper(info, receiver); } function createPrivateIdentifierAccessHelper(info: PrivateIdentifierInfo, receiver: Expression): Expression { @@ -1146,9 +1188,10 @@ export function transformClassFields(context: TransformationContext): (x: Source } } if (shouldTransformSuperInStaticInitializers && + currentClassElement && isSuperProperty(node) && isIdentifier(node.name) && - currentStaticPropertyDeclarationOrStaticBlock && + isStaticPropertyDeclarationOrClassStaticBlock(currentClassElement) && lexicalEnvironment?.data) { const { classConstructor, superClassReference, facts } = lexicalEnvironment.data; if (facts & ClassFacts.ClassWasDecorated) { @@ -1171,8 +1214,9 @@ export function transformClassFields(context: TransformationContext): (x: Source function visitElementAccessExpression(node: ElementAccessExpression) { if (shouldTransformSuperInStaticInitializers && + currentClassElement && isSuperProperty(node) && - currentStaticPropertyDeclarationOrStaticBlock && + isStaticPropertyDeclarationOrClassStaticBlock(currentClassElement) && lexicalEnvironment?.data) { const { classConstructor, superClassReference, facts } = lexicalEnvironment.data; if (facts & ClassFacts.ClassWasDecorated) { @@ -1203,6 +1247,7 @@ export function transformClassFields(context: TransformationContext): (x: Source let info: PrivateIdentifierInfo | undefined; if (info = accessPrivateIdentifier(operand.name)) { const receiver = visitNode(operand.expression, visitor, isExpression); + ensureDynamicThisIfNeeded(receiver); const { readExpression, initializeExpression } = createCopiableReceiverExpr(receiver); let expression: Expression = createPrivateIdentifierAccess(info, readExpression); @@ -1224,8 +1269,9 @@ export function transformClassFields(context: TransformationContext): (x: Source } } else if (shouldTransformSuperInStaticInitializers && + currentClassElement && isSuperProperty(operand) && - currentStaticPropertyDeclarationOrStaticBlock && + isStaticPropertyDeclarationOrClassStaticBlock(currentClassElement) && lexicalEnvironment?.data) { // converts `++super.a` into `(Reflect.set(_baseTemp, "a", (_a = Reflect.get(_baseTemp, "a", _classTemp), _b = ++_a), _classTemp), _b)` // converts `++super[f()]` into `(Reflect.set(_baseTemp, _a = f(), (_b = Reflect.get(_baseTemp, _a, _classTemp), _c = ++_b), _classTemp), _c)` @@ -1299,6 +1345,9 @@ export function transformClassFields(context: TransformationContext): (x: Source function createCopiableReceiverExpr(receiver: Expression): { readExpression: Expression; initializeExpression: Expression | undefined } { const clone = nodeIsSynthesized(receiver) ? receiver : factory.cloneNode(receiver); + if (receiver.kind === SyntaxKind.ThisKeyword && noSubstitution.has(receiver)) { + noSubstitution.add(clone); + } if (isSimpleInlineableExpression(receiver)) { return { readExpression: clone, initializeExpression: undefined }; } @@ -1332,8 +1381,9 @@ export function transformClassFields(context: TransformationContext): (x: Source } if (shouldTransformSuperInStaticInitializers && + currentClassElement && isSuperProperty(node.expression) && - currentStaticPropertyDeclarationOrStaticBlock && + isStaticPropertyDeclarationOrClassStaticBlock(currentClassElement) && lexicalEnvironment?.data?.classConstructor) { // super.x() // super[x]() @@ -1369,8 +1419,9 @@ export function transformClassFields(context: TransformationContext): (x: Source ); } if (shouldTransformSuperInStaticInitializers && + currentClassElement && isSuperProperty(node.tag) && - currentStaticPropertyDeclarationOrStaticBlock && + isStaticPropertyDeclarationOrClassStaticBlock(currentClassElement) && lexicalEnvironment?.data?.classConstructor) { // converts `` super.f`x` `` into `` Reflect.get(_baseTemp, "f", _classTemp).bind(_classTemp)`x` `` @@ -1398,7 +1449,7 @@ export function transformClassFields(context: TransformationContext): (x: Source if (shouldTransformPrivateElementsOrClassStaticBlocks) { startLexicalEnvironment(); - let statements = setCurrentStaticPropertyDeclarationOrStaticBlockAnd( + let statements = setCurrentClassElementAnd( node, statements => visitNodes(statements, visitor, isStatement), node.body.statements @@ -1506,8 +1557,9 @@ export function transformClassFields(context: TransformationContext): (x: Source } } else if (shouldTransformSuperInStaticInitializers && + currentClassElement && isSuperProperty(node.left) && - currentStaticPropertyDeclarationOrStaticBlock && + isStaticPropertyDeclarationOrClassStaticBlock(currentClassElement) && lexicalEnvironment?.data) { // super.x = ... // super[x] = ... @@ -1641,6 +1693,7 @@ export function transformClassFields(context: TransformationContext): (x: Source function createPrivateIdentifierAssignment(info: PrivateIdentifierInfo, receiver: Expression, right: Expression, operator: AssignmentOperator): Expression { receiver = visitNode(receiver, visitor, isExpression); right = visitNode(right, visitor, isExpression); + ensureDynamicThisIfNeeded(receiver); if (isCompoundAssignment(operator)) { const { readExpression, initializeExpression } = createCopiableReceiverExpr(receiver); @@ -2420,7 +2473,7 @@ export function transformClassFields(context: TransformationContext): (x: Source * @param receiver The object receiving the property assignment. */ function transformProperty(property: PropertyDeclaration, receiver: LeftHandSideExpression) { - const savedCurrentStaticPropertyDeclarationOrStaticBlock = currentStaticPropertyDeclarationOrStaticBlock; + const savedCurrentClassElement = currentClassElement; const transformed = transformPropertyWorker(property, receiver); if (transformed && hasStaticModifier(property) && lexicalEnvironment?.data?.facts) { // capture the lexical environment for the member @@ -2429,7 +2482,7 @@ export function transformClassFields(context: TransformationContext): (x: Source setSourceMapRange(transformed, getSourceMapRange(property.name)); lexicalEnvironmentMap.set(getOriginalNode(property), lexicalEnvironment); } - currentStaticPropertyDeclarationOrStaticBlock = savedCurrentStaticPropertyDeclarationOrStaticBlock; + currentClassElement = savedCurrentClassElement; return transformed; } @@ -2458,7 +2511,7 @@ export function transformClassFields(context: TransformationContext): (x: Source property.name; if (hasStaticModifier(property)) { - currentStaticPropertyDeclarationOrStaticBlock = property; + currentClassElement = property; } const initializerVisitor: Visitor = @@ -2931,8 +2984,9 @@ export function transformClassFields(context: TransformationContext): (x: Source return wrapPrivateIdentifierForDestructuringTarget(node); } else if (shouldTransformSuperInStaticInitializers && + currentClassElement && isSuperProperty(node) && - currentStaticPropertyDeclarationOrStaticBlock && + isStaticPropertyDeclarationOrClassStaticBlock(currentClassElement) && lexicalEnvironment?.data) { const { classConstructor, superClassReference, facts } = lexicalEnvironment.data; if (facts & ClassFacts.ClassWasDecorated) { @@ -3164,6 +3218,7 @@ export function transformClassFields(context: TransformationContext): (x: Source */ function onSubstituteNode(hint: EmitHint, node: Node) { node = previousOnSubstituteNode(hint, node); + if (hint === EmitHint.Expression) { return substituteExpression(node as Expression); } @@ -3181,7 +3236,9 @@ export function transformClassFields(context: TransformationContext): (x: Source } function substituteThisExpression(node: ThisExpression) { - if (enabledSubstitutions & ClassPropertySubstitutionFlags.ClassStaticThisOrSuperReference && lexicalEnvironment?.data) { + if (enabledSubstitutions & ClassPropertySubstitutionFlags.ClassStaticThisOrSuperReference && + lexicalEnvironment?.data && + !noSubstitution.has(node)) { const { facts, classConstructor, classThis } = lexicalEnvironment.data; if (facts & ClassFacts.ClassWasDecorated && legacyDecorators) { return factory.createParenthesizedExpression(factory.createVoidZero()); @@ -3264,3 +3321,11 @@ function isPrivateIdentifierInExpression(node: BinaryExpression): node is Privat return isPrivateIdentifier(node.left) && node.operatorToken.kind === SyntaxKind.InKeyword; } + +function isStaticPropertyDeclaration(node: Node): node is PropertyDeclaration { + return isPropertyDeclaration(node) && hasStaticModifier(node); +} + +function isStaticPropertyDeclarationOrClassStaticBlock(node: Node): node is ClassStaticBlockDeclaration | PropertyDeclaration { + return isClassStaticBlockDeclaration(node) || isStaticPropertyDeclaration(node); +} \ No newline at end of file diff --git a/tests/baselines/reference/esDecorators-classDeclaration-classThisReference(target=es2015).js b/tests/baselines/reference/esDecorators-classDeclaration-classThisReference(target=es2015).js index 086bc280bb0..215a9c4eff6 100644 --- a/tests/baselines/reference/esDecorators-classDeclaration-classThisReference(target=es2015).js +++ b/tests/baselines/reference/esDecorators-classDeclaration-classThisReference(target=es2015).js @@ -19,8 +19,8 @@ let C = (() => { let _classExtraInitializers = []; let _classThis; var C = _classThis = class { - static get a() { return __classPrivateFieldGet(_classThis, _classThis, "f", _a_accessor_storage); } - static set a(value) { __classPrivateFieldSet(_classThis, _classThis, value, "f", _a_accessor_storage); } + static get a() { return __classPrivateFieldGet(this, _classThis, "f", _a_accessor_storage); } + static set a(value) { __classPrivateFieldSet(this, _classThis, value, "f", _a_accessor_storage); } static m() { this; } static get g() { return this; } }; diff --git a/tests/baselines/reference/esDecorators-classDeclaration-fields-staticAccessor(target=es2015).js b/tests/baselines/reference/esDecorators-classDeclaration-fields-staticAccessor(target=es2015).js index 620c62869b4..1d114efb9c1 100644 --- a/tests/baselines/reference/esDecorators-classDeclaration-fields-staticAccessor(target=es2015).js +++ b/tests/baselines/reference/esDecorators-classDeclaration-fields-staticAccessor(target=es2015).js @@ -31,12 +31,12 @@ let C = (() => { let _static_member_decorators_1; let _static_member_initializers_1 = []; return _a = class C { - static get field1() { return __classPrivateFieldGet(_a, _a, "f", _C_field1_accessor_storage); } - static set field1(value) { __classPrivateFieldSet(_a, _a, value, "f", _C_field1_accessor_storage); } - static get ["field2"]() { return __classPrivateFieldGet(_a, _a, "f", _C__a_accessor_storage); } - static set ["field2"](value) { __classPrivateFieldSet(_a, _a, value, "f", _C__a_accessor_storage); } - static get [(_static_field1_decorators = [dec(1)], _static_member_decorators = [dec(2)], _static_member_decorators_1 = [dec(3)], _b = __propKey(field3))]() { return __classPrivateFieldGet(_a, _a, "f", _C__b_accessor_storage); } - static set [_b](value) { __classPrivateFieldSet(_a, _a, value, "f", _C__b_accessor_storage); } + static get field1() { return __classPrivateFieldGet(this, _a, "f", _C_field1_accessor_storage); } + static set field1(value) { __classPrivateFieldSet(this, _a, value, "f", _C_field1_accessor_storage); } + static get ["field2"]() { return __classPrivateFieldGet(this, _a, "f", _C__a_accessor_storage); } + static set ["field2"](value) { __classPrivateFieldSet(this, _a, value, "f", _C__a_accessor_storage); } + static get [(_static_field1_decorators = [dec(1)], _static_member_decorators = [dec(2)], _static_member_decorators_1 = [dec(3)], _b = __propKey(field3))]() { return __classPrivateFieldGet(this, _a, "f", _C__b_accessor_storage); } + static set [_b](value) { __classPrivateFieldSet(this, _a, value, "f", _C__b_accessor_storage); } }, (() => { __esDecorate(_a, null, _static_field1_decorators, { kind: "accessor", name: "field1", static: true, private: false, access: { has: obj => "field1" in obj, get: obj => obj.field1, set: (obj, value) => { obj.field1 = value; } } }, _static_field1_initializers, _staticExtraInitializers); @@ -56,8 +56,8 @@ let D = (() => { let _classExtraInitializers = []; let _classThis; var D = _classThis = class { - static get field1() { return __classPrivateFieldGet(_classThis, _classThis, "f", _field1_accessor_storage); } - static set field1(value) { __classPrivateFieldSet(_classThis, _classThis, value, "f", _field1_accessor_storage); } + static get field1() { return __classPrivateFieldGet(this, _classThis, "f", _field1_accessor_storage); } + static set field1(value) { __classPrivateFieldSet(this, _classThis, value, "f", _field1_accessor_storage); } }; __setFunctionName(_classThis, "D"); (() => {