[
https://issues.apache.org/jira/browse/GROOVY-12353?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18111714#comment-18111714
]
ASF GitHub Bot commented on GROOVY-12353:
-----------------------------------------
Copilot commented on code in PR #2877:
URL: https://github.com/apache/groovy/pull/2877#discussion_r3937641155
##########
src/main/java/org/apache/groovy/parser/antlr4/GroovySyntaxError.java:
##########
@@ -19,7 +19,14 @@
package org.apache.groovy.parser.antlr4;
/**
- * Represents a syntax error of groovy program
+ * Represents a syntax error of a Groovy program, raised by the lexer or
parser.
+ * <p>
+ * The message is the diagnostic as produced by the recogniser (for example
+ * {@code Unclosed string literal} or {@code Unexpected character: '\u200b'}).
Review Comment:
Java translates Unicode escapes before parsing comments, so this Javadoc
example renders an actual invisible U+200B rather than the literal `\u200b`
emitted by the diagnostic. Escape the backslash through `\u005c` so the
generated API documentation shows the intended text.
##########
src/main/java/org/apache/groovy/parser/antlr4/internal/AbstractFriendlyErrorStrategy.java:
##########
@@ -49,24 +71,107 @@ abstract class AbstractFriendlyErrorStrategy extends
DefaultErrorStrategy {
}
/**
- * Prefer a precise "Missing …" delimiter diagnostic when the token stream
- * clearly indicates an unclosed / incomplete construct; otherwise fall
- * back to the generic message.
+ * Prefer a relocated "Missing …" closer when the token stream supports it;
+ * otherwise refine the generic {@code Unexpected input: ...} fallback
+ * (reserved keyword, sole expected punctuation, unexpected EOF).
*/
private void reportFriendlyError(final Parser recognizer, final
RecognitionException e, final String fallbackMessage) {
- MissingDelimiterDiagnostic.Hit hit = null;
try {
- // Incomplete / synthetic contexts can leave token indices out of
range.
- hit =
MissingDelimiterDiagnostic.locate(recognizer.getInputStream(), e);
+ // Incomplete / synthetic contexts can leave token indices out of
range,
+ // and getExpectedTokens() can reject an invalid ATN state number.
+ MissingDelimiterDiagnostic.Hit hit =
MissingDelimiterDiagnostic.locate(recognizer.getInputStream(), e);
+ if (hit != null) {
+ recognizer.notifyErrorListeners(hit.at, hit.message, e);
+ return;
+ }
+ notifyErrorListeners(recognizer, refineFallbackMessage(e,
fallbackMessage), e);
} catch (IndexOutOfBoundsException | IllegalArgumentException ignored)
{
- // Fall through to the generic message. Catch only locate()'s known
- // defensive failures — never listener-side fatals (e.g.
addFatalError).
+ notifyErrorListeners(recognizer, fallbackMessage, e);
Review Comment:
The guarded block now includes both listener-dispatch calls. If an error
listener throws `IllegalArgumentException` or `IndexOutOfBoundsException`, that
exception is mistaken for a diagnostic lookup failure and the listener is
invoked a second time with the fallback message, masking the original failure
and potentially duplicating side effects. Keep only `locate`/message refinement
inside the defensive catch and dispatch after it.
##########
src/antlr/GroovyLexer.g4:
##########
@@ -997,9 +997,17 @@ WS : ([ \t]+ | LineEscape+) -> skip
NL : LineTerminator { ignoreTokenInsideParens(); }
;
-// Multiple-line comments (including groovydoc comments)
+// Multiple-line comments (including groovydoc comments).
+// The EOF alternative is an error-path-only match: a well-formed comment
+// takes the first alt and never reaches it (unlike parser error alternatives,
+// which GROOVY-9588 showed can pollute prediction). javac reports
+// "unclosed comment" at the opener; requireUnclosedComment keeps the caret
there.
+// Type is set in an action rather than `-> type(NL)` so the rule may have
+// more than one outermost alternative (ANTLR requires `->` commands to be
+// last on a single outermost alt).
ML_COMMENT
- : '/*' .*? '*/' { addComment(0);
ignoreMultiLineCommentConditionally(); } -> type(NL)
+ : '/*' .*? '*/' { addComment(0);
ignoreMultiLineCommentConditionally(); setType(NL); }
+ | '/*' .*? EOF { requireUnclosedComment(errorIgnored);
addComment(0); setType(NL); }
Review Comment:
ANTLR lexers prefer the alternative that consumes the most input. When a
valid `/* ... */` is followed by more source, the new EOF alternative can
consume the closing delimiter and everything through EOF, so it wins over the
shorter first alternative, reports a valid comment as unclosed, and swallows
the remaining source. Put the `*/`/EOF choice after one shared non-greedy loop
so `*/` terminates the token as soon as it is encountered.
> Improve remaining common syntax error messages (unclosed literals, unexpected
> characters, missing punctuation, reserved keywords)
> ---------------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12353
> URL: https://issues.apache.org/jira/browse/GROOVY-12353
> Project: Groovy
> Issue Type: Improvement
> Reporter: Daniel Sun
> Priority: Major
>
> h3. Problem
> After GROOVY-12169 and GROOVY-12171, several everyday syntax mistakes still
> produce a generic *Unexpected input* or *Unexpected character* message that
> does not name the actual problem.
> Invisible characters (zero-width space, BOM, NUL, form feed) appear as an
> empty glyph in quotes. Unclosed quotes and comments are reported as an
> unexpected quote or slash rather than as an unclosed literal. {{if true}}
> without parentheses, {{x ? y}}, and {{const x = 1}} look like random
> unexpected tokens instead of a missing {{(}} / {{:}} or an unimplemented
> keyword.
> h3. Examples (before)
> * Source:
> {noformat}
> println 'Hello
> {noformat}
> Report:
> {noformat}
> Unexpected character: ''' @ line 1, column 9.
> {noformat}
> (caret on the opening quote)
> * Source:
> {noformat}
> /* comment
> {noformat}
> Report:
> {noformat}
> Unexpected input: '/'
> {noformat}
> (no mention of an unclosed comment)
> * Source: {{def}} + zero-width space + {{name = null}}
> Report:
> {noformat}
> Unexpected character: ''
> {noformat}
> (the offending character is invisible)
> * Source:
> {noformat}
> if true { x = 1 }
> {noformat}
> Report:
> {noformat}
> Unexpected input: 'true'
> {noformat}
> * Source:
> {noformat}
> const x = 1
> {noformat}
> Report:
> {noformat}
> Unexpected input: 'const'
> {noformat}
> * Source:
> {noformat}
> def n = 1_
> {noformat}
> Report:
> {noformat}
> Number ending with underscores is invalid @ line 1, column 10 @ line 1,
> column 10.
> {noformat}
> (position duplicated)
> * Source:
> {noformat}
> def m(int... a, int b) {}
> {noformat}
> Report:
> {noformat}
> The var-arg parameter strs must be the last parameter
> {noformat}
> (hard-coded name {{strs}})
> h3. Root cause
> * Lexer {{UNEXPECTED_CHAR}} inlined the raw character with only a
> quote-escape, so control / format characters vanish in the message. An
> unexpected quote is almost always an unclosed string, but the message never
> said so.
> * Unclosed block comments failed the comment rule and were retokenised as
> {{/}}, so the parser saw an unexpected slash.
> * Parser fallback wording was still ANTLR's *Unexpected input*, even when
> the expected set was a single punctuation token (open paren, colon, {{>}}) or
> the offending token was a reserved keyword ({{const}}, {{goto}}, {{else}},
> {{catch}}, {{finally}}, {{case}}) or EOF.
> * Lexer {{require(..., true)}} appended {{@ line N, column M}} to
> {{GroovySyntaxError}}, and {{SyntaxException}} appended the same location
> again.
> * {{AstBuilder}} used a hard-coded parameter name {{strs}} in the
> varargs-not-last diagnostic.
> Grammar-level parser error alternatives are not an option: GROOVY-9588 showed
> they enlarge the ATN and slow successful parses.
> h3. Goal
> Give javac-aligned, developer-facing sentences for these common mistakes,
> with an accurate caret, without reintroducing parser error alternatives on
> the hot path.
> h3. Approach
> Error-path-only, two layers:
> * Lexer ({{GroovyLexer.g4}} / {{AbstractLexer}}): unexpected quote becomes
> *Unclosed string literal*; unclosed block comment becomes *Unclosed comment*
> at the opener (same-rule EOF alternative, not a second lexer rule); other
> unexpected characters via {{getCharErrorDisplay}} (shared with the GString
> {{$}} path from GROOVY-12171). Stop attaching position text on lexer
> {{require}} calls so {{SyntaxException}} is the only source of {{@ line N,
> column M}}.
> * Parser ({{AbstractFriendlyErrorStrategy}}): {{MissingDelimiterDiagnostic}}
> still relocates the caret for a missing closer (GROOVY-12169). Everything
> else only refines the fallback sentence and keeps ANTLR's offending token:
> reserved/misplaced keyword, then a singleton expected punctuation token
> ({{Missing '('}}, {{Missing ':'}}, {{Missing '>'}}, ...), then *Unexpected
> end of input* for EOF.
> {{AstBuilder}} reports the actual varargs parameter name.
> h3. Expected result (after)
> * unclosed single-quoted string becomes *Unclosed string literal*
> * unclosed block comment becomes *Unclosed comment* (caret on the opener)
> * zero-width space in an identifier becomes {{Unexpected character:
> '\u200b'}}
> * {{if true}} without parentheses becomes {{Missing '('}}
> * {{x ? y}} becomes {{Missing ':'}}
> * a generic type missing {{>}} before {{(}} becomes {{Missing '>'}}
> * {{const x = 1}} becomes {{'const' is not supported; use 'val' or 'static
> final' instead}}
> * {{goto label}} becomes {{'goto' is not supported}}
> * stray {{else}} / {{catch}} / {{case}} become {{'else' without 'if'}} /
> {{'catch' without 'try'}} / {{'case' outside of switch}}
> * {{throw}} at EOF becomes *Unexpected end of input*
> * {{def n = 1_}} becomes *Number ending with underscores is invalid*
> (position once)
> * {{def m(int... a, int b)}} with a later parameter becomes {{The var-arg
> parameter a must be the last parameter}}
> Valid programs are unchanged. Successful parses never enter these helpers.
> h3. Related
> GROOVY-12169, GROOVY-12171, GROOVY-9588, GROOVY-10146
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
