Only bind module.exports if no local definition exists (#25869)

* Only bind module.exports if no local definition exists

Note that this uses `lookupSymbolForNameWorker`, which is really a
best-effort check since it only knows about symbols that it has already
encountered.

As a side-effect, even when `module` is bound as part of a
`module.exports` reference, it only declares it once instead of one
declaration per reference.

* Only type module.exports inside module files

It is an error inside script files, but the binder sometimes creates a
ModuleExports symbol because we doesn't know whether we have a commonjs
module until after binding is done.

* Only bind module.exports in a commonjs module

Note that this, too, is a best-effort check since evidence of
commonjs-ness may be found after a *reference* to module.exports. (A
reference to module.exports alone is not enough evidence that a file is
commonjs. It has to have an assignment to it.)

Author: sandersn

src/compiler/binder.ts

@@ -2072,7 +2072,10 @@ namespace ts {
                     if (isSpecialPropertyDeclaration(node as PropertyAccessExpression)) {
                         bindSpecialPropertyDeclaration(node as PropertyAccessExpression);
                     }
-                    if (isInJavaScriptFile(node) && isModuleExportsPropertyAccessExpression(node as PropertyAccessExpression)) {
+                    if (isInJavaScriptFile(node) &&
+                        file.commonJsModuleIndicator &&
+                        isModuleExportsPropertyAccessExpression(node as PropertyAccessExpression) &&
+                        !lookupSymbolForNameWorker(container, "module" as __String)) {
                         declareSymbol(container.locals!, /*parent*/ undefined, (node as PropertyAccessExpression).expression as Identifier,
                             SymbolFlags.FunctionScopedVariable | SymbolFlags.ModuleExports, SymbolFlags.FunctionScopedVariableExcludes);
                     }

tests/baselines/reference/conflictingCommonJSES2015Exports.errors.txt

@@ -0,0 +1,12 @@
+tests/cases/conformance/salsa/bug24934.js(2,1): error TS2304: Cannot find name 'module'.
+
+
+==== tests/cases/conformance/salsa/bug24934.js (1 errors) ====
+    export function abc(a, b, c) { return 5; }
+    module.exports = { abc };
+    ~~~~~~
+!!! error TS2304: Cannot find name 'module'.
+==== tests/cases/conformance/salsa/use.js (0 errors) ====
+    import { abc } from './bug24934';
+    abc(1, 2, 3);
+    

tests/baselines/reference/conflictingCommonJSES2015Exports.symbols

@@ -6,9 +6,6 @@ export function abc(a, b, c) { return 5; }
 >c : Symbol(c, Decl(bug24934.js, 0, 25))
 
 module.exports = { abc };
->module.exports : Symbol("tests/cases/conformance/salsa/bug24934", Decl(bug24934.js, 0, 0))
->module : Symbol(module, Decl(bug24934.js, 0, 42))
->exports : Symbol("tests/cases/conformance/salsa/bug24934", Decl(bug24934.js, 0, 0))
 >abc : Symbol(abc, Decl(bug24934.js, 1, 18))
 
 === tests/cases/conformance/salsa/use.js ===

tests/baselines/reference/conflictingCommonJSES2015Exports.types

@@ -7,10 +7,10 @@ export function abc(a, b, c) { return 5; }
 >5 : 5
 
 module.exports = { abc };
->module.exports = { abc } : typeof import("tests/cases/conformance/salsa/bug24934")
->module.exports : typeof import("tests/cases/conformance/salsa/bug24934")
->module : { "tests/cases/conformance/salsa/bug24934": typeof import("tests/cases/conformance/salsa/bug24934"); }
->exports : typeof import("tests/cases/conformance/salsa/bug24934")
+>module.exports = { abc } : any
+>module.exports : any
+>module : any
+>exports : any
 >{ abc } : { abc: (a: any, b: any, c: any) => number; }
 >abc : (a: any, b: any, c: any) => number
 

tests/baselines/reference/contextualTypedSpecialAssignment.symbols

@@ -102,7 +102,7 @@ exports.x
 module.exports.y = {
 >module.exports.y : Symbol(y, Decl(test.js, 45, 9))
 >module.exports : Symbol(y, Decl(test.js, 45, 9))
->module : Symbol(module, Decl(test.js, 45, 9), Decl(test.js, 51, 1))
+>module : Symbol(module, Decl(test.js, 45, 9))
 >exports : Symbol("tests/cases/conformance/salsa/test", Decl(test.js, 0, 0))
 >y : Symbol(y, Decl(test.js, 45, 9))
 
@@ -116,7 +116,7 @@ module.exports.y = {
 module.exports.y
 >module.exports.y : Symbol(y, Decl(test.js, 45, 9))
 >module.exports : Symbol("tests/cases/conformance/salsa/test", Decl(test.js, 0, 0))
->module : Symbol(module, Decl(test.js, 45, 9), Decl(test.js, 51, 1))
+>module : Symbol(module, Decl(test.js, 45, 9))
 >exports : Symbol("tests/cases/conformance/salsa/test", Decl(test.js, 0, 0))
 >y : Symbol(y, Decl(test.js, 45, 9))
 

tests/baselines/reference/jsdocTypeFromChainedAssignment2.symbols

@@ -43,12 +43,12 @@ exports.f = exports.g = function fg(n) {
 module.exports.h = module.exports.i = function hi(mom) {
 >module.exports.h : Symbol(h, Decl(mod.js, 3, 1))
 >module.exports : Symbol(h, Decl(mod.js, 3, 1))
->module : Symbol(module, Decl(mod.js, 3, 1), Decl(mod.js, 5, 18))
+>module : Symbol(module, Decl(mod.js, 3, 1))
 >exports : Symbol("tests/cases/conformance/jsdoc/mod", Decl(mod.js, 0, 0))
 >h : Symbol(h, Decl(mod.js, 3, 1))
 >module.exports.i : Symbol(i, Decl(mod.js, 5, 18))
 >module.exports : Symbol(i, Decl(mod.js, 5, 18))
->module : Symbol(module, Decl(mod.js, 3, 1), Decl(mod.js, 5, 18))
+>module : Symbol(module, Decl(mod.js, 3, 1))
 >exports : Symbol("tests/cases/conformance/jsdoc/mod", Decl(mod.js, 0, 0))
 >i : Symbol(i, Decl(mod.js, 5, 18))
 >hi : Symbol(hi, Decl(mod.js, 5, 37))

tests/baselines/reference/moduleExportAlias.symbols

@@ -121,7 +121,7 @@ exports.func2 = function () { };
 var moduleExportsAlias = module.exports;
 >moduleExportsAlias : Symbol(moduleExportsAlias, Decl(b.js, 4, 3))
 >module.exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 
 moduleExportsAlias.func3 = function () { };
@@ -132,15 +132,15 @@ moduleExportsAlias.func3 = function () { };
 module.exports.func4 = function () { };
 >module.exports.func4 : Symbol(func4, Decl(b.js, 5, 43))
 >module.exports : Symbol(func4, Decl(b.js, 5, 43))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >func4 : Symbol(func4, Decl(b.js, 5, 43))
 
 var multipleDeclarationAlias1 = exports = module.exports;
 >multipleDeclarationAlias1 : Symbol(multipleDeclarationAlias1, Decl(b.js, 8, 3))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >module.exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 
 multipleDeclarationAlias1.func5 = function () { };
@@ -151,7 +151,7 @@ multipleDeclarationAlias1.func5 = function () { };
 var multipleDeclarationAlias2 = module.exports = exports;
 >multipleDeclarationAlias2 : Symbol(multipleDeclarationAlias2, Decl(b.js, 11, 3))
 >module.exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 
@@ -177,7 +177,7 @@ var multipleDeclarationAlias4 = someOtherVariable = module.exports;
 >multipleDeclarationAlias4 : Symbol(multipleDeclarationAlias4, Decl(b.js, 18, 3))
 >someOtherVariable : Symbol(someOtherVariable, Decl(b.js, 14, 3))
 >module.exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 
 multipleDeclarationAlias4.func8 = function () { };
@@ -188,7 +188,7 @@ multipleDeclarationAlias4.func8 = function () { };
 var multipleDeclarationAlias5 = module.exports = exports = {};
 >multipleDeclarationAlias5 : Symbol(multipleDeclarationAlias5, Decl(b.js, 21, 3))
 >module.exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 
@@ -201,7 +201,7 @@ var multipleDeclarationAlias6 = exports = module.exports = {};
 >multipleDeclarationAlias6 : Symbol(multipleDeclarationAlias6, Decl(b.js, 24, 3))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >module.exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 
 multipleDeclarationAlias6.func10 = function () { };
@@ -212,7 +212,7 @@ multipleDeclarationAlias6.func10 = function () { };
 exports = module.exports = someOtherVariable = {};
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >module.exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >someOtherVariable : Symbol(someOtherVariable, Decl(b.js, 14, 3))
 
@@ -224,14 +224,14 @@ exports.func11 = function () { };
 module.exports.func12 = function () { };
 >module.exports.func12 : Symbol(func12, Decl(b.js, 28, 33), Decl(b.js, 32, 33))
 >module.exports : Symbol(func12, Decl(b.js, 28, 33), Decl(b.js, 32, 33))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >func12 : Symbol(func12, Decl(b.js, 28, 33), Decl(b.js, 32, 33))
 
 exports = module.exports = someOtherVariable = {};
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >module.exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >someOtherVariable : Symbol(someOtherVariable, Decl(b.js, 14, 3))
 
@@ -243,14 +243,14 @@ exports.func11 = function () { };
 module.exports.func12 = function () { };
 >module.exports.func12 : Symbol(func12, Decl(b.js, 28, 33), Decl(b.js, 32, 33))
 >module.exports : Symbol(func12, Decl(b.js, 28, 33), Decl(b.js, 32, 33))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >func12 : Symbol(func12, Decl(b.js, 28, 33), Decl(b.js, 32, 33))
 
 exports = module.exports = {};
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >module.exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 
 exports.func13 = function () { };
@@ -261,14 +261,14 @@ exports.func13 = function () { };
 module.exports.func14 = function () { };
 >module.exports.func14 : Symbol(func14, Decl(b.js, 36, 33))
 >module.exports : Symbol(func14, Decl(b.js, 36, 33))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >func14 : Symbol(func14, Decl(b.js, 36, 33))
 
 exports = module.exports = {};
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >module.exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 
 exports.func15 = function () { };
@@ -279,13 +279,13 @@ exports.func15 = function () { };
 module.exports.func16 = function () { };
 >module.exports.func16 : Symbol(func16, Decl(b.js, 40, 33))
 >module.exports : Symbol(func16, Decl(b.js, 40, 33))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >func16 : Symbol(func16, Decl(b.js, 40, 33))
 
 module.exports = exports = {};
 >module.exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 
@@ -297,13 +297,13 @@ exports.func17 = function () { };
 module.exports.func18 = function () { };
 >module.exports.func18 : Symbol(func18, Decl(b.js, 44, 33))
 >module.exports : Symbol(func18, Decl(b.js, 44, 33))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >func18 : Symbol(func18, Decl(b.js, 44, 33))
 
 module.exports = {};
 >module.exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 
 exports.func19 = function () { };
@@ -314,7 +314,7 @@ exports.func19 = function () { };
 module.exports.func20 = function () { };
 >module.exports.func20 : Symbol(func20, Decl(b.js, 48, 33))
 >module.exports : Symbol(func20, Decl(b.js, 48, 33))
->module : Symbol(module, Decl(b.js, 4, 24), Decl(b.js, 5, 43), Decl(b.js, 8, 41), Decl(b.js, 11, 31), Decl(b.js, 18, 51) ... and 14 more)
+>module : Symbol(module, Decl(b.js, 4, 24))
 >exports : Symbol("tests/cases/conformance/salsa/b", Decl(b.js, 0, 0))
 >func20 : Symbol(func20, Decl(b.js, 48, 33))
 

tests/baselines/reference/moduleExportAlias4.symbols

@@ -14,7 +14,7 @@ module.exports = class C {}
 module.exports.D = class D { }
 >module.exports.D : Symbol(D)
 >module.exports : Symbol(D, Decl(bug24024.js, 2, 27))
->module : Symbol(module, Decl(bug24024.js, 1, 31), Decl(bug24024.js, 2, 27))
+>module : Symbol(module, Decl(bug24024.js, 1, 31))
 >exports : Symbol("tests/cases/conformance/salsa/bug24024", Decl(bug24024.js, 0, 0))
 >D : Symbol(D, Decl(bug24024.js, 2, 27))
 >D : Symbol(D, Decl(bug24024.js, 3, 18))

tests/baselines/reference/moduleExportAssignment.symbols

@@ -38,7 +38,7 @@ npmlog.on('hi') // both references should see EE.on
 module.exports.on('hi') // here too
 >module.exports.on : Symbol(EE.on, Decl(npmlog.js, 0, 10))
 >module.exports : Symbol("tests/cases/conformance/salsa/npmlog", Decl(npmlog.js, 0, 0))
->module : Symbol(module, Decl(npmlog.js, 4, 12), Decl(npmlog.js, 6, 15), Decl(npmlog.js, 9, 12), Decl(npmlog.js, 11, 8))
+>module : Symbol(module, Decl(npmlog.js, 4, 12))
 >exports : Symbol("tests/cases/conformance/salsa/npmlog", Decl(npmlog.js, 0, 0))
 >on : Symbol(EE.on, Decl(npmlog.js, 0, 10))
 
@@ -50,7 +50,7 @@ npmlog.x = 1
 module.exports.y = 2
 >module.exports.y : Symbol(y, Decl(npmlog.js, 9, 12))
 >module.exports : Symbol(y, Decl(npmlog.js, 9, 12))
->module : Symbol(module, Decl(npmlog.js, 4, 12), Decl(npmlog.js, 6, 15), Decl(npmlog.js, 9, 12), Decl(npmlog.js, 11, 8))
+>module : Symbol(module, Decl(npmlog.js, 4, 12))
 >exports : Symbol("tests/cases/conformance/salsa/npmlog", Decl(npmlog.js, 0, 0))
 >y : Symbol(y, Decl(npmlog.js, 9, 12))
 
@@ -62,7 +62,7 @@ npmlog.y
 module.exports.x
 >module.exports.x : Symbol(x, Decl(npmlog.js, 7, 23))
 >module.exports : Symbol("tests/cases/conformance/salsa/npmlog", Decl(npmlog.js, 0, 0))
->module : Symbol(module, Decl(npmlog.js, 4, 12), Decl(npmlog.js, 6, 15), Decl(npmlog.js, 9, 12), Decl(npmlog.js, 11, 8))
+>module : Symbol(module, Decl(npmlog.js, 4, 12))
 >exports : Symbol("tests/cases/conformance/salsa/npmlog", Decl(npmlog.js, 0, 0))
 >x : Symbol(x, Decl(npmlog.js, 7, 23))
 

tests/baselines/reference/moduleExportAssignment2.symbols

@@ -9,7 +9,7 @@ var npm = module.exports = function (tree) {
 module.exports.asReadInstalled = function (tree) {
 >module.exports.asReadInstalled : Symbol(asReadInstalled, Decl(npm.js, 1, 1))
 >module.exports : Symbol(asReadInstalled, Decl(npm.js, 1, 1))
->module : Symbol(module, Decl(npm.js, 0, 9), Decl(npm.js, 1, 1))
+>module : Symbol(module, Decl(npm.js, 0, 9))
 >exports : Symbol("tests/cases/conformance/salsa/npm", Decl(npm.js, 0, 0))
 >asReadInstalled : Symbol(asReadInstalled, Decl(npm.js, 1, 1))
 >tree : Symbol(tree, Decl(npm.js, 2, 43))