[jira] [Updated] (GROOVY-12169) Improve syntax error message and caret position for missing ')'

["Daniel Sun (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/GROOVY-12169?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Daniel Sun updated GROOVY-12169:
--------------------------------
    Description: 
h3. Problem

When a Groovy source is missing a closing parenthesis {{{}){}}}, the ANTLR4 parser often reports a generic *Unexpected input* message at a *misleading* location (frequently the opening {{(}} or an earlier token), instead of *Missing ')'* at the place where {{)}} should be inserted or where the construct first went wrong.

This makes simple typos hard to fix, especially with casts, method calls, and formal parameter lists.

h3. Root cause
 * ANTLR4 does not recover missing tokens as helpfully as the old Antlr2 path.
 * Grammar-level *error alternatives* on {{rparen}} can produce {*}Missing ')'{*}, but they enlarge the ATN and hurt successful-parse performance. They were removed in the GROOVY-9588 work for that reason.
 * With {{BailErrorStrategy}} + generic mismatch reporting, prediction often fails a deep parenthesised alternative and falls back to a shorter parse, so the surface error points far from the real omission.

h3. Goal

Report *Missing ')'* with an accurate caret when the token stream clearly indicates a missing or incomplete {{{}){}}}, *without* reintroducing grammar error alternatives on the hot (successful) parse path.
h3. Approach

Error-path-only diagnostics in the existing error strategy:
 * {{DescriptiveErrorStrategy}} — on {{InputMismatch}} / {{{}NoViableAlt{}}}, prefer a missing-paren diagnostic over the generic message.
 * {{MissingParenDiagnostic}} — single linear scan of the (fully filled) token stream after failure; *no* work on successful parses.

Detection order (most specific first):
 # RecognitionException whose *sole* expected token is {{{}RPAREN{}}}, and net paren depth > 0 (avoids false positives like {{foo(1;2;3)}} where parens are balanced but {{;}} is wrong).
 # Cast / parenthesised-type pattern: {{(type <expr-start>}} without the cast's {{)}} (e.g. {{{}(int 123){}}}).
 # Unclosed {{(}} at EOF or at a structural hard-stop such as an outer {{{}}{}}}.

