Skip to content
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

Gh 41788 incorrect output for esprivate with nested class in esnext #42663

Merged
39 changes: 32 additions & 7 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ namespace ts {
const compilerOptions = host.getCompilerOptions();
const languageVersion = getEmitScriptTarget(compilerOptions);
const moduleKind = getEmitModuleKind(compilerOptions);
const useDefineForClassFields = getUseDefineForClassFields(compilerOptions);
const allowSyntheticDefaultImports = getAllowSyntheticDefaultImports(compilerOptions);
const strictNullChecks = getStrictOptionValue(compilerOptions, "strictNullChecks");
const strictFunctionTypes = getStrictOptionValue(compilerOptions, "strictFunctionTypes");
Expand Down Expand Up @@ -1501,7 +1502,7 @@ namespace ts {
}
else if (isParameterPropertyDeclaration(declaration, declaration.parent)) {
// foo = this.bar is illegal in esnext+useDefineForClassFields when bar is a parameter property
return !(compilerOptions.target === ScriptTarget.ESNext && !!compilerOptions.useDefineForClassFields
return !(compilerOptions.target === ScriptTarget.ESNext && useDefineForClassFields
&& getContainingClass(declaration) === getContainingClass(usage)
&& isUsedInFunctionOrInstanceProperty(usage, declaration));
}
Expand Down Expand Up @@ -1532,7 +1533,7 @@ namespace ts {
return true;
}
if (isUsedInFunctionOrInstanceProperty(usage, declaration)) {
if (compilerOptions.target === ScriptTarget.ESNext && !!compilerOptions.useDefineForClassFields
if (compilerOptions.target === ScriptTarget.ESNext && useDefineForClassFields
&& getContainingClass(declaration)
&& (isPropertyDeclaration(declaration) || isParameterPropertyDeclaration(declaration, declaration.parent))) {
return !isPropertyImmediatelyReferencedWithinDeclaration(declaration, usage, /*stopAtAnyPropertyDeclaration*/ true);
Expand Down Expand Up @@ -1680,7 +1681,7 @@ namespace ts {
case SyntaxKind.PropertyDeclaration:
// static properties in classes introduce temporary variables
if (hasStaticModifier(node)) {
return target < ScriptTarget.ESNext || !compilerOptions.useDefineForClassFields;
return target < ScriptTarget.ESNext || !useDefineForClassFields;
}
return requiresScopeChangeWorker((node as PropertyDeclaration).name);
default:
Expand Down Expand Up @@ -2098,7 +2099,7 @@ namespace ts {

// Perform extra checks only if error reporting was requested
if (nameNotFoundMessage) {
if (propertyWithInvalidInitializer && !(compilerOptions.target === ScriptTarget.ESNext && compilerOptions.useDefineForClassFields)) {
if (propertyWithInvalidInitializer && !(compilerOptions.target === ScriptTarget.ESNext && useDefineForClassFields)) {
// We have a match, but the reference occurred within a property initializer and the identifier also binds
// to a local variable in the constructor where the code will be emitted. Note that this is actually allowed
// with ESNext+useDefineForClassFields because the scope semantics are different.
Expand Down Expand Up @@ -24183,7 +24184,7 @@ namespace ts {
break;
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.PropertySignature:
if (hasSyntacticModifier(container, ModifierFlags.Static) && !(compilerOptions.target === ScriptTarget.ESNext && compilerOptions.useDefineForClassFields)) {
if (hasSyntacticModifier(container, ModifierFlags.Static) && !(compilerOptions.target === ScriptTarget.ESNext && useDefineForClassFields)) {
error(node, Diagnostics.this_cannot_be_referenced_in_a_static_property_initializer);
// do not return here so in case if lexical this is captured - it will be reflected in flags on NodeLinks
}
Expand Down Expand Up @@ -26980,6 +26981,30 @@ namespace ts {
if (assignmentKind && lexicallyScopedSymbol && lexicallyScopedSymbol.valueDeclaration && isMethodDeclaration(lexicallyScopedSymbol.valueDeclaration)) {
grammarErrorOnNode(right, Diagnostics.Cannot_assign_to_private_method_0_Private_methods_are_not_writable, idText(right));
}

if (lexicallyScopedSymbol?.valueDeclaration && (compilerOptions.target === ScriptTarget.ESNext && !useDefineForClassFields)) {
const lexicalClass = getContainingClass(lexicallyScopedSymbol.valueDeclaration);
const parentStaticFieldInitializer = findAncestor(node, (n) => {
if (n === lexicalClass) return "quit";
if (isPropertyDeclaration(n.parent) && hasStaticModifier(n.parent) && n.parent.initializer === n && n.parent.parent === lexicalClass) {
return true;
}
return false;
});
if (parentStaticFieldInitializer) {
const parentStaticFieldInitializerSymbol = getSymbolOfNode(parentStaticFieldInitializer.parent);
Debug.assert(parentStaticFieldInitializerSymbol, "Initializer without declaration symbol");
const diagnostic = error(node,
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,
symbolName(lexicallyScopedSymbol));
addRelatedInfo(diagnostic,
createDiagnosticForNode(parentStaticFieldInitializer.parent,
Diagnostics.Initializer_for_property_0,
symbolName(parentStaticFieldInitializerSymbol))
);
}
}

if (isAnyLike) {
if (lexicallyScopedSymbol) {
return apparentType;
Expand Down Expand Up @@ -33054,7 +33079,7 @@ namespace ts {
// - The constructor declares parameter properties
// or the containing class declares instance member variables with initializers.
const superCallShouldBeFirst =
(compilerOptions.target !== ScriptTarget.ESNext || !compilerOptions.useDefineForClassFields) &&
(compilerOptions.target !== ScriptTarget.ESNext || !useDefineForClassFields) &&
(some((<ClassDeclaration>node.parent).members, isInstancePropertyWithInitializerOrPrivateIdentifierProperty) ||
some(node.parameters, p => hasSyntacticModifier(p, ModifierFlags.ParameterPropertyModifier)));

Expand Down Expand Up @@ -37059,7 +37084,7 @@ namespace ts {
Diagnostics._0_is_defined_as_a_property_in_class_1_but_is_overridden_here_in_2_as_an_accessor;
error(getNameOfDeclaration(derived.valueDeclaration) || derived.valueDeclaration, errorMessage, symbolToString(base), typeToString(baseType), typeToString(type));
}
else if (compilerOptions.useDefineForClassFields) {
else if (useDefineForClassFields) {
const uninitialized = derived.declarations?.find(d => d.kind === SyntaxKind.PropertyDeclaration && !(d as PropertyDeclaration).initializer);
if (uninitialized
&& !(derived.flags & SymbolFlags.Transient)
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3312,6 +3312,14 @@
"category": "Error",
"code": 2809
},
"Property '{0}' may not be used in a static property's initializer in the same class when 'target' is 'esnext' and 'useDefineForClassFields' is 'false'.": {
"category": "Error",
"code": 2810
},
"Initializer for property '{0}'": {
"category": "Error",
"code": 2811
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down
10 changes: 5 additions & 5 deletions src/compiler/transformers/classFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ namespace ts {
const resolver = context.getEmitResolver();
const compilerOptions = context.getCompilerOptions();
const languageVersion = getEmitScriptTarget(compilerOptions);
const useDefineForClassFields = getUseDefineForClassFields(compilerOptions);

const shouldTransformPrivateElements = languageVersion < ScriptTarget.ESNext;

Expand Down Expand Up @@ -138,7 +139,7 @@ namespace ts {
function transformSourceFile(node: SourceFile) {
const options = context.getCompilerOptions();
if (node.isDeclarationFile
|| options.useDefineForClassFields && options.target === ScriptTarget.ESNext) {
|| useDefineForClassFields && options.target === ScriptTarget.ESNext) {
return node;
}
const visited = visitEachChild(node, visitor, context);
Expand Down Expand Up @@ -340,7 +341,7 @@ namespace ts {
// Create a temporary variable to store a computed property name (if necessary).
// If it's not inlineable, then we emit an expression after the class which assigns
// the property name to the temporary variable.
const expr = getPropertyNameExpressionIfNeeded(node.name, !!node.initializer || !!context.getCompilerOptions().useDefineForClassFields);
const expr = getPropertyNameExpressionIfNeeded(node.name, !!node.initializer || useDefineForClassFields);
if (expr && !isSimpleInlineableExpression(expr)) {
getPendingExpressions().push(expr);
}
Expand Down Expand Up @@ -829,7 +830,7 @@ namespace ts {
if (hasStaticModifier(member) || hasSyntacticModifier(getOriginalNode(member), ModifierFlags.Abstract)) {
return false;
}
if (context.getCompilerOptions().useDefineForClassFields) {
if (useDefineForClassFields) {
// If we are using define semantics and targeting ESNext or higher,
// then we don't need to transform any class properties.
return languageVersion < ScriptTarget.ESNext;
Expand Down Expand Up @@ -865,7 +866,6 @@ namespace ts {
}

function transformConstructorBody(node: ClassDeclaration | ClassExpression, constructor: ConstructorDeclaration | undefined, isDerivedClass: boolean) {
const useDefineForClassFields = context.getCompilerOptions().useDefineForClassFields;
let properties = getProperties(node, /*requireInitializer*/ false, /*isStatic*/ false);
if (!useDefineForClassFields) {
properties = filter(properties, property => !!property.initializer || isPrivateIdentifier(property.name));
Expand Down Expand Up @@ -1000,7 +1000,7 @@ namespace ts {
*/
function transformProperty(property: PropertyDeclaration, receiver: LeftHandSideExpression) {
// We generate a name here in order to reuse the value cached by the relocated computed name expression (which uses the same generated name)
const emitAssignment = !context.getCompilerOptions().useDefineForClassFields;
const emitAssignment = !useDefineForClassFields;
const propertyName = isComputedPropertyName(property.name) && !isSimpleInlineableExpression(property.name.expression)
? factory.updateComputedPropertyName(property.name, factory.getGeneratedNameForNode(property.name))
: property.name;
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/transformers/es2015.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1626,7 +1626,7 @@ namespace ts {
const memberFunction = transformFunctionLikeToExpression(member, /*location*/ member, /*name*/ undefined, container);
const propertyName = visitNode(member.name, visitor, isPropertyName);
let e: Expression;
if (!isPrivateIdentifier(propertyName) && context.getCompilerOptions().useDefineForClassFields) {
if (!isPrivateIdentifier(propertyName) && getUseDefineForClassFields(context.getCompilerOptions())) {
const name = isComputedPropertyName(propertyName) ? propertyName.expression
: isIdentifier(propertyName) ? factory.createStringLiteral(unescapeLeadingUnderscores(propertyName.escapedText))
: propertyName;
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6071,6 +6071,10 @@ namespace ts {
return compilerOptions.allowJs === undefined ? !!compilerOptions.checkJs : compilerOptions.allowJs;
}

export function getUseDefineForClassFields(compilerOptions: CompilerOptions): boolean {
return compilerOptions.useDefineForClassFields === undefined ? compilerOptions.target === ScriptTarget.ESNext : compilerOptions.useDefineForClassFields;
}

export function compilerOptionsAffectSemanticDiagnostics(newOptions: CompilerOptions, oldOptions: CompilerOptions): boolean {
return oldOptions !== newOptions &&
semanticDiagnosticsOptionDeclarations.some(option => !isJsonEqual(getCompilerOptionValue(oldOptions, option), getCompilerOptionValue(newOptions, option)));
Expand Down
1 change: 1 addition & 0 deletions src/testRunner/unittests/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ namespace ts {
compilerOptions: {
target: ScriptTarget.ESNext,
newLine: NewLineKind.CarriageReturnLineFeed,
useDefineForClassFields: false,
}
}).outputText;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

//// [classWithStaticFieldInParameterBindingPattern.js]
// https://github.com/microsoft/TypeScript/issues/36295
((_a) => { var _b; var { [(_b = class {
},
_b.x = 1,
_b).x]: b = "" } = _a; })();
(({ [class {
static x = 1;
}.x]: b = "" }) => { })();
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

//// [classWithStaticFieldInParameterInitializer.js]
// https://github.com/microsoft/TypeScript/issues/36295
((b) => { var _a; if (b === void 0) { b = (_a = class {
},
_a.x = 1,
_a); } })();
((b = class {
static x = 1;
}) => { })();
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,7 @@ class A {

//// [privateNameAndStaticInitializer.js]
class A {
constructor() {
this.#foo = 1;
this.#prop = 2;
}
#foo;
#prop;
#foo = 1;
static inst = new A();
#prop = 2;
}
A.inst = new A();
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,15 @@ new A().test();

//// [privateNameComputedPropertyName1.js]
class A {
#a = 'a';
#b;
#c = 'c';
#d;
#e = '';
constructor() {
this.#a = 'a';
this.#c = 'c';
this.#e = '';
this.#b = 'b';
this.#d = 'd';
}
#a;
#b;
#c;
#d;
#e;
test() {
const data = { a: 'a', b: 'b', c: 'c', d: 'd', e: 'e' };
const { [this.#a]: a, [this.#b]: b, [this.#c]: c, [this.#d]: d, [this.#e = 'e']: e, } = data;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@ console.log(getX(new A));
//// [privateNameComputedPropertyName2.js]
let getX;
class A {
constructor() {
this.#x = 100;
}
#x;
#x = 100;
[(getX = (a) => a.#x, "_")]() { }
}
console.log(getX(new A));
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,14 @@ console.log(new Foo("NAME").getValue(100));

//// [privateNameComputedPropertyName3.js]
class Foo {
#name;
constructor(name) {
this.#name = name;
}
#name;
getValue(x) {
const obj = this;
class Bar {
constructor() {
this.#y = 100;
}
#y;
#y = 100;
[obj.#name]() {
return x + this.#y;
}
Expand Down
Loading