[
https://issues.apache.org/jira/browse/GROOVY-12694?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18138384#comment-18138384
]
ASF GitHub Bot commented on GROOVY-12694:
-----------------------------------------
codeconsole opened a new pull request, #3215:
URL: https://github.com/apache/groovy/pull/3215
Fixes GROOVY-12694.
What changed
The "unclosed cast" heuristic in MissingDelimiterDiagnostic (strategy 2,
fromCastPattern) now ignores a `(` whose matching `)` comes before the point
where the parser failed. That point is the offending token, or, for a
NoViableAltException, the start token of the failed decision when that is
earlier. When the exception carries no position (null exception or unindexed
tokens) the scan is unchanged. The bookkeeping is one linear pass over the
default-channel tokens up to the failure point with a stack of open parens, so
the error path stays linear.
Why
The scan looked at the whole token stream for `( type <literal | identifier
| (>` and reported the earliest match as a cast missing its `)`, without asking
whether that group had already been closed. Groups the parser had long accepted
match that shape: a record or method header with a primitive-typed parameter
(`record Circle(double radius)`, `def foo(int x)`), a classic `for (int i = 0;
...)` header, or a parenthesised call such as `(Math.max(1, 2))`. Any syntax
error later in the file was then reported as `Missing ')'` on that earlier
line. The reported case is an unsupported type pattern in a switch case label
(`case Circle c ->`) after a record declaration: `Missing ')' @ line 1, column
22` inside the well-formed record header instead of `Unexpected input: 'String
s = switch (o) { case Circle c' @ line 3, column 37`, which is what the same
switch gets after a plain class.
Real unclosed casts keep their diagnostics because a group that still
contains the failure point, or has no closer at all, is still scanned: `println
((int 123)` keeps `Missing ')'` at `123`, and the existing `for (int i=0; ...)`
call-argument case in ParserNegativeSyntaxTest (where the offending token is
the `(` of the group itself) keeps its caret.
Testing
- ParserNegativeSyntaxTest: two new end-to-end cases (record declaration
followed by `case Circle c ->`; a for header, a primitive-typed parameter and a
parenthesised call followed by a later syntax error).
- MissingDelimiterDiagnosticTest: cases for a group closed before the
failure (with and without another unclosed delimiter), a group holding the
offending token, the NoViableAltException start token, and a failure without a
position. The existing reflection tests for fromCastPattern pass the new second
argument.
- The new end-to-end cases and the two "closed before the failure" unit
cases fail on master without the main change and pass with it.
- `./gradlew :test --tests
org.apache.groovy.parser.antlr4.internal.MissingDelimiterDiagnosticTest --tests
org.apache.groovy.parser.antlr4.ParserNegativeSyntaxTest` on JDK 21: 82 + 235
tests, 0 failures. Without the main change the same run fails the four
behavioural cases named above (plus the reflection tests, which then see the
old one-argument signature).
- `./gradlew :test --tests 'org.apache.groovy.parser.antlr4.*'`: 14 classes,
828 tests, 0 failures, 1 skipped.
- `./gradlew :checkstyleMain`: no new findings in
MissingDelimiterDiagnostic.java; the only entry for the file is the
pre-existing cyclomatic-complexity warning on fromCastPattern (15, unchanged
from master because the candidate filtering lives in the new helper).
- The installed `groovyc --check` from this branch reports the reproducer at
`3:37: Unexpected input: 'String s = switch (o) { case Circle c'`; 6.0.0,
6.0.1-SNAPSHOT and master before the change report `1:22: Missing ')'`.
Backport
The affected code is identical on GROOVY_6_0_X; the change applies cleanly
there and is suggested for 6.0.1.
> Parser reports Missing ')' at an earlier closed paren group instead of the
> actual syntax error
> ----------------------------------------------------------------------------------------------
>
> Key: GROOVY-12694
> URL: https://issues.apache.org/jira/browse/GROOVY-12694
> Project: Groovy
> Issue Type: Bug
> Components: parser
> Affects Versions: 6.0.0
> Reporter: Scott Murphy Heiberg
> Priority: Major
>
> *Environment:* macOS 26.5 (Darwin 25.5.0), JDK 21.0.12 (BellSoft Liberica);
> diagnostics taken from {{groovyc --check --error-format=short}}. Also
> reproduces on 6.0.1-SNAPSHOT (GROOVY_6_0_X at 7c0dcf17dd) and master
> 7.0.0-SNAPSHOT (d86ea7eb85).
> h3. Description
> A Java-style type pattern in a switch case label ({{case Circle c ->}}) is
> not Groovy syntax, so the script below must fail to parse. When a record
> declaration precedes the switch, however, the error is reported as {{Missing
> ')'}} inside the record header on line 1 -- which parses fine on its own --
> instead of at the offending case label on line 3.
> h3. Reproducer
> {code:groovy}
> record Circle(double radius) {}
> Object o = new Circle(1)
> String s = switch (o) { case Circle c -> 'circle'; default -> 'other' }
> println s
> {code}
> h3. Actual
> {noformat}
> record.groovy:1:22: error: Missing ')'
> {noformat}
> Column 22 is {{radius}} in the record header. Replacing {{case Circle c ->}}
> with {{case Circle ->}} makes the file compile, so there is nothing missing
> on line 1.
> h3. Expected
> The diagnostic at the unsupported case label, which is what the same switch
> gets when a plain class ({{class Circle { double radius }}}) precedes it
> instead of the record:
> {noformat}
> record.groovy:3:37: error: Unexpected input: 'String s = switch (o) { case
> Circle c'
> {noformat}
> h3. Scope
> The record is incidental. Any parenthesised group of the shape {{( <type>
> <literal | identifier | (>}} that the parser has already accepted is reported
> as a missing {{)}} whenever a syntax error occurs later in the file:
> {code:groovy}
> def foo(int x) { x }
> def y = 1 +
> {code}
> {noformat}
> Missing ')' @ line 1, column 13
> {noformat}
> Expected {{Unexpected input: '+' @ line 2, column 11}}, which is what the
> second line reports on its own. The same happens with {{for (int i = 0; i <
> 3; i++) { }}} or {{def y = (Math.max(1, 2))}} on the first line.
> h3. Cause
> {{org.apache.groovy.parser.antlr4.internal.MissingDelimiterDiagnostic#fromCastPattern}}
> (the "unclosed cast" heuristic added for the friendlier missing-delimiter
> messages) scans the whole token stream for {{'(' type <expr-start>}} and
> reports the earliest match, without checking whether that group's {{)}} was
> already matched before the point where the parser failed. A formal parameter
> list with a primitive-typed parameter, a classic {{for}} header or a
> parenthesised call all match that pattern. Restricting the scan to groups
> that are still open at the failure point (the offending token, or the start
> token of a failed {{NoViableAltException}} decision) fixes the position while
> keeping the real unclosed-cast diagnostics ({{println ((int 123)}} still
> reports at {{123}}).
> A pull request with regression tests follows.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
