Skip to content

Commit c302893

Browse files
rbucktonrbuckton
authored
Merge pull request microsoft#11286 from Microsoft/noEmitExtraVars
Do not emit extra var decls for merged enums/namespaces
2 parents 0971212 + 8d47511 commit c302893

188 files changed

Lines changed: 747 additions & 941 deletions

File tree

Some content is hidden

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

‎src/compiler/transformers/ts.ts‎

Lines changed: 63 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ namespace ts {
5151
let currentNamespace: ModuleDeclaration;
5252
let currentNamespaceContainerName: Identifier;
5353
let currentScope: SourceFile | Block | ModuleBlock | CaseBlock;
54+
let currentScopeFirstDeclarationsOfName: Map<Node>;
5455
let currentSourceFileExternalHelpersModuleName: Identifier;
5556

5657
/**
@@ -100,15 +101,19 @@ namespace ts {
100101
function saveStateAndInvoke<T>(node: Node, f: (node: Node) => T): T {
101102
// Save state
102103
const savedCurrentScope = currentScope;
104+
const savedCurrentScopeFirstDeclarationsOfName = currentScopeFirstDeclarationsOfName;
103105

104106
// Handle state changes before visiting a node.
105107
onBeforeVisitNode(node);
106108

107109
const visited = f(node);
108110

109111
// Restore state
110-
currentScope = savedCurrentScope;
112+
if (currentScope !== savedCurrentScope) {
113+
currentScopeFirstDeclarationsOfName = savedCurrentScopeFirstDeclarationsOfName;
114+
}
111115

116+
currentScope = savedCurrentScope;
112117
return visited;
113118
}
114119

@@ -414,6 +419,16 @@ namespace ts {
414419
case SyntaxKind.ModuleBlock:
415420
case SyntaxKind.Block:
416421
currentScope = <SourceFile | CaseBlock | ModuleBlock | Block>node;
422+
currentScopeFirstDeclarationsOfName = undefined;
423+
break;
424+
425+
case SyntaxKind.ClassDeclaration:
426+
case SyntaxKind.FunctionDeclaration:
427+
if (hasModifier(node, ModifierFlags.Ambient)) {
428+
break;
429+
}
430+
431+
recordEmittedDeclarationInScope(node);
417432
break;
418433
}
419434
}
@@ -2502,10 +2517,12 @@ namespace ts {
25022517
}
25032518

25042519
function shouldEmitVarForEnumDeclaration(node: EnumDeclaration | ModuleDeclaration) {
2505-
return !hasModifier(node, ModifierFlags.Export)
2506-
|| (isES6ExportedDeclaration(node) && isFirstDeclarationOfKind(node, node.kind));
2520+
return isFirstEmittedDeclarationInScope(node)
2521+
&& (!hasModifier(node, ModifierFlags.Export)
2522+
|| isES6ExportedDeclaration(node));
25072523
}
25082524

2525+
25092526
/*
25102527
* Adds a trailing VariableStatement for an enum or module declaration.
25112528
*/
@@ -2543,6 +2560,7 @@ namespace ts {
25432560
// If needed, we should emit a variable declaration for the enum. If we emit
25442561
// a leading variable declaration, we should not emit leading comments for the
25452562
// enum body.
2563+
recordEmittedDeclarationInScope(node);
25462564
if (shouldEmitVarForEnumDeclaration(node)) {
25472565
addVarForEnumOrModuleDeclaration(statements, node);
25482566

@@ -2679,20 +2697,48 @@ namespace ts {
26792697
return isInstantiatedModule(node, compilerOptions.preserveConstEnums || compilerOptions.isolatedModules);
26802698
}
26812699

2682-
function isModuleMergedWithES6Class(node: ModuleDeclaration) {
2683-
return languageVersion === ScriptTarget.ES6
2684-
&& isMergedWithClass(node);
2685-
}
2686-
26872700
function isES6ExportedDeclaration(node: Node) {
26882701
return isExternalModuleExport(node)
26892702
&& moduleKind === ModuleKind.ES6;
26902703
}
26912704

2705+
/**
2706+
* Records that a declaration was emitted in the current scope, if it was the first
2707+
* declaration for the provided symbol.
2708+
*
2709+
* NOTE: if there is ever a transformation above this one, we may not be able to rely
2710+
* on symbol names.
2711+
*/
2712+
function recordEmittedDeclarationInScope(node: Node) {
2713+
const name = node.symbol && node.symbol.name;
2714+
if (name) {
2715+
if (!currentScopeFirstDeclarationsOfName) {
2716+
currentScopeFirstDeclarationsOfName = createMap<Node>();
2717+
}
2718+
2719+
if (!(name in currentScopeFirstDeclarationsOfName)) {
2720+
currentScopeFirstDeclarationsOfName[name] = node;
2721+
}
2722+
}
2723+
}
2724+
2725+
/**
2726+
* Determines whether a declaration is the first declaration with the same name emitted
2727+
* in the current scope.
2728+
*/
2729+
function isFirstEmittedDeclarationInScope(node: Node) {
2730+
if (currentScopeFirstDeclarationsOfName) {
2731+
const name = node.symbol && node.symbol.name;
2732+
if (name) {
2733+
return currentScopeFirstDeclarationsOfName[name] === node;
2734+
}
2735+
}
2736+
2737+
return false;
2738+
}
2739+
26922740
function shouldEmitVarForModuleDeclaration(node: ModuleDeclaration) {
2693-
return !isModuleMergedWithES6Class(node)
2694-
&& (!isES6ExportedDeclaration(node)
2695-
|| isFirstDeclarationOfKind(node, node.kind));
2741+
return isFirstEmittedDeclarationInScope(node);
26962742
}
26972743

26982744
/**
@@ -2768,6 +2814,7 @@ namespace ts {
27682814
// If needed, we should emit a variable declaration for the module. If we emit
27692815
// a leading variable declaration, we should not emit leading comments for the
27702816
// module body.
2817+
recordEmittedDeclarationInScope(node);
27712818
if (shouldEmitVarForModuleDeclaration(node)) {
27722819
addVarForEnumOrModuleDeclaration(statements, node);
27732820
// We should still emit the comments if we are emitting a system module.
@@ -2837,8 +2884,10 @@ namespace ts {
28372884
function transformModuleBody(node: ModuleDeclaration, namespaceLocalName: Identifier): Block {
28382885
const savedCurrentNamespaceContainerName = currentNamespaceContainerName;
28392886
const savedCurrentNamespace = currentNamespace;
2887+
const savedCurrentScopeFirstDeclarationsOfName = currentScopeFirstDeclarationsOfName;
28402888
currentNamespaceContainerName = namespaceLocalName;
28412889
currentNamespace = node;
2890+
currentScopeFirstDeclarationsOfName = undefined;
28422891

28432892
const statements: Statement[] = [];
28442893
startLexicalEnvironment();
@@ -2865,10 +2914,12 @@ namespace ts {
28652914
const moduleBlock = <ModuleBlock>getInnerMostModuleDeclarationFromDottedModule(node).body;
28662915
statementsLocation = moveRangePos(moduleBlock.statements, -1);
28672916
}
2868-
addRange(statements, endLexicalEnvironment());
28692917

2918+
addRange(statements, endLexicalEnvironment());
28702919
currentNamespaceContainerName = savedCurrentNamespaceContainerName;
28712920
currentNamespace = savedCurrentNamespace;
2921+
currentScopeFirstDeclarationsOfName = savedCurrentScopeFirstDeclarationsOfName;
2922+
28722923
const block = createBlock(
28732924
createNodeArray(
28742925
statements,

‎tests/baselines/reference/ClassAndModuleThatMergeWithModuleMemberThatUsesClassTypeParameter.js‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ var clodule1 = (function () {
5656
}
5757
return clodule1;
5858
}());
59-
var clodule1;
6059
(function (clodule1) {
6160
function f(x) { }
6261
})(clodule1 || (clodule1 = {}));
@@ -65,7 +64,6 @@ var clodule2 = (function () {
6564
}
6665
return clodule2;
6766
}());
68-
var clodule2;
6967
(function (clodule2) {
7068
var x;
7169
var D = (function () {
@@ -79,7 +77,6 @@ var clodule3 = (function () {
7977
}
8078
return clodule3;
8179
}());
82-
var clodule3;
8380
(function (clodule3) {
8481
clodule3.y = { id: T };
8582
})(clodule3 || (clodule3 = {}));
@@ -88,7 +85,6 @@ var clodule4 = (function () {
8885
}
8986
return clodule4;
9087
}());
91-
var clodule4;
9288
(function (clodule4) {
9389
var D = (function () {
9490
function D() {

‎tests/baselines/reference/ClassAndModuleThatMergeWithModulesExportedGenericFunctionAndGenericClassStaticFunctionOfTheSameName.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ var clodule = (function () {
2222
clodule.fn = function (id) { };
2323
return clodule;
2424
}());
25-
var clodule;
2625
(function (clodule) {
2726
// error: duplicate identifier expected
2827
function fn(x, y) {

‎tests/baselines/reference/ClassAndModuleThatMergeWithModulesExportedGenericFunctionAndNonGenericClassStaticFunctionOfTheSameName.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ var clodule = (function () {
2222
clodule.fn = function (id) { };
2323
return clodule;
2424
}());
25-
var clodule;
2625
(function (clodule) {
2726
// error: duplicate identifier expected
2827
function fn(x, y) {

‎tests/baselines/reference/ClassAndModuleThatMergeWithModulesExportedStaticFunctionUsingClassPrivateStatics.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ var clodule = (function () {
2222
clodule.sfn = function (id) { return 42; };
2323
return clodule;
2424
}());
25-
var clodule;
2625
(function (clodule) {
2726
// error: duplicate identifier expected
2827
function fn(x, y) {

‎tests/baselines/reference/ClassAndModuleThatMergeWithStaticFunctionAndExportedFunctionThatShareAName.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ var Point = (function () {
3131
Point.Origin = function () { return { x: 0, y: 0 }; }; // unexpected error here bug 840246
3232
return Point;
3333
}());
34-
var Point;
3534
(function (Point) {
3635
function Origin() { return null; } //expected duplicate identifier error
3736
Point.Origin = Origin;
@@ -47,7 +46,6 @@ var A;
4746
return Point;
4847
}());
4948
A.Point = Point;
50-
var Point;
5149
(function (Point) {
5250
function Origin() { return ""; } //expected duplicate identifier error
5351
Point.Origin = Origin;

‎tests/baselines/reference/ClassAndModuleThatMergeWithStaticFunctionAndNonExportedFunctionThatShareAName.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ var Point = (function () {
3131
Point.Origin = function () { return { x: 0, y: 0 }; };
3232
return Point;
3333
}());
34-
var Point;
3534
(function (Point) {
3635
function Origin() { return ""; } // not an error, since not exported
3736
})(Point || (Point = {}));
@@ -46,7 +45,6 @@ var A;
4645
return Point;
4746
}());
4847
A.Point = Point;
49-
var Point;
5048
(function (Point) {
5149
function Origin() { return ""; } // not an error since not exported
5250
})(Point = A.Point || (A.Point = {}));

‎tests/baselines/reference/ClassAndModuleThatMergeWithStaticVariableAndExportedVarThatShareAName.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ var Point = (function () {
3131
return Point;
3232
}());
3333
Point.Origin = { x: 0, y: 0 };
34-
var Point;
3534
(function (Point) {
3635
Point.Origin = ""; //expected duplicate identifier error
3736
})(Point || (Point = {}));
@@ -46,7 +45,6 @@ var A;
4645
}());
4746
Point.Origin = { x: 0, y: 0 };
4847
A.Point = Point;
49-
var Point;
5048
(function (Point) {
5149
Point.Origin = ""; //expected duplicate identifier error
5250
})(Point = A.Point || (A.Point = {}));

‎tests/baselines/reference/ClassAndModuleThatMergeWithStaticVariableAndNonExportedVarThatShareAName.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ var Point = (function () {
3131
return Point;
3232
}());
3333
Point.Origin = { x: 0, y: 0 };
34-
var Point;
3534
(function (Point) {
3635
var Origin = ""; // not an error, since not exported
3736
})(Point || (Point = {}));
@@ -46,7 +45,6 @@ var A;
4645
}());
4746
Point.Origin = { x: 0, y: 0 };
4847
A.Point = Point;
49-
var Point;
5048
(function (Point) {
5149
var Origin = ""; // not an error since not exported
5250
})(Point = A.Point || (A.Point = {}));

‎tests/baselines/reference/ClassAndModuleWithSameNameAndCommonRoot.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ var A = (function () {
7676
}
7777
return A;
7878
}());
79-
var A;
8079
(function (A) {
8180
A.Instance = new A();
8281
})(A || (A = {}));

0 commit comments

Comments
 (0)