From 699dc4f9bb452646b72883378bc7401e013ae390 Mon Sep 17 00:00:00 2001 From: Rado Kirov Date: Thu, 19 May 2016 17:15:26 -0700 Subject: [PATCH 1/5] do not emit double assigment of class expressions. Closure compiler ES6 support rejects expressions of the form `let A = B = class {}` With a small rewriting of the emitted code the case of decorated and staticly referenced classes, TypeScript ES6 emit satisfies this requirement. Before: let C_1; let C = C_1 = class C {}; After: let C_1 = class C {}; let C_1 = C; --- src/compiler/emitter.ts | 46 +++++++++++-------- .../reference/decoratorOnClass5.es6.js | 4 +- .../reference/decoratorOnClass6.es6.js | 4 +- .../reference/decoratorOnClass7.es6.js | 4 +- 4 files changed, 34 insertions(+), 24 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index f27557a9136..f225b4cacf8 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -5359,17 +5359,17 @@ const _super = (function (geti, seti) { // // TypeScript | Javascript // --------------------------------|------------------------------------ - // @dec | let C_1; - // class C { | let C = C_1 = class C { - // static x() { return C.y; } | static x() { return C_1.y; } - // static y = 1; | } + // @dec | let C_1 = class C { + // class C { | static x() { return C_1.y; } + // static x() { return C.y; } | } + // static y = 1; | let C = C_1; // } | C.y = 1; // | C = C_1 = __decorate([dec], C); // --------------------------------|------------------------------------ - // @dec | let C_1; - // export class C { | export let C = C_1 = class C { - // static x() { return C.y; } | static x() { return C_1.y; } - // static y = 1; | } + // @dec | let C_1 = class C { + // export class C { | static x() { return C_1.y; } + // static x() { return C.y; } | } + // static y = 1; | export let C = C_1; // } | C.y = 1; // | C = C_1 = __decorate([dec], C); // --------------------------------------------------------------------- @@ -5398,10 +5398,10 @@ const _super = (function (geti, seti) { // // TypeScript | Javascript // --------------------------------|------------------------------------ - // @dec | let C_1; - // export default class C { | let C = C_1 = class C { - // static x() { return C.y; } | static x() { return C_1.y; } - // static y = 1; | } + // @dec | let C_1 = class C { + // export default class C { | static x() { return C_1.y; } + // static x() { return C.y; } | }; + // static y = 1; | let C = C_1; // } | C.y = 1; // | C = C_1 = __decorate([dec], C); // | export default C; @@ -5410,25 +5410,25 @@ const _super = (function (geti, seti) { // // NOTE: we reuse the same rewriting logic for cases when targeting ES6 and module kind is System. - // Because of hoisting top level class declaration need to be emitted as class expressions. + // Because of hoisting top level class declaration need to be emitted as class expressions. // Double bind case is only required if node is decorated. if (isDecorated && resolver.getNodeCheckFlags(node) & NodeCheckFlags.ClassWithBodyScopedClassBinding) { decoratedClassAlias = unescapeIdentifier(makeUniqueName(node.name ? node.name.text : "default")); decoratedClassAliases[getNodeId(node)] = decoratedClassAlias; - write(`let ${decoratedClassAlias};`); - writeLine(); } - if (isES6ExportedDeclaration(node) && !(node.flags & NodeFlags.Default)) { + if (isES6ExportedDeclaration(node) && !(node.flags & NodeFlags.Default) && decoratedClassAlias === undefined) { write("export "); } if (!isHoistedDeclarationInSystemModule) { write("let "); } - emitDeclarationName(node); if (decoratedClassAlias !== undefined) { - write(` = ${decoratedClassAlias}`); + write(`${decoratedClassAlias}`); + } + else { + emitDeclarationName(node); } write(" = "); @@ -5490,6 +5490,16 @@ const _super = (function (geti, seti) { emitToken(SyntaxKind.CloseBraceToken, node.members.end); if (rewriteAsClassExpression) { + if (decoratedClassAlias !== undefined) { + write(";"); + writeLine(); + if (isES6ExportedDeclaration(node) && !(node.flags & NodeFlags.Default)) { + write("export "); + } + write("let "); + emitDeclarationName(node); + write(` = ${decoratedClassAlias}`); + } decoratedClassAliases[getNodeId(node)] = undefined; write(";"); } diff --git a/tests/baselines/reference/decoratorOnClass5.es6.js b/tests/baselines/reference/decoratorOnClass5.es6.js index 58c5f0d4f29..e34740d6517 100644 --- a/tests/baselines/reference/decoratorOnClass5.es6.js +++ b/tests/baselines/reference/decoratorOnClass5.es6.js @@ -16,10 +16,10 @@ var __decorate = (this && this.__decorate) || function (decorators, target, key, else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r; return c > 3 && r && Object.defineProperty(target, key, r), r; }; -let C_1; -let C = C_1 = class C { +let C_1 = class C { static x() { return C_1.y; } }; +let C = C_1; C.y = 1; C = C_1 = __decorate([ dec diff --git a/tests/baselines/reference/decoratorOnClass6.es6.js b/tests/baselines/reference/decoratorOnClass6.es6.js index 03b8ace2b5b..0861774bfc0 100644 --- a/tests/baselines/reference/decoratorOnClass6.es6.js +++ b/tests/baselines/reference/decoratorOnClass6.es6.js @@ -16,10 +16,10 @@ var __decorate = (this && this.__decorate) || function (decorators, target, key, else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r; return c > 3 && r && Object.defineProperty(target, key, r), r; }; -let C_1; -export let C = C_1 = class C { +let C_1 = class C { static x() { return C_1.y; } }; +export let C = C_1; C.y = 1; C = C_1 = __decorate([ dec diff --git a/tests/baselines/reference/decoratorOnClass7.es6.js b/tests/baselines/reference/decoratorOnClass7.es6.js index 3ffd0f1574f..7f111ea4abf 100644 --- a/tests/baselines/reference/decoratorOnClass7.es6.js +++ b/tests/baselines/reference/decoratorOnClass7.es6.js @@ -16,10 +16,10 @@ var __decorate = (this && this.__decorate) || function (decorators, target, key, else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r; return c > 3 && r && Object.defineProperty(target, key, r), r; }; -let C_1; -let C = C_1 = class C { +let C_1 = class C { static x() { return C_1.y; } }; +let C = C_1; C.y = 1; C = C_1 = __decorate([ dec From cc5b103b2ee59a61241cd8cc942497f048f7012b Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 26 May 2016 09:45:40 -0700 Subject: [PATCH 2/5] Fix localeCompare differences in node versions --- src/services/navigationBar.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index bcf0726ffd5..1c3dcb0a585 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -140,7 +140,7 @@ namespace ts.NavigationBar { function sortNodes(nodes: Node[]): Node[] { return nodes.slice(0).sort((n1: Declaration, n2: Declaration) => { if (n1.name && n2.name) { - return getPropertyNameForPropertyNameNode(n1.name).localeCompare(getPropertyNameForPropertyNameNode(n2.name)); + return localeCompareFix(getPropertyNameForPropertyNameNode(n1.name), getPropertyNameForPropertyNameNode(n2.name)); } else if (n1.name) { return 1; @@ -152,6 +152,16 @@ namespace ts.NavigationBar { return n1.kind - n2.kind; } }); + + // node 0.10 treats "a" as greater than "B". + // For consistency, sort alphabetically, falling back to which is lower-case. + function localeCompareFix(a: string, b: string) { + const cmp = a.toLowerCase().localeCompare(b.toLowerCase()); + if (cmp !== 0) + return cmp; + // Return the *opposite* of the `<` operator, which works the same in node 0.10 and 6.0. + return a < b ? 1 : a > b ? -1 : 0; + } } function addTopLevelNodes(nodes: Node[], topLevelNodes: Node[]): void { From f012159220edaa716bf39c297b334486c687d354 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 26 May 2016 09:59:45 -0700 Subject: [PATCH 3/5] Revert previous commit --- src/services/navigationBar.ts | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 1c3dcb0a585..bcf0726ffd5 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -140,7 +140,7 @@ namespace ts.NavigationBar { function sortNodes(nodes: Node[]): Node[] { return nodes.slice(0).sort((n1: Declaration, n2: Declaration) => { if (n1.name && n2.name) { - return localeCompareFix(getPropertyNameForPropertyNameNode(n1.name), getPropertyNameForPropertyNameNode(n2.name)); + return getPropertyNameForPropertyNameNode(n1.name).localeCompare(getPropertyNameForPropertyNameNode(n2.name)); } else if (n1.name) { return 1; @@ -152,16 +152,6 @@ namespace ts.NavigationBar { return n1.kind - n2.kind; } }); - - // node 0.10 treats "a" as greater than "B". - // For consistency, sort alphabetically, falling back to which is lower-case. - function localeCompareFix(a: string, b: string) { - const cmp = a.toLowerCase().localeCompare(b.toLowerCase()); - if (cmp !== 0) - return cmp; - // Return the *opposite* of the `<` operator, which works the same in node 0.10 and 6.0. - return a < b ? 1 : a > b ? -1 : 0; - } } function addTopLevelNodes(nodes: Node[], topLevelNodes: Node[]): void { From fdd5c06b630287c03bbd0c5508eb96cea4834240 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 26 May 2016 12:18:19 -0700 Subject: [PATCH 4/5] Include type aliases as childItems in navigation bar --- src/services/navigationBar.ts | 3 +++ tests/cases/fourslash/navigationBarItemsTypeAlias.ts | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index bcf0726ffd5..0cb431de668 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -104,6 +104,7 @@ namespace ts.NavigationBar { case SyntaxKind.ImportEqualsDeclaration: case SyntaxKind.ImportSpecifier: case SyntaxKind.ExportSpecifier: + case SyntaxKind.TypeAliasDeclaration: childNodes.push(node); break; } @@ -326,6 +327,8 @@ namespace ts.NavigationBar { case SyntaxKind.InterfaceDeclaration: return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.interfaceElement); + case SyntaxKind.TypeAliasDeclaration: + return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.typeElement); case SyntaxKind.CallSignature: return createItem(node, "()", ts.ScriptElementKind.callSignatureElement); diff --git a/tests/cases/fourslash/navigationBarItemsTypeAlias.ts b/tests/cases/fourslash/navigationBarItemsTypeAlias.ts index 50923256533..448a959380f 100644 --- a/tests/cases/fourslash/navigationBarItemsTypeAlias.ts +++ b/tests/cases/fourslash/navigationBarItemsTypeAlias.ts @@ -2,5 +2,6 @@ ////type T = number | string; -verify.navigationBarCount(1); +verify.navigationBarCount(3); verify.navigationBarContains("T", "type"); +verify.navigationBarChildItem("", "T", "type"); From abfcdd2cfdbc7abc149496c43f49e43e910eb8ea Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Thu, 26 May 2016 12:42:35 -0700 Subject: [PATCH 5/5] Symbol for property assignments in Salsa/ES6 constructors Previously no symbol at all was created, meaning that Salsa didn't track properties in ES6 code. --- src/compiler/binder.ts | 18 +++++++++++++----- .../cases/fourslash/renameJsThisProperty03.ts | 14 ++++++++++++++ .../cases/fourslash/renameJsThisProperty04.ts | 14 ++++++++++++++ 3 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 tests/cases/fourslash/renameJsThisProperty03.ts create mode 100644 tests/cases/fourslash/renameJsThisProperty04.ts diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 19fdf79a24a..6f0f1ac7863 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -1880,12 +1880,20 @@ namespace ts { } function bindThisPropertyAssignment(node: BinaryExpression) { - // Declare a 'member' in case it turns out the container was an ES5 class - if (container.kind === SyntaxKind.FunctionExpression || container.kind === SyntaxKind.FunctionDeclaration) { - container.symbol.members = container.symbol.members || {}; - // It's acceptable for multiple 'this' assignments of the same identifier to occur - declareSymbol(container.symbol.members, container.symbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes & ~SymbolFlags.Property); + // Declare a 'member' in case it turns out the container was an ES5 class or ES6 constructor + let assignee: Node; + if (container.kind === SyntaxKind.FunctionDeclaration || container.kind === SyntaxKind.FunctionDeclaration) { + assignee = container; } + else if (container.kind === SyntaxKind.Constructor) { + assignee = container.parent; + } + else { + return; + } + assignee.symbol.members = assignee.symbol.members || {}; + // It's acceptable for multiple 'this' assignments of the same identifier to occur + declareSymbol(assignee.symbol.members, assignee.symbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes & ~SymbolFlags.Property); } function bindPrototypePropertyAssignment(node: BinaryExpression) { diff --git a/tests/cases/fourslash/renameJsThisProperty03.ts b/tests/cases/fourslash/renameJsThisProperty03.ts new file mode 100644 index 00000000000..f2176538578 --- /dev/null +++ b/tests/cases/fourslash/renameJsThisProperty03.ts @@ -0,0 +1,14 @@ +/// + +// @allowJs: true +// @Filename: a.js +////class C { +//// constructor(y) { +//// this./**/[|x|] = y; +//// } +////} +////var t = new C(12); +////t.[|x|] = 11; + +goTo.marker(); +verify.renameLocations( /*findInStrings*/ false, /*findInComments*/ false); diff --git a/tests/cases/fourslash/renameJsThisProperty04.ts b/tests/cases/fourslash/renameJsThisProperty04.ts new file mode 100644 index 00000000000..e307022c66f --- /dev/null +++ b/tests/cases/fourslash/renameJsThisProperty04.ts @@ -0,0 +1,14 @@ +/// + +// @allowJs: true +// @Filename: a.js +////class C { +//// constructor(y) { +//// this.[|x|] = y; +//// } +////} +////var t = new C(12); +////t./**/[|x|] = 11; + +goTo.marker(); +verify.renameLocations( /*findInStrings*/ false, /*findInComments*/ false);