Skip to content

Commit 3b74503

Browse files
rbucktonrbuckton
committed
Merge pull request #2750 from Microsoft/fixES6Clodule
Elide var when emitting a module merged with an ES6 class
2 parents fb44a23 + 3eb0a3a commit 3b74503

File tree

8 files changed

+213
-15
lines changed

8 files changed

+213
-15
lines changed

‎src/compiler/checker.ts

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10434,13 +10434,29 @@ module ts {
1043410434
function getFirstNonAmbientClassOrFunctionDeclaration(symbol: Symbol): Declaration {
1043510435
let declarations = symbol.declarations;
1043610436
for (let declaration of declarations) {
10437-
if ((declaration.kind === SyntaxKind.ClassDeclaration || (declaration.kind === SyntaxKind.FunctionDeclaration && nodeIsPresent((<FunctionLikeDeclaration>declaration).body))) && !isInAmbientContext(declaration)) {
10437+
if ((declaration.kind === SyntaxKind.ClassDeclaration ||
10438+
(declaration.kind === SyntaxKind.FunctionDeclaration && nodeIsPresent((<FunctionLikeDeclaration>declaration).body))) &&
10439+
!isInAmbientContext(declaration)) {
1043810440
return declaration;
1043910441
}
1044010442
}
1044110443
return undefined;
1044210444
}
1044310445

10446+
function inSameLexicalScope(node1: Node, node2: Node) {
10447+
let container1 = getEnclosingBlockScopeContainer(node1);
10448+
let container2 = getEnclosingBlockScopeContainer(node2);
10449+
if (isGlobalSourceFile(container1)) {
10450+
return isGlobalSourceFile(container2);
10451+
}
10452+
else if (isGlobalSourceFile(container2)) {
10453+
return false;
10454+
}
10455+
else {
10456+
return container1 === container2;
10457+
}
10458+
}
10459+
1044410460
function checkModuleDeclaration(node: ModuleDeclaration) {
1044510461
if (produceDiagnostics) {
1044610462
// Grammar checking
@@ -10460,15 +10476,23 @@ module ts {
1046010476
&& symbol.declarations.length > 1
1046110477
&& !isInAmbientContext(node)
1046210478
&& isInstantiatedModule(node, compilerOptions.preserveConstEnums || compilerOptions.separateCompilation)) {
10463-
let classOrFunc = getFirstNonAmbientClassOrFunctionDeclaration(symbol);
10464-
if (classOrFunc) {
10465-
if (getSourceFileOfNode(node) !== getSourceFileOfNode(classOrFunc)) {
10479+
let firstNonAmbientClassOrFunc = getFirstNonAmbientClassOrFunctionDeclaration(symbol);
10480+
if (firstNonAmbientClassOrFunc) {
10481+
if (getSourceFileOfNode(node) !== getSourceFileOfNode(firstNonAmbientClassOrFunc)) {
1046610482
error(node.name, Diagnostics.A_module_declaration_cannot_be_in_a_different_file_from_a_class_or_function_with_which_it_is_merged);
1046710483
}
10468-
else if (node.pos < classOrFunc.pos) {
10484+
else if (node.pos < firstNonAmbientClassOrFunc.pos) {
1046910485
error(node.name, Diagnostics.A_module_declaration_cannot_be_located_prior_to_a_class_or_function_with_which_it_is_merged);
1047010486
}
1047110487
}
10488+
10489+
// if the module merges with a class declaration in the same lexical scope,
10490+
// we need to track this to ensure the correct emit.
10491+
let mergedClass = getDeclarationOfKind(symbol, SyntaxKind.ClassDeclaration);
10492+
if (mergedClass &&
10493+
inSameLexicalScope(node, mergedClass)) {
10494+
getNodeLinks(node).flags |= NodeCheckFlags.LexicalModuleMergesWithClass;
10495+
}
1047210496
}
1047310497

1047410498
// Checks for ambient external modules.

‎src/compiler/diagnosticInformationMap.generated.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
// <auto-generated />
22
/// <reference path="types.ts" />
3-
/* @internal */
43
module ts {
54
export var Diagnostics = {
65
Unterminated_string_literal: { code: 1002, category: DiagnosticCategory.Error, key: "Unterminated string literal." },

‎src/compiler/emitter.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4211,6 +4211,10 @@ var __param = this.__param || function(index, decorator) { return function (targ
42114211
return isInstantiatedModule(node, compilerOptions.preserveConstEnums || compilerOptions.separateCompilation);
42124212
}
42134213

4214+
function isModuleMergedWithES6Class(node: ModuleDeclaration) {
4215+
return languageVersion === ScriptTarget.ES6 && !!(resolver.getNodeCheckFlags(node) & NodeCheckFlags.LexicalModuleMergesWithClass);
4216+
}
4217+
42144218
function emitModuleDeclaration(node: ModuleDeclaration) {
42154219
// Emit only if this module is non-ambient.
42164220
let shouldEmit = shouldEmitModuleDeclaration(node);
@@ -4219,15 +4223,19 @@ var __param = this.__param || function(index, decorator) { return function (targ
42194223
return emitOnlyPinnedOrTripleSlashComments(node);
42204224
}
42214225

4222-
emitStart(node);
4223-
if (isES6ExportedDeclaration(node)) {
4224-
write("export ");
4226+
if (!isModuleMergedWithES6Class(node)) {
4227+
emitStart(node);
4228+
if (isES6ExportedDeclaration(node)) {
4229+
write("export ");
4230+
}
4231+
4232+
write("var ");
4233+
emit(node.name);
4234+
write(";");
4235+
emitEnd(node);
4236+
writeLine();
42254237
}
4226-
write("var ");
4227-
emit(node.name);
4228-
write(";");
4229-
emitEnd(node);
4230-
writeLine();
4238+
42314239
emitStart(node);
42324240
write("(function (");
42334241
emitStart(node.name);

‎src/compiler/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1405,6 +1405,7 @@ module ts {
14051405
BlockScopedBindingInLoop = 0x00000100,
14061406
EmitDecorate = 0x00000200, // Emit __decorate
14071407
EmitParam = 0x00000400, // Emit __param helper for decorators
1408+
LexicalModuleMergesWithClass = 0x00000800, // Instantiated lexical module declaration is merged with a previous class declaration.
14081409
}
14091410

14101411
/* @internal */

‎src/compiler/utilities.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,10 @@ module ts {
211211
isCatchClauseVariableDeclaration(declaration);
212212
}
213213

214+
// Gets the nearest enclosing block scope container that has the provided node
215+
// as a descendant, that is not the provided node.
214216
export function getEnclosingBlockScopeContainer(node: Node): Node {
215-
let current = node;
217+
let current = node.parent;
216218
while (current) {
217219
if (isFunctionLike(current)) {
218220
return current;

‎tests/baselines/reference/ClassAndModuleWithSameNameAndCommonRootES6.errors.txt

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
tests/cases/conformance/internalModules/DeclarationMerging/module.ts(2,19): error TS2433: A module declaration cannot be in a different file from a class or function with which it is merged
2+
3+
4+
==== tests/cases/conformance/internalModules/DeclarationMerging/class.ts (0 errors) ====
5+
module X.Y {
6+
export class Point {
7+
constructor(x: number, y: number) {
8+
this.x = x;
9+
this.y = y;
10+
}
11+
x: number;
12+
y: number;
13+
}
14+
}
15+
16+
==== tests/cases/conformance/internalModules/DeclarationMerging/module.ts (1 errors) ====
17+
module X.Y {
18+
export module Point {
19+
~~~~~
20+
!!! error TS2433: A module declaration cannot be in a different file from a class or function with which it is merged
21+
export var Origin = new Point(0, 0);
22+
}
23+
}
24+
25+
==== tests/cases/conformance/internalModules/DeclarationMerging/test.ts (0 errors) ====
26+
//var cl: { x: number; y: number; }
27+
var cl = new X.Y.Point(1,1);
28+
var cl = X.Y.Point.Origin; // error not expected here same as bug 83996 ?
29+
30+
31+
==== tests/cases/conformance/internalModules/DeclarationMerging/simple.ts (0 errors) ====
32+
class A {
33+
id: string;
34+
}
35+
36+
module A {
37+
export var Instance = new A();
38+
}
39+
40+
// ensure merging works as expected
41+
var a = A.Instance;
42+
var a = new A();
43+
var a: { id: string };
44+

‎tests/baselines/reference/ClassAndModuleWithSameNameAndCommonRootES6.js

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
//// [tests/cases/conformance/internalModules/DeclarationMerging/ClassAndModuleWithSameNameAndCommonRootES6.ts] ////
2+
3+
//// [class.ts]
4+
module X.Y {
5+
export class Point {
6+
constructor(x: number, y: number) {
7+
this.x = x;
8+
this.y = y;
9+
}
10+
x: number;
11+
y: number;
12+
}
13+
}
14+
15+
//// [module.ts]
16+
module X.Y {
17+
export module Point {
18+
export var Origin = new Point(0, 0);
19+
}
20+
}
21+
22+
//// [test.ts]
23+
//var cl: { x: number; y: number; }
24+
var cl = new X.Y.Point(1,1);
25+
var cl = X.Y.Point.Origin; // error not expected here same as bug 83996 ?
26+
27+
28+
//// [simple.ts]
29+
class A {
30+
id: string;
31+
}
32+
33+
module A {
34+
export var Instance = new A();
35+
}
36+
37+
// ensure merging works as expected
38+
var a = A.Instance;
39+
var a = new A();
40+
var a: { id: string };
41+
42+
43+
//// [class.js]
44+
var X;
45+
(function (X) {
46+
var Y;
47+
(function (Y) {
48+
class Point {
49+
constructor(x, y) {
50+
this.x = x;
51+
this.y = y;
52+
}
53+
}
54+
Y.Point = Point;
55+
})(Y = X.Y || (X.Y = {}));
56+
})(X || (X = {}));
57+
//// [module.js]
58+
var X;
59+
(function (X) {
60+
var Y;
61+
(function (Y) {
62+
var Point;
63+
(function (Point) {
64+
Point.Origin = new Point(0, 0);
65+
})(Point = Y.Point || (Y.Point = {}));
66+
})(Y = X.Y || (X.Y = {}));
67+
})(X || (X = {}));
68+
//// [test.js]
69+
//var cl: { x: number; y: number; }
70+
var cl = new X.Y.Point(1, 1);
71+
var cl = X.Y.Point.Origin; // error not expected here same as bug 83996 ?
72+
//// [simple.js]
73+
class A {
74+
}
75+
(function (A) {
76+
A.Instance = new A();
77+
})(A || (A = {}));
78+
// ensure merging works as expected
79+
var a = A.Instance;
80+
var a = new A();
81+
var a;

‎tests/cases/conformance/internalModules/DeclarationMerging/ClassAndModuleWithSameNameAndCommonRootES6.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// @target: ES6
2+
// @filename: class.ts
3+
module X.Y {
4+
export class Point {
5+
constructor(x: number, y: number) {
6+
this.x = x;
7+
this.y = y;
8+
}
9+
x: number;
10+
y: number;
11+
}
12+
}
13+
14+
// @filename: module.ts
15+
module X.Y {
16+
export module Point {
17+
export var Origin = new Point(0, 0);
18+
}
19+
}
20+
21+
// @filename: test.ts
22+
//var cl: { x: number; y: number; }
23+
var cl = new X.Y.Point(1,1);
24+
var cl = X.Y.Point.Origin; // error not expected here same as bug 83996 ?
25+
26+
27+
// @filename: simple.ts
28+
class A {
29+
id: string;
30+
}
31+
32+
module A {
33+
export var Instance = new A();
34+
}
35+
36+
// ensure merging works as expected
37+
var a = A.Instance;
38+
var a = new A();
39+
var a: { id: string };

0 commit comments

Comments
 (0)