From 48e985d72f12af17d39b47140491a8b52ccb29df Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Tue, 10 Nov 2015 11:35:03 -0800 Subject: [PATCH 1/4] Prevent resolveName from ignoring default exports --- src/compiler/checker.ts | 42 ++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 706ed1c65a3..05ced970793 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -487,8 +487,9 @@ namespace ts { (location.kind === SyntaxKind.ModuleDeclaration && (location).name.kind === SyntaxKind.StringLiteral)) { // It's an external module. Because of module/namespace merging, a module's exports are in scope, - // yet we never want to treat an export specifier as putting a member in scope. Therefore, - // if the name we find is purely an export specifier, it is not actually considered in scope. + // yet we never want to treat an export specifier as putting a member in scope, except 'default'. + // Therefore, if the name we find is purely an export specifier, it is not actually considered in scope, + // unless it is 'default'. // Two things to note about this: // 1. We have to check this without calling getSymbol. The problem with calling getSymbol // on an export specifier is that it might find the export specifier itself, and try to @@ -497,15 +498,16 @@ namespace ts { // 2. We check === SymbolFlags.Alias in order to check that the symbol is *purely* // an alias. If we used &, we'd be throwing out symbols that have non alias aspects, // which is not the desired behavior. + const isLocalSymbolForExportDefault = doesNameResolveToLocalSymbolForExportDefault(moduleExports, meaning, name); if (hasProperty(moduleExports, name) && moduleExports[name].flags === SymbolFlags.Alias && + !isLocalSymbolForExportDefault && getDeclarationOfKind(moduleExports[name], SyntaxKind.ExportSpecifier)) { break; } - result = moduleExports["default"]; - const localSymbol = getLocalSymbolForExportDefault(result); - if (result && localSymbol && (result.flags & meaning) && localSymbol.name === name) { + if (isLocalSymbolForExportDefault) { + result = moduleExports["default"]; break loop; } result = undefined; @@ -674,6 +676,12 @@ namespace ts { return result; } + function doesNameResolveToLocalSymbolForExportDefault(exports: SymbolTable, meaning: SymbolFlags, name: string): boolean { + const defaultExport = exports["default"]; + const localSymbol = getLocalSymbolForExportDefault(defaultExport); + return defaultExport && (defaultExport.flags & meaning) && localSymbol && localSymbol.name === name; + } + function checkResolvedBlockScopedVariable(result: Symbol, errorLocation: Node): void { Debug.assert((result.flags & SymbolFlags.BlockScopedVariable) !== 0); // Block-scoped variables cannot be used before their definition @@ -4124,12 +4132,12 @@ namespace ts { // We only support expressions that are simple qualified names. For other expressions this produces undefined. const typeNameOrExpression = node.kind === SyntaxKind.TypeReference ? (node).typeName : isSupportedExpressionWithTypeArguments(node) ? (node).expression : - undefined; + undefined; const symbol = typeNameOrExpression && resolveEntityName(typeNameOrExpression, SymbolFlags.Type) || unknownSymbol; const type = symbol === unknownSymbol ? unknownType : symbol.flags & (SymbolFlags.Class | SymbolFlags.Interface) ? getTypeFromClassOrInterfaceReference(node, symbol) : - symbol.flags & SymbolFlags.TypeAlias ? getTypeFromTypeAliasReference(node, symbol) : - getTypeFromNonGenericTypeReference(node, symbol); + symbol.flags & SymbolFlags.TypeAlias ? getTypeFromTypeAliasReference(node, symbol) : + getTypeFromNonGenericTypeReference(node, symbol); // Cache both the resolved symbol and the resolved type. The resolved symbol is needed in when we check the // type reference in checkTypeReferenceOrExpressionWithTypeArguments. links.resolvedSymbol = symbol; @@ -11309,7 +11317,7 @@ namespace ts { // Abstract methods can't have an implementation -- in particular, they don't need one. if (!isExportSymbolInsideModule && lastSeenNonAmbientDeclaration && !lastSeenNonAmbientDeclaration.body && - !(lastSeenNonAmbientDeclaration.flags & NodeFlags.Abstract) ) { + !(lastSeenNonAmbientDeclaration.flags & NodeFlags.Abstract)) { reportImplementationExpectedError(lastSeenNonAmbientDeclaration); } @@ -14221,8 +14229,8 @@ namespace ts { if (className) { copySymbol(location.symbol, meaning); } - // fall through; this fall-through is necessary because we would like to handle - // type parameter inside class expression similar to how we handle it in classDeclaration and interface Declaration + // fall through; this fall-through is necessary because we would like to handle + // type parameter inside class expression similar to how we handle it in classDeclaration and interface Declaration case SyntaxKind.ClassDeclaration: case SyntaxKind.InterfaceDeclaration: // If we didn't come from static member of class or interface, @@ -14378,8 +14386,8 @@ namespace ts { return resolveEntityName(entityName, meaning); } else if ((entityName.parent.kind === SyntaxKind.JsxOpeningElement) || - (entityName.parent.kind === SyntaxKind.JsxSelfClosingElement) || - (entityName.parent.kind === SyntaxKind.JsxClosingElement)) { + (entityName.parent.kind === SyntaxKind.JsxSelfClosingElement) || + (entityName.parent.kind === SyntaxKind.JsxClosingElement)) { return getJsxElementTagSymbol(entityName.parent); } else if (isExpression(entityName)) { @@ -14446,8 +14454,8 @@ namespace ts { : getSymbolOfPartOfRightHandSideOfImportEquals(node); } else if (node.parent.kind === SyntaxKind.BindingElement && - node.parent.parent.kind === SyntaxKind.ObjectBindingPattern && - node === (node.parent).propertyName) { + node.parent.parent.kind === SyntaxKind.ObjectBindingPattern && + node === (node.parent).propertyName) { const typeOfPattern = getTypeOfNode(node.parent.parent); const propertyDeclaration = typeOfPattern && getPropertyOfType(typeOfPattern, (node).text); @@ -14484,7 +14492,7 @@ namespace ts { (node.parent).moduleSpecifier === node)) { return resolveExternalModuleName(node, node); } - // Fall through + // Fall through case SyntaxKind.NumericLiteral: // index access @@ -14680,7 +14688,7 @@ namespace ts { if (links.isNestedRedeclaration === undefined) { const container = getEnclosingBlockScopeContainer(symbol.valueDeclaration); links.isNestedRedeclaration = isStatementWithLocals(container) && - !!resolveName(container.parent, symbol.name, SymbolFlags.Value, /*nameNotFoundMessage*/ undefined, /*nameArg*/ undefined); + !!resolveName(container.parent, symbol.name, SymbolFlags.Value, /*nameNotFoundMessage*/ undefined, /*nameArg*/ undefined); } return links.isNestedRedeclaration; } From e30a01db0fb06dc93177f194cfd919ca4d4baec3 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Tue, 10 Nov 2015 11:44:56 -0800 Subject: [PATCH 2/4] Add test case and accept baseline --- .../reference/reExportDefaultExport.js | 27 +++++++++++++++++++ .../reference/reExportDefaultExport.symbols | 22 +++++++++++++++ .../reference/reExportDefaultExport.types | 24 +++++++++++++++++ .../es6/modules/reExportDefaultExport.ts | 15 +++++++++++ 4 files changed, 88 insertions(+) create mode 100644 tests/baselines/reference/reExportDefaultExport.js create mode 100644 tests/baselines/reference/reExportDefaultExport.symbols create mode 100644 tests/baselines/reference/reExportDefaultExport.types create mode 100644 tests/cases/conformance/es6/modules/reExportDefaultExport.ts diff --git a/tests/baselines/reference/reExportDefaultExport.js b/tests/baselines/reference/reExportDefaultExport.js new file mode 100644 index 00000000000..dbe3dfdbbae --- /dev/null +++ b/tests/baselines/reference/reExportDefaultExport.js @@ -0,0 +1,27 @@ +//// [tests/cases/conformance/es6/modules/reExportDefaultExport.ts] //// + +//// [m1.ts] + +export default function f() { +} +export {f}; + + +//// [m2.ts] +import foo from "./m1"; +import {f} from "./m1"; + +f(); +foo(); + +//// [m1.js] +function f() { +} +Object.defineProperty(exports, "__esModule", { value: true }); +exports.default = f; +exports.f = f; +//// [m2.js] +var m1_1 = require("./m1"); +var m1_2 = require("./m1"); +m1_2.f(); +m1_1.default(); diff --git a/tests/baselines/reference/reExportDefaultExport.symbols b/tests/baselines/reference/reExportDefaultExport.symbols new file mode 100644 index 00000000000..cd0d6e493c9 --- /dev/null +++ b/tests/baselines/reference/reExportDefaultExport.symbols @@ -0,0 +1,22 @@ +=== tests/cases/conformance/es6/modules/m1.ts === + +export default function f() { +>f : Symbol(f, Decl(m1.ts, 0, 0)) +} +export {f}; +>f : Symbol(f, Decl(m1.ts, 3, 8)) + + +=== tests/cases/conformance/es6/modules/m2.ts === +import foo from "./m1"; +>foo : Symbol(foo, Decl(m2.ts, 0, 6)) + +import {f} from "./m1"; +>f : Symbol(f, Decl(m2.ts, 1, 8)) + +f(); +>f : Symbol(f, Decl(m2.ts, 1, 8)) + +foo(); +>foo : Symbol(foo, Decl(m2.ts, 0, 6)) + diff --git a/tests/baselines/reference/reExportDefaultExport.types b/tests/baselines/reference/reExportDefaultExport.types new file mode 100644 index 00000000000..f775571ac2c --- /dev/null +++ b/tests/baselines/reference/reExportDefaultExport.types @@ -0,0 +1,24 @@ +=== tests/cases/conformance/es6/modules/m1.ts === + +export default function f() { +>f : () => void +} +export {f}; +>f : () => void + + +=== tests/cases/conformance/es6/modules/m2.ts === +import foo from "./m1"; +>foo : () => void + +import {f} from "./m1"; +>f : () => void + +f(); +>f() : void +>f : () => void + +foo(); +>foo() : void +>foo : () => void + diff --git a/tests/cases/conformance/es6/modules/reExportDefaultExport.ts b/tests/cases/conformance/es6/modules/reExportDefaultExport.ts new file mode 100644 index 00000000000..730435fd4ee --- /dev/null +++ b/tests/cases/conformance/es6/modules/reExportDefaultExport.ts @@ -0,0 +1,15 @@ +// @module: commonjs +// @target: ES5 + +// @filename: m1.ts +export default function f() { +} +export {f}; + + +// @filename: m2.ts +import foo from "./m1"; +import {f} from "./m1"; + +f(); +foo(); \ No newline at end of file From e62603ba8522eca6529eccfd84480e8a1a4c7a6e Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 11 Nov 2015 08:51:37 -0800 Subject: [PATCH 3/4] Move export default check before module merge one As suggested by Anders. --- src/compiler/checker.ts | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 05ced970793..2bbed26f23c 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -486,10 +486,19 @@ namespace ts { if (location.kind === SyntaxKind.SourceFile || (location.kind === SyntaxKind.ModuleDeclaration && (location).name.kind === SyntaxKind.StringLiteral)) { - // It's an external module. Because of module/namespace merging, a module's exports are in scope, - // yet we never want to treat an export specifier as putting a member in scope, except 'default'. - // Therefore, if the name we find is purely an export specifier, it is not actually considered in scope, - // unless it is 'default'. + // It's an external module. First see if the module has an export default and if the local + // name of that export default matches. + if (result = moduleExports["default"]) { + const localSymbol = getLocalSymbolForExportDefault(result); + if (localSymbol && (result.flags & meaning) && localSymbol.name === name) { + break loop; + } + result = undefined; + } + + // Because of module/namespace merging, a module's exports are in scope, + // yet we never want to treat an export specifier as putting a member in scope. + // Therefore, if the name we find is purely an export specifier, it is not actually considered in scope. // Two things to note about this: // 1. We have to check this without calling getSymbol. The problem with calling getSymbol // on an export specifier is that it might find the export specifier itself, and try to @@ -498,19 +507,11 @@ namespace ts { // 2. We check === SymbolFlags.Alias in order to check that the symbol is *purely* // an alias. If we used &, we'd be throwing out symbols that have non alias aspects, // which is not the desired behavior. - const isLocalSymbolForExportDefault = doesNameResolveToLocalSymbolForExportDefault(moduleExports, meaning, name); if (hasProperty(moduleExports, name) && moduleExports[name].flags === SymbolFlags.Alias && - !isLocalSymbolForExportDefault && getDeclarationOfKind(moduleExports[name], SyntaxKind.ExportSpecifier)) { break; } - - if (isLocalSymbolForExportDefault) { - result = moduleExports["default"]; - break loop; - } - result = undefined; } if (result = getSymbol(moduleExports, name, meaning & SymbolFlags.ModuleMember)) { From 9472c0be9951bd2926d6e2200f8ce8c28da13dc4 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 11 Nov 2015 10:53:24 -0800 Subject: [PATCH 4/4] Remove now-unused function `doesNameResolveToLocalSymbolForExportDefault` is no longer used, so delete it. --- src/compiler/checker.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 2bbed26f23c..175807ac686 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -677,12 +677,6 @@ namespace ts { return result; } - function doesNameResolveToLocalSymbolForExportDefault(exports: SymbolTable, meaning: SymbolFlags, name: string): boolean { - const defaultExport = exports["default"]; - const localSymbol = getLocalSymbolForExportDefault(defaultExport); - return defaultExport && (defaultExport.flags & meaning) && localSymbol && localSymbol.name === name; - } - function checkResolvedBlockScopedVariable(result: Symbol, errorLocation: Node): void { Debug.assert((result.flags & SymbolFlags.BlockScopedVariable) !== 0); // Block-scoped variables cannot be used before their definition