From efb6db87576845d108386774c2c70a92ebe5b07c Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 25 Jul 2014 18:10:27 -0700 Subject: [PATCH 1/3] Gracefully handle 'catch' and 'finally' blocks without a preceding 'try' block. Fixes #216. As a note of this fix, when a 'catch' block is followed by a 'finally' block, only the 'catch' keyword gets an error reported on it. --- .../diagnosticInformationMap.generated.ts | 2 + src/compiler/diagnosticMessages.json | 9 ++++ src/compiler/parser.ts | 44 ++++++++++++++++++- .../invalidTryStatements2.errors.txt | 8 +--- .../reference/parserMissingToken1.errors.txt | 4 +- 5 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/compiler/diagnosticInformationMap.generated.ts b/src/compiler/diagnosticInformationMap.generated.ts index 098d908ea6e..8a79bb20325 100644 --- a/src/compiler/diagnosticInformationMap.generated.ts +++ b/src/compiler/diagnosticInformationMap.generated.ts @@ -160,6 +160,8 @@ module ts { Constructor_implementation_expected: { code: 2240, category: DiagnosticCategory.Error, key: "Constructor implementation expected." }, An_export_assignment_cannot_be_used_in_a_module_with_other_exported_elements: { code: 2245, category: DiagnosticCategory.Error, key: "An export assignment cannot be used in a module with other exported elements." }, A_parameter_property_is_only_allowed_in_a_constructor_implementation: { code: 2246, category: DiagnosticCategory.Error, key: "A parameter property is only allowed in a constructor implementation." }, + A_catch_clause_must_be_preceded_by_a_try_statement: { code: 2249, category: DiagnosticCategory.Error, key: "A 'catch' clause must be preceded by a 'try' statement." }, + A_finally_block_must_be_preceded_by_a_try_statement: { code: 2250, category: DiagnosticCategory.Error, key: "A 'finally' block must be preceded by a 'try' statement." }, Circular_definition_of_import_alias_0: { code: 3000, category: DiagnosticCategory.Error, key: "Circular definition of import alias '{0}'." }, Cannot_find_name_0: { code: 3001, category: DiagnosticCategory.Error, key: "Cannot find name '{0}'." }, Module_0_has_no_exported_member_1: { code: 3002, category: DiagnosticCategory.Error, key: "Module '{0}' has no exported member '{1}'." }, diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 24fcdfa51e4..93966c1b440 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -632,6 +632,15 @@ "category": "Error", "code": 2246 }, + "A 'catch' clause must be preceded by a 'try' statement.": { + "category": "Error", + "code": 2249 + }, + "A 'finally' block must be preceded by a 'try' statement.": { + "category": "Error", + "code": 2250 + }, + "Circular definition of import alias '{0}'.": { "category": "Error", "code": 3000 diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 20bd662166d..0ffe2f5cfbd 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -2542,6 +2542,35 @@ module ts { return finishNode(node); } + // This function is used for parsing 'catch'/'finally' blocks + // in spite of them missing a 'try' statement. + function parseCatchOrFinallyBlocksMissingTryStatement(): TryStatement { + + Debug.assert(token === SyntaxKind.CatchKeyword || token === SyntaxKind.FinallyKeyword, + "'parseCatchOrFinallyBlocksMissingTryStatement' should only be called when the current token is a 'catch' or 'finally' keyword."); + + // We're just going to return a bogus TryStatement. + var node = createNode(SyntaxKind.TryStatement); + node.tryBlock = createNode(SyntaxKind.Block); + node.tryBlock.statements = createMissingList(); + + if (token === SyntaxKind.CatchKeyword) { + error(Diagnostics.A_catch_clause_must_be_preceded_by_a_try_statement); + node.catchBlock = parseCatchBlock(); + } + + if (token === SyntaxKind.FinallyKeyword) { + // Only report an error on the 'finally' block if we haven't on the 'catch' block. + if (node.catchBlock === undefined) { + error(Diagnostics.A_finally_block_must_be_preceded_by_a_try_statement); + } + + node.finallyBlock = parseTokenAndBlock(SyntaxKind.FinallyKeyword, SyntaxKind.FinallyBlock); + } + + return finishNode(node); + } + function parseTokenAndBlock(token: SyntaxKind, kind: SyntaxKind): Block { var pos = getNodePos(); parseExpected(token); @@ -2646,6 +2675,10 @@ module ts { case SyntaxKind.ThrowKeyword: case SyntaxKind.TryKeyword: case SyntaxKind.DebuggerKeyword: + // 'catch' and 'finally' do not actually indicate that the code is part of a statement, + // however, we say they are here so that we may gracefully parse them and error later. + case SyntaxKind.CatchKeyword: + case SyntaxKind.FinallyKeyword: return true; case SyntaxKind.InterfaceKeyword: case SyntaxKind.ClassKeyword: @@ -2653,13 +2686,17 @@ module ts { case SyntaxKind.EnumKeyword: // When followed by an identifier, these do not start a statement but might // instead be following declarations - if (isDeclaration()) return false; + if (isDeclaration()) { + return false; + } case SyntaxKind.PublicKeyword: case SyntaxKind.PrivateKeyword: case SyntaxKind.StaticKeyword: // When followed by an identifier or keyword, these do not start a statement but // might instead be following type members - if (lookAhead(() => nextToken() >= SyntaxKind.Identifier)) return false; + if (lookAhead(() => nextToken() >= SyntaxKind.Identifier)) { + return false; + } default: return isExpression(); } @@ -2697,6 +2734,9 @@ module ts { return parseThrowStatement(); case SyntaxKind.TryKeyword: return parseTryStatement(); + case SyntaxKind.CatchKeyword: + case SyntaxKind.FinallyKeyword: + return parseCatchOrFinallyBlocksMissingTryStatement(); case SyntaxKind.DebuggerKeyword: return parseDebuggerStatement(); default: diff --git a/tests/baselines/reference/invalidTryStatements2.errors.txt b/tests/baselines/reference/invalidTryStatements2.errors.txt index fcb8ad3560d..4c36ce6a6b7 100644 --- a/tests/baselines/reference/invalidTryStatements2.errors.txt +++ b/tests/baselines/reference/invalidTryStatements2.errors.txt @@ -1,4 +1,4 @@ -==== tests/cases/conformance/statements/tryStatements/invalidTryStatements2.ts (4 errors) ==== +==== tests/cases/conformance/statements/tryStatements/invalidTryStatements2.ts (2 errors) ==== function fn() { try { } catch { // syntax error, missing '(x)' @@ -8,11 +8,7 @@ catch(x) { } // error missing try ~~~~~ -!!! Statement expected. - ~ -!!! '=>' expected. +!!! A 'catch' clause must be preceded by a 'try' statement. finally{ } // error missing try - ~~~~~~~ -!!! Statement expected. } \ No newline at end of file diff --git a/tests/baselines/reference/parserMissingToken1.errors.txt b/tests/baselines/reference/parserMissingToken1.errors.txt index 670da2ef5c1..35e5e605cb5 100644 --- a/tests/baselines/reference/parserMissingToken1.errors.txt +++ b/tests/baselines/reference/parserMissingToken1.errors.txt @@ -1,6 +1,8 @@ -==== tests/cases/conformance/parser/ecmascript5/MissingTokens/parserMissingToken1.ts (2 errors) ==== +==== tests/cases/conformance/parser/ecmascript5/MissingTokens/parserMissingToken1.ts (3 errors) ==== a / finally ~~~~~~~ !!! Expression expected. + +!!! '{' expected. ~ !!! Cannot find name 'a'. \ No newline at end of file From 0939f77d77d624e634d0d3c216437ded9f23bc70 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 25 Jul 2014 18:25:43 -0700 Subject: [PATCH 2/3] Added tests for missing 'try' parsing --- .../invalidTryStatements2.errors.txt | 31 +++++++++++++++++-- .../tryStatements/invalidTryStatements2.ts | 21 ++++++++++++- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/tests/baselines/reference/invalidTryStatements2.errors.txt b/tests/baselines/reference/invalidTryStatements2.errors.txt index 4c36ce6a6b7..bedc9af1adc 100644 --- a/tests/baselines/reference/invalidTryStatements2.errors.txt +++ b/tests/baselines/reference/invalidTryStatements2.errors.txt @@ -1,4 +1,4 @@ -==== tests/cases/conformance/statements/tryStatements/invalidTryStatements2.ts (2 errors) ==== +==== tests/cases/conformance/statements/tryStatements/invalidTryStatements2.ts (6 errors) ==== function fn() { try { } catch { // syntax error, missing '(x)' @@ -10,5 +10,32 @@ ~~~~~ !!! A 'catch' clause must be preceded by a 'try' statement. - finally{ } // error missing try + finally{ } // potential error; can be absorbed by the 'catch' + } + + function fn2() { + finally { } // error missing try + ~~~~~~~ +!!! A 'finally' block must be preceded by a 'try' statement. + catch (x) { } // error missing try + ~~~~~ +!!! A 'catch' clause must be preceded by a 'try' statement. + + // no error + try { + } + finally { + } + + // error missing try + finally { + ~~~~~~~ +!!! A 'finally' block must be preceded by a 'try' statement. + } + + // error missing try + catch (x) { + ~~~~~ +!!! A 'catch' clause must be preceded by a 'try' statement. + } } \ No newline at end of file diff --git a/tests/cases/conformance/statements/tryStatements/invalidTryStatements2.ts b/tests/cases/conformance/statements/tryStatements/invalidTryStatements2.ts index de37313e92a..6937e509845 100644 --- a/tests/cases/conformance/statements/tryStatements/invalidTryStatements2.ts +++ b/tests/cases/conformance/statements/tryStatements/invalidTryStatements2.ts @@ -5,5 +5,24 @@ function fn() { catch(x) { } // error missing try - finally{ } // error missing try + finally{ } // potential error; can be absorbed by the 'catch' +} + +function fn2() { + finally { } // error missing try + catch (x) { } // error missing try + + // no error + try { + } + finally { + } + + // error missing try + finally { + } + + // error missing try + catch (x) { + } } \ No newline at end of file From 79735b44315722f8b9d42e7b14da675058d8cc61 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Sat, 26 Jul 2014 21:17:53 -0700 Subject: [PATCH 3/3] Simplified error recovery by just using 'parseTryStatement'. --- .../diagnosticInformationMap.generated.ts | 2 -- src/compiler/diagnosticMessages.json | 10 +----- src/compiler/parser.ts | 33 ++----------------- .../invalidTryStatements2.errors.txt | 10 +++--- 4 files changed, 8 insertions(+), 47 deletions(-) diff --git a/src/compiler/diagnosticInformationMap.generated.ts b/src/compiler/diagnosticInformationMap.generated.ts index 8a79bb20325..098d908ea6e 100644 --- a/src/compiler/diagnosticInformationMap.generated.ts +++ b/src/compiler/diagnosticInformationMap.generated.ts @@ -160,8 +160,6 @@ module ts { Constructor_implementation_expected: { code: 2240, category: DiagnosticCategory.Error, key: "Constructor implementation expected." }, An_export_assignment_cannot_be_used_in_a_module_with_other_exported_elements: { code: 2245, category: DiagnosticCategory.Error, key: "An export assignment cannot be used in a module with other exported elements." }, A_parameter_property_is_only_allowed_in_a_constructor_implementation: { code: 2246, category: DiagnosticCategory.Error, key: "A parameter property is only allowed in a constructor implementation." }, - A_catch_clause_must_be_preceded_by_a_try_statement: { code: 2249, category: DiagnosticCategory.Error, key: "A 'catch' clause must be preceded by a 'try' statement." }, - A_finally_block_must_be_preceded_by_a_try_statement: { code: 2250, category: DiagnosticCategory.Error, key: "A 'finally' block must be preceded by a 'try' statement." }, Circular_definition_of_import_alias_0: { code: 3000, category: DiagnosticCategory.Error, key: "Circular definition of import alias '{0}'." }, Cannot_find_name_0: { code: 3001, category: DiagnosticCategory.Error, key: "Cannot find name '{0}'." }, Module_0_has_no_exported_member_1: { code: 3002, category: DiagnosticCategory.Error, key: "Module '{0}' has no exported member '{1}'." }, diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 93966c1b440..4769121297a 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -632,14 +632,6 @@ "category": "Error", "code": 2246 }, - "A 'catch' clause must be preceded by a 'try' statement.": { - "category": "Error", - "code": 2249 - }, - "A 'finally' block must be preceded by a 'try' statement.": { - "category": "Error", - "code": 2250 - }, "Circular definition of import alias '{0}'.": { "category": "Error", @@ -1246,4 +1238,4 @@ "category": "Error", "code": -9999999 } -} \ No newline at end of file +} diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 0ffe2f5cfbd..27a1906d62a 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -2542,35 +2542,6 @@ module ts { return finishNode(node); } - // This function is used for parsing 'catch'/'finally' blocks - // in spite of them missing a 'try' statement. - function parseCatchOrFinallyBlocksMissingTryStatement(): TryStatement { - - Debug.assert(token === SyntaxKind.CatchKeyword || token === SyntaxKind.FinallyKeyword, - "'parseCatchOrFinallyBlocksMissingTryStatement' should only be called when the current token is a 'catch' or 'finally' keyword."); - - // We're just going to return a bogus TryStatement. - var node = createNode(SyntaxKind.TryStatement); - node.tryBlock = createNode(SyntaxKind.Block); - node.tryBlock.statements = createMissingList(); - - if (token === SyntaxKind.CatchKeyword) { - error(Diagnostics.A_catch_clause_must_be_preceded_by_a_try_statement); - node.catchBlock = parseCatchBlock(); - } - - if (token === SyntaxKind.FinallyKeyword) { - // Only report an error on the 'finally' block if we haven't on the 'catch' block. - if (node.catchBlock === undefined) { - error(Diagnostics.A_finally_block_must_be_preceded_by_a_try_statement); - } - - node.finallyBlock = parseTokenAndBlock(SyntaxKind.FinallyKeyword, SyntaxKind.FinallyBlock); - } - - return finishNode(node); - } - function parseTokenAndBlock(token: SyntaxKind, kind: SyntaxKind): Block { var pos = getNodePos(); parseExpected(token); @@ -2733,10 +2704,10 @@ module ts { case SyntaxKind.ThrowKeyword: return parseThrowStatement(); case SyntaxKind.TryKeyword: - return parseTryStatement(); + // Include the next two for error recovery. case SyntaxKind.CatchKeyword: case SyntaxKind.FinallyKeyword: - return parseCatchOrFinallyBlocksMissingTryStatement(); + return parseTryStatement(); case SyntaxKind.DebuggerKeyword: return parseDebuggerStatement(); default: diff --git a/tests/baselines/reference/invalidTryStatements2.errors.txt b/tests/baselines/reference/invalidTryStatements2.errors.txt index bedc9af1adc..a600210a605 100644 --- a/tests/baselines/reference/invalidTryStatements2.errors.txt +++ b/tests/baselines/reference/invalidTryStatements2.errors.txt @@ -8,7 +8,7 @@ catch(x) { } // error missing try ~~~~~ -!!! A 'catch' clause must be preceded by a 'try' statement. +!!! 'try' expected. finally{ } // potential error; can be absorbed by the 'catch' } @@ -16,10 +16,10 @@ function fn2() { finally { } // error missing try ~~~~~~~ -!!! A 'finally' block must be preceded by a 'try' statement. +!!! 'try' expected. catch (x) { } // error missing try ~~~~~ -!!! A 'catch' clause must be preceded by a 'try' statement. +!!! 'try' expected. // no error try { @@ -30,12 +30,12 @@ // error missing try finally { ~~~~~~~ -!!! A 'finally' block must be preceded by a 'try' statement. +!!! 'try' expected. } // error missing try catch (x) { ~~~~~ -!!! A 'catch' clause must be preceded by a 'try' statement. +!!! 'try' expected. } } \ No newline at end of file