Skip to content

Fix Computed Property Name Bindings #17

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17446,6 +17446,23 @@ namespace ts {
const links = getNodeLinks(node.expression);
if (!links.resolvedType) {
links.resolvedType = checkExpression(node.expression);

if (isPropertyDeclaration(node.parent) && isClassLike(node.parent.parent)) {
const container = getEnclosingBlockScopeContainer(node);
let current = container;
let containedInIterationStatement = false;
while (current && !nodeStartsNewLexicalEnvironment(current)) {
if (isIterationStatement(current, /*lookInLabeledStatements*/ false)) {
containedInIterationStatement = true;
break;
}
current = current.parent;
}
if (containedInIterationStatement) {
getNodeLinks(current).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
}
links.flags |= NodeCheckFlags.BlockScopedBindingInLoop;
}
// This will allow types number, string, symbol or any. It will also allow enums, the unknown
// type, and any union of these types (like string | number).
if (links.resolvedType.flags & TypeFlags.Nullable ||
Expand Down
14 changes: 12 additions & 2 deletions src/compiler/transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ namespace ts {
let lexicalEnvironmentFunctionDeclarations: FunctionDeclaration[];
let lexicalEnvironmentVariableDeclarationsStack: VariableDeclaration[][] = [];
let lexicalEnvironmentFunctionDeclarationsStack: FunctionDeclaration[][] = [];
let lexicalEnvironmentScopingStack: LexicalEnvironmentScoping[] = [];
let lexicalEnvironmentScoping: LexicalEnvironmentScoping;
let lexicalEnvironmentStackOffset = 0;
let lexicalEnvironmentSuspended = false;
let emitHelpers: EmitHelper[] | undefined;
Expand Down Expand Up @@ -258,7 +260,7 @@ namespace ts {
* Starts a new lexical environment. Any existing hoisted variable or function declarations
* are pushed onto a stack, and the related storage variables are reset.
*/
function startLexicalEnvironment(): void {
function startLexicalEnvironment(scoping: LexicalEnvironmentScoping = LexicalEnvironmentScoping.Function): void {
Debug.assert(state > TransformationState.Uninitialized, "Cannot modify the lexical environment during initialization.");
Debug.assert(state < TransformationState.Completed, "Cannot modify the lexical environment after transformation has completed.");
Debug.assert(!lexicalEnvironmentSuspended, "Lexical environment is suspended.");
Expand All @@ -267,11 +269,13 @@ namespace ts {
// stack size variable. This allows us to reuse existing array slots we've
// already allocated between transformations to avoid allocation and GC overhead during
// transformation.
lexicalEnvironmentScopingStack[lexicalEnvironmentStackOffset] = lexicalEnvironmentScoping;
lexicalEnvironmentVariableDeclarationsStack[lexicalEnvironmentStackOffset] = lexicalEnvironmentVariableDeclarations;
lexicalEnvironmentFunctionDeclarationsStack[lexicalEnvironmentStackOffset] = lexicalEnvironmentFunctionDeclarations;
lexicalEnvironmentStackOffset++;
lexicalEnvironmentVariableDeclarations = undefined!;
lexicalEnvironmentFunctionDeclarations = undefined!;
lexicalEnvironmentScoping = scoping;
}

/** Suspends the current lexical environment, usually after visiting a parameter list. */
Expand Down Expand Up @@ -308,7 +312,10 @@ namespace ts {
if (lexicalEnvironmentVariableDeclarations) {
const statement = createVariableStatement(
/*modifiers*/ undefined,
createVariableDeclarationList(lexicalEnvironmentVariableDeclarations)
createVariableDeclarationList(
lexicalEnvironmentVariableDeclarations,
lexicalEnvironmentScoping === LexicalEnvironmentScoping.Block ? NodeFlags.Let : undefined
)
);

if (!statements) {
Expand All @@ -324,9 +331,11 @@ namespace ts {
lexicalEnvironmentStackOffset--;
lexicalEnvironmentVariableDeclarations = lexicalEnvironmentVariableDeclarationsStack[lexicalEnvironmentStackOffset];
lexicalEnvironmentFunctionDeclarations = lexicalEnvironmentFunctionDeclarationsStack[lexicalEnvironmentStackOffset];
lexicalEnvironmentScoping = lexicalEnvironmentScopingStack[lexicalEnvironmentStackOffset];
if (lexicalEnvironmentStackOffset === 0) {
lexicalEnvironmentVariableDeclarationsStack = [];
lexicalEnvironmentFunctionDeclarationsStack = [];
lexicalEnvironmentScopingStack = [];
}
return statements;
}
Expand Down Expand Up @@ -358,6 +367,7 @@ namespace ts {
lexicalEnvironmentVariableDeclarationsStack = undefined!;
lexicalEnvironmentFunctionDeclarations = undefined!;
lexicalEnvironmentFunctionDeclarationsStack = undefined!;
lexicalEnvironmentScopingStack = undefined!;
onSubstituteNode = undefined!;
onEmitNode = undefined!;
emitHelpers = undefined;
Expand Down
18 changes: 17 additions & 1 deletion src/compiler/transformers/ts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,10 @@ namespace ts {
* @param node The node to visit.
*/
function visitorWorker(node: Node): VisitResult<Node> {
if (node.transformFlags & TransformFlags.TypeScript) {
if (node.kind === SyntaxKind.Block && node.transformFlags & TransformFlags.AssertTypeScript) {
return visitBlock(node as Block);
}
else if (node.transformFlags & TransformFlags.TypeScript) {
// This node is explicitly marked as TypeScript, so we should transform the node.
return visitTypeScript(node);
}
Expand Down Expand Up @@ -560,6 +563,19 @@ namespace ts {
}
}

function visitBlock(node: Block): Block {
startLexicalEnvironment(LexicalEnvironmentScoping.Block);
node = visitEachChild(node, visitor, context);
const declarations = endLexicalEnvironment();
if (some(declarations)) {
return updateBlock(
node,
mergeLexicalEnvironment(node.statements, declarations)
);
}
return node;
}

function visitSourceFile(node: SourceFile) {
const alwaysStrict = getStrictOptionValue(compilerOptions, "alwaysStrict") &&
!(isExternalModule(node) && moduleKind >= ModuleKind.ES2015) &&
Expand Down
7 changes: 6 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5212,6 +5212,11 @@ namespace ts {
writeFile: WriteFileCallback;
}

export const enum LexicalEnvironmentScoping {
Function,
Block
}

export interface TransformationContext {
/*@internal*/ getEmitResolver(): EmitResolver;
/*@internal*/ getEmitHost(): EmitHost;
Expand All @@ -5220,7 +5225,7 @@ namespace ts {
getCompilerOptions(): CompilerOptions;

/** Starts a new lexical environment. */
startLexicalEnvironment(): void;
startLexicalEnvironment(scoping?: LexicalEnvironmentScoping): void;

/** Suspends the current lexical environment, usually after visiting a parameter list. */
suspendLexicalEnvironment(): void;
Expand Down
6 changes: 5 additions & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2752,11 +2752,15 @@ declare namespace ts {
MappedTypeParameter = 3,
Unspecified = 4
}
enum LexicalEnvironmentScoping {
Function = 0,
Block = 1
}
interface TransformationContext {
/** Gets the compiler options supplied to the transformer. */
getCompilerOptions(): CompilerOptions;
/** Starts a new lexical environment. */
startLexicalEnvironment(): void;
startLexicalEnvironment(scoping?: LexicalEnvironmentScoping): void;
/** Suspends the current lexical environment, usually after visiting a parameter list. */
suspendLexicalEnvironment(): void;
/** Resumes a suspended lexical environment, usually before visiting a function body. */
Expand Down
6 changes: 5 additions & 1 deletion tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2752,11 +2752,15 @@ declare namespace ts {
MappedTypeParameter = 3,
Unspecified = 4
}
enum LexicalEnvironmentScoping {
Function = 0,
Block = 1
}
interface TransformationContext {
/** Gets the compiler options supplied to the transformer. */
getCompilerOptions(): CompilerOptions;
/** Starts a new lexical environment. */
startLexicalEnvironment(): void;
startLexicalEnvironment(scoping?: LexicalEnvironmentScoping): void;
/** Suspends the current lexical environment, usually after visiting a parameter list. */
suspendLexicalEnvironment(): void;
/** Resumes a suspended lexical environment, usually before visiting a function body. */
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/classBlockScoping.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ function f(b: boolean) {

//// [classBlockScoping.js]
function f(b) {
var _a;
var Foo;
if (b) {
var _a = void 0;
Foo = (_a = /** @class */ (function () {
function Foo() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ for (let i = 0; i < 3; i++) {
arr.forEach(C => console.log(C.y()));

//// [classExpressionWithStaticProperties3.js]
var _a;
var arr = [];
var _loop_1 = function (i) {
var _a = void 0;
arr.push((_a = /** @class */ (function () {
function C() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ for (let i = 0; i < 3; i++) {
arr.forEach(C => console.log(C.y()));

//// [classExpressionWithStaticPropertiesES63.js]
var _a;
const arr = [];
for (let i = 0; i < 3; i++) {
let _a;
arr.push((_a = class C {
},
_a.x = i,
Expand Down
17 changes: 17 additions & 0 deletions tests/baselines/reference/computedPropertyNames52_ES5.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
tests/cases/conformance/es6/computedProperties/computedPropertyNames52_ES5.ts(5,13): error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.


==== tests/cases/conformance/es6/computedProperties/computedPropertyNames52_ES5.ts (1 errors) ====
const classes = [];
for (let i = 0; i <= 10; ++i) {
classes.push(
class A {
[i] = "my property";
~~~
!!! error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
}
);
}
for (const clazz of classes) {
console.log(Object.getOwnPropertyNames(new clazz()));
}
33 changes: 33 additions & 0 deletions tests/baselines/reference/computedPropertyNames52_ES5.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//// [computedPropertyNames52_ES5.ts]
const classes = [];
for (let i = 0; i <= 10; ++i) {
classes.push(
class A {
[i] = "my property";
}
);
}
for (const clazz of classes) {
console.log(Object.getOwnPropertyNames(new clazz()));
}

//// [computedPropertyNames52_ES5.js]
var classes = [];
var _loop_1 = function (i) {
var _a = void 0, _b = void 0;
classes.push((_b = /** @class */ (function () {
function A() {
this[_a] = "my property";
}
return A;
}()),
_a = i,
_b));
};
for (var i = 0; i <= 10; ++i) {
_loop_1(i);
}
for (var _i = 0, classes_1 = classes; _i < classes_1.length; _i++) {
var clazz = classes_1[_i];
console.log(Object.getOwnPropertyNames(new clazz()));
}
36 changes: 36 additions & 0 deletions tests/baselines/reference/computedPropertyNames52_ES5.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
=== tests/cases/conformance/es6/computedProperties/computedPropertyNames52_ES5.ts ===
const classes = [];
>classes : Symbol(classes, Decl(computedPropertyNames52_ES5.ts, 0, 5))

for (let i = 0; i <= 10; ++i) {
>i : Symbol(i, Decl(computedPropertyNames52_ES5.ts, 1, 8))
>i : Symbol(i, Decl(computedPropertyNames52_ES5.ts, 1, 8))
>i : Symbol(i, Decl(computedPropertyNames52_ES5.ts, 1, 8))

classes.push(
>classes.push : Symbol(Array.push, Decl(lib.es5.d.ts, --, --))
>classes : Symbol(classes, Decl(computedPropertyNames52_ES5.ts, 0, 5))
>push : Symbol(Array.push, Decl(lib.es5.d.ts, --, --))

class A {
>A : Symbol(A, Decl(computedPropertyNames52_ES5.ts, 2, 17))

[i] = "my property";
>[i] : Symbol(A[i], Decl(computedPropertyNames52_ES5.ts, 3, 17))
>i : Symbol(i, Decl(computedPropertyNames52_ES5.ts, 1, 8))
}
);
}
for (const clazz of classes) {
>clazz : Symbol(clazz, Decl(computedPropertyNames52_ES5.ts, 8, 10))
>classes : Symbol(classes, Decl(computedPropertyNames52_ES5.ts, 0, 5))

console.log(Object.getOwnPropertyNames(new clazz()));
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>Object.getOwnPropertyNames : Symbol(ObjectConstructor.getOwnPropertyNames, Decl(lib.es5.d.ts, --, --))
>Object : Symbol(Object, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
>getOwnPropertyNames : Symbol(ObjectConstructor.getOwnPropertyNames, Decl(lib.es5.d.ts, --, --))
>clazz : Symbol(clazz, Decl(computedPropertyNames52_ES5.ts, 8, 10))
}
47 changes: 47 additions & 0 deletions tests/baselines/reference/computedPropertyNames52_ES5.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
=== tests/cases/conformance/es6/computedProperties/computedPropertyNames52_ES5.ts ===
const classes = [];
>classes : any[]
>[] : undefined[]

for (let i = 0; i <= 10; ++i) {
>i : number
>0 : 0
>i <= 10 : boolean
>i : number
>10 : 10
>++i : number
>i : number

classes.push(
>classes.push( class A { [i] = "my property"; } ) : number
>classes.push : (...items: any[]) => number
>classes : any[]
>push : (...items: any[]) => number

class A {
>class A { [i] = "my property"; } : typeof A
>A : typeof A

[i] = "my property";
>[i] : string
>i : number
>"my property" : "my property"
}
);
}
for (const clazz of classes) {
>clazz : any
>classes : any[]

console.log(Object.getOwnPropertyNames(new clazz()));
>console.log(Object.getOwnPropertyNames(new clazz())) : void
>console.log : (message?: any, ...optionalParams: any[]) => void
>console : Console
>log : (message?: any, ...optionalParams: any[]) => void
>Object.getOwnPropertyNames(new clazz()) : string[]
>Object.getOwnPropertyNames : (o: any) => string[]
>Object : ObjectConstructor
>getOwnPropertyNames : (o: any) => string[]
>new clazz() : any
>clazz : any
}
17 changes: 17 additions & 0 deletions tests/baselines/reference/computedPropertyNames52_ES6.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
tests/cases/conformance/es6/computedProperties/computedPropertyNames52_ES6.ts(5,13): error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.


==== tests/cases/conformance/es6/computedProperties/computedPropertyNames52_ES6.ts (1 errors) ====
const classes = [];
for (let i = 0; i <= 10; ++i) {
classes.push(
class A {
[i] = "my property";
~~~
!!! error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
}
);
}
for (const clazz of classes) {
console.log(Object.getOwnPropertyNames(new clazz()));
}
28 changes: 28 additions & 0 deletions tests/baselines/reference/computedPropertyNames52_ES6.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//// [computedPropertyNames52_ES6.ts]
const classes = [];
for (let i = 0; i <= 10; ++i) {
classes.push(
class A {
[i] = "my property";
}
);
}
for (const clazz of classes) {
console.log(Object.getOwnPropertyNames(new clazz()));
}

//// [computedPropertyNames52_ES6.js]
const classes = [];
for (let i = 0; i <= 10; ++i) {
let _a, _b;
classes.push((_b = class A {
constructor() {
this[_a] = "my property";
}
},
_a = i,
_b));
}
for (const clazz of classes) {
console.log(Object.getOwnPropertyNames(new clazz()));
}
Loading