Token stream is force-filled to EOF before scanning so a trailing {{)}} is visible even when the parser failed earlier inside the list.
h3. Expected result (after)
||Source||Expected message||
|{{println ((int 123)}}|{{Missing ')'}} at the start of {{123}}|
|{\{def x() { println((int) 123 }}}|{{Missing ')'}} at the insertion point after {{123}}|
|{\{def m( { }}}|{{Missing ')'}} at {{'
Unknown macro: \{'}
}|
|{{foo(1, 2}}|{{Missing ')'}} at end of line|
|{{def f(int x}}|{{Missing ')'}} after {{x}}|
|{{println ((int) 123}}|{{Missing ')'}} at end of line|

Balanced but invalid constructs (e.g. {{{}[].bar(1;2;3){}}}) must *not* be rewritten as {*}Missing ')'{*}; keep the generic unexpected-input (or other) message.

  was:
h3. Problem

When a Groovy source is missing a closing parenthesis {{{}){}}}, the ANTLR4 parser often reports a generic *Unexpected input* message at a *misleading* location (frequently the opening {{(}} or an earlier token), instead of *Missing ')'* at the place where {{)}} should be inserted or where the construct first went wrong.

This makes simple typos hard to fix, especially with casts, method calls, and formal parameter lists.
h3. Examples (before)
||Source||Reported (unhelpful)||
|{{println ((int 123)}}|{{Unexpected input: '('}} at the *opening* parenthesis|
|{\{def x() { println((int) 123 }}}|{{Unexpected input: '('}} at the call's opening parenthesis|
|{\{def m( { }}}|{{Unexpected input: '{'}} (closer, but still not "Missing ')'")|
h3. Root cause
 * ANTLR4 does not recover missing tokens as helpfully as the old Antlr2 path.
 * Grammar-level *error alternatives* on {{rparen}} can produce {*}Missing ')'{*}, but they enlarge the ATN and hurt successful-parse performance. They were removed in the GROOVY-9588 work for that reason.
 * With {{BailErrorStrategy}} + generic mismatch reporting, prediction often fails a deep parenthesised alternative and falls back to a shorter parse, so the surface error points far from the real omission.

h3. Goal

Report *Missing ')'* with an accurate caret when the token stream clearly indicates a missing or incomplete {{{}){}}}, *without* reintroducing grammar error alternatives on the hot (successful) parse path.
h3. Approach

Error-path-only diagnostics in the existing error strategy:
 * {{DescriptiveErrorStrategy}} — on {{InputMismatch}} / {{{}NoViableAlt{}}}, prefer a missing-paren diagnostic over the generic message.
 * {{MissingParenDiagnostic}} — single linear scan of the (fully filled) token stream after failure; *no* work on successful parses.

Detection order (most specific first):
 # RecognitionException whose *sole* expected token is {{{}RPAREN{}}}, and net paren depth > 0 (avoids false positives like {{foo(1;2;3)}} where parens are balanced but {{;}} is wrong).
 # Cast / parenthesised-type pattern: {{(type <expr-start>}} without the cast's {{)}} (e.g. {{{}(int 123){}}}).
 # Unclosed {{(}} at EOF or at a structural hard-stop such as an outer {{{}}{}}}.

Token stream is force-filled to EOF before scanning so a trailing {{)}} is visible even when the parser failed earlier inside the list.
h3. Expected result (after)
||Source||Expected message||
|{{println ((int 123)}}|{{Missing ')'}} at the start of {{123}}|
|{\{def x() { println((int) 123 }}}|{{Missing ')'}} at the insertion point after {{123}}|
|{\{def m( { }}}|{{Missing ')'}} at {{'
Unknown macro: \{'}
}|
|{{foo(1, 2}}|{{Missing ')'}} at end of line|
|{{def f(int x}}|{{Missing ')'}} after {{x}}|
|{{println ((int) 123}}|{{Missing ')'}} at end of line|

Balanced but invalid constructs (e.g. {{{}[].bar(1;2;3){}}}) must *not* be rewritten as {*}Missing ')'{*}; keep the generic unexpected-input (or other) message.


> Improve syntax error message and caret position for missing ')'
> ---------------------------------------------------------------
>
>                 Key: GROOVY-12169
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12169
>             Project: Groovy
>          Issue Type: Improvement
>            Reporter: Daniel Sun
>            Priority: Major
>
> h3. Problem
> When a Groovy source is missing a closing parenthesis {{{}){}}}, the ANTLR4 parser often reports a generic *Unexpected input* message at a *misleading* location (frequently the opening {{(}} or an earlier token), instead of *Missing ')'* at the place where {{)}} should be inserted or where the construct first went wrong.
> This makes simple typos hard to fix, especially with casts, method calls, and formal parameter lists.
> h3. Root cause
>  * ANTLR4 does not recover missing tokens as helpfully as the old Antlr2 path.
>  * Grammar-level *error alternatives* on {{rparen}} can produce {*}Missing ')'{*}, but they enlarge the ATN and hurt successful-parse performance. They were removed in the GROOVY-9588 work for that reason.
>  * With {{BailErrorStrategy}} + generic mismatch reporting, prediction often fails a deep parenthesised alternative and falls back to a shorter parse, so the surface error points far from the real omission.
> h3. Goal
> Report *Missing ')'* with an accurate caret when the token stream clearly indicates a missing or incomplete {{{}){}}}, *without* reintroducing grammar error alternatives on the hot (successful) parse path.
> h3. Approach
> Error-path-only diagnostics in the existing error strategy:
>  * {{DescriptiveErrorStrategy}} — on {{InputMismatch}} / {{{}NoViableAlt{}}}, prefer a missing-paren diagnostic over the generic message.
>  * {{MissingParenDiagnostic}} — single linear scan of the (fully filled) token stream after failure; *no* work on successful parses.
> Detection order (most specific first):
>  # RecognitionException whose *sole* expected token is {{{}RPAREN{}}}, and net paren depth > 0 (avoids false positives like {{foo(1;2;3)}} where parens are balanced but {{;}} is wrong).
>  # Cast / parenthesised-type pattern: {{(type <expr-start>}} without the cast's {{)}} (e.g. {{{}(int 123){}}}).
>  # Unclosed {{(}} at EOF or at a structural hard-stop such as an outer {{{}}{}}}.
> Token stream is force-filled to EOF before scanning so a trailing {{)}} is visible even when the parser failed earlier inside the list.
> h3. Expected result (after)
> ||Source||Expected message||
> |{{println ((int 123)}}|{{Missing ')'}} at the start of {{123}}|
> |{\{def x() { println((int) 123 }}}|{{Missing ')'}} at the insertion point after {{123}}|
> |{\{def m( { }}}|{{Missing ')'}} at {{'
> Unknown macro: \{'}
> }|
> |{{foo(1, 2}}|{{Missing ')'}} at end of line|
> |{{def f(int x}}|{{Missing ')'}} after {{x}}|
> |{{println ((int) 123}}|{{Missing ')'}} at end of line|
> Balanced but invalid constructs (e.g. {{{}[].bar(1;2;3){}}}) must *not* be rewritten as {*}Missing ')'{*}; keep the generic unexpected-input (or other) message.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)