From a56b272d3891916a8debe36087fe7563bbade164 Mon Sep 17 00:00:00 2001
From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
Date: Thu, 14 Jun 2018 11:18:23 -0700
Subject: [PATCH] In JS, fix crash with in a file with conflicting
 ES2015/commonjs exports (#24960)

* fix crash with conflicting ES2015/commonjs modules

* Refactor based on PR comments
---
 src/compiler/binder.ts                        | 18 ++++++++-----
 .../conflictingCommonJSES2015Exports.symbols  | 18 +++++++++++++
 .../conflictingCommonJSES2015Exports.types    | 27 +++++++++++++++++++
 .../salsa/conflictingCommonJSES2015Exports.ts |  9 +++++++
 4 files changed, 65 insertions(+), 7 deletions(-)
 create mode 100644 tests/baselines/reference/conflictingCommonJSES2015Exports.symbols
 create mode 100644 tests/baselines/reference/conflictingCommonJSES2015Exports.types
 create mode 100644 tests/cases/conformance/salsa/conflictingCommonJSES2015Exports.ts

diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts
index 432b63dca1e..e6970df6811 100644
--- a/src/compiler/binder.ts
+++ b/src/compiler/binder.ts
@@ -2305,18 +2305,22 @@ namespace ts {
         }
 
         function setCommonJsModuleIndicator(node: Node) {
+            if (file.externalModuleIndicator) {
+                return false;
+            }
             if (!file.commonJsModuleIndicator) {
                 file.commonJsModuleIndicator = node;
-                if (!file.externalModuleIndicator) {
-                    bindSourceFileAsExternalModule();
-                }
+                bindSourceFileAsExternalModule();
             }
+            return true;
         }
 
         function bindExportsPropertyAssignment(node: BinaryExpression) {
             // When we create a property via 'exports.foo = bar', the 'exports.foo' property access
             // expression is the declaration
-            setCommonJsModuleIndicator(node);
+            if (!setCommonJsModuleIndicator(node)) {
+                return;
+            }
             const lhs = node.left as PropertyAccessEntityNameExpression;
             const symbol = forEachIdentifierInEntityName(lhs.expression, /*parent*/ undefined, (id, symbol) => {
                 if (symbol) {
@@ -2337,15 +2341,15 @@ namespace ts {
             // is still pointing to 'module.exports'.
             // We do not want to consider this as 'export=' since a module can have only one of these.
             // Similarly we do not want to treat 'module.exports = exports' as an 'export='.
+            if (!setCommonJsModuleIndicator(node)) {
+                return;
+            }
             const assignedExpression = getRightMostAssignedExpression(node.right);
             if (isEmptyObjectLiteral(assignedExpression) || container === file && isExportsOrModuleExportsOrAlias(file, assignedExpression)) {
-                // Mark it as a module in case there are no other exports in the file
-                setCommonJsModuleIndicator(node);
                 return;
             }
 
             // 'module.exports = expr' assignment
-            setCommonJsModuleIndicator(node);
             const flags = exportAssignmentIsAlias(node)
                 ? SymbolFlags.Alias // An export= with an EntityNameExpression or a ClassExpression exports all meanings of that identifier or class
                 : SymbolFlags.Property | SymbolFlags.ExportValue | SymbolFlags.ValueModule;
diff --git a/tests/baselines/reference/conflictingCommonJSES2015Exports.symbols b/tests/baselines/reference/conflictingCommonJSES2015Exports.symbols
new file mode 100644
index 00000000000..b022439bf31
--- /dev/null
+++ b/tests/baselines/reference/conflictingCommonJSES2015Exports.symbols
@@ -0,0 +1,18 @@
+=== tests/cases/conformance/salsa/bug24934.js ===
+export function abc(a, b, c) { return 5; }
+>abc : Symbol(abc, Decl(bug24934.js, 0, 0))
+>a : Symbol(a, Decl(bug24934.js, 0, 20))
+>b : Symbol(b, Decl(bug24934.js, 0, 22))
+>c : Symbol(c, Decl(bug24934.js, 0, 25))
+
+module.exports = { abc };
+>module : Symbol(module)
+>abc : Symbol(abc, Decl(bug24934.js, 1, 18))
+
+=== tests/cases/conformance/salsa/use.js ===
+import { abc } from './bug24934';
+>abc : Symbol(abc, Decl(use.js, 0, 8))
+
+abc(1, 2, 3);
+>abc : Symbol(abc, Decl(use.js, 0, 8))
+
diff --git a/tests/baselines/reference/conflictingCommonJSES2015Exports.types b/tests/baselines/reference/conflictingCommonJSES2015Exports.types
new file mode 100644
index 00000000000..a9dc1ce80bd
--- /dev/null
+++ b/tests/baselines/reference/conflictingCommonJSES2015Exports.types
@@ -0,0 +1,27 @@
+=== tests/cases/conformance/salsa/bug24934.js ===
+export function abc(a, b, c) { return 5; }
+>abc : (a: any, b: any, c: any) => number
+>a : any
+>b : any
+>c : any
+>5 : 5
+
+module.exports = { abc };
+>module.exports = { abc } : { [x: string]: any; abc: (a: any, b: any, c: any) => number; }
+>module.exports : any
+>module : any
+>exports : any
+>{ abc } : { [x: string]: any; abc: (a: any, b: any, c: any) => number; }
+>abc : (a: any, b: any, c: any) => number
+
+=== tests/cases/conformance/salsa/use.js ===
+import { abc } from './bug24934';
+>abc : (a: any, b: any, c: any) => number
+
+abc(1, 2, 3);
+>abc(1, 2, 3) : number
+>abc : (a: any, b: any, c: any) => number
+>1 : 1
+>2 : 2
+>3 : 3
+
diff --git a/tests/cases/conformance/salsa/conflictingCommonJSES2015Exports.ts b/tests/cases/conformance/salsa/conflictingCommonJSES2015Exports.ts
new file mode 100644
index 00000000000..35acd416e79
--- /dev/null
+++ b/tests/cases/conformance/salsa/conflictingCommonJSES2015Exports.ts
@@ -0,0 +1,9 @@
+// @checkJs: true
+// @allowJS: true
+// @noEmit: true
+// @Filename: bug24934.js
+export function abc(a, b, c) { return 5; }
+module.exports = { abc };
+// @Filename: use.js
+import { abc } from './bug24934';
+abc(1, 2, 3);