Skip to content

Commit

Permalink
Grammar error on export type * (#37064)
Browse files Browse the repository at this point in the history
* Recognize `export type *` syntax, but disallow it

* Add more comments to test

* Revert recognizing invalid forms as type-only

* Revert more
  • Loading branch information
andrewbranch authored Feb 28, 2020
1 parent f1457c1 commit 0a6ee77
Show file tree
Hide file tree
Showing 8 changed files with 164 additions and 4 deletions.
20 changes: 16 additions & 4 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1936,10 +1936,11 @@ namespace ts {
if (!isValidTypeOnlyAliasUseSite(useSite)) {
const typeOnlyDeclaration = getTypeOnlyAliasDeclaration(symbol);
if (typeOnlyDeclaration) {
const message = typeOnlyDeclaration.kind === SyntaxKind.ExportSpecifier
const isExport = typeOnlyDeclarationIsExport(typeOnlyDeclaration);
const message = isExport
? Diagnostics._0_cannot_be_used_as_a_value_because_it_was_exported_using_export_type
: Diagnostics._0_cannot_be_used_as_a_value_because_it_was_imported_using_import_type;
const relatedMessage = typeOnlyDeclaration.kind === SyntaxKind.ExportSpecifier
const relatedMessage = isExport
? Diagnostics._0_was_exported_here
: Diagnostics._0_was_imported_here;
const unescapedName = unescapeLeadingUnderscores(name);
Expand Down Expand Up @@ -2286,12 +2287,14 @@ namespace ts {
function checkAndReportErrorForResolvingImportAliasToTypeOnlySymbol(node: ImportEqualsDeclaration, resolved: Symbol | undefined) {
if (markSymbolOfAliasDeclarationIfTypeOnly(node, /*immediateTarget*/ undefined, resolved, /*overwriteEmpty*/ false)) {
const typeOnlyDeclaration = getTypeOnlyAliasDeclaration(getSymbolOfNode(node))!;
const message = typeOnlyDeclaration.kind === SyntaxKind.ExportSpecifier
const isExport = typeOnlyDeclarationIsExport(typeOnlyDeclaration);
const message = isExport
? Diagnostics.An_import_alias_cannot_reference_a_declaration_that_was_exported_using_export_type
: Diagnostics.An_import_alias_cannot_reference_a_declaration_that_was_imported_using_import_type;
const relatedMessage = typeOnlyDeclaration.kind === SyntaxKind.ExportSpecifier
const relatedMessage = isExport
? Diagnostics._0_was_exported_here
: Diagnostics._0_was_imported_here;

// Non-null assertion is safe because the optionality comes from ImportClause,
// but if an ImportClause was the typeOnlyDeclaration, it had to have a `name`.
const name = unescapeLeadingUnderscores(typeOnlyDeclaration.name!.escapedText);
Expand Down Expand Up @@ -33598,6 +33601,7 @@ namespace ts {
checkExternalEmitHelpers(node, ExternalEmitHelpers.CreateBinding);
}

checkGrammarExportDeclaration(node);
if (!node.moduleSpecifier || checkExternalImportOrExportDeclaration(node)) {
if (node.exportClause) {
// export { x, y }
Expand Down Expand Up @@ -33630,6 +33634,14 @@ namespace ts {
}
}

function checkGrammarExportDeclaration(node: ExportDeclaration): boolean {
const isTypeOnlyExportStar = node.isTypeOnly && node.exportClause?.kind !== SyntaxKind.NamedExports;
if (isTypeOnlyExportStar) {
grammarErrorOnNode(node, Diagnostics.Only_named_exports_may_use_export_type);
}
return !isTypeOnlyExportStar;
}

function checkGrammarModuleElementContext(node: Statement, errorMessage: DiagnosticMessage): boolean {
const isInAppropriateContext = node.parent.kind === SyntaxKind.SourceFile || node.parent.kind === SyntaxKind.ModuleBlock || node.parent.kind === SyntaxKind.ModuleDeclaration;
if (!isInAppropriateContext) {
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,10 @@
"category": "Error",
"code": 1382
},
"Only named exports may use 'export type'.": {
"category": "Error",
"code": 1383
},

"The types of '{0}' are incompatible between these types.": {
"category": "Error",
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6220,6 +6220,10 @@ namespace ts {
|| !isExpressionNode(useSite);
}

export function typeOnlyDeclarationIsExport(typeOnlyDeclaration: Node) {
return typeOnlyDeclaration.kind === SyntaxKind.ExportSpecifier;
}

function isPartOfPossiblyValidTypeOrAbstractComputedPropertyName(node: Node) {
while (node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.PropertyAccessExpression) {
node = node.parent;
Expand Down
25 changes: 25 additions & 0 deletions tests/baselines/reference/exportNamespace4.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
tests/cases/conformance/externalModules/typeOnly/b.ts(1,1): error TS1383: Only named exports may use 'export type'.
tests/cases/conformance/externalModules/typeOnly/c.ts(1,1): error TS1383: Only named exports may use 'export type'.


==== tests/cases/conformance/externalModules/typeOnly/a.ts (0 errors) ====
export class A {}

==== tests/cases/conformance/externalModules/typeOnly/b.ts (1 errors) ====
export type * from './a'; // Grammar error
~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1383: Only named exports may use 'export type'.

==== tests/cases/conformance/externalModules/typeOnly/c.ts (1 errors) ====
export type * as ns from './a'; // Grammar error
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1383: Only named exports may use 'export type'.

==== tests/cases/conformance/externalModules/typeOnly/d.ts (0 errors) ====
import { A } from './b';
A;

==== tests/cases/conformance/externalModules/typeOnly/e.ts (0 errors) ====
import { ns } from './c';
ns.A;

45 changes: 45 additions & 0 deletions tests/baselines/reference/exportNamespace4.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
//// [tests/cases/conformance/externalModules/typeOnly/exportNamespace4.ts] ////

//// [a.ts]
export class A {}

//// [b.ts]
export type * from './a'; // Grammar error

//// [c.ts]
export type * as ns from './a'; // Grammar error

//// [d.ts]
import { A } from './b';
A;

//// [e.ts]
import { ns } from './c';
ns.A;


//// [a.js]
"use strict";
exports.__esModule = true;
var A = /** @class */ (function () {
function A() {
}
return A;
}());
exports.A = A;
//// [b.js]
"use strict";
exports.__esModule = true;
//// [c.js]
"use strict";
exports.__esModule = true;
//// [d.js]
"use strict";
exports.__esModule = true;
var b_1 = require("./b");
b_1.A;
//// [e.js]
"use strict";
exports.__esModule = true;
var c_1 = require("./c");
c_1.ns.A;
27 changes: 27 additions & 0 deletions tests/baselines/reference/exportNamespace4.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
=== tests/cases/conformance/externalModules/typeOnly/a.ts ===
export class A {}
>A : Symbol(A, Decl(a.ts, 0, 0))

=== tests/cases/conformance/externalModules/typeOnly/b.ts ===
export type * from './a'; // Grammar error
No type information for this code.
No type information for this code.=== tests/cases/conformance/externalModules/typeOnly/c.ts ===
export type * as ns from './a'; // Grammar error
>ns : Symbol(ns, Decl(c.ts, 0, 11))

=== tests/cases/conformance/externalModules/typeOnly/d.ts ===
import { A } from './b';
>A : Symbol(A, Decl(d.ts, 0, 8))

A;
>A : Symbol(A, Decl(d.ts, 0, 8))

=== tests/cases/conformance/externalModules/typeOnly/e.ts ===
import { ns } from './c';
>ns : Symbol(ns, Decl(e.ts, 0, 8))

ns.A;
>ns.A : Symbol(ns.A, Decl(a.ts, 0, 0))
>ns : Symbol(ns, Decl(e.ts, 0, 8))
>A : Symbol(ns.A, Decl(a.ts, 0, 0))

27 changes: 27 additions & 0 deletions tests/baselines/reference/exportNamespace4.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
=== tests/cases/conformance/externalModules/typeOnly/a.ts ===
export class A {}
>A : A

=== tests/cases/conformance/externalModules/typeOnly/b.ts ===
export type * from './a'; // Grammar error
No type information for this code.
No type information for this code.=== tests/cases/conformance/externalModules/typeOnly/c.ts ===
export type * as ns from './a'; // Grammar error
>ns : typeof ns

=== tests/cases/conformance/externalModules/typeOnly/d.ts ===
import { A } from './b';
>A : typeof A

A;
>A : typeof A

=== tests/cases/conformance/externalModules/typeOnly/e.ts ===
import { ns } from './c';
>ns : typeof ns

ns.A;
>ns.A : typeof ns.A
>ns : typeof ns
>A : typeof ns.A

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// @Filename: a.ts
export class A {}

// @Filename: b.ts
export type * from './a'; // Grammar error

// @Filename: c.ts
export type * as ns from './a'; // Grammar error

// @Filename: d.ts
import { A } from './b';
A;

// @Filename: e.ts
import { ns } from './c';
ns.A;

0 comments on commit 0a6ee77

Please sign in to comment.