From f6303652d2939480db836464f8657dc08ecb6ba7 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Fri, 7 May 2021 08:09:38 -0700 Subject: [PATCH] Improve errors for incorrectly nested export default (#43967) * Improve errors for incorrectly nested export default The compiler and services don't handle incorrectly nested `export default` well right now: ```ts export = (x,y) => { export default { } } ``` Asking for document highlights, find all references or quick info on 'export' or 'default' cause a crash. After the crash is fixed, the error message is confusing and wrong: "An export assignment cannot be used outside a module." This PR: 1. Skips document highlights for incorrectly nested export default. 2. Skips find all refs for incorrectly nested export default. 3. Switches the fallback binding for incorrectly nested export default from Alias to Property. Neither is correct, but Property doesn't cause a crash in alias resolution. 4. Improves the error message to reflect a post-ES module world, which has export default and 'module' means 'ES module', not 'namespace'. Fixes #40082 and the related bugs mentioned above. * address PR comments --- src/compiler/binder.ts | 4 +- src/compiler/checker.ts | 5 +- src/compiler/diagnosticMessages.json | 6 +- src/services/findAllReferences.ts | 4 +- src/services/importTracker.ts | 20 +- src/services/symbolDisplay.ts | 2 +- .../moduleElementsInWrongContext.errors.txt | 8 +- .../moduleElementsInWrongContext2.errors.txt | 8 +- .../moduleElementsInWrongContext3.errors.txt | 8 +- ...nfoNestedExportEqualExportDefault.baseline | 334 ++++++++++++++++++ .../fourslash/documentHighlights_40082.ts | 11 + ...quickInfoNestedExportEqualExportDefault.ts | 6 + 12 files changed, 387 insertions(+), 29 deletions(-) create mode 100644 tests/baselines/reference/quickInfoNestedExportEqualExportDefault.baseline create mode 100644 tests/cases/fourslash/documentHighlights_40082.ts create mode 100644 tests/cases/fourslash/quickInfoNestedExportEqualExportDefault.ts diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index d42035879e4..4e850f4eea9 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -2768,8 +2768,8 @@ namespace ts { function bindExportAssignment(node: ExportAssignment) { if (!container.symbol || !container.symbol.exports) { - // Export assignment in some sort of block construct - bindAnonymousDeclaration(node, SymbolFlags.Alias, getDeclarationName(node)!); + // Incorrect export assignment in some sort of block construct + bindAnonymousDeclaration(node, SymbolFlags.Value, getDeclarationName(node)!); } else { const flags = exportAssignmentIsAlias(node) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 026a9d7bc90..a5fe578757d 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -38315,7 +38315,10 @@ namespace ts { } function checkExportAssignment(node: ExportAssignment) { - if (checkGrammarModuleElementContext(node, Diagnostics.An_export_assignment_can_only_be_used_in_a_module)) { + const illegalContextMessage = node.isExportEquals + ? Diagnostics.An_export_assignment_must_be_at_the_top_level_of_a_file_or_module_declaration + : Diagnostics.A_default_export_must_be_at_the_top_level_of_a_file_or_module_declaration; + if (checkGrammarModuleElementContext(node, illegalContextMessage)) { // If we hit an export assignment in an illegal context, just bail out to avoid cascading errors. return; } diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index f4fbe932e93..155e176eaa7 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -743,7 +743,7 @@ "category": "Error", "code": 1230 }, - "An export assignment can only be used in a module.": { + "An export assignment must be at the top level of a file or module declaration.": { "category": "Error", "code": 1231 }, @@ -847,6 +847,10 @@ "category": "Error", "code": 1257 }, + "A default export must be at the top level of a file or module declaration.": { + "category": "Error", + "code": 1258 + }, "Module '{0}' can only be default-imported using the '{1}' flag": { "category": "Error", "code": 1259 diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index a1269738435..fa7dfa188e8 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -921,9 +921,9 @@ namespace ts.FindAllReferences { // When renaming at an export specifier, rename the export and not the thing being exported. getReferencesAtExportSpecifier(exportSpecifier.name, symbol, exportSpecifier, state.createSearch(node, originalSymbol, /*comingFrom*/ undefined), state, /*addReferencesHere*/ true, /*alwaysGetReferences*/ true); } - else if (node && node.kind === SyntaxKind.DefaultKeyword && symbol.escapedName === InternalSymbolName.Default) { + else if (node && node.kind === SyntaxKind.DefaultKeyword && symbol.escapedName === InternalSymbolName.Default && symbol.parent) { addReference(node, symbol, state); - searchForImportsOfExport(node, symbol, { exportingModuleSymbol: Debug.checkDefined(symbol.parent, "Expected export symbol to have a parent"), exportKind: ExportKind.Default }, state); + searchForImportsOfExport(node, symbol, { exportingModuleSymbol: symbol.parent, exportKind: ExportKind.Default }, state); } else { const search = state.createSearch(node, symbol, /*comingFrom*/ undefined, { allSearchSymbols: node ? populateSearchSymbolSet(symbol, node, checker, options.use === FindReferencesUse.Rename, !!options.providePrefixAndSuffixTextForRename, !!options.implementations) : [symbol] }); diff --git a/src/services/importTracker.ts b/src/services/importTracker.ts index e93b99ef63b..d3c95854f9b 100644 --- a/src/services/importTracker.ts +++ b/src/services/importTracker.ts @@ -465,13 +465,13 @@ namespace ts.FindAllReferences { function getExport(): ExportedSymbol | ImportedSymbol | undefined { const { parent } = node; - const grandParent = parent.parent; + const grandparent = parent.parent; if (symbol.exportSymbol) { if (parent.kind === SyntaxKind.PropertyAccessExpression) { // When accessing an export of a JS module, there's no alias. The symbol will still be flagged as an export even though we're at the use. // So check that we are at the declaration. - return symbol.declarations?.some(d => d === parent) && isBinaryExpression(grandParent) - ? getSpecialPropertyExport(grandParent, /*useLhsSymbol*/ false) + return symbol.declarations?.some(d => d === parent) && isBinaryExpression(grandparent) + ? getSpecialPropertyExport(grandparent, /*useLhsSymbol*/ false) : undefined; } else { @@ -502,26 +502,26 @@ namespace ts.FindAllReferences { return getExportAssignmentExport(parent); } // If we are in `export = class A {};` (or `export = class A {};`) at `A`, `parent.parent` is the export assignment. - else if (isExportAssignment(grandParent)) { - return getExportAssignmentExport(grandParent); + else if (isExportAssignment(grandparent)) { + return getExportAssignmentExport(grandparent); } // Similar for `module.exports =` and `exports.A =`. else if (isBinaryExpression(parent)) { return getSpecialPropertyExport(parent, /*useLhsSymbol*/ true); } - else if (isBinaryExpression(grandParent)) { - return getSpecialPropertyExport(grandParent, /*useLhsSymbol*/ true); + else if (isBinaryExpression(grandparent)) { + return getSpecialPropertyExport(grandparent, /*useLhsSymbol*/ true); } else if (isJSDocTypedefTag(parent)) { return exportInfo(symbol, ExportKind.Named); } } - function getExportAssignmentExport(ex: ExportAssignment): ExportedSymbol { + function getExportAssignmentExport(ex: ExportAssignment): ExportedSymbol | undefined { // Get the symbol for the `export =` node; its parent is the module it's the export of. - const exportingModuleSymbol = Debug.checkDefined(ex.symbol.parent, "Expected export symbol to have a parent"); + if (!ex.symbol.parent) return undefined; const exportKind = ex.isExportEquals ? ExportKind.ExportEquals : ExportKind.Default; - return { kind: ImportExport.Export, symbol, exportInfo: { exportingModuleSymbol, exportKind } }; + return { kind: ImportExport.Export, symbol, exportInfo: { exportingModuleSymbol: ex.symbol.parent, exportKind } }; } function getSpecialPropertyExport(node: BinaryExpression, useLhsSymbol: boolean): ExportedSymbol | undefined { diff --git a/src/services/symbolDisplay.ts b/src/services/symbolDisplay.ts index fc99fd745d2..52085ef72c5 100644 --- a/src/services/symbolDisplay.ts +++ b/src/services/symbolDisplay.ts @@ -174,7 +174,7 @@ namespace ts.SymbolDisplay { } let signature: Signature | undefined; - type = isThisExpression ? typeChecker.getTypeAtLocation(location) : typeChecker.getTypeOfSymbolAtLocation(symbol.exportSymbol || symbol, location); + type = isThisExpression ? typeChecker.getTypeAtLocation(location) : typeChecker.getTypeOfSymbolAtLocation(symbol, location); if (location.parent && location.parent.kind === SyntaxKind.PropertyAccessExpression) { const right = (location.parent).name; diff --git a/tests/baselines/reference/moduleElementsInWrongContext.errors.txt b/tests/baselines/reference/moduleElementsInWrongContext.errors.txt index 0ab411e03c4..146a6dcd951 100644 --- a/tests/baselines/reference/moduleElementsInWrongContext.errors.txt +++ b/tests/baselines/reference/moduleElementsInWrongContext.errors.txt @@ -2,11 +2,11 @@ tests/cases/compiler/moduleElementsInWrongContext.ts(2,5): error TS1235: A names tests/cases/compiler/moduleElementsInWrongContext.ts(3,5): error TS1235: A namespace declaration is only allowed in a namespace or module. tests/cases/compiler/moduleElementsInWrongContext.ts(7,5): error TS1235: A namespace declaration is only allowed in a namespace or module. tests/cases/compiler/moduleElementsInWrongContext.ts(9,5): error TS1234: An ambient module declaration is only allowed at the top level in a file. -tests/cases/compiler/moduleElementsInWrongContext.ts(13,5): error TS1231: An export assignment can only be used in a module. +tests/cases/compiler/moduleElementsInWrongContext.ts(13,5): error TS1231: An export assignment must be at the top level of a file or module declaration. tests/cases/compiler/moduleElementsInWrongContext.ts(17,5): error TS1233: An export declaration can only be used in a module. tests/cases/compiler/moduleElementsInWrongContext.ts(18,5): error TS1233: An export declaration can only be used in a module. tests/cases/compiler/moduleElementsInWrongContext.ts(19,5): error TS1233: An export declaration can only be used in a module. -tests/cases/compiler/moduleElementsInWrongContext.ts(20,5): error TS1231: An export assignment can only be used in a module. +tests/cases/compiler/moduleElementsInWrongContext.ts(20,5): error TS1258: A default export must be at the top level of a file or module declaration. tests/cases/compiler/moduleElementsInWrongContext.ts(21,5): error TS1184: Modifiers cannot appear here. tests/cases/compiler/moduleElementsInWrongContext.ts(22,5): error TS1184: Modifiers cannot appear here. tests/cases/compiler/moduleElementsInWrongContext.ts(23,5): error TS1232: An import declaration can only be used in a namespace or module. @@ -40,7 +40,7 @@ tests/cases/compiler/moduleElementsInWrongContext.ts(28,5): error TS1232: An imp export = M; ~~~~~~ -!!! error TS1231: An export assignment can only be used in a module. +!!! error TS1231: An export assignment must be at the top level of a file or module declaration. var v; function foo() { } @@ -55,7 +55,7 @@ tests/cases/compiler/moduleElementsInWrongContext.ts(28,5): error TS1232: An imp !!! error TS1233: An export declaration can only be used in a module. export default v; ~~~~~~ -!!! error TS1231: An export assignment can only be used in a module. +!!! error TS1258: A default export must be at the top level of a file or module declaration. export default class C { } ~~~~~~ !!! error TS1184: Modifiers cannot appear here. diff --git a/tests/baselines/reference/moduleElementsInWrongContext2.errors.txt b/tests/baselines/reference/moduleElementsInWrongContext2.errors.txt index 223f14684fe..f858bf9923e 100644 --- a/tests/baselines/reference/moduleElementsInWrongContext2.errors.txt +++ b/tests/baselines/reference/moduleElementsInWrongContext2.errors.txt @@ -2,11 +2,11 @@ tests/cases/compiler/moduleElementsInWrongContext2.ts(2,5): error TS1235: A name tests/cases/compiler/moduleElementsInWrongContext2.ts(3,5): error TS1235: A namespace declaration is only allowed in a namespace or module. tests/cases/compiler/moduleElementsInWrongContext2.ts(7,5): error TS1235: A namespace declaration is only allowed in a namespace or module. tests/cases/compiler/moduleElementsInWrongContext2.ts(9,5): error TS1234: An ambient module declaration is only allowed at the top level in a file. -tests/cases/compiler/moduleElementsInWrongContext2.ts(13,5): error TS1231: An export assignment can only be used in a module. +tests/cases/compiler/moduleElementsInWrongContext2.ts(13,5): error TS1231: An export assignment must be at the top level of a file or module declaration. tests/cases/compiler/moduleElementsInWrongContext2.ts(17,5): error TS1233: An export declaration can only be used in a module. tests/cases/compiler/moduleElementsInWrongContext2.ts(18,5): error TS1233: An export declaration can only be used in a module. tests/cases/compiler/moduleElementsInWrongContext2.ts(19,5): error TS1233: An export declaration can only be used in a module. -tests/cases/compiler/moduleElementsInWrongContext2.ts(20,5): error TS1231: An export assignment can only be used in a module. +tests/cases/compiler/moduleElementsInWrongContext2.ts(20,5): error TS1258: A default export must be at the top level of a file or module declaration. tests/cases/compiler/moduleElementsInWrongContext2.ts(21,5): error TS1184: Modifiers cannot appear here. tests/cases/compiler/moduleElementsInWrongContext2.ts(22,5): error TS1184: Modifiers cannot appear here. tests/cases/compiler/moduleElementsInWrongContext2.ts(23,5): error TS1232: An import declaration can only be used in a namespace or module. @@ -40,7 +40,7 @@ tests/cases/compiler/moduleElementsInWrongContext2.ts(28,5): error TS1232: An im export = M; ~~~~~~ -!!! error TS1231: An export assignment can only be used in a module. +!!! error TS1231: An export assignment must be at the top level of a file or module declaration. var v; function foo() { } @@ -55,7 +55,7 @@ tests/cases/compiler/moduleElementsInWrongContext2.ts(28,5): error TS1232: An im !!! error TS1233: An export declaration can only be used in a module. export default v; ~~~~~~ -!!! error TS1231: An export assignment can only be used in a module. +!!! error TS1258: A default export must be at the top level of a file or module declaration. export default class C { } ~~~~~~ !!! error TS1184: Modifiers cannot appear here. diff --git a/tests/baselines/reference/moduleElementsInWrongContext3.errors.txt b/tests/baselines/reference/moduleElementsInWrongContext3.errors.txt index 5d4b190bb65..df78fa08741 100644 --- a/tests/baselines/reference/moduleElementsInWrongContext3.errors.txt +++ b/tests/baselines/reference/moduleElementsInWrongContext3.errors.txt @@ -2,11 +2,11 @@ tests/cases/compiler/moduleElementsInWrongContext3.ts(3,9): error TS1235: A name tests/cases/compiler/moduleElementsInWrongContext3.ts(4,9): error TS1235: A namespace declaration is only allowed in a namespace or module. tests/cases/compiler/moduleElementsInWrongContext3.ts(8,9): error TS1235: A namespace declaration is only allowed in a namespace or module. tests/cases/compiler/moduleElementsInWrongContext3.ts(10,9): error TS1234: An ambient module declaration is only allowed at the top level in a file. -tests/cases/compiler/moduleElementsInWrongContext3.ts(14,9): error TS1231: An export assignment can only be used in a module. +tests/cases/compiler/moduleElementsInWrongContext3.ts(14,9): error TS1231: An export assignment must be at the top level of a file or module declaration. tests/cases/compiler/moduleElementsInWrongContext3.ts(18,9): error TS1233: An export declaration can only be used in a module. tests/cases/compiler/moduleElementsInWrongContext3.ts(19,9): error TS1233: An export declaration can only be used in a module. tests/cases/compiler/moduleElementsInWrongContext3.ts(20,9): error TS1233: An export declaration can only be used in a module. -tests/cases/compiler/moduleElementsInWrongContext3.ts(21,9): error TS1231: An export assignment can only be used in a module. +tests/cases/compiler/moduleElementsInWrongContext3.ts(21,9): error TS1258: A default export must be at the top level of a file or module declaration. tests/cases/compiler/moduleElementsInWrongContext3.ts(22,9): error TS1184: Modifiers cannot appear here. tests/cases/compiler/moduleElementsInWrongContext3.ts(23,9): error TS1184: Modifiers cannot appear here. tests/cases/compiler/moduleElementsInWrongContext3.ts(24,9): error TS1232: An import declaration can only be used in a namespace or module. @@ -41,7 +41,7 @@ tests/cases/compiler/moduleElementsInWrongContext3.ts(29,9): error TS1232: An im export = M; ~~~~~~ -!!! error TS1231: An export assignment can only be used in a module. +!!! error TS1231: An export assignment must be at the top level of a file or module declaration. var v; function foo() { } @@ -56,7 +56,7 @@ tests/cases/compiler/moduleElementsInWrongContext3.ts(29,9): error TS1232: An im !!! error TS1233: An export declaration can only be used in a module. export default v; ~~~~~~ -!!! error TS1231: An export assignment can only be used in a module. +!!! error TS1258: A default export must be at the top level of a file or module declaration. export default class C { } ~~~~~~ !!! error TS1184: Modifiers cannot appear here. diff --git a/tests/baselines/reference/quickInfoNestedExportEqualExportDefault.baseline b/tests/baselines/reference/quickInfoNestedExportEqualExportDefault.baseline new file mode 100644 index 00000000000..159b8b81519 --- /dev/null +++ b/tests/baselines/reference/quickInfoNestedExportEqualExportDefault.baseline @@ -0,0 +1,334 @@ +[ + { + "marker": { + "fileName": "/tests/cases/fourslash/quickInfoNestedExportEqualExportDefault.ts", + "position": 41, + "name": "1" + }, + "quickInfo": { + "kind": "enum member", + "kindModifiers": "export", + "textSpan": { + "start": 35, + "length": 6 + }, + "displayParts": [ + { + "text": "class", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "(Anonymous function)", + "kind": "functionName" + }, + { + "text": ".", + "kind": "punctuation" + }, + { + "text": "default", + "kind": "localName" + }, + { + "text": "\n", + "kind": "lineBreak" + }, + { + "text": "enum", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "(Anonymous function)", + "kind": "functionName" + }, + { + "text": ".", + "kind": "punctuation" + }, + { + "text": "default", + "kind": "localName" + }, + { + "text": "\n", + "kind": "lineBreak" + }, + { + "text": "module", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "(Anonymous function)", + "kind": "functionName" + }, + { + "text": ".", + "kind": "punctuation" + }, + { + "text": "default", + "kind": "localName" + }, + { + "text": "\n", + "kind": "lineBreak" + }, + { + "text": "(", + "kind": "punctuation" + }, + { + "text": "enum member", + "kind": "text" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "(Anonymous function)", + "kind": "functionName" + }, + { + "text": ".", + "kind": "punctuation" + }, + { + "text": "default", + "kind": "localName" + }, + { + "text": "\n", + "kind": "lineBreak" + }, + { + "text": "(", + "kind": "punctuation" + }, + { + "text": "enum member", + "kind": "text" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "(Anonymous function)", + "kind": "functionName" + }, + { + "text": ".", + "kind": "punctuation" + }, + { + "text": "default", + "kind": "localName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "{", + "kind": "punctuation" + }, + { + "text": "}", + "kind": "punctuation" + } + ], + "documentation": [] + } + }, + { + "marker": { + "fileName": "/tests/cases/fourslash/quickInfoNestedExportEqualExportDefault.ts", + "position": 49, + "name": "2" + }, + "quickInfo": { + "kind": "enum member", + "kindModifiers": "export", + "textSpan": { + "start": 42, + "length": 7 + }, + "displayParts": [ + { + "text": "class", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "(Anonymous function)", + "kind": "functionName" + }, + { + "text": ".", + "kind": "punctuation" + }, + { + "text": "default", + "kind": "localName" + }, + { + "text": "\n", + "kind": "lineBreak" + }, + { + "text": "enum", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "(Anonymous function)", + "kind": "functionName" + }, + { + "text": ".", + "kind": "punctuation" + }, + { + "text": "default", + "kind": "localName" + }, + { + "text": "\n", + "kind": "lineBreak" + }, + { + "text": "module", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "(Anonymous function)", + "kind": "functionName" + }, + { + "text": ".", + "kind": "punctuation" + }, + { + "text": "default", + "kind": "localName" + }, + { + "text": "\n", + "kind": "lineBreak" + }, + { + "text": "(", + "kind": "punctuation" + }, + { + "text": "enum member", + "kind": "text" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "(Anonymous function)", + "kind": "functionName" + }, + { + "text": ".", + "kind": "punctuation" + }, + { + "text": "default", + "kind": "localName" + }, + { + "text": "\n", + "kind": "lineBreak" + }, + { + "text": "(", + "kind": "punctuation" + }, + { + "text": "enum member", + "kind": "text" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "(Anonymous function)", + "kind": "functionName" + }, + { + "text": ".", + "kind": "punctuation" + }, + { + "text": "default", + "kind": "localName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "{", + "kind": "punctuation" + }, + { + "text": "}", + "kind": "punctuation" + } + ], + "documentation": [] + } + } +] \ No newline at end of file diff --git a/tests/cases/fourslash/documentHighlights_40082.ts b/tests/cases/fourslash/documentHighlights_40082.ts new file mode 100644 index 00000000000..2d9e44c2226 --- /dev/null +++ b/tests/cases/fourslash/documentHighlights_40082.ts @@ -0,0 +1,11 @@ +/// +// #40082 + +// @checkJs: true +//// export = (state, messages) => { +//// export [|default|] { +//// } +//// } + +const [r] = test.ranges(); +verify.documentHighlightsOf(r, [r]); diff --git a/tests/cases/fourslash/quickInfoNestedExportEqualExportDefault.ts b/tests/cases/fourslash/quickInfoNestedExportEqualExportDefault.ts new file mode 100644 index 00000000000..4f19ebe8dfe --- /dev/null +++ b/tests/cases/fourslash/quickInfoNestedExportEqualExportDefault.ts @@ -0,0 +1,6 @@ +/// +//// export = (state, messages) => { +//// export/*1*/ default/*2*/ { +//// } +//// } +verify.baselineQuickInfo()