Skip to content

Commit 4c01b2f

Browse files
rbucktonrbuckton
authored
Fix incorrect 'this' substitution in decorated static 'accessor' fields (microsoft#54474)
1 parent d495200 commit 4c01b2f

File tree

3 files changed

+105
-40
lines changed

3 files changed

+105
-40
lines changed

‎src/compiler/transformers/classFields.ts‎

Lines changed: 95 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ import {
7373
Identifier,
7474
InKeyword,
7575
InternalEmitFlags,
76+
isAccessor,
7677
isAccessorModifier,
7778
isArrayBindingOrAssignmentElement,
7879
isArrayLiteralExpression,
@@ -414,8 +415,11 @@ export function transformClassFields(context: TransformationContext): (x: Source
414415
let lexicalEnvironment: LexicalEnv | undefined;
415416
const lexicalEnvironmentMap = new Map<Node, LexicalEnv>();
416417

418+
// Nodes that should not be replaced during emit substitution.
419+
const noSubstitution = new Set<Node>();
420+
417421
let currentClassContainer: ClassLikeDeclaration | undefined;
418-
let currentStaticPropertyDeclarationOrStaticBlock: PropertyDeclaration | ClassStaticBlockDeclaration | undefined;
422+
let currentClassElement: ClassElement | undefined;
419423
let shouldSubstituteThisWithClassThis = false;
420424
let previousShouldSubstituteThisWithClassThis = false;
421425

@@ -497,13 +501,19 @@ export function transformClassFields(context: TransformationContext): (x: Source
497501
return visitForStatement(node as ForStatement);
498502
case SyntaxKind.FunctionDeclaration:
499503
case SyntaxKind.FunctionExpression:
504+
// If we are descending into a new scope, clear the current class element
505+
return setCurrentClassElementAnd(
506+
/*classElement*/ undefined,
507+
fallbackVisitor,
508+
node
509+
);
500510
case SyntaxKind.Constructor:
501511
case SyntaxKind.MethodDeclaration:
502512
case SyntaxKind.GetAccessor:
503513
case SyntaxKind.SetAccessor: {
504-
// If we are descending into a new scope, clear the current static property or block
505-
return setCurrentStaticPropertyDeclarationOrStaticBlockAnd(
506-
/*current*/ undefined,
514+
// If we are descending into a class element, set the class element
515+
return setCurrentClassElementAnd(
516+
node as ConstructorDeclaration | MethodDeclaration | GetAccessorDeclaration | SetAccessorDeclaration,
507517
fallbackVisitor,
508518
node
509519
);
@@ -582,21 +592,27 @@ export function transformClassFields(context: TransformationContext): (x: Source
582592
function classElementVisitor(node: Node): VisitResult<Node | undefined> {
583593
switch (node.kind) {
584594
case SyntaxKind.Constructor:
585-
return visitConstructorDeclaration(node as ConstructorDeclaration);
595+
return setCurrentClassElementAnd(
596+
node as ConstructorDeclaration,
597+
visitConstructorDeclaration,
598+
node as ConstructorDeclaration);
586599
case SyntaxKind.GetAccessor:
587600
case SyntaxKind.SetAccessor:
588601
case SyntaxKind.MethodDeclaration:
589-
return setCurrentStaticPropertyDeclarationOrStaticBlockAnd(
590-
/*current*/ undefined,
602+
return setCurrentClassElementAnd(
603+
node as MethodDeclaration | AccessorDeclaration,
591604
visitMethodOrAccessorDeclaration,
592605
node as MethodDeclaration | AccessorDeclaration);
593606
case SyntaxKind.PropertyDeclaration:
594-
return setCurrentStaticPropertyDeclarationOrStaticBlockAnd(
595-
/*current*/ undefined,
607+
return setCurrentClassElementAnd(
608+
node as PropertyDeclaration,
596609
visitPropertyDeclaration,
597610
node as PropertyDeclaration);
598611
case SyntaxKind.ClassStaticBlockDeclaration:
599-
return visitClassStaticBlockDeclaration(node as ClassStaticBlockDeclaration);
612+
return setCurrentClassElementAnd(
613+
node as ClassStaticBlockDeclaration,
614+
visitClassStaticBlockDeclaration,
615+
node as ClassStaticBlockDeclaration);
600616
case SyntaxKind.ComputedPropertyName:
601617
return visitComputedPropertyName(node as ComputedPropertyName);
602618
case SyntaxKind.SemicolonClassElement:
@@ -881,16 +897,19 @@ export function transformClassFields(context: TransformationContext): (x: Source
881897
return undefined;
882898
}
883899

884-
function setCurrentStaticPropertyDeclarationOrStaticBlockAnd<T, U>(
885-
current: ClassStaticBlockDeclaration | PropertyDeclaration | undefined,
900+
function setCurrentClassElementAnd<T, U>(
901+
classElement: ClassElement | undefined,
886902
visitor: (arg: T) => U,
887903
arg: T,
888904
) {
889-
const savedCurrentStaticPropertyDeclarationOrStaticBlock = currentStaticPropertyDeclarationOrStaticBlock;
890-
currentStaticPropertyDeclarationOrStaticBlock = current;
891-
const result = visitor(arg);
892-
currentStaticPropertyDeclarationOrStaticBlock = savedCurrentStaticPropertyDeclarationOrStaticBlock;
893-
return result;
905+
if (classElement !== currentClassElement) {
906+
const savedCurrentClassElement = currentClassElement;
907+
currentClassElement = classElement;
908+
const result = visitor(arg);
909+
currentClassElement = savedCurrentClassElement;
910+
return result;
911+
}
912+
return visitor(arg);
894913
}
895914

896915
function getHoistedFunctionName(node: MethodDeclaration | AccessorDeclaration) {
@@ -1095,8 +1114,31 @@ export function transformClassFields(context: TransformationContext): (x: Source
10951114
return transformFieldInitializer(node);
10961115
}
10971116

1117+
function shouldForceDynamicThis() {
1118+
return !!currentClassElement &&
1119+
hasStaticModifier(currentClassElement) &&
1120+
isAccessor(currentClassElement) &&
1121+
isAutoAccessorPropertyDeclaration(getOriginalNode(currentClassElement));
1122+
}
1123+
1124+
/**
1125+
* Prevent substitution of `this` to `_classThis` in static getters and setters that wrap `accessor` fields.
1126+
*/
1127+
function ensureDynamicThisIfNeeded(node: Expression) {
1128+
if (shouldForceDynamicThis()) {
1129+
// do not substitute `this` with `_classThis` when `this`
1130+
// should be bound dynamically.
1131+
const innerExpression = skipOuterExpressions(node);
1132+
if (innerExpression.kind === SyntaxKind.ThisKeyword) {
1133+
noSubstitution.add(innerExpression);
1134+
}
1135+
}
1136+
}
1137+
10981138
function createPrivateIdentifierAccess(info: PrivateIdentifierInfo, receiver: Expression): Expression {
1099-
return createPrivateIdentifierAccessHelper(info, visitNode(receiver, visitor, isExpression));
1139+
receiver = visitNode(receiver, visitor, isExpression);
1140+
ensureDynamicThisIfNeeded(receiver);
1141+
return createPrivateIdentifierAccessHelper(info, receiver);
11001142
}
11011143

11021144
function createPrivateIdentifierAccessHelper(info: PrivateIdentifierInfo, receiver: Expression): Expression {
@@ -1146,9 +1188,10 @@ export function transformClassFields(context: TransformationContext): (x: Source
11461188
}
11471189
}
11481190
if (shouldTransformSuperInStaticInitializers &&
1191+
currentClassElement &&
11491192
isSuperProperty(node) &&
11501193
isIdentifier(node.name) &&
1151-
currentStaticPropertyDeclarationOrStaticBlock &&
1194+
isStaticPropertyDeclarationOrClassStaticBlock(currentClassElement) &&
11521195
lexicalEnvironment?.data) {
11531196
const { classConstructor, superClassReference, facts } = lexicalEnvironment.data;
11541197
if (facts & ClassFacts.ClassWasDecorated) {
@@ -1171,8 +1214,9 @@ export function transformClassFields(context: TransformationContext): (x: Source
11711214

11721215
function visitElementAccessExpression(node: ElementAccessExpression) {
11731216
if (shouldTransformSuperInStaticInitializers &&
1217+
currentClassElement &&
11741218
isSuperProperty(node) &&
1175-
currentStaticPropertyDeclarationOrStaticBlock &&
1219+
isStaticPropertyDeclarationOrClassStaticBlock(currentClassElement) &&
11761220
lexicalEnvironment?.data) {
11771221
const { classConstructor, superClassReference, facts } = lexicalEnvironment.data;
11781222
if (facts & ClassFacts.ClassWasDecorated) {
@@ -1203,6 +1247,7 @@ export function transformClassFields(context: TransformationContext): (x: Source
12031247
let info: PrivateIdentifierInfo | undefined;
12041248
if (info = accessPrivateIdentifier(operand.name)) {
12051249
const receiver = visitNode(operand.expression, visitor, isExpression);
1250+
ensureDynamicThisIfNeeded(receiver);
12061251
const { readExpression, initializeExpression } = createCopiableReceiverExpr(receiver);
12071252

12081253
let expression: Expression = createPrivateIdentifierAccess(info, readExpression);
@@ -1224,8 +1269,9 @@ export function transformClassFields(context: TransformationContext): (x: Source
12241269
}
12251270
}
12261271
else if (shouldTransformSuperInStaticInitializers &&
1272+
currentClassElement &&
12271273
isSuperProperty(operand) &&
1228-
currentStaticPropertyDeclarationOrStaticBlock &&
1274+
isStaticPropertyDeclarationOrClassStaticBlock(currentClassElement) &&
12291275
lexicalEnvironment?.data) {
12301276
// converts `++super.a` into `(Reflect.set(_baseTemp, "a", (_a = Reflect.get(_baseTemp, "a", _classTemp), _b = ++_a), _classTemp), _b)`
12311277
// 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
12991345

13001346
function createCopiableReceiverExpr(receiver: Expression): { readExpression: Expression; initializeExpression: Expression | undefined } {
13011347
const clone = nodeIsSynthesized(receiver) ? receiver : factory.cloneNode(receiver);
1348+
if (receiver.kind === SyntaxKind.ThisKeyword && noSubstitution.has(receiver)) {
1349+
noSubstitution.add(clone);
1350+
}
13021351
if (isSimpleInlineableExpression(receiver)) {
13031352
return { readExpression: clone, initializeExpression: undefined };
13041353
}
@@ -1332,8 +1381,9 @@ export function transformClassFields(context: TransformationContext): (x: Source
13321381
}
13331382

13341383
if (shouldTransformSuperInStaticInitializers &&
1384+
currentClassElement &&
13351385
isSuperProperty(node.expression) &&
1336-
currentStaticPropertyDeclarationOrStaticBlock &&
1386+
isStaticPropertyDeclarationOrClassStaticBlock(currentClassElement) &&
13371387
lexicalEnvironment?.data?.classConstructor) {
13381388
// super.x()
13391389
// super[x]()
@@ -1369,8 +1419,9 @@ export function transformClassFields(context: TransformationContext): (x: Source
13691419
);
13701420
}
13711421
if (shouldTransformSuperInStaticInitializers &&
1422+
currentClassElement &&
13721423
isSuperProperty(node.tag) &&
1373-
currentStaticPropertyDeclarationOrStaticBlock &&
1424+
isStaticPropertyDeclarationOrClassStaticBlock(currentClassElement) &&
13741425
lexicalEnvironment?.data?.classConstructor) {
13751426

13761427
// converts `` super.f`x` `` into `` Reflect.get(_baseTemp, "f", _classTemp).bind(_classTemp)`x` ``
@@ -1398,7 +1449,7 @@ export function transformClassFields(context: TransformationContext): (x: Source
13981449

13991450
if (shouldTransformPrivateElementsOrClassStaticBlocks) {
14001451
startLexicalEnvironment();
1401-
let statements = setCurrentStaticPropertyDeclarationOrStaticBlockAnd(
1452+
let statements = setCurrentClassElementAnd(
14021453
node,
14031454
statements => visitNodes(statements, visitor, isStatement),
14041455
node.body.statements
@@ -1506,8 +1557,9 @@ export function transformClassFields(context: TransformationContext): (x: Source
15061557
}
15071558
}
15081559
else if (shouldTransformSuperInStaticInitializers &&
1560+
currentClassElement &&
15091561
isSuperProperty(node.left) &&
1510-
currentStaticPropertyDeclarationOrStaticBlock &&
1562+
isStaticPropertyDeclarationOrClassStaticBlock(currentClassElement) &&
15111563
lexicalEnvironment?.data) {
15121564
// super.x = ...
15131565
// super[x] = ...
@@ -1641,6 +1693,7 @@ export function transformClassFields(context: TransformationContext): (x: Source
16411693
function createPrivateIdentifierAssignment(info: PrivateIdentifierInfo, receiver: Expression, right: Expression, operator: AssignmentOperator): Expression {
16421694
receiver = visitNode(receiver, visitor, isExpression);
16431695
right = visitNode(right, visitor, isExpression);
1696+
ensureDynamicThisIfNeeded(receiver);
16441697

16451698
if (isCompoundAssignment(operator)) {
16461699
const { readExpression, initializeExpression } = createCopiableReceiverExpr(receiver);
@@ -2420,7 +2473,7 @@ export function transformClassFields(context: TransformationContext): (x: Source
24202473
* @param receiver The object receiving the property assignment.
24212474
*/
24222475
function transformProperty(property: PropertyDeclaration, receiver: LeftHandSideExpression) {
2423-
const savedCurrentStaticPropertyDeclarationOrStaticBlock = currentStaticPropertyDeclarationOrStaticBlock;
2476+
const savedCurrentClassElement = currentClassElement;
24242477
const transformed = transformPropertyWorker(property, receiver);
24252478
if (transformed && hasStaticModifier(property) && lexicalEnvironment?.data?.facts) {
24262479
// capture the lexical environment for the member
@@ -2429,7 +2482,7 @@ export function transformClassFields(context: TransformationContext): (x: Source
24292482
setSourceMapRange(transformed, getSourceMapRange(property.name));
24302483
lexicalEnvironmentMap.set(getOriginalNode(property), lexicalEnvironment);
24312484
}
2432-
currentStaticPropertyDeclarationOrStaticBlock = savedCurrentStaticPropertyDeclarationOrStaticBlock;
2485+
currentClassElement = savedCurrentClassElement;
24332486
return transformed;
24342487
}
24352488

@@ -2458,7 +2511,7 @@ export function transformClassFields(context: TransformationContext): (x: Source
24582511
property.name;
24592512

24602513
if (hasStaticModifier(property)) {
2461-
currentStaticPropertyDeclarationOrStaticBlock = property;
2514+
currentClassElement = property;
24622515
}
24632516

24642517
const initializerVisitor: Visitor =
@@ -2931,8 +2984,9 @@ export function transformClassFields(context: TransformationContext): (x: Source
29312984
return wrapPrivateIdentifierForDestructuringTarget(node);
29322985
}
29332986
else if (shouldTransformSuperInStaticInitializers &&
2987+
currentClassElement &&
29342988
isSuperProperty(node) &&
2935-
currentStaticPropertyDeclarationOrStaticBlock &&
2989+
isStaticPropertyDeclarationOrClassStaticBlock(currentClassElement) &&
29362990
lexicalEnvironment?.data) {
29372991
const { classConstructor, superClassReference, facts } = lexicalEnvironment.data;
29382992
if (facts & ClassFacts.ClassWasDecorated) {
@@ -3164,6 +3218,7 @@ export function transformClassFields(context: TransformationContext): (x: Source
31643218
*/
31653219
function onSubstituteNode(hint: EmitHint, node: Node) {
31663220
node = previousOnSubstituteNode(hint, node);
3221+
31673222
if (hint === EmitHint.Expression) {
31683223
return substituteExpression(node as Expression);
31693224
}
@@ -3181,7 +3236,9 @@ export function transformClassFields(context: TransformationContext): (x: Source
31813236
}
31823237

31833238
function substituteThisExpression(node: ThisExpression) {
3184-
if (enabledSubstitutions & ClassPropertySubstitutionFlags.ClassStaticThisOrSuperReference && lexicalEnvironment?.data) {
3239+
if (enabledSubstitutions & ClassPropertySubstitutionFlags.ClassStaticThisOrSuperReference &&
3240+
lexicalEnvironment?.data &&
3241+
!noSubstitution.has(node)) {
31853242
const { facts, classConstructor, classThis } = lexicalEnvironment.data;
31863243
if (facts & ClassFacts.ClassWasDecorated && legacyDecorators) {
31873244
return factory.createParenthesizedExpression(factory.createVoidZero());
@@ -3264,3 +3321,11 @@ function isPrivateIdentifierInExpression(node: BinaryExpression): node is Privat
32643321
return isPrivateIdentifier(node.left)
32653322
&& node.operatorToken.kind === SyntaxKind.InKeyword;
32663323
}
3324+
3325+
function isStaticPropertyDeclaration(node: Node): node is PropertyDeclaration {
3326+
return isPropertyDeclaration(node) && hasStaticModifier(node);
3327+
}
3328+
3329+
function isStaticPropertyDeclarationOrClassStaticBlock(node: Node): node is ClassStaticBlockDeclaration | PropertyDeclaration {
3330+
return isClassStaticBlockDeclaration(node) || isStaticPropertyDeclaration(node);
3331+
}

‎tests/baselines/reference/esDecorators-classDeclaration-classThisReference(target=es2015).js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ let C = (() => {
1919
let _classExtraInitializers = [];
2020
let _classThis;
2121
var C = _classThis = class {
22-
static get a() { return __classPrivateFieldGet(_classThis, _classThis, "f", _a_accessor_storage); }
23-
static set a(value) { __classPrivateFieldSet(_classThis, _classThis, value, "f", _a_accessor_storage); }
22+
static get a() { return __classPrivateFieldGet(this, _classThis, "f", _a_accessor_storage); }
23+
static set a(value) { __classPrivateFieldSet(this, _classThis, value, "f", _a_accessor_storage); }
2424
static m() { this; }
2525
static get g() { return this; }
2626
};

0 commit comments

Comments
 (0)