Fix stack overflow in merge symbol (microsoft#24134)

sandersn · web-flow · commit 0ba8998c8200 · 2018-05-15T12:49:54.000-07:00
* JS initializers use original valueDecl, not mutated

target's valueDeclaration is set to the source's if it is not present.
This makes it incorrect to use getJSInitializerSymbol because it relies
on the symbol's valueDeclaration.

This fix just saves the original valueDeclaration before mutating and
uses that.

* Compare merged targetInitializer to target

Instead of the unmerged one

* Add test of stack overflow
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -892,6 +892,7 @@ namespace ts {
         function mergeSymbol(target: Symbol, source: Symbol) {
             if (!(target.flags & getExcludedSymbolFlags(source.flags)) ||
                 (source.flags | target.flags) & SymbolFlags.JSContainer) {
+                const targetValueDeclaration = target.valueDeclaration;
                 Debug.assert(!!(target.flags & SymbolFlags.Transient));
                 // Javascript static-property-assignment declarations always merge, even though they are also values
                 if (source.flags & SymbolFlags.ValueModule && target.flags & SymbolFlags.ValueModule && target.constEnumOnlyModule && !source.constEnumOnlyModule) {
@@ -916,12 +917,13 @@ namespace ts {
                 }
                 if ((source.flags | target.flags) & SymbolFlags.JSContainer) {
                     const sourceInitializer = getJSInitializerSymbol(source);
-                    let targetInitializer = getJSInitializerSymbol(target);
+                    const init = getDeclaredJavascriptInitializer(targetValueDeclaration) || getAssignedJavascriptInitializer(targetValueDeclaration);
+                    let targetInitializer = init && init.symbol ? init.symbol : target;
+                    if (!(targetInitializer.flags & SymbolFlags.Transient)) {
+                        const mergedInitializer = getMergedSymbol(targetInitializer);
+                        targetInitializer = mergedInitializer === targetInitializer ? cloneSymbol(targetInitializer) : mergedInitializer;
+                    }
                     if (sourceInitializer !== source || targetInitializer !== target) {
-                        if (!(targetInitializer.flags & SymbolFlags.Transient)) {
-                            const mergedInitializer = getMergedSymbol(targetInitializer);
-                            targetInitializer = mergedInitializer === targetInitializer ? cloneSymbol(targetInitializer) : mergedInitializer;
-                        }
                         mergeSymbol(targetInitializer, sourceInitializer);
                     }
                 }
diff --git a/tests/baselines/reference/jsContainerMergeJsContainer.symbols b/tests/baselines/reference/jsContainerMergeJsContainer.symbols
@@ -0,0 +1,18 @@
+=== tests/cases/conformance/salsa/a.js ===
+// #24131
+const a = {};
+>a : Symbol(a, Decl(a.js, 1, 5), Decl(a.js, 1, 13), Decl(b.js, 0, 0))
+
+a.d = function() {};
+>a.d : Symbol(d, Decl(a.js, 1, 13), Decl(b.js, 0, 2))
+>a : Symbol(a, Decl(a.js, 1, 5), Decl(a.js, 1, 13), Decl(b.js, 0, 0))
+>d : Symbol(d, Decl(a.js, 1, 13), Decl(b.js, 0, 2))
+
+=== tests/cases/conformance/salsa/b.js ===
+a.d.prototype = {};
+>a.d.prototype : Symbol(d.prototype, Decl(b.js, 0, 0))
+>a.d : Symbol(d, Decl(a.js, 1, 13), Decl(b.js, 0, 2))
+>a : Symbol(a, Decl(a.js, 1, 5), Decl(a.js, 1, 13), Decl(b.js, 0, 0))
+>d : Symbol(d, Decl(a.js, 1, 13), Decl(b.js, 0, 2))
+>prototype : Symbol(d.prototype, Decl(b.js, 0, 0))
+
diff --git a/tests/baselines/reference/jsContainerMergeJsContainer.types b/tests/baselines/reference/jsContainerMergeJsContainer.types
@@ -0,0 +1,23 @@
+=== tests/cases/conformance/salsa/a.js ===
+// #24131
+const a = {};
+>a : { [x: string]: any; d: typeof d; }
+>{} : { [x: string]: any; d: typeof d; }
+
+a.d = function() {};
+>a.d = function() {} : typeof d
+>a.d : typeof d
+>a : { [x: string]: any; d: typeof d; }
+>d : typeof d
+>function() {} : typeof d
+
+=== tests/cases/conformance/salsa/b.js ===
+a.d.prototype = {};
+>a.d.prototype = {} : { [x: string]: any; }
+>a.d.prototype : { [x: string]: any; }
+>a.d : typeof d
+>a : { [x: string]: any; d: typeof d; }
+>d : typeof d
+>prototype : { [x: string]: any; }
+>{} : { [x: string]: any; }
+
diff --git a/tests/cases/conformance/salsa/jsContainerMergeJsContainer.ts b/tests/cases/conformance/salsa/jsContainerMergeJsContainer.ts
@@ -0,0 +1,10 @@
+// @noEmit: true
+// @allowJs: true
+// @checkJs: true
+
+// #24131
+// @Filename: a.js
+const a = {};
+a.d = function() {};
+// @Filename: b.js
+a.d.prototype = {};
