Improve errors for incorrectly nested export default (#43967)

sandersn · web-flow · commit f6303652d293 · 2021-05-07T08:09:38.000-07:00
* 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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 = (<PropertyAccessExpression>location.parent).name;
diff --git 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
@@ -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
@@ -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
diff --git a/tests/cases/fourslash/documentHighlights_40082.ts b/tests/cases/fourslash/documentHighlights_40082.ts
diff --git a/tests/cases/fourslash/quickInfoNestedExportEqualExportDefault.ts b/tests/cases/fourslash/quickInfoNestedExportEqualExportDefault.ts

Original file line number	Diff line number	Diff line change
`@@ -38315,7 +38315,10 @@ namespace ts {`
`38315`	`38315`	`}`
`38316`	`38316`
`38317`	`38317`	`function checkExportAssignment(node: ExportAssignment) {`
`38318`		`- if (checkGrammarModuleElementContext(node, Diagnostics.An_export_assignment_can_only_be_used_in_a_module)) {`
	`38318`	`+ const illegalContextMessage = node.isExportEquals`
	`38319`	`+ ? Diagnostics.An_export_assignment_must_be_at_the_top_level_of_a_file_or_module_declaration`
	`38320`	`+ : Diagnostics.A_default_export_must_be_at_the_top_level_of_a_file_or_module_declaration;`
	`38321`	`+ if (checkGrammarModuleElementContext(node, illegalContextMessage)) {`
`38319`	`38322`	`// If we hit an export assignment in an illegal context, just bail out to avoid cascading errors.`
`38320`	`38323`	`return;`
`38321`	`38324`	`}`
Original file line number	Diff line number	Diff line change
`@@ -921,9 +921,9 @@ namespace ts.FindAllReferences {`
`921`	`921`	`// When renaming at an export specifier, rename the export and not the thing being exported.`
`922`	`922`	`getReferencesAtExportSpecifier(exportSpecifier.name, symbol, exportSpecifier, state.createSearch(node, originalSymbol, /comingFrom/ undefined), state, /addReferencesHere/ true, /alwaysGetReferences/ true);`
`923`	`923`	`}`
`924`		`- else if (node && node.kind === SyntaxKind.DefaultKeyword && symbol.escapedName === InternalSymbolName.Default) {`
	`924`	`+ else if (node && node.kind === SyntaxKind.DefaultKeyword && symbol.escapedName === InternalSymbolName.Default && symbol.parent) {`
`925`	`925`	`addReference(node, symbol, state);`
`926`		`- searchForImportsOfExport(node, symbol, { exportingModuleSymbol: Debug.checkDefined(symbol.parent, "Expected export symbol to have a parent"), exportKind: ExportKind.Default }, state);`
	`926`	`+ searchForImportsOfExport(node, symbol, { exportingModuleSymbol: symbol.parent, exportKind: ExportKind.Default }, state);`
`927`	`927`	`}`
`928`	`928`	`else {`
`929`	`929`	`const search = state.createSearch(node, symbol, /comingFrom/ undefined, { allSearchSymbols: node ? populateSearchSymbolSet(symbol, node, checker, options.use === FindReferencesUse.Rename, !!options.providePrefixAndSuffixTextForRename, !!options.implementations) : [symbol] });`
Original file line number	Diff line number	Diff line change
`@@ -174,7 +174,7 @@ namespace ts.SymbolDisplay {`
`174`	`174`	`}`
`175`	`175`
`176`	`176`	`let signature: Signature \| undefined;`
`177`		`- type = isThisExpression ? typeChecker.getTypeAtLocation(location) : typeChecker.getTypeOfSymbolAtLocation(symbol.exportSymbol \|\| symbol, location);`
	`177`	`+ type = isThisExpression ? typeChecker.getTypeAtLocation(location) : typeChecker.getTypeOfSymbolAtLocation(symbol, location);`
`178`	`178`
`179`	`179`	`if (location.parent && location.parent.kind === SyntaxKind.PropertyAccessExpression) {`
`180`	`180`	`const right = (<PropertyAccessExpression>location.parent).name;`