Skip to content

Commit 03a0b65

Browse files
mkubilaykmkubilayk
authored
leave bad code in for #constructor and duplicate private names (#58)
1 parent 55a7097 commit 03a0b65

File tree

4 files changed

+233
-74
lines changed

4 files changed

+233
-74
lines changed

‎src/compiler/transformers/classFields.ts

Lines changed: 63 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ namespace ts {
2424
* Stores if the identifier is static or not
2525
*/
2626
isStatic: boolean;
27+
/**
28+
* Stores if the identifier declaration is valid or not. Reserved names (e.g. #constructor)
29+
* or duplicate identifiers are considered invalid.
30+
*/
31+
isValid: boolean;
2732
}
2833
interface PrivateIdentifierAccessorInfo extends PrivateIdentifierInfoBase {
2934
kind: PrivateIdentifierKind.Accessor;
@@ -260,6 +265,13 @@ namespace ts {
260265
return visitEachChild(node, classElementVisitor, context);
261266
}
262267

268+
// leave invalid code untransformed
269+
const info = accessPrivateIdentifier(node.name);
270+
Debug.assert(info, "Undeclared private name for property declaration.");
271+
if (!info.isValid) {
272+
return node;
273+
}
274+
263275
const functionName = getHoistedFunctionName(node);
264276
if (functionName) {
265277
getPendingExpressions().push(
@@ -303,17 +315,27 @@ namespace ts {
303315

304316
function visitPropertyDeclaration(node: PropertyDeclaration) {
305317
Debug.assert(!some(node.decorators));
306-
if (!shouldTransformPrivateElements && isPrivateIdentifier(node.name)) {
307-
// Initializer is elided as the field is initialized in transformConstructor.
308-
return factory.updatePropertyDeclaration(
309-
node,
310-
/*decorators*/ undefined,
311-
visitNodes(node.modifiers, visitor, isModifier),
312-
node.name,
313-
/*questionOrExclamationToken*/ undefined,
314-
/*type*/ undefined,
315-
/*initializer*/ undefined
316-
);
318+
319+
if (isPrivateIdentifier(node.name)) {
320+
if (!shouldTransformPrivateElements) {
321+
// Initializer is elided as the field is initialized in transformConstructor.
322+
return factory.updatePropertyDeclaration(
323+
node,
324+
/*decorators*/ undefined,
325+
visitNodes(node.modifiers, visitor, isModifier),
326+
node.name,
327+
/*questionOrExclamationToken*/ undefined,
328+
/*type*/ undefined,
329+
/*initializer*/ undefined
330+
);
331+
}
332+
333+
// leave invalid code untransformed
334+
const info = accessPrivateIdentifier(node.name);
335+
Debug.assert(info, "Undeclared private name for property declaration.");
336+
if (!info.isValid) {
337+
return node;
338+
}
317339
}
318340
// Create a temporary variable to store a computed property name (if necessary).
319341
// If it's not inlineable, then we emit an expression after the class which assigns
@@ -1162,55 +1184,62 @@ namespace ts {
11621184
const env = getPrivateIdentifierEnvironment();
11631185
const { weakSetName, classConstructor } = env;
11641186
const assignmentExpressions: Expression[] = [];
1187+
1188+
const privateName = node.name.escapedText;
1189+
const previousInfo = env.identifiers.get(privateName);
1190+
const isValid = !isReservedPrivateName(node.name) && previousInfo === undefined;
1191+
11651192
if (hasStaticModifier(node)) {
11661193
Debug.assert(classConstructor, "weakSetName should be set in private identifier environment");
11671194
if (isPropertyDeclaration(node)) {
11681195
const variableName = createHoistedVariableForPrivateName(text, node);
1169-
env.identifiers.set(node.name.escapedText, {
1196+
env.identifiers.set(privateName, {
11701197
kind: PrivateIdentifierKind.Field,
11711198
variableName,
11721199
brandCheckIdentifier: classConstructor,
11731200
isStatic: true,
1201+
isValid,
11741202
});
11751203
}
11761204
else if (isMethodDeclaration(node)) {
11771205
const functionName = createHoistedVariableForPrivateName(text, node);
1178-
env.identifiers.set(node.name.escapedText, {
1206+
env.identifiers.set(privateName, {
11791207
kind: PrivateIdentifierKind.Method,
11801208
methodName: functionName,
11811209
brandCheckIdentifier: classConstructor,
11821210
isStatic: true,
1211+
isValid,
11831212
});
11841213
}
11851214
else if (isGetAccessorDeclaration(node)) {
11861215
const getterName = createHoistedVariableForPrivateName(text + "_get", node);
1187-
const previousInfo = env.identifiers.get(node.name.escapedText);
1188-
if (previousInfo?.kind === PrivateIdentifierKind.Accessor) {
1216+
if (previousInfo?.kind === PrivateIdentifierKind.Accessor && previousInfo.isStatic && !previousInfo.getterName) {
11891217
previousInfo.getterName = getterName;
11901218
}
11911219
else {
1192-
env.identifiers.set(node.name.escapedText, {
1220+
env.identifiers.set(privateName, {
11931221
kind: PrivateIdentifierKind.Accessor,
11941222
getterName,
11951223
setterName: undefined,
11961224
brandCheckIdentifier: classConstructor,
11971225
isStatic: true,
1226+
isValid,
11981227
});
11991228
}
12001229
}
12011230
else if (isSetAccessorDeclaration(node)) {
12021231
const setterName = createHoistedVariableForPrivateName(text + "_set", node);
1203-
const previousInfo = env.identifiers.get(node.name.escapedText);
1204-
if (previousInfo?.kind === PrivateIdentifierKind.Accessor) {
1232+
if (previousInfo?.kind === PrivateIdentifierKind.Accessor && previousInfo.isStatic && !previousInfo.setterName) {
12051233
previousInfo.setterName = setterName;
12061234
}
12071235
else {
1208-
env.identifiers.set(node.name.escapedText, {
1236+
env.identifiers.set(privateName, {
12091237
kind: PrivateIdentifierKind.Accessor,
12101238
getterName: undefined,
12111239
setterName,
12121240
brandCheckIdentifier: classConstructor,
12131241
isStatic: true,
1242+
isValid,
12141243
});
12151244
}
12161245
}
@@ -1220,11 +1249,12 @@ namespace ts {
12201249
}
12211250
else if (isPropertyDeclaration(node)) {
12221251
const weakMapName = createHoistedVariableForPrivateName(text, node);
1223-
env.identifiers.set(node.name.escapedText, {
1252+
env.identifiers.set(privateName, {
12241253
kind: PrivateIdentifierKind.Field,
12251254
brandCheckIdentifier: weakMapName,
12261255
isStatic: false,
1227-
variableName: undefined
1256+
variableName: undefined,
1257+
isValid,
12281258
});
12291259

12301260
assignmentExpressions.push(factory.createAssignment(
@@ -1239,46 +1269,48 @@ namespace ts {
12391269
else if (isMethodDeclaration(node)) {
12401270
Debug.assert(weakSetName, "weakSetName should be set in private identifier environment");
12411271

1242-
env.identifiers.set(node.name.escapedText, {
1272+
env.identifiers.set(privateName, {
12431273
kind: PrivateIdentifierKind.Method,
12441274
methodName: createHoistedVariableForPrivateName(text, node),
12451275
brandCheckIdentifier: weakSetName,
12461276
isStatic: false,
1277+
isValid,
12471278
});
12481279
}
12491280
else if (isAccessor(node)) {
12501281
Debug.assert(weakSetName, "weakSetName should be set in private identifier environment");
1251-
const previousInfo = env.identifiers.get(node.name.escapedText);
12521282

12531283
if (isGetAccessor(node)) {
12541284
const getterName = createHoistedVariableForPrivateName(text + "_get", node);
12551285

1256-
if (previousInfo?.kind === PrivateIdentifierKind.Accessor) {
1286+
if (previousInfo?.kind === PrivateIdentifierKind.Accessor && !previousInfo.isStatic && !previousInfo.getterName) {
12571287
previousInfo.getterName = getterName;
12581288
}
12591289
else {
1260-
env.identifiers.set(node.name.escapedText, {
1290+
env.identifiers.set(privateName, {
12611291
kind: PrivateIdentifierKind.Accessor,
12621292
getterName,
12631293
setterName: undefined,
12641294
brandCheckIdentifier: weakSetName,
12651295
isStatic: false,
1296+
isValid,
12661297
});
12671298
}
12681299
}
12691300
else {
12701301
const setterName = createHoistedVariableForPrivateName(text + "_set", node);
12711302

1272-
if (previousInfo?.kind === PrivateIdentifierKind.Accessor) {
1303+
if (previousInfo?.kind === PrivateIdentifierKind.Accessor && !previousInfo.isStatic && !previousInfo.setterName) {
12731304
previousInfo.setterName = setterName;
12741305
}
12751306
else {
1276-
env.identifiers.set(node.name.escapedText, {
1307+
env.identifiers.set(privateName, {
12771308
kind: PrivateIdentifierKind.Accessor,
12781309
getterName: undefined,
12791310
setterName,
12801311
brandCheckIdentifier: weakSetName,
12811312
isStatic: false,
1313+
isValid,
12821314
});
12831315
}
12841316
}
@@ -1475,4 +1507,8 @@ namespace ts {
14751507
[receiver]
14761508
);
14771509
}
1510+
1511+
function isReservedPrivateName(node: PrivateIdentifier) {
1512+
return node.escapedText === "#constructor";
1513+
}
14781514
}

‎tests/baselines/reference/privateNameConstructorReserved.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,6 @@ class A {
1010
constructor() {
1111
_A_instances.add(this);
1212
}
13+
#constructor() { } // Error: `#constructor` is a reserved word.
1314
}
14-
_A_instances = new WeakSet(), _A_constructor = function _A_constructor() { };
15+
_A_instances = new WeakSet();

0 commit comments

Comments
 (0)