Fix crash when checking module exports for export=

Also make maxNodeModuleJsDepth default to 0 so that incorrect tsconfigs now let the compiler spend less time compiling JS that is found in node_modules (especially since most people will already have the d.ts and want ignore the JS anyway). jsconfig still defaults to 2.
microsoft · Aug 25, 2016 · c0309fa · c0309fa
1 parent 93de502
commit c0309fa
Show file tree

Hide file tree

Showing 11 changed files with 123 additions and 7 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -1023,8 +1023,8 @@ namespace ts {
             }
         }
 
-        function getDeclarationOfAliasSymbol(symbol: Symbol): Declaration {
-            return findMap(symbol.declarations, d => isAliasSymbolDeclaration(d) ? d : undefined);
+        function getDeclarationOfAliasSymbol(symbol: Symbol): Declaration | undefined {
+            return forEach(symbol.declarations, d => isAliasSymbolDeclaration(d) ? d : undefined);
         }
 
         function getTargetOfImportEqualsDeclaration(node: ImportEqualsDeclaration): Symbol {
@@ -1191,6 +1191,7 @@ namespace ts {
             if (!links.target) {
                 links.target = resolvingSymbol;
                 const node = getDeclarationOfAliasSymbol(symbol);
+                Debug.assert(!!node);
                 const target = getTargetOfAliasDeclaration(node);
                 if (links.target === resolvingSymbol) {
                     links.target = target || unknownSymbol;
@@ -1226,6 +1227,7 @@ namespace ts {
             if (!links.referenced) {
                 links.referenced = true;
                 const node = getDeclarationOfAliasSymbol(symbol);
+                Debug.assert(!!node);
                 if (node.kind === SyntaxKind.ExportAssignment) {
                     // export default <symbol>
                     checkExpressionCached((<ExportAssignment>node).expression);

diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts
@@ -885,7 +885,7 @@ namespace ts {
     function convertCompilerOptionsFromJsonWorker(jsonOptions: any,
         basePath: string, errors: Diagnostic[], configFileName?: string): CompilerOptions {
 
-        const options: CompilerOptions = getBaseFileName(configFileName) === "jsconfig.json" ? { allowJs: true } : {};
+        const options: CompilerOptions = getBaseFileName(configFileName) === "jsconfig.json" ? { allowJs: true, maxNodeModuleJsDepth: 2 } : {};
         convertOptionsFromJson(optionDeclarations, jsonOptions, basePath, options, Diagnostics.Unknown_compiler_option_0, errors);
         return options;
     }

diff --git a/src/compiler/program.ts b/src/compiler/program.ts
@@ -1101,7 +1101,7 @@ namespace ts {
         // - This calls resolveModuleNames, and then calls findSourceFile for each resolved module.
         // As all these operations happen - and are nested - within the createProgram call, they close over the below variables.
         // The current resolution depth is tracked by incrementing/decrementing as the depth first search progresses.
-        const maxNodeModulesJsDepth = typeof options.maxNodeModuleJsDepth === "number" ? options.maxNodeModuleJsDepth : 2;
+        const maxNodeModulesJsDepth = typeof options.maxNodeModuleJsDepth === "number" ? options.maxNodeModuleJsDepth : 0;
         let currentNodeModulesDepth = 0;
 
         // If a module has some of its imports skipped due to being at the depth limit under node_modules, then track

diff --git a/src/harness/unittests/convertCompilerOptionsFromJson.ts b/src/harness/unittests/convertCompilerOptionsFromJson.ts
@@ -403,6 +403,7 @@ namespace ts {
                 {
                     compilerOptions: <CompilerOptions>{
                         allowJs: true,
+                        maxNodeModuleJsDepth: 2,
                         module: ModuleKind.CommonJS,
                         target: ScriptTarget.ES5,
                         noImplicitAny: false,
@@ -429,6 +430,7 @@ namespace ts {
                 {
                     compilerOptions: <CompilerOptions>{
                         allowJs: false,
+                        maxNodeModuleJsDepth: 2,
                         module: ModuleKind.CommonJS,
                         target: ScriptTarget.ES5,
                         noImplicitAny: false,
@@ -450,7 +452,8 @@ namespace ts {
                 {
                     compilerOptions:
                     {
-                        allowJs: true
+                        allowJs: true,
+                        maxNodeModuleJsDepth: 2
                     },
                     errors: [{
                         file: undefined,
@@ -469,7 +472,8 @@ namespace ts {
                 {
                     compilerOptions:
                     {
-                        allowJs: true
+                        allowJs: true,
+                        maxNodeModuleJsDepth: 2
                     },
                     errors: <Diagnostic[]>[]
                 }

diff --git a/tests/baselines/reference/errorForConflictingExportEqualsValue.errors.txt b/tests/baselines/reference/errorForConflictingExportEqualsValue.errors.txt
@@ -0,0 +1,9 @@
+tests/cases/compiler/errorForConflictingExportEqualsValue.ts(2,1): error TS2309: An export assignment cannot be used in a module with other exported elements.
+
+
+==== tests/cases/compiler/errorForConflictingExportEqualsValue.ts (1 errors) ====
+    export var x;
+    export = {};
+    ~~~~~~~~~~~~
+!!! error TS2309: An export assignment cannot be used in a module with other exported elements.
+
diff --git a/tests/baselines/reference/errorForConflictingExportEqualsValue.js b/tests/baselines/reference/errorForConflictingExportEqualsValue.js
@@ -0,0 +1,8 @@
+//// [errorForConflictingExportEqualsValue.ts]
+export var x;
+export = {};
+
+
+//// [errorForConflictingExportEqualsValue.js]
+"use strict";
+module.exports = {};
diff --git a/tests/baselines/reference/maxNodeModuleJsDepthDefaultsToZero.errors.txt b/tests/baselines/reference/maxNodeModuleJsDepthDefaultsToZero.errors.txt
@@ -0,0 +1,26 @@
+c:/root/index.ts(4,5): error TS2339: Property 'y' does not exist on type 'typeof "shortid"'.
+
+
+==== c:/root/index.ts (1 errors) ====
+    /// <reference path="c:/root/typings/index.d.ts" />
+    import * as foo from "shortid";
+    foo.x // found in index.d.ts
+    foo.y // ignored from shortid/index.ts
+        ~
+!!! error TS2339: Property 'y' does not exist on type 'typeof "shortid"'.
+
+
+==== c:/root/node_modules/shortid/node_modules/z/index.js (0 errors) ====
+    // z will not be found because maxNodeModulesJsDepth: 0
+    module.exports = { z: 'no' };
+
+==== c:/root/node_modules/shortid/index.js (0 errors) ====
+    var z = require('z');
+    var y = { y: 'foo' };
+    module.exports = y;
+
+==== c:/root/typings/index.d.ts (0 errors) ====
+    declare module "shortid" {
+        export var x: number;
+    }
+
diff --git a/tests/baselines/reference/maxNodeModuleJsDepthDefaultsToZero.trace.json b/tests/baselines/reference/maxNodeModuleJsDepthDefaultsToZero.trace.json
@@ -0,0 +1,28 @@
+[
+    "======== Resolving module 'shortid' from 'c:/root/index.ts'. ========",
+    "Explicitly specified module resolution kind: 'NodeJs'.",
+    "Loading module 'shortid' from 'node_modules' folder.",
+    "File 'c:/root/node_modules/shortid.ts' does not exist.",
+    "File 'c:/root/node_modules/shortid.tsx' does not exist.",
+    "File 'c:/root/node_modules/shortid.d.ts' does not exist.",
+    "File 'c:/root/node_modules/shortid.js' does not exist.",
+    "File 'c:/root/node_modules/shortid.jsx' does not exist.",
+    "File 'c:/root/node_modules/shortid/package.json' does not exist.",
+    "File 'c:/root/node_modules/shortid/index.ts' does not exist.",
+    "File 'c:/root/node_modules/shortid/index.tsx' does not exist.",
+    "File 'c:/root/node_modules/shortid/index.d.ts' does not exist.",
+    "File 'c:/root/node_modules/shortid/index.js' exist - use it as a name resolution result.",
+    "File 'c:/root/node_modules/@types/shortid.ts' does not exist.",
+    "File 'c:/root/node_modules/@types/shortid.tsx' does not exist.",
+    "File 'c:/root/node_modules/@types/shortid.d.ts' does not exist.",
+    "File 'c:/root/node_modules/@types/shortid.js' does not exist.",
+    "File 'c:/root/node_modules/@types/shortid.jsx' does not exist.",
+    "File 'c:/root/node_modules/@types/shortid/package.json' does not exist.",
+    "File 'c:/root/node_modules/@types/shortid/index.ts' does not exist.",
+    "File 'c:/root/node_modules/@types/shortid/index.tsx' does not exist.",
+    "File 'c:/root/node_modules/@types/shortid/index.d.ts' does not exist.",
+    "File 'c:/root/node_modules/@types/shortid/index.js' does not exist.",
+    "File 'c:/root/node_modules/@types/shortid/index.jsx' does not exist.",
+    "Resolving real path for 'c:/root/node_modules/shortid/index.js', result 'c:/root/node_modules/shortid/index.js'",
+    "======== Module name 'shortid' was successfully resolved to 'c:/root/node_modules/shortid/index.js'. ========"
+]
diff --git a/tests/cases/compiler/errorForConflictingExportEqualsValue.ts b/tests/cases/compiler/errorForConflictingExportEqualsValue.ts
@@ -0,0 +1,2 @@
+export var x;
+export = {};
diff --git a/tests/cases/compiler/maxNodeModuleJsDepthDefaultsToZero.ts b/tests/cases/compiler/maxNodeModuleJsDepthDefaultsToZero.ts
@@ -0,0 +1,36 @@
+// @module: commonjs
+// @moduleResolution: node
+// @allowJs: true
+// @traceResolution: true
+// @noEmit: true
+
+// @filename: c:/root/tsconfig.json
+{
+    "compileOnSave": true,
+    "compilerOptions": {
+        "module": "commonjs",
+        "moduleResolution": "node",
+        "outDir": "bin"
+    },
+    "exclude": [ "node_modules" ]
+}
+// @filename: c:/root/node_modules/shortid/node_modules/z/index.js
+// z will not be found because maxNodeModulesJsDepth: 0
+module.exports = { z: 'no' };
+
+// @filename: c:/root/node_modules/shortid/index.js
+var z = require('z');
+var y = { y: 'foo' };
+module.exports = y;
+
+// @filename: c:/root/typings/index.d.ts
+declare module "shortid" {
+    export var x: number;
+}
+
+// @filename: c:/root/index.ts
+/// <reference path="c:/root/typings/index.d.ts" />
+import * as foo from "shortid";
+foo.x // found in index.d.ts
+foo.y // ignored from shortid/index.ts
+
diff --git a/tests/cases/projects/NodeModulesSearch/importHigher/tsconfig.json b/tests/cases/projects/NodeModulesSearch/importHigher/tsconfig.json
@@ -2,6 +2,7 @@
   "compilerOptions": { 
     "allowJs": true,
     "declaration": false,
-    "moduleResolution": "node"
+    "moduleResolution": "node",
+    "maxNodeModuleJsDepth": 2
   }
 }