Skip to content

Commit 2484210

Browse files
dragomirtitiandragomirtitian
authored
Gh 41788 incorrect output for esprivate with nested class in esnext (microsoft#42663)
* If target:esnext,then useDefineForClassFields: true will now be the default. * Added error if a private identifier is used in a static a initializer if target:ESNext and useDefineForClassFields:false. * Added test for new useDefineForClassFields default and error message. * Fixed tests after changing the default of useDefineForClassFields to true for target esnext * Fixed code review suggestions. * Updated error message. * Added missing static check for the containing property. Fixed other code review issues.
1 parent 2bb54dc commit 2484210

File tree

64 files changed

+1109
-54
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

64 files changed

+1109
-54
lines changed

‎src/compiler/checker.ts

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,7 @@ namespace ts {
346346
const compilerOptions = host.getCompilerOptions();
347347
const languageVersion = getEmitScriptTarget(compilerOptions);
348348
const moduleKind = getEmitModuleKind(compilerOptions);
349+
const useDefineForClassFields = getUseDefineForClassFields(compilerOptions);
349350
const allowSyntheticDefaultImports = getAllowSyntheticDefaultImports(compilerOptions);
350351
const strictNullChecks = getStrictOptionValue(compilerOptions, "strictNullChecks");
351352
const strictFunctionTypes = getStrictOptionValue(compilerOptions, "strictFunctionTypes");
@@ -1501,7 +1502,7 @@ namespace ts {
15011502
}
15021503
else if (isParameterPropertyDeclaration(declaration, declaration.parent)) {
15031504
// foo = this.bar is illegal in esnext+useDefineForClassFields when bar is a parameter property
1504-
return !(compilerOptions.target === ScriptTarget.ESNext && !!compilerOptions.useDefineForClassFields
1505+
return !(compilerOptions.target === ScriptTarget.ESNext && useDefineForClassFields
15051506
&& getContainingClass(declaration) === getContainingClass(usage)
15061507
&& isUsedInFunctionOrInstanceProperty(usage, declaration));
15071508
}
@@ -1532,7 +1533,7 @@ namespace ts {
15321533
return true;
15331534
}
15341535
if (isUsedInFunctionOrInstanceProperty(usage, declaration)) {
1535-
if (compilerOptions.target === ScriptTarget.ESNext && !!compilerOptions.useDefineForClassFields
1536+
if (compilerOptions.target === ScriptTarget.ESNext && useDefineForClassFields
15361537
&& getContainingClass(declaration)
15371538
&& (isPropertyDeclaration(declaration) || isParameterPropertyDeclaration(declaration, declaration.parent))) {
15381539
return !isPropertyImmediatelyReferencedWithinDeclaration(declaration, usage, /*stopAtAnyPropertyDeclaration*/ true);
@@ -1680,7 +1681,7 @@ namespace ts {
16801681
case SyntaxKind.PropertyDeclaration:
16811682
// static properties in classes introduce temporary variables
16821683
if (hasStaticModifier(node)) {
1683-
return target < ScriptTarget.ESNext || !compilerOptions.useDefineForClassFields;
1684+
return target < ScriptTarget.ESNext || !useDefineForClassFields;
16841685
}
16851686
return requiresScopeChangeWorker((node as PropertyDeclaration).name);
16861687
default:
@@ -2098,7 +2099,7 @@ namespace ts {
20982099

20992100
// Perform extra checks only if error reporting was requested
21002101
if (nameNotFoundMessage) {
2101-
if (propertyWithInvalidInitializer && !(compilerOptions.target === ScriptTarget.ESNext && compilerOptions.useDefineForClassFields)) {
2102+
if (propertyWithInvalidInitializer && !(compilerOptions.target === ScriptTarget.ESNext && useDefineForClassFields)) {
21022103
// We have a match, but the reference occurred within a property initializer and the identifier also binds
21032104
// to a local variable in the constructor where the code will be emitted. Note that this is actually allowed
21042105
// with ESNext+useDefineForClassFields because the scope semantics are different.
@@ -24260,7 +24261,7 @@ namespace ts {
2426024261
break;
2426124262
case SyntaxKind.PropertyDeclaration:
2426224263
case SyntaxKind.PropertySignature:
24263-
if (hasSyntacticModifier(container, ModifierFlags.Static) && !(compilerOptions.target === ScriptTarget.ESNext && compilerOptions.useDefineForClassFields)) {
24264+
if (hasSyntacticModifier(container, ModifierFlags.Static) && !(compilerOptions.target === ScriptTarget.ESNext && useDefineForClassFields)) {
2426424265
error(node, Diagnostics.this_cannot_be_referenced_in_a_static_property_initializer);
2426524266
// do not return here so in case if lexical this is captured - it will be reflected in flags on NodeLinks
2426624267
}
@@ -27057,6 +27058,30 @@ namespace ts {
2705727058
if (assignmentKind && lexicallyScopedSymbol && lexicallyScopedSymbol.valueDeclaration && isMethodDeclaration(lexicallyScopedSymbol.valueDeclaration)) {
2705827059
grammarErrorOnNode(right, Diagnostics.Cannot_assign_to_private_method_0_Private_methods_are_not_writable, idText(right));
2705927060
}
27061+
27062+
if (lexicallyScopedSymbol?.valueDeclaration && (compilerOptions.target === ScriptTarget.ESNext && !useDefineForClassFields)) {
27063+
const lexicalClass = getContainingClass(lexicallyScopedSymbol.valueDeclaration);
27064+
const parentStaticFieldInitializer = findAncestor(node, (n) => {
27065+
if (n === lexicalClass) return "quit";
27066+
if (isPropertyDeclaration(n.parent) && hasStaticModifier(n.parent) && n.parent.initializer === n && n.parent.parent === lexicalClass) {
27067+
return true;
27068+
}
27069+
return false;
27070+
});
27071+
if (parentStaticFieldInitializer) {
27072+
const parentStaticFieldInitializerSymbol = getSymbolOfNode(parentStaticFieldInitializer.parent);
27073+
Debug.assert(parentStaticFieldInitializerSymbol, "Initializer without declaration symbol");
27074+
const diagnostic = error(node,
27075+
Diagnostics.Property_0_may_not_be_used_in_a_static_property_s_initializer_in_the_same_class_when_target_is_esnext_and_useDefineForClassFields_is_false,
27076+
symbolName(lexicallyScopedSymbol));
27077+
addRelatedInfo(diagnostic,
27078+
createDiagnosticForNode(parentStaticFieldInitializer.parent,
27079+
Diagnostics.Initializer_for_property_0,
27080+
symbolName(parentStaticFieldInitializerSymbol))
27081+
);
27082+
}
27083+
}
27084+
2706027085
if (isAnyLike) {
2706127086
if (lexicallyScopedSymbol) {
2706227087
return apparentType;
@@ -33134,7 +33159,7 @@ namespace ts {
3313433159
// - The constructor declares parameter properties
3313533160
// or the containing class declares instance member variables with initializers.
3313633161
const superCallShouldBeFirst =
33137-
(compilerOptions.target !== ScriptTarget.ESNext || !compilerOptions.useDefineForClassFields) &&
33162+
(compilerOptions.target !== ScriptTarget.ESNext || !useDefineForClassFields) &&
3313833163
(some((<ClassDeclaration>node.parent).members, isInstancePropertyWithInitializerOrPrivateIdentifierProperty) ||
3313933164
some(node.parameters, p => hasSyntacticModifier(p, ModifierFlags.ParameterPropertyModifier)));
3314033165

@@ -37142,7 +37167,7 @@ namespace ts {
3714237167
Diagnostics._0_is_defined_as_a_property_in_class_1_but_is_overridden_here_in_2_as_an_accessor;
3714337168
error(getNameOfDeclaration(derived.valueDeclaration) || derived.valueDeclaration, errorMessage, symbolToString(base), typeToString(baseType), typeToString(type));
3714437169
}
37145-
else if (compilerOptions.useDefineForClassFields) {
37170+
else if (useDefineForClassFields) {
3714637171
const uninitialized = derived.declarations?.find(d => d.kind === SyntaxKind.PropertyDeclaration && !(d as PropertyDeclaration).initializer);
3714737172
if (uninitialized
3714837173
&& !(derived.flags & SymbolFlags.Transient)

‎src/compiler/diagnosticMessages.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3312,6 +3312,14 @@
33123312
"category": "Error",
33133313
"code": 2809
33143314
},
3315+
"Property '{0}' may not be used in a static property's initializer in the same class when 'target' is 'esnext' and 'useDefineForClassFields' is 'false'.": {
3316+
"category": "Error",
3317+
"code": 2810
3318+
},
3319+
"Initializer for property '{0}'": {
3320+
"category": "Error",
3321+
"code": 2811
3322+
},
33153323

33163324
"Import declaration '{0}' is using private name '{1}'.": {
33173325
"category": "Error",

‎src/compiler/transformers/classFields.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ namespace ts {
108108
const resolver = context.getEmitResolver();
109109
const compilerOptions = context.getCompilerOptions();
110110
const languageVersion = getEmitScriptTarget(compilerOptions);
111+
const useDefineForClassFields = getUseDefineForClassFields(compilerOptions);
111112

112113
const shouldTransformPrivateElements = languageVersion < ScriptTarget.ESNext;
113114

@@ -138,7 +139,7 @@ namespace ts {
138139
function transformSourceFile(node: SourceFile) {
139140
const options = context.getCompilerOptions();
140141
if (node.isDeclarationFile
141-
|| options.useDefineForClassFields && options.target === ScriptTarget.ESNext) {
142+
|| useDefineForClassFields && options.target === ScriptTarget.ESNext) {
142143
return node;
143144
}
144145
const visited = visitEachChild(node, visitor, context);
@@ -340,7 +341,7 @@ namespace ts {
340341
// Create a temporary variable to store a computed property name (if necessary).
341342
// If it's not inlineable, then we emit an expression after the class which assigns
342343
// the property name to the temporary variable.
343-
const expr = getPropertyNameExpressionIfNeeded(node.name, !!node.initializer || !!context.getCompilerOptions().useDefineForClassFields);
344+
const expr = getPropertyNameExpressionIfNeeded(node.name, !!node.initializer || useDefineForClassFields);
344345
if (expr && !isSimpleInlineableExpression(expr)) {
345346
getPendingExpressions().push(expr);
346347
}
@@ -829,7 +830,7 @@ namespace ts {
829830
if (hasStaticModifier(member) || hasSyntacticModifier(getOriginalNode(member), ModifierFlags.Abstract)) {
830831
return false;
831832
}
832-
if (context.getCompilerOptions().useDefineForClassFields) {
833+
if (useDefineForClassFields) {
833834
// If we are using define semantics and targeting ESNext or higher,
834835
// then we don't need to transform any class properties.
835836
return languageVersion < ScriptTarget.ESNext;
@@ -865,7 +866,6 @@ namespace ts {
865866
}
866867

867868
function transformConstructorBody(node: ClassDeclaration | ClassExpression, constructor: ConstructorDeclaration | undefined, isDerivedClass: boolean) {
868-
const useDefineForClassFields = context.getCompilerOptions().useDefineForClassFields;
869869
let properties = getProperties(node, /*requireInitializer*/ false, /*isStatic*/ false);
870870
if (!useDefineForClassFields) {
871871
properties = filter(properties, property => !!property.initializer || isPrivateIdentifier(property.name));
@@ -1000,7 +1000,7 @@ namespace ts {
10001000
*/
10011001
function transformProperty(property: PropertyDeclaration, receiver: LeftHandSideExpression) {
10021002
// We generate a name here in order to reuse the value cached by the relocated computed name expression (which uses the same generated name)
1003-
const emitAssignment = !context.getCompilerOptions().useDefineForClassFields;
1003+
const emitAssignment = !useDefineForClassFields;
10041004
const propertyName = isComputedPropertyName(property.name) && !isSimpleInlineableExpression(property.name.expression)
10051005
? factory.updateComputedPropertyName(property.name, factory.getGeneratedNameForNode(property.name))
10061006
: property.name;

‎src/compiler/transformers/es2015.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1626,7 +1626,7 @@ namespace ts {
16261626
const memberFunction = transformFunctionLikeToExpression(member, /*location*/ member, /*name*/ undefined, container);
16271627
const propertyName = visitNode(member.name, visitor, isPropertyName);
16281628
let e: Expression;
1629-
if (!isPrivateIdentifier(propertyName) && context.getCompilerOptions().useDefineForClassFields) {
1629+
if (!isPrivateIdentifier(propertyName) && getUseDefineForClassFields(context.getCompilerOptions())) {
16301630
const name = isComputedPropertyName(propertyName) ? propertyName.expression
16311631
: isIdentifier(propertyName) ? factory.createStringLiteral(unescapeLeadingUnderscores(propertyName.escapedText))
16321632
: propertyName;

‎src/compiler/utilities.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6071,6 +6071,10 @@ namespace ts {
60716071
return compilerOptions.allowJs === undefined ? !!compilerOptions.checkJs : compilerOptions.allowJs;
60726072
}
60736073

6074+
export function getUseDefineForClassFields(compilerOptions: CompilerOptions): boolean {
6075+
return compilerOptions.useDefineForClassFields === undefined ? compilerOptions.target === ScriptTarget.ESNext : compilerOptions.useDefineForClassFields;
6076+
}
6077+
60746078
export function compilerOptionsAffectSemanticDiagnostics(newOptions: CompilerOptions, oldOptions: CompilerOptions): boolean {
60756079
return oldOptions !== newOptions &&
60766080
semanticDiagnosticsOptionDeclarations.some(option => !isJsonEqual(getCompilerOptionValue(oldOptions, option), getCompilerOptionValue(newOptions, option)));

‎src/testRunner/unittests/transform.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ namespace ts {
176176
compilerOptions: {
177177
target: ScriptTarget.ESNext,
178178
newLine: NewLineKind.CarriageReturnLineFeed,
179+
useDefineForClassFields: false,
179180
}
180181
}).outputText;
181182
});

‎tests/baselines/reference/classWithStaticFieldInParameterBindingPattern(target=esnext).js

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
//// [classWithStaticFieldInParameterBindingPattern.js]
66
// https://github.com/microsoft/TypeScript/issues/36295
7-
((_a) => { var _b; var { [(_b = class {
8-
},
9-
_b.x = 1,
10-
_b).x]: b = "" } = _a; })();
7+
(({ [class {
8+
static x = 1;
9+
}.x]: b = "" }) => { })();

‎tests/baselines/reference/classWithStaticFieldInParameterInitializer(target=esnext).js

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
//// [classWithStaticFieldInParameterInitializer.js]
66
// https://github.com/microsoft/TypeScript/issues/36295
7-
((b) => { var _a; if (b === void 0) { b = (_a = class {
8-
},
9-
_a.x = 1,
10-
_a); } })();
7+
((b = class {
8+
static x = 1;
9+
}) => { })();

‎tests/baselines/reference/privateNameAndStaticInitializer(target=esnext).js

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,7 @@ class A {
99

1010
//// [privateNameAndStaticInitializer.js]
1111
class A {
12-
constructor() {
13-
this.#foo = 1;
14-
this.#prop = 2;
15-
}
16-
#foo;
17-
#prop;
12+
#foo = 1;
13+
static inst = new A();
14+
#prop = 2;
1815
}
19-
A.inst = new A();

‎tests/baselines/reference/privateNameComputedPropertyName1(target=esnext).js

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,15 @@ new A().test();
3939

4040
//// [privateNameComputedPropertyName1.js]
4141
class A {
42+
#a = 'a';
43+
#b;
44+
#c = 'c';
45+
#d;
46+
#e = '';
4247
constructor() {
43-
this.#a = 'a';
44-
this.#c = 'c';
45-
this.#e = '';
4648
this.#b = 'b';
4749
this.#d = 'd';
4850
}
49-
#a;
50-
#b;
51-
#c;
52-
#d;
53-
#e;
5451
test() {
5552
const data = { a: 'a', b: 'b', c: 'c', d: 'd', e: 'e' };
5653
const { [this.#a]: a, [this.#b]: b, [this.#c]: c, [this.#d]: d, [this.#e = 'e']: e, } = data;

0 commit comments

Comments
 (0)