Skip to content

Parser error type identifier #4878

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

Closed
wants to merge 7 commits into from
Closed
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
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2477,5 +2477,13 @@
"A type assertion expression is not allowed in the left-hand side of an exponentiation expression. Consider enclosing the expression in parentheses.": {
"category": "Error",
"code": 17007
},
"Invalid type. To avoid ambiguity, add parentheses: '{0}'": {
"category": "Error",
"code": 17008
},
"Identifier expected, reserved word '{0}' not allowed here.": {
"category": "Error",
"code": 17009
}
}
107 changes: 95 additions & 12 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ namespace ts {
return new (getNodeConstructor(kind))(pos, end);
}

interface ContextualError {
callback?: (node: Node) => void;
count: number;
}

function visitNode<T>(cbNode: (node: Node) => T, node: Node): T {
if (node) {
return cbNode(node);
Expand Down Expand Up @@ -526,6 +531,9 @@ namespace ts {
// attached to the EOF token.
let parseErrorBeforeNextFinishedNode = false;

// For more detailed errors, a contextual error node & callback may be used
let contextualError: ContextualError = { count: 0 };

export function parseSourceFile(fileName: string, _sourceText: string, languageVersion: ScriptTarget, _syntaxCursor: IncrementalParser.SyntaxCursor, setParentNodes?: boolean): SourceFile {
initializeState(fileName, _sourceText, languageVersion, _syntaxCursor);

Expand All @@ -548,6 +556,7 @@ namespace ts {

contextFlags = isJavaScript(fileName) ? ParserContextFlags.JavaScriptFile : ParserContextFlags.None;
parseErrorBeforeNextFinishedNode = false;
contextualError = { count: 0 };

// Initialize and prime the scanner before parsing the source elements.
scanner.setText(sourceText);
Expand Down Expand Up @@ -804,7 +813,10 @@ namespace ts {
function parseErrorAtPosition(start: number, length: number, message: DiagnosticMessage, arg0?: any): void {
// Don't report another error if it would just be at the same position as the last error.
let lastError = lastOrUndefined(parseDiagnostics);
if (!lastError || start !== lastError.start) {
if (contextualError.callback) {
contextualError.count++;
}
else if (!lastError || start !== lastError.start) {
parseDiagnostics.push(createFileDiagnostic(sourceFile, start, length, message, arg0));
}

Expand Down Expand Up @@ -866,6 +878,7 @@ namespace ts {
// descent nature of our parser. However, we still store this here just so we can
// assert that that invariant holds.
let saveContextFlags = contextFlags;
let saveContextErrorCount = contextualError.count;

// If we're only looking ahead, then tell the scanner to only lookahead as well.
// Otherwise, if we're actually speculatively parsing, then tell the scanner to do the
Expand All @@ -882,6 +895,7 @@ namespace ts {
token = saveToken;
parseDiagnostics.length = saveParseDiagnosticsLength;
parseErrorBeforeNextFinishedNode = saveParseErrorBeforeNextFinishedNode;
contextualError.count = saveContextErrorCount;
}

return result;
Expand Down Expand Up @@ -923,6 +937,57 @@ namespace ts {
return token > SyntaxKind.LastReservedWord;
}

// ERRORS
function invalidTypeReferenceOrTypePredicate(node: TypeReferenceNode | TypePredicateNode): void {
if (node.kind === SyntaxKind.TypePredicate) {
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 an after-the-fact grammar check. Can you explain why you couldn't just make this part of a grammar check in checker.ts?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

if ((<TypePredicateNode>node).type.parserContextFlags & ParserContextFlags.ThisNodeHasError) {
parseErrorAtPosition((<TypePredicateNode>node).type.pos, 0, Diagnostics.Type_expected);
}
return;
}
let typeName = (<TypeReferenceNode>node).typeName;
if (typeName.flags & NodeFlags.Missing) {
onInvalidTypeReference(typeName);
}
if ((<QualifiedName>typeName).right && ((<QualifiedName>typeName).right.flags & NodeFlags.Missing)) {
onInvalidTypeQualifiedName();
}
}

function onInvalidTypeReference(node: Identifier | QualifiedName): void {
let needsParentheses = true;
if (isStartOfFunctionType()) {
parseFunctionOrConstructorType(SyntaxKind.FunctionType);
}
else if (token === SyntaxKind.NewKeyword) {
parseFunctionOrConstructorType(SyntaxKind.ConstructorType);
}
else {
needsParentheses = false;
}
if (needsParentheses) {
let hint = "(" + sourceFile.text.substring(node.pos, scanner.getStartPos()).trim() + ")";
parseErrorAtPosition(node.pos, scanner.getStartPos() - node.pos, Diagnostics.Invalid_type_To_avoid_ambiguity_add_parentheses_Colon_0, hint);
}
else {
parseErrorAtCurrentToken(Diagnostics.Type_expected);
}
}

function onInvalidTypeQualifiedName(): void {
if (scanner.isReservedWord()) {
parseErrorAtCurrentToken(Diagnostics.Identifier_expected_reserved_word_0_not_allowed_here, tokenToString(token));
}
else {
if (scanner.hasPrecedingLineBreak()) {
parseErrorAtPosition(scanner.getStartPos(), 0, Diagnostics.Identifier_expected);
}
else {
parseErrorAtCurrentToken(Diagnostics.Identifier_expected);
}
}
}

function parseExpected(kind: SyntaxKind, diagnosticMessage?: DiagnosticMessage, shouldAdvance = true): boolean {
if (token === kind) {
if (shouldAdvance) {
Expand Down Expand Up @@ -1013,7 +1078,20 @@ namespace ts {
parseErrorBeforeNextFinishedNode = false;
node.parserContextFlags |= ParserContextFlags.ThisNodeHasError;
}
return node;
}

function finishNodeContextualError<T extends Node>(node: T, end?: number): T {
finishNode(node, end);

let errorCallback = contextualError.count ? contextualError.callback : undefined;
contextualError.callback = undefined;

// If contextual error, use callback for more detailed error(s)
if (errorCallback) {
errorCallback(node);
contextualError.count = 0;
}
return node;
}

Expand All @@ -1026,6 +1104,7 @@ namespace ts {
}

let result = createNode(kind, scanner.getStartPos());
result.flags |= NodeFlags.Missing;
(<Identifier>result).text = "";
return finishNode(result);
}
Expand Down Expand Up @@ -1814,7 +1893,7 @@ namespace ts {
return entity;
}

function parseRightSideOfDot(allowIdentifierNames: boolean): Identifier {
function parseRightSideOfDot(allowReservedWords: boolean): Identifier {
// Technically a keyword is valid here as all identifiers and keywords are identifier names.
// However, often we'll encounter this in error situations when the identifier or keyword
// is actually starting another valid construct.
Expand Down Expand Up @@ -1844,8 +1923,7 @@ namespace ts {
return <Identifier>createMissingNode(SyntaxKind.Identifier, /*reportAtCurrentToken*/ true, Diagnostics.Identifier_expected);
}
}

return allowIdentifierNames ? parseIdentifierName() : parseIdentifier();
return allowReservedWords ? parseIdentifierName() : parseIdentifier();
}

function parseTemplateExpression(): TemplateExpression {
Expand Down Expand Up @@ -1922,20 +2000,25 @@ namespace ts {
// TYPES

function parseTypeReferenceOrTypePredicate(): TypeReferenceNode | TypePredicateNode {
let typeName = parseEntityName(/*allowReservedWords*/ false, Diagnostics.Type_expected);
contextualError.callback = invalidTypeReferenceOrTypePredicate;

let typeName = parseEntityName(/*allowReservedWords*/ false);
if (typeName.kind === SyntaxKind.Identifier && token === SyntaxKind.IsKeyword && !scanner.hasPrecedingLineBreak()) {
nextToken();
let node = <TypePredicateNode>createNode(SyntaxKind.TypePredicate, typeName.pos);
node.parameterName = <Identifier>typeName;
node.type = parseType();
return finishNode(node);
let predicateNode = <TypePredicateNode>createNode(SyntaxKind.TypePredicate, typeName.pos);
predicateNode.parameterName = <Identifier>typeName;
predicateNode.type = parseType();
return finishNodeContextualError(predicateNode);
}

let node = <TypeReferenceNode>createNode(SyntaxKind.TypeReference, typeName.pos);
node.typeName = typeName;
if (!scanner.hasPrecedingLineBreak() && token === SyntaxKind.LessThanToken) {
node.typeArguments = parseBracketedList(ParsingContext.TypeArguments, parseType, SyntaxKind.LessThanToken, SyntaxKind.GreaterThanToken);
if ((typeName.flags & NodeFlags.Missing) === 0) {
Copy link
Member

Choose a reason for hiding this comment

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

As @CyrusNajmabadi mentioned, we already had a way of checking if something was missing, so we shouldn't introduce another.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

if (token === SyntaxKind.LessThanToken && !scanner.hasPrecedingLineBreak()) {
node.typeArguments = parseBracketedList(ParsingContext.TypeArguments, parseType, SyntaxKind.LessThanToken, SyntaxKind.GreaterThanToken);
}
}
return finishNode(node);
return finishNodeContextualError(node);
}

function parseTypeQuery(): TypeQueryNode {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ namespace ts {
Namespace = 0x00020000, // Namespace declaration
ExportContext = 0x00040000, // Export context (initialized by binding)
ContainsThis = 0x00080000, // Interface contains references to "this"
Missing = 0x00100000, // Node which is invalid/missing
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is what we want.


Modifier = Export | Ambient | Public | Private | Protected | Static | Abstract | Default | Async,
AccessibilityModifier = Public | Private | Protected,
Expand Down
66 changes: 66 additions & 0 deletions tests/baselines/reference/errorParserTypeIdentifier.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
tests/cases/compiler/errorParserTypeIdentifier.ts(3,26): error TS17008: Invalid type. To avoid ambiguity, add parentheses: '(new() => void)'
tests/cases/compiler/errorParserTypeIdentifier.ts(5,16): error TS17008: Invalid type. To avoid ambiguity, add parentheses: '(new () => void)'
tests/cases/compiler/errorParserTypeIdentifier.ts(7,16): error TS17008: Invalid type. To avoid ambiguity, add parentheses: '(new () => void)'
tests/cases/compiler/errorParserTypeIdentifier.ts(8,16): error TS17008: Invalid type. To avoid ambiguity, add parentheses: '(new () => void | number)'
tests/cases/compiler/errorParserTypeIdentifier.ts(9,16): error TS17008: Invalid type. To avoid ambiguity, add parentheses: '(<T>(...) => void | void)'
tests/cases/compiler/errorParserTypeIdentifier.ts(9,24): error TS1003: Identifier expected.
tests/cases/compiler/errorParserTypeIdentifier.ts(12,8): error TS2503: Cannot find namespace 'TypeModule1'.
tests/cases/compiler/errorParserTypeIdentifier.ts(12,20): error TS1003: Identifier expected.
tests/cases/compiler/errorParserTypeIdentifier.ts(17,8): error TS2503: Cannot find namespace 'x'.
tests/cases/compiler/errorParserTypeIdentifier.ts(17,10): error TS17009: Identifier expected, reserved word 'void' not allowed here.
tests/cases/compiler/errorParserTypeIdentifier.ts(17,14): error TS1109: Expression expected.
tests/cases/compiler/errorParserTypeIdentifier.ts(18,13): error TS1110: Type expected.
tests/cases/compiler/errorParserTypeIdentifier.ts(20,1): error TS2304: Cannot find name 'Foo'.
tests/cases/compiler/errorParserTypeIdentifier.ts(20,8): error TS1110: Type expected.


==== tests/cases/compiler/errorParserTypeIdentifier.ts (14 errors) ====

// Union or intersection ambiguity
function test(x: string | new() => void) { }
~~~~~~~~~~~~~~
!!! error TS17008: Invalid type. To avoid ambiguity, add parentheses: '(new() => void)'

let a: string | new () => void;
~~~~~~~~~~~~~~~
!!! error TS17008: Invalid type. To avoid ambiguity, add parentheses: '(new () => void)'
let b: string | (new () => void);
let c: string | new () => void /* not part of error */
~~~~~~~~~~~~~~~
!!! error TS17008: Invalid type. To avoid ambiguity, add parentheses: '(new () => void)'
let d: string | new () => void | number;
~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS17008: Invalid type. To avoid ambiguity, add parentheses: '(new () => void | number)'
let e: string | <T>(...) => void | void;
~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS17008: Invalid type. To avoid ambiguity, add parentheses: '(<T>(...) => void | void)'
~
!!! error TS1003: Identifier expected.

// Missing identifier or keyword
let f: TypeModule1.
~~~~~~~~~~~
!!! error TS2503: Cannot find namespace 'TypeModule1'.

!!! error TS1003: Identifier expected.

module TypeModule2 {
}

let g: x.void;
~
!!! error TS2503: Cannot find namespace 'x'.
~~~~
!!! error TS17009: Identifier expected, reserved word 'void' not allowed here.
~
!!! error TS1109: Expression expected.
let h = (a: ) => {
~
!!! error TS1110: Type expected.
}
Foo<a, , b>();
~~~
!!! error TS2304: Cannot find name 'Foo'.
~
!!! error TS1110: Type expected.

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

// Union or intersection ambiguity
function test(x: string | new() => void) { }

let a: string | new () => void;
let b: string | (new () => void);
let c: string | new () => void /* not part of error */
let d: string | new () => void | number;
let e: string | <T>(...) => void | void;

// Missing identifier or keyword
let f: TypeModule1.

module TypeModule2 {
}

let g: x.void;
let h = (a: ) => {
}
Foo<a, , b>();


//// [errorParserTypeIdentifier.js]
// Union or intersection ambiguity
function test(x) { }
var a;
var b;
var c; /* not part of error */
var d;
var e;
// Missing identifier or keyword
var f;
var g = void ;
var h = function (a) {
};
Foo();
6 changes: 3 additions & 3 deletions tests/baselines/reference/parserSkippedTokens20.errors.txt
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
tests/cases/conformance/parser/ecmascript5/SkippedTokens/parserSkippedTokens20.ts(1,8): error TS2304: Cannot find name 'X'.
tests/cases/conformance/parser/ecmascript5/SkippedTokens/parserSkippedTokens20.ts(1,12): error TS1127: Invalid character.
tests/cases/conformance/parser/ecmascript5/SkippedTokens/parserSkippedTokens20.ts(1,12): error TS1005: ',' expected.
tests/cases/conformance/parser/ecmascript5/SkippedTokens/parserSkippedTokens20.ts(1,13): error TS1005: '>' expected.


==== tests/cases/conformance/parser/ecmascript5/SkippedTokens/parserSkippedTokens20.ts (3 errors) ====
var v: X<T \
~
!!! error TS2304: Cannot find name 'X'.

!!! error TS1127: Invalid character.
~
!!! error TS1005: ',' expected.

!!! error TS1005: '>' expected.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
tests/cases/conformance/parser/ecmascript5/ErrorRecovery/parserUnterminatedGeneric1.ts(2,23): error TS2304: Cannot find name 'IPromise'.
tests/cases/conformance/parser/ecmascript5/ErrorRecovery/parserUnterminatedGeneric1.ts(2,45): error TS2304: Cannot find name 'IPromise'.
tests/cases/conformance/parser/ecmascript5/ErrorRecovery/parserUnterminatedGeneric1.ts(2,54): error TS1005: '>' expected.
tests/cases/conformance/parser/ecmascript5/ErrorRecovery/parserUnterminatedGeneric1.ts(2,54): error TS1005: '}' expected.


==== tests/cases/conformance/parser/ecmascript5/ErrorRecovery/parserUnterminatedGeneric1.ts (3 errors) ====
Expand All @@ -11,4 +11,4 @@ tests/cases/conformance/parser/ecmascript5/ErrorRecovery/parserUnterminatedGener
~~~~~~~~
!!! error TS2304: Cannot find name 'IPromise'.

!!! error TS1005: '>' expected.
!!! error TS1005: '}' expected.
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 a regression.

Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ tests/cases/conformance/parser/ecmascript5/ErrorRecovery/parserUnterminatedGener
tests/cases/conformance/parser/ecmascript5/ErrorRecovery/parserUnterminatedGeneric2.ts(4,43): error TS2304: Cannot find name 'any'.
tests/cases/conformance/parser/ecmascript5/ErrorRecovery/parserUnterminatedGeneric2.ts(8,23): error TS2304: Cannot find name 'IPromise'.
tests/cases/conformance/parser/ecmascript5/ErrorRecovery/parserUnterminatedGeneric2.ts(8,45): error TS2304: Cannot find name 'IPromise'.
tests/cases/conformance/parser/ecmascript5/ErrorRecovery/parserUnterminatedGeneric2.ts(8,54): error TS1005: '>' expected.
tests/cases/conformance/parser/ecmascript5/ErrorRecovery/parserUnterminatedGeneric2.ts(8,54): error TS1005: '}' expected.


==== tests/cases/conformance/parser/ecmascript5/ErrorRecovery/parserUnterminatedGeneric2.ts (15 errors) ====
Expand Down Expand Up @@ -53,4 +53,4 @@ tests/cases/conformance/parser/ecmascript5/ErrorRecovery/parserUnterminatedGener
~~~~~~~~
!!! error TS2304: Cannot find name 'IPromise'.

!!! error TS1005: '>' expected.
!!! error TS1005: '}' expected.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
tests/cases/conformance/parser/ecmascript5/parservoidInQualifiedName2.ts(1,9): error TS2503: Cannot find namespace 'x'.
tests/cases/conformance/parser/ecmascript5/parservoidInQualifiedName2.ts(1,11): error TS1003: Identifier expected.
tests/cases/conformance/parser/ecmascript5/parservoidInQualifiedName2.ts(1,11): error TS17009: Identifier expected, reserved word 'void' not allowed here.
tests/cases/conformance/parser/ecmascript5/parservoidInQualifiedName2.ts(1,15): error TS1109: Expression expected.


Expand All @@ -8,6 +8,6 @@ tests/cases/conformance/parser/ecmascript5/parservoidInQualifiedName2.ts(1,15):
~
!!! error TS2503: Cannot find namespace 'x'.
~~~~
!!! error TS1003: Identifier expected.
!!! error TS17009: Identifier expected, reserved word 'void' not allowed here.
~
!!! error TS1109: Expression expected.
Loading