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.
--
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]
