From 0ba8998c8200621ff5de11329bf74c85f3019869 Mon Sep 17 00:00:00 2001
From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
Date: Tue, 15 May 2018 12:49:54 -0700
Subject: [PATCH] Fix stack overflow in merge symbol (#24134)

* 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
---
 src/compiler/checker.ts                       | 12 ++++++----
 .../jsContainerMergeJsContainer.symbols       | 18 +++++++++++++++
 .../jsContainerMergeJsContainer.types         | 23 +++++++++++++++++++
 .../salsa/jsContainerMergeJsContainer.ts      | 10 ++++++++
 4 files changed, 58 insertions(+), 5 deletions(-)
 create mode 100644 tests/baselines/reference/jsContainerMergeJsContainer.symbols
 create mode 100644 tests/baselines/reference/jsContainerMergeJsContainer.types
 create mode 100644 tests/cases/conformance/salsa/jsContainerMergeJsContainer.ts

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index b85acc03d46..5e9a0ae9dc8 100644
--- 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
new file mode 100644
index 00000000000..1f4728d50de
--- /dev/null
+++ 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
new file mode 100644
index 00000000000..fa5c74f5b05
--- /dev/null
+++ 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
new file mode 100644
index 00000000000..3d6cbb35b75
--- /dev/null
+++ 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 = {};