From e7915f165b62960d16917d34a3f37bc182516db8 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 9 Oct 2015 14:54:58 -0700 Subject: [PATCH 1/7] Specifically check for differing declaration names. --- src/compiler/checker.ts | 82 ++++++++++++++++++++++++++++++++++------- 1 file changed, 69 insertions(+), 13 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 228b1067bb01a..51f59825e1b62 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11061,7 +11061,7 @@ namespace ts { let someHaveQuestionToken = false; let allHaveQuestionToken = true; let hasOverloads = false; - let bodyDeclaration: FunctionLikeDeclaration; + let implementationDeclaration: FunctionLikeDeclaration; // should be the first declaration with a body let lastSeenNonAmbientDeclaration: FunctionLikeDeclaration; let previousDeclaration: FunctionLikeDeclaration; @@ -11120,6 +11120,7 @@ namespace ts { // names and consistency of modifiers are verified when we check local symbol let isExportSymbolInsideModule = symbol.parent && symbol.parent.flags & SymbolFlags.Module; let duplicateFunctionDeclaration = false; + let duplicateDefaultExports = false; let multipleConstructorImplementation = false; for (let current of declarations) { let node = current; @@ -11143,12 +11144,13 @@ namespace ts { someHaveQuestionToken = someHaveQuestionToken || hasQuestionToken(node); allHaveQuestionToken = allHaveQuestionToken && hasQuestionToken(node); - if (nodeIsPresent(node.body) && bodyDeclaration) { + if (nodeIsPresent(node.body) && implementationDeclaration) { if (isConstructor) { multipleConstructorImplementation = true; } else { duplicateFunctionDeclaration = true; + duplicateDefaultExports = !!(node.flags & NodeFlags.Default); } } else if (!isExportSymbolInsideModule && previousDeclaration && previousDeclaration.parent === node.parent && previousDeclaration.end !== node.pos) { @@ -11156,8 +11158,8 @@ namespace ts { } if (nodeIsPresent(node.body)) { - if (!bodyDeclaration) { - bodyDeclaration = node; + if (!implementationDeclaration) { + implementationDeclaration = node; } } else { @@ -11173,15 +11175,19 @@ namespace ts { } if (multipleConstructorImplementation) { - forEach(declarations, declaration => { + for (let declaration of declarations) { error(declaration, Diagnostics.Multiple_constructor_implementations_are_not_allowed); - }); + } } if (duplicateFunctionDeclaration) { - forEach(declarations, declaration => { - error(declaration.name, Diagnostics.Duplicate_function_implementation); - }); + let message = duplicateDefaultExports && areAllFunctionDeclarationsWhereSomeNamesDiffer(declarations) ? + Diagnostics.A_module_cannot_have_multiple_default_exports : + Diagnostics.Duplicate_function_implementation; + + for (let declaration of declarations) { + error(declaration.name, message); + } } // Abstract methods can't have an implementation -- in particular, they don't need one. @@ -11191,12 +11197,12 @@ namespace ts { } if (hasOverloads) { - checkFlagAgreementBetweenOverloads(declarations, bodyDeclaration, flagsToCheck, someNodeFlags, allNodeFlags); - checkQuestionTokenAgreementBetweenOverloads(declarations, bodyDeclaration, someHaveQuestionToken, allHaveQuestionToken); + checkFlagAgreementBetweenOverloads(declarations, implementationDeclaration, flagsToCheck, someNodeFlags, allNodeFlags); + checkQuestionTokenAgreementBetweenOverloads(declarations, implementationDeclaration, someHaveQuestionToken, allHaveQuestionToken); - if (bodyDeclaration) { + if (implementationDeclaration) { let signatures = getSignaturesOfSymbol(symbol); - let bodySignature = getSignatureFromDeclaration(bodyDeclaration); + let bodySignature = getSignatureFromDeclaration(implementationDeclaration); // If the implementation signature has string literals, we will have reported an error in // checkSpecializedSignatureDeclaration if (!bodySignature.hasStringLiterals) { @@ -11225,6 +11231,56 @@ namespace ts { } } + /** + * Determines if all declarations in the given list are function declarations, and that + * all the declarations have the same name. This is useful for how we report errors. + * + * For instance, if a user has + * + * export default function foo() { } + * export default function bar() { } + * + * We want to tell the user that there are multiple default exports. However, if a user has + * + * export default function foo() { } + * export default function foo() { } + * + * It is probably more accurate to say that there are duplicate function implementations. + * + * @param declarations + */ + function areAllFunctionDeclarationsWhereSomeNamesDiffer(declarations: Declaration[]) { + let numDeclarations = declarations.length; + Debug.assert(numDeclarations > 0, "Expected at least one declaration."); + + let firstDeclaration = declarations[0]; + if (firstDeclaration.kind !== SyntaxKind.FunctionDeclaration) { + return false; + } + + let firstName = (firstDeclaration as FunctionDeclaration).name; + for (let i = 1; i < numDeclarations; i++) { + let currentDeclaration = declarations[i]; + if (currentDeclaration.kind !== SyntaxKind.FunctionDeclaration) { + return false; + } + + let currentName = (currentDeclaration as FunctionDeclaration).name; + if (currentName) { + if (!firstName || currentName.text !== firstName.text) { + // Either 'firstName' must be undefined or the text must differ. + return true; + } + } + else if (firstName) { + // 'currentName' is not defined but 'firstName' is. + return true; + } + } + + return false; + } + function checkExportsOnMergedDeclarations(node: Node): void { if (!produceDiagnostics) { return; From 3cf37584ed434be6af9e3129b21469989e8ef905 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 9 Oct 2015 14:57:52 -0700 Subject: [PATCH 2/7] Accepted baselines. --- .../reference/multipleDefaultExports02.errors.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/baselines/reference/multipleDefaultExports02.errors.txt b/tests/baselines/reference/multipleDefaultExports02.errors.txt index 235f8f7c3e374..d6b9009b60381 100644 --- a/tests/baselines/reference/multipleDefaultExports02.errors.txt +++ b/tests/baselines/reference/multipleDefaultExports02.errors.txt @@ -1,18 +1,18 @@ -tests/cases/conformance/es6/modules/m1.ts(2,25): error TS2393: Duplicate function implementation. -tests/cases/conformance/es6/modules/m1.ts(6,25): error TS2393: Duplicate function implementation. +tests/cases/conformance/es6/modules/m1.ts(2,25): error TS2528: A module cannot have multiple default exports. +tests/cases/conformance/es6/modules/m1.ts(6,25): error TS2528: A module cannot have multiple default exports. ==== tests/cases/conformance/es6/modules/m1.ts (2 errors) ==== export default function foo() { ~~~ -!!! error TS2393: Duplicate function implementation. +!!! error TS2528: A module cannot have multiple default exports. } export default function bar() { ~~~ -!!! error TS2393: Duplicate function implementation. +!!! error TS2528: A module cannot have multiple default exports. } From bc6949ee8b14439b34c504dea0734e30712adff9 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 9 Oct 2015 15:00:33 -0700 Subject: [PATCH 3/7] Added test for duplicate default exports with no name. --- .../es6/modules/multipleDefaultExports05.ts | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 tests/cases/conformance/es6/modules/multipleDefaultExports05.ts diff --git a/tests/cases/conformance/es6/modules/multipleDefaultExports05.ts b/tests/cases/conformance/es6/modules/multipleDefaultExports05.ts new file mode 100644 index 0000000000000..e90b175d779bc --- /dev/null +++ b/tests/cases/conformance/es6/modules/multipleDefaultExports05.ts @@ -0,0 +1,38 @@ +// @module: commonjs +// @target: ES5 + +// @filename: m1.ts +export default function () { + +} + +export default function () { + +} + +// @filename: m2.ts +export default function () { + +} + +export default class { + +} + +// @filename: m3.ts +export default function () { + +} + +export default class C { + +} + +// @filename: m4.ts +export default function f() { + +} + +export default class { + +} \ No newline at end of file From 1fe55873da9ccf0f3f659bfd5a0e1dde7316b3b8 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 9 Oct 2015 15:02:35 -0700 Subject: [PATCH 4/7] Accepted baselines. --- .../multipleDefaultExports05.errors.txt | 58 +++++++++++++ .../reference/multipleDefaultExports05.js | 84 +++++++++++++++++++ 2 files changed, 142 insertions(+) create mode 100644 tests/baselines/reference/multipleDefaultExports05.errors.txt create mode 100644 tests/baselines/reference/multipleDefaultExports05.js diff --git a/tests/baselines/reference/multipleDefaultExports05.errors.txt b/tests/baselines/reference/multipleDefaultExports05.errors.txt new file mode 100644 index 0000000000000..35ad6f9c40af2 --- /dev/null +++ b/tests/baselines/reference/multipleDefaultExports05.errors.txt @@ -0,0 +1,58 @@ +error TS2393: Duplicate function implementation. +tests/cases/conformance/es6/modules/m2.ts(1,1): error TS2528: A module cannot have multiple default exports. +tests/cases/conformance/es6/modules/m2.ts(5,1): error TS2528: A module cannot have multiple default exports. +tests/cases/conformance/es6/modules/m3.ts(1,1): error TS2528: A module cannot have multiple default exports. +tests/cases/conformance/es6/modules/m3.ts(5,22): error TS2528: A module cannot have multiple default exports. +tests/cases/conformance/es6/modules/m4.ts(1,25): error TS2528: A module cannot have multiple default exports. +tests/cases/conformance/es6/modules/m4.ts(5,1): error TS2528: A module cannot have multiple default exports. + + +!!! error TS2393: Duplicate function implementation. +==== tests/cases/conformance/es6/modules/m1.ts (0 errors) ==== + + export default function () { + + } + + export default function () { + + } + +==== tests/cases/conformance/es6/modules/m2.ts (2 errors) ==== + export default function () { + ~~~~~~ +!!! error TS2528: A module cannot have multiple default exports. + + } + + export default class { + ~~~~~~ +!!! error TS2528: A module cannot have multiple default exports. + + } + +==== tests/cases/conformance/es6/modules/m3.ts (2 errors) ==== + export default function () { + ~~~~~~ +!!! error TS2528: A module cannot have multiple default exports. + + } + + export default class C { + ~ +!!! error TS2528: A module cannot have multiple default exports. + + } + +==== tests/cases/conformance/es6/modules/m4.ts (2 errors) ==== + export default function f() { + ~ +!!! error TS2528: A module cannot have multiple default exports. + + } + + export default class { + ~~~~~~ +!!! error TS2528: A module cannot have multiple default exports. + + } \ No newline at end of file diff --git a/tests/baselines/reference/multipleDefaultExports05.js b/tests/baselines/reference/multipleDefaultExports05.js new file mode 100644 index 0000000000000..67458900bf9d2 --- /dev/null +++ b/tests/baselines/reference/multipleDefaultExports05.js @@ -0,0 +1,84 @@ +//// [tests/cases/conformance/es6/modules/multipleDefaultExports05.ts] //// + +//// [m1.ts] + +export default function () { + +} + +export default function () { + +} + +//// [m2.ts] +export default function () { + +} + +export default class { + +} + +//// [m3.ts] +export default function () { + +} + +export default class C { + +} + +//// [m4.ts] +export default function f() { + +} + +export default class { + +} + +//// [m1.js] +function default_1() { +} +Object.defineProperty(exports, "__esModule", { value: true }); +exports.default = default_1; +function default_2() { +} +Object.defineProperty(exports, "__esModule", { value: true }); +exports.default = default_2; +//// [m2.js] +function default_1() { +} +Object.defineProperty(exports, "__esModule", { value: true }); +exports.default = default_1; +var default_2 = (function () { + function default_2() { + } + return default_2; +})(); +Object.defineProperty(exports, "__esModule", { value: true }); +exports.default = default_2; +//// [m3.js] +function default_1() { +} +Object.defineProperty(exports, "__esModule", { value: true }); +exports.default = default_1; +var C = (function () { + function C() { + } + return C; +})(); +Object.defineProperty(exports, "__esModule", { value: true }); +exports.default = C; +//// [m4.js] +function f() { +} +Object.defineProperty(exports, "__esModule", { value: true }); +exports.default = f; +var default_1 = (function () { + function default_1() { + } + return default_1; +})(); +Object.defineProperty(exports, "__esModule", { value: true }); +exports.default = default_1; From 2d6af5b7f725818df6d938ce7cdedcc7ad49d2dd Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 9 Oct 2015 15:05:38 -0700 Subject: [PATCH 5/7] Use the declaration if the name is not available. --- src/compiler/checker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 51f59825e1b62..8ad4dcce8b4cb 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11186,7 +11186,7 @@ namespace ts { Diagnostics.Duplicate_function_implementation; for (let declaration of declarations) { - error(declaration.name, message); + error(declaration.name || declaration, message); } } From e4b3bd297d381618309f1d4f7d0d4f3863474b67 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 9 Oct 2015 15:06:04 -0700 Subject: [PATCH 6/7] Accepted baselines. --- .../reference/multipleDefaultExports05.errors.txt | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/baselines/reference/multipleDefaultExports05.errors.txt b/tests/baselines/reference/multipleDefaultExports05.errors.txt index 35ad6f9c40af2..5edcfc914c77d 100644 --- a/tests/baselines/reference/multipleDefaultExports05.errors.txt +++ b/tests/baselines/reference/multipleDefaultExports05.errors.txt @@ -1,4 +1,5 @@ -error TS2393: Duplicate function implementation. +tests/cases/conformance/es6/modules/m1.ts(2,1): error TS2393: Duplicate function implementation. +tests/cases/conformance/es6/modules/m1.ts(6,1): error TS2393: Duplicate function implementation. tests/cases/conformance/es6/modules/m2.ts(1,1): error TS2528: A module cannot have multiple default exports. tests/cases/conformance/es6/modules/m2.ts(5,1): error TS2528: A module cannot have multiple default exports. tests/cases/conformance/es6/modules/m3.ts(1,1): error TS2528: A module cannot have multiple default exports. @@ -7,14 +8,17 @@ tests/cases/conformance/es6/modules/m4.ts(1,25): error TS2528: A module cannot h tests/cases/conformance/es6/modules/m4.ts(5,1): error TS2528: A module cannot have multiple default exports. -!!! error TS2393: Duplicate function implementation. -==== tests/cases/conformance/es6/modules/m1.ts (0 errors) ==== +==== tests/cases/conformance/es6/modules/m1.ts (2 errors) ==== export default function () { + ~~~~~~ +!!! error TS2393: Duplicate function implementation. } export default function () { + ~~~~~~ +!!! error TS2393: Duplicate function implementation. } From c7e20aa49b70173fa7a68398276e5b72e60f7d0d Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Thu, 15 Oct 2015 13:02:30 -0700 Subject: [PATCH 7/7] Get rid of local, just use the flag. --- src/compiler/checker.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 8ad4dcce8b4cb..01fc97314c263 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11120,7 +11120,6 @@ namespace ts { // names and consistency of modifiers are verified when we check local symbol let isExportSymbolInsideModule = symbol.parent && symbol.parent.flags & SymbolFlags.Module; let duplicateFunctionDeclaration = false; - let duplicateDefaultExports = false; let multipleConstructorImplementation = false; for (let current of declarations) { let node = current; @@ -11150,7 +11149,11 @@ namespace ts { } else { duplicateFunctionDeclaration = true; - duplicateDefaultExports = !!(node.flags & NodeFlags.Default); + + // We expect both implementations to agree in defaultness for later on. + if ((node.flags & NodeFlags.Default) !== (implementationDeclaration.flags & NodeFlags.Default)) { + Debug.fail("Expected first and current implementations of and current to agree in export default flag."); + } } } else if (!isExportSymbolInsideModule && previousDeclaration && previousDeclaration.parent === node.parent && previousDeclaration.end !== node.pos) { @@ -11181,7 +11184,7 @@ namespace ts { } if (duplicateFunctionDeclaration) { - let message = duplicateDefaultExports && areAllFunctionDeclarationsWhereSomeNamesDiffer(declarations) ? + let message = implementationDeclaration.flags & NodeFlags.Default && areAllFunctionDeclarationsWhereSomeNamesDiffer(declarations) ? Diagnostics.A_module_cannot_have_multiple_default_exports : Diagnostics.Duplicate_function_implementation;