Skip to content

fix(54760): Report error for 'declare type' followed by newline #54761

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.

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 6 commits into from
Jul 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7094,11 +7094,17 @@ namespace Parser {
case SyntaxKind.ProtectedKeyword:
case SyntaxKind.PublicKeyword:
case SyntaxKind.ReadonlyKeyword:
const previousToken = token();
nextToken();
// ASI takes effect for this modifier.
if (scanner.hasPrecedingLineBreak()) {
return false;
}
if (previousToken === SyntaxKind.DeclareKeyword && token() === SyntaxKind.TypeKeyword) {
// If we see 'declare type', then commit to parsing a type alias. parseTypeAliasDeclaration will
// report Line_break_not_permitted_here if needed.
return true;
}
continue;

case SyntaxKind.GlobalKeyword:
Expand Down Expand Up @@ -8092,6 +8098,9 @@ namespace Parser {

function parseTypeAliasDeclaration(pos: number, hasJSDoc: boolean, modifiers: NodeArray<ModifierLike> | undefined): TypeAliasDeclaration {
parseExpected(SyntaxKind.TypeKeyword);
if (scanner.hasPrecedingLineBreak()) {
parseErrorAtCurrentToken(Diagnostics.Line_break_not_permitted_here);
}
const name = parseIdentifier();
const typeParameters = parseTypeParameters();
parseExpected(SyntaxKind.EqualsToken);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
typeAliasDeclareKeywordNewlines.ts(5,1): error TS1142: Line break not permitted here.


==== typeAliasDeclareKeywordNewlines.ts (1 errors) ====
var declare: string, type: number;

// The following is invalid but should declare a type alias named 'T1':
declare type /*unexpected newline*/
T1 = null;
~~
!!! error TS1142: Line break not permitted here.
Comment on lines +8 to +11
Copy link
Contributor Author

@strager strager Jun 24, 2023

Choose a reason for hiding this comment

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

Question for reviewers: I don't like the placement of the diagnostic, but I don't know TypeScript diagnostic conventions. Is here fine or should it point to the type keyword on the previous line?

Copy link
Member

Choose a reason for hiding this comment

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

You can give a specialized diagnostic, but I think this is actually fine as-is.

const t1: T1 = null; // Assert that T1 is the null type.

let T: null;
// The following should use a variable named 'declare', use a variable named
// 'type', and assign to a variable named 'T'.
declare /*ASI*/
type /*ASI*/
T = null;

// The following should use a variable named 'declare' and declare a type alias
// named 'T2':
declare /*ASI*/
type T2 = null;
const t2: T2 = null; // Assert that T2 is the null type.

37 changes: 37 additions & 0 deletions tests/baselines/reference/typeAliasDeclareKeywordNewlines.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//// [tests/cases/compiler/typeAliasDeclareKeywordNewlines.ts] ////

//// [typeAliasDeclareKeywordNewlines.ts]
var declare: string, type: number;

// The following is invalid but should declare a type alias named 'T1':
declare type /*unexpected newline*/
T1 = null;
const t1: T1 = null; // Assert that T1 is the null type.

let T: null;
// The following should use a variable named 'declare', use a variable named
// 'type', and assign to a variable named 'T'.
declare /*ASI*/
type /*ASI*/
T = null;

// The following should use a variable named 'declare' and declare a type alias
// named 'T2':
declare /*ASI*/
type T2 = null;
const t2: T2 = null; // Assert that T2 is the null type.


//// [typeAliasDeclareKeywordNewlines.js]
var declare, type;
var t1 = null; // Assert that T1 is the null type.
var T;
// The following should use a variable named 'declare', use a variable named
// 'type', and assign to a variable named 'T'.
declare; /*ASI*/
type; /*ASI*/
T = null;
// The following should use a variable named 'declare' and declare a type alias
// named 'T2':
declare; /*ASI*/
var t2 = null; // Assert that T2 is the null type.
42 changes: 42 additions & 0 deletions tests/baselines/reference/typeAliasDeclareKeywordNewlines.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
//// [tests/cases/compiler/typeAliasDeclareKeywordNewlines.ts] ////

=== typeAliasDeclareKeywordNewlines.ts ===
var declare: string, type: number;
>declare : Symbol(declare, Decl(typeAliasDeclareKeywordNewlines.ts, 0, 3))
>type : Symbol(type, Decl(typeAliasDeclareKeywordNewlines.ts, 0, 20))

// The following is invalid but should declare a type alias named 'T1':
declare type /*unexpected newline*/
T1 = null;
>T1 : Symbol(T1, Decl(typeAliasDeclareKeywordNewlines.ts, 0, 34))

const t1: T1 = null; // Assert that T1 is the null type.
>t1 : Symbol(t1, Decl(typeAliasDeclareKeywordNewlines.ts, 5, 5))
>T1 : Symbol(T1, Decl(typeAliasDeclareKeywordNewlines.ts, 0, 34))

let T: null;
>T : Symbol(T, Decl(typeAliasDeclareKeywordNewlines.ts, 7, 3))

// The following should use a variable named 'declare', use a variable named
// 'type', and assign to a variable named 'T'.
declare /*ASI*/
>declare : Symbol(declare, Decl(typeAliasDeclareKeywordNewlines.ts, 0, 3))

type /*ASI*/
>type : Symbol(type, Decl(typeAliasDeclareKeywordNewlines.ts, 0, 20))

T = null;
>T : Symbol(T, Decl(typeAliasDeclareKeywordNewlines.ts, 7, 3))

// The following should use a variable named 'declare' and declare a type alias
// named 'T2':
declare /*ASI*/
>declare : Symbol(declare, Decl(typeAliasDeclareKeywordNewlines.ts, 0, 3))

type T2 = null;
>T2 : Symbol(T2, Decl(typeAliasDeclareKeywordNewlines.ts, 16, 7))

const t2: T2 = null; // Assert that T2 is the null type.
>t2 : Symbol(t2, Decl(typeAliasDeclareKeywordNewlines.ts, 18, 5))
>T2 : Symbol(T2, Decl(typeAliasDeclareKeywordNewlines.ts, 16, 7))

41 changes: 41 additions & 0 deletions tests/baselines/reference/typeAliasDeclareKeywordNewlines.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
//// [tests/cases/compiler/typeAliasDeclareKeywordNewlines.ts] ////

=== typeAliasDeclareKeywordNewlines.ts ===
var declare: string, type: number;
>declare : string
>type : number

// The following is invalid but should declare a type alias named 'T1':
declare type /*unexpected newline*/
T1 = null;
>T1 : null

const t1: T1 = null; // Assert that T1 is the null type.
>t1 : null

let T: null;
>T : null

// The following should use a variable named 'declare', use a variable named
// 'type', and assign to a variable named 'T'.
declare /*ASI*/
>declare : string

type /*ASI*/
>type : number

T = null;
>T = null : null
>T : null

// The following should use a variable named 'declare' and declare a type alias
// named 'T2':
declare /*ASI*/
>declare : string

type T2 = null;
>T2 : null

const t2: T2 = null; // Assert that T2 is the null type.
>t2 : null

19 changes: 19 additions & 0 deletions tests/cases/compiler/typeAliasDeclareKeywordNewlines.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
var declare: string, type: number;

// The following is invalid but should declare a type alias named 'T1':
declare type /*unexpected newline*/
T1 = null;
const t1: T1 = null; // Assert that T1 is the null type.

let T: null;
// The following should use a variable named 'declare', use a variable named
// 'type', and assign to a variable named 'T'.
declare /*ASI*/
type /*ASI*/
T = null;

// The following should use a variable named 'declare' and declare a type alias
// named 'T2':
declare /*ASI*/
type T2 = null;
const t2: T2 = null; // Assert that T2 is the null type.