daniellansun commented on PR #2877:
URL: https://github.com/apache/groovy/pull/2877#issuecomment-5558264020
> Looking good with some minor aspects:
>
> * Minor merge conflict in `ErrorStrategyTest` to pass new lintGate
> * A closed string with an illegal escape is now misreported. x =
"C:\Users\me" gives "Unclosed string literal", where master gave the unhelpful
but not wrong "Unexpected character". I don't see this as a blocker but would
be nice to fix if possible. An AI assessment proposes: _a scan-ahead in
`AbstractLexer.unexpectedCharacterMessage` to report an illegal escape instead_.
> * AI assessment but I agree we probably just don't need to make the claim:
_The adoc and javadoc claim val is "preferred over final in Groovy 6". The spec
does not say that anywhere else, and val is behind a system property (default
on). Suggest dropping the "preferred" clause._
@paulk-asert Thanks Paul — all three notes were on target. Addressed as
follows.
## lintGate / `ErrorStrategyTest`
> Minor merge conflict in `ErrorStrategyTest` to pass new lintGate
`parser(...)` still used fully qualified `org.antlr.v4.runtime.CharStream` /
`ANTLRErrorStrategy` even though both types are imported. Master’s lintGate
(`FullyQualifiedName`) rejects that. Switched the parameters to the simple
names, matching master.
Also cleaned the same gate on this PR’s javadoc (`{@link SyntaxException}`
instead of a package-qualified `{@link}`) and a `java.util.Arrays` FQN in
`AbstractLexerTest`. `:checkstyleGate` is green locally.
## Illegal escape in a closed string
> `x = "C:\Users\me"` gives "Unclosed string literal", where master gave
> "Unexpected character"
Agreed — the literal *is* closed; `\U` is just not a Groovy `EscapeSequence`,
so `StringLiteral` fails and the opener was reported as unclosed.
On the `UNEXPECTED_CHAR` path only, we now scan-ahead from the quote (same
rules as `GroovyLexer.g4` `EscapeSequence`: `\btnfrs"'\\`, `\$`, octal,
backslash-u plus four ASCII hex digits, line continuation). If we hit an
illegal `\`, the diagnostic is `Illegal escape character: '\U'` with the
caret
on the backslash. A quote with no such escape is still
`Unclosed string literal`. Successful tokenisation never runs the scan.
An unclosed quote that also contains an illegal escape is reported as the
illegal escape (left-to-right first error) rather than as unclosed.
## `val` “preferred over `final`”
> The spec does not say that anywhere else, and val is behind a system
> property (default on). Suggest dropping the "preferred" clause.
Dropped. The `const` diagnostic still points at `val` (locals) and
`static final` (class constants); the adoc and the `keywordMessage` javadoc
no longer call `val` preferred. `core-syntax.adoc` now also notes that an
unknown letter escape such as `\U` in `"C:\Users"` is `Illegal escape
character`.
Happy to take another look if anything still feels off.
--
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]
