Skip to content

Improve errors for incorrectly nested export default #43967

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 7, 2021
Merged

Improve errors for incorrectly nested export default #43967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
661ec87
Improve errors for incorrectly nested export default
sandersn May 5, 2021
f0f8dfa
address PR comments
sandersn May 7, 2021
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
4 changes: 2 additions & 2 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)!);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in hindsight, creating an alias for an incorrect declaration is extremely likely to fail later.

// Incorrect export assignment in some sort of block construct
bindAnonymousDeclaration(node, SymbolFlags.Value, getDeclarationName(node)!);
}
else {
const flags = exportAssignmentIsAlias(node)
Expand Down
5 changes: 4 additions & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38301,7 +38301,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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is weird because node.isExportEquals looks like the second argument at a glance.

? 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;
}
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/services/findAllReferences.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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] });
Expand Down
20 changes: 10 additions & 10 deletions src/services/importTracker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,13 +465,13 @@ namespace ts.FindAllReferences {

function getExport(): ExportedSymbol | ImportedSymbol | undefined {
const { parent } = node;
const grandParent = parent.parent;
const grandparent = parent.parent;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grandparent is one word, not "grand parent". That's just insulting.

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 {
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/services/symbolDisplay.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ namespace ts.SymbolDisplay {
}

let signature: Signature | undefined;
type = isThisExpression ? typeChecker.getTypeAtLocation(location) : typeChecker.getTypeOfSymbolAtLocation(symbol.exportSymbol || symbol, location);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first line of getTypeOfSymbolAtLocation does symbol = symbol.exportSymbol || symbol, so we don't need to do it here.

type = isThisExpression ? typeChecker.getTypeAtLocation(location) : typeChecker.getTypeOfSymbolAtLocation(symbol, location);

if (location.parent && location.parent.kind === SyntaxKind.PropertyAccessExpression) {
const right = (<PropertyAccessExpression>location.parent).name;
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/moduleElementsInWrongContext.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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() { }
Expand All @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/moduleElementsInWrongContext2.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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() { }
Expand All @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/moduleElementsInWrongContext3.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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() { }
Expand All @@ -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.
Expand Down
Loading