Copilot commented on code in PR #2944:
URL: https://github.com/apache/groovy/pull/2944#discussion_r4220037277
##########
src/antlr/GroovyLexer.g4:
##########
@@ -289,13 +304,24 @@ options {
// ยง3.10.5 String Literals
+// A quote that never closes is UNEXPECTED_CHAR: nothing else matches it.
+// `/` is also DIV, so an unclosed `/.../` is an EOF alternative of this rule.
+// isUnclosedSlashy() is the first edge of that alternative, before any body
+// character is read. `*` is safe because the follower is EOF, not `/`:
+// `//` is not an empty slashy. EOF stays here so a second rule cannot
+// swallow source after a real closer.
Review Comment:
This comment reads as if unclosed quoted strings are handled purely as
`UNEXPECTED_CHAR`, while the PR adds/extends tests expecting `Unclosed string
literal` messaging. If unclosed quotes are intentionally diagnosed via
`UNEXPECTED_CHAR` (but mapped to `Unclosed string literal` elsewhere), consider
clarifying that mapping here to avoid misleading future maintainers.
##########
src/antlr/GroovyLexer.g4:
##########
@@ -48,6 +48,8 @@ options {
private boolean errorIgnored;
private long tokenIndex;
private int lastTokenType;
+ /** Last default-channel token whose type is not {@code NL}. */
+ private int lastRealTokenType;
Review Comment:
`lastRealTokenType` is used to make lexing decisions but is not initialized
here. That makes behavior depend on the Java default (0 / `Token.INVALID_TYPE`)
or any prior state if the lexer instance is reused. Initializing (and resetting
alongside `lastTokenType`) to a well-defined value like `Token.EOF` would make
`isUnclosedSlashy()` deterministic in leading-newline / fresh-input scenarios.
##########
src/antlr/GroovyLexer.g4:
##########
@@ -109,6 +114,16 @@ options {
return !REGEX_CHECK_SET.get(this.lastTokenType);
}
+ /**
+ * A {@code /} that reaches EOF is an unclosed slashy string only when the
+ * previous real token (ignoring {@code NL}) could not be the left operand
+ * of division. {@code 9 \n / \n 3} stays division; {@code s = /hello}
does not.
+ */
+ private boolean isUnclosedSlashy() {
+ int decision = this.lastTokenType == NL ? this.lastRealTokenType :
this.lastTokenType;
+ return !REGEX_CHECK_SET.get(decision);
+ }
Review Comment:
`lastRealTokenType` is used to make lexing decisions but is not initialized
here. That makes behavior depend on the Java default (0 / `Token.INVALID_TYPE`)
or any prior state if the lexer instance is reused. Initializing (and resetting
alongside `lastTokenType`) to a well-defined value like `Token.EOF` would make
`isUnclosedSlashy()` deterministic in leading-newline / fresh-input scenarios.
##########
src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java:
##########
@@ -2594,11 +2594,13 @@ public Expression visitCommandExpression(final
CommandExpressionContext ctx) {
&& baseExpr instanceof BinaryExpression bin
&& !"[".equals(bin.getOperation().getText())) {
// `List<Integer name` is parsed as the comparison `List <
Integer` plus a
- // command argument. javac: `'>' expected`.
+ // following argument list. javac: `'>' expected`. Unmatched `>`
in the
+ // error strategy keeps ANTLR's offender; point at that list, not
at `List`.
+ // An identifier after `T < U` is argumentList, not
commandArgument.
if ("<".equals(bin.getOperation().getText())
&& looksLikeTypeName(bin.getLeftExpression())
&& looksLikeTypeName(bin.getRightExpression())) {
- throw createParsingFailedException("Missing '>'", bin);
+ throw createParsingFailedException("Missing '>'",
ctx.enhancedArgumentListInPar());
Review Comment:
`ctx.enhancedArgumentListInPar()` may be null for some command-expression
shapes, which would risk an NPE (or a mis-positioned error) inside
`createParsingFailedException`. Consider guarding and falling back to a
non-null anchor (e.g., `ctx.expression()` or `bin`) when
`enhancedArgumentListInPar()` is absent, while still preferring the
argument-list node when it exists.
--
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]
