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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
