Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash when checking module exports for export= #10538

Merged
merged 3 commits into from
Aug 25, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 6 additions & 2 deletions src/harness/unittests/convertCompilerOptionsFromJson.ts
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ namespace ts {
{
compilerOptions: <CompilerOptions>{
allowJs: true,
maxNodeModuleJsDepth: 2,
module: ModuleKind.CommonJS,
target: ScriptTarget.ES5,
noImplicitAny: false,
Expand All @@ -429,6 +430,7 @@ namespace ts {
{
compilerOptions: <CompilerOptions>{
allowJs: false,
maxNodeModuleJsDepth: 2,
module: ModuleKind.CommonJS,
target: ScriptTarget.ES5,
noImplicitAny: false,
Expand All @@ -450,7 +452,8 @@ namespace ts {
{
compilerOptions:
{
allowJs: true
allowJs: true,
maxNodeModuleJsDepth: 2
},
errors: [{
file: undefined,
Expand All @@ -469,7 +472,8 @@ namespace ts {
{
compilerOptions:
{
allowJs: true
allowJs: true,
maxNodeModuleJsDepth: 2
},
errors: <Diagnostic[]>[]
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//// [errorForConflictingExportEqualsValue.ts]
export var x;
export = {};


//// [errorForConflictingExportEqualsValue.js]
"use strict";
module.exports = {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/index.ts(4,5): error TS2339: Property 'y' does not exist on type 'typeof "shortid"'.


==== /index.ts (1 errors) ====
/// <reference path="/typings/index.d.ts" />
import * as foo from "shortid";
foo.x // found in index.d.ts
foo.y // ignored from shortid/index.js
~
!!! error TS2339: Property 'y' does not exist on type 'typeof "shortid"'.


==== /node_modules/shortid/node_modules/z/index.js (0 errors) ====
// z will not be found because maxNodeModulesJsDepth: 0
module.exports = { z: 'no' };

==== /node_modules/shortid/index.js (0 errors) ====
var z = require('z');
var y = { y: 'foo' };
module.exports = y;

==== /typings/index.d.ts (0 errors) ====
declare module "shortid" {
export var x: number;
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
[
"======== Resolving module 'shortid' from '/index.ts'. ========",
"Explicitly specified module resolution kind: 'NodeJs'.",
"Loading module 'shortid' from 'node_modules' folder.",
"File '/node_modules/shortid.ts' does not exist.",
"File '/node_modules/shortid.tsx' does not exist.",
"File '/node_modules/shortid.d.ts' does not exist.",
"File '/node_modules/shortid.js' does not exist.",
"File '/node_modules/shortid.jsx' does not exist.",
"File '/node_modules/shortid/package.json' does not exist.",
"File '/node_modules/shortid/index.ts' does not exist.",
"File '/node_modules/shortid/index.tsx' does not exist.",
"File '/node_modules/shortid/index.d.ts' does not exist.",
"File '/node_modules/shortid/index.js' exist - use it as a name resolution result.",
"File '/node_modules/@types/shortid.ts' does not exist.",
"File '/node_modules/@types/shortid.tsx' does not exist.",
"File '/node_modules/@types/shortid.d.ts' does not exist.",
"File '/node_modules/@types/shortid.js' does not exist.",
"File '/node_modules/@types/shortid.jsx' does not exist.",
"File '/node_modules/@types/shortid/package.json' does not exist.",
"File '/node_modules/@types/shortid/index.ts' does not exist.",
"File '/node_modules/@types/shortid/index.tsx' does not exist.",
"File '/node_modules/@types/shortid/index.d.ts' does not exist.",
"File '/node_modules/@types/shortid/index.js' does not exist.",
"File '/node_modules/@types/shortid/index.jsx' does not exist.",
"Resolving real path for '/node_modules/shortid/index.js', result '/node_modules/shortid/index.js'",
"======== Module name 'shortid' was successfully resolved to '/node_modules/shortid/index.js'. ========"
]
2 changes: 2 additions & 0 deletions tests/cases/compiler/errorForConflictingExportEqualsValue.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export var x;
export = {};
36 changes: 36 additions & 0 deletions tests/cases/compiler/maxNodeModuleJsDepthDefaultsToZero.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// @module: commonjs
// @moduleResolution: node
// @allowJs: true
// @traceResolution: true
// @noEmit: true

// @filename: /tsconfig.json
{
"compileOnSave": true,
"compilerOptions": {
"module": "commonjs",
"moduleResolution": "node",
"outDir": "bin"
},
"exclude": [ "node_modules" ]
}
// @filename: /node_modules/shortid/node_modules/z/index.js
// z will not be found because maxNodeModulesJsDepth: 0
module.exports = { z: 'no' };

// @filename: /node_modules/shortid/index.js
var z = require('z');
var y = { y: 'foo' };
module.exports = y;

// @filename: /typings/index.d.ts
declare module "shortid" {
export var x: number;
}

// @filename: /index.ts
/// <reference path="/typings/index.d.ts" />
import * as foo from "shortid";
foo.x // found in index.d.ts
foo.y // ignored from shortid/index.js

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"compilerOptions": {
"allowJs": true,
"declaration": false,
"moduleResolution": "node"
"moduleResolution": "node",
"maxNodeModuleJsDepth": 2
}
}