From f2d153176879c76f1e1ee4e587662b8f1f5f1a0b Mon Sep 17 00:00:00 2001 From: Eli Barzilay Date: Wed, 29 Jul 2020 15:32:52 -0400 Subject: [PATCH] Fix location for duplicate function implementation errors Use only the relevant declarations (by collecting them in the for loop), and use `declaration` if `getNameOfDeclaration` didn't work (useful for `export default` with anonymous functions). Fixes #39804. Also, use `nodeIsPresent` once, and a random `?.`. --- src/compiler/checker.ts | 15 +++++++++------ ...portDefaultInterfaceAndTwoFunctions.errors.txt | 13 +++++++++++++ .../exportDefaultInterfaceAndTwoFunctions.js | 13 +++++++++++++ .../exportDefaultInterfaceAndTwoFunctions.symbols | 8 ++++++++ .../exportDefaultInterfaceAndTwoFunctions.types | 10 ++++++++++ .../exportDefaultInterfaceAndTwoFunctions.ts | 3 +++ 6 files changed, 56 insertions(+), 6 deletions(-) create mode 100644 tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.errors.txt create mode 100644 tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.js create mode 100644 tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.symbols create mode 100644 tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.types create mode 100644 tests/cases/compiler/exportDefaultInterfaceAndTwoFunctions.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 5c31002cf27..c97692c96f4 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -31280,6 +31280,7 @@ namespace ts { let duplicateFunctionDeclaration = false; let multipleConstructorImplementation = false; let hasNonAmbientClass = false; + const functionDeclarations = [] as Declaration[]; for (const current of declarations) { const node = current; const inAmbientContext = node.flags & NodeFlags.Ambient; @@ -31300,13 +31301,15 @@ namespace ts { } if (node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.MethodDeclaration || node.kind === SyntaxKind.MethodSignature || node.kind === SyntaxKind.Constructor) { + functionDeclarations.push(node); const currentNodeFlags = getEffectiveDeclarationFlags(node, flagsToCheck); someNodeFlags |= currentNodeFlags; allNodeFlags &= currentNodeFlags; someHaveQuestionToken = someHaveQuestionToken || hasQuestionToken(node); allHaveQuestionToken = allHaveQuestionToken && hasQuestionToken(node); + const bodyIsPresent = nodeIsPresent((node as FunctionLikeDeclaration).body); - if (nodeIsPresent((node as FunctionLikeDeclaration).body) && bodyDeclaration) { + if (bodyIsPresent && bodyDeclaration) { if (isConstructor) { multipleConstructorImplementation = true; } @@ -31314,11 +31317,11 @@ namespace ts { duplicateFunctionDeclaration = true; } } - else if (previousDeclaration && previousDeclaration.parent === node.parent && previousDeclaration.end !== node.pos) { + else if (previousDeclaration?.parent === node.parent && previousDeclaration.end !== node.pos) { reportImplementationExpectedError(previousDeclaration); } - if (nodeIsPresent((node as FunctionLikeDeclaration).body)) { + if (bodyIsPresent) { if (!bodyDeclaration) { bodyDeclaration = node as FunctionLikeDeclaration; } @@ -31336,14 +31339,14 @@ namespace ts { } if (multipleConstructorImplementation) { - forEach(declarations, declaration => { + forEach(functionDeclarations, declaration => { error(declaration, Diagnostics.Multiple_constructor_implementations_are_not_allowed); }); } if (duplicateFunctionDeclaration) { - forEach(declarations, declaration => { - error(getNameOfDeclaration(declaration), Diagnostics.Duplicate_function_implementation); + forEach(functionDeclarations, declaration => { + error(getNameOfDeclaration(declaration) || declaration, Diagnostics.Duplicate_function_implementation); }); } diff --git a/tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.errors.txt b/tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.errors.txt new file mode 100644 index 00000000000..9f1f731b6a7 --- /dev/null +++ b/tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.errors.txt @@ -0,0 +1,13 @@ +tests/cases/compiler/exportDefaultInterfaceAndTwoFunctions.ts(2,1): error TS2393: Duplicate function implementation. +tests/cases/compiler/exportDefaultInterfaceAndTwoFunctions.ts(3,1): error TS2393: Duplicate function implementation. + + +==== tests/cases/compiler/exportDefaultInterfaceAndTwoFunctions.ts (2 errors) ==== + export default interface A { a: string; } + export default function() { return 1; } + ~~~~~~ +!!! error TS2393: Duplicate function implementation. + export default function() { return 2; } + ~~~~~~ +!!! error TS2393: Duplicate function implementation. + \ No newline at end of file diff --git a/tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.js b/tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.js new file mode 100644 index 00000000000..e2d4f28561f --- /dev/null +++ b/tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.js @@ -0,0 +1,13 @@ +//// [exportDefaultInterfaceAndTwoFunctions.ts] +export default interface A { a: string; } +export default function() { return 1; } +export default function() { return 2; } + + +//// [exportDefaultInterfaceAndTwoFunctions.js] +"use strict"; +exports.__esModule = true; +function default_1() { return 1; } +exports["default"] = default_1; +function default_2() { return 2; } +exports["default"] = default_2; diff --git a/tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.symbols b/tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.symbols new file mode 100644 index 00000000000..1f43e7bfcbd --- /dev/null +++ b/tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.symbols @@ -0,0 +1,8 @@ +=== tests/cases/compiler/exportDefaultInterfaceAndTwoFunctions.ts === +export default interface A { a: string; } +>A : Symbol(A, Decl(exportDefaultInterfaceAndTwoFunctions.ts, 0, 41), Decl(exportDefaultInterfaceAndTwoFunctions.ts, 1, 39), Decl(exportDefaultInterfaceAndTwoFunctions.ts, 0, 0)) +>a : Symbol(A.a, Decl(exportDefaultInterfaceAndTwoFunctions.ts, 0, 28)) + +export default function() { return 1; } +export default function() { return 2; } + diff --git a/tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.types b/tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.types new file mode 100644 index 00000000000..4d2d15abefa --- /dev/null +++ b/tests/baselines/reference/exportDefaultInterfaceAndTwoFunctions.types @@ -0,0 +1,10 @@ +=== tests/cases/compiler/exportDefaultInterfaceAndTwoFunctions.ts === +export default interface A { a: string; } +>a : string + +export default function() { return 1; } +>1 : 1 + +export default function() { return 2; } +>2 : 2 + diff --git a/tests/cases/compiler/exportDefaultInterfaceAndTwoFunctions.ts b/tests/cases/compiler/exportDefaultInterfaceAndTwoFunctions.ts new file mode 100644 index 00000000000..65841d28141 --- /dev/null +++ b/tests/cases/compiler/exportDefaultInterfaceAndTwoFunctions.ts @@ -0,0 +1,3 @@ +export default interface A { a: string; } +export default function() { return 1; } +export default function() { return 2; }