[
https://issues.apache.org/jira/browse/GROOVY-10355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18106341#comment-18106341
]
ASF GitHub Bot commented on GROOVY-10355:
-----------------------------------------
daniellansun commented on PR #2817:
URL: https://github.com/apache/groovy/pull/2817#issuecomment-5357850974
A couple of things I wasn’t sure about — I may well have missed the intended
scope.
**`in` grouping.** `repairBinaryKeywordCast` uses the whole command argument
as the RHS. With `x = 1`, `list = [1]`:
- `x in list && true` is `(x in list) && true` → `true`
- `def r = (x) in list && true` becomes `x in (list && true)` → `false`
- `if ((x) in list && true)` takes the same path
On master the parenthesized command forms don’t compile. After the rewrite
they compile, but with the inverted meaning. `return (x) in list && true` is
already grouped the other way — `return` parses a normal expression, not a
command.
Would it be reasonable to peel `&&` / `==` / a second `in` off the RHS, the
way `combineRebalancing` does for `+/-`? If the sketch is only meant to cover a
simple name on the right, it might help to say so — the commit currently reads
as the full relational meaning.
**`as`.** `(x) as Long` works; `(x) as List<String>`, `(x) as String[]`, and
`(x) as Long ?: 0` still fail to resolve `x`. For `List<String>` the argument
is already a `ClassExpression` — `right.getType()` might be enough. The other
two look harder (expression vs `coercionType`).
**Source positions.** Inner nodes from `combineRebalancing` show up as
`line=-1` (e.g. the inner `+` in `"A" + (b) + "C"`). A `configureAST` on each
new `BinaryExpression` would probably sort that out.
**`CAST_RESOLVE_HINT`.** It’s a new public constant; the parser only needs
the string. `@Internal` (or a shared internal key) might keep it off the public
surface.
**Comments / tests.** The text says “lowercase-initial”; the check is
`!isUpperCase` (`_foo` is treated as a value). And the `in` grouping above
isn’t in the suite yet — something like `assert ((x) in list && true)` would
have caught it. `((x) as Long)` already works on master because of the extra
parens; `(x) as List<String>` as a statement would cover the `as` gap. `(p)++`
isn’t a cast, so it passes without the rewrite.
> Compiler interpret variable name as class name when in parentheses.
> --------------------------------------------------------------------
>
> Key: GROOVY-10355
> URL: https://issues.apache.org/jira/browse/GROOVY-10355
> Project: Groovy
> Issue Type: Bug
> Components: parser-antlr4
> Affects Versions: 3.0.0, 4.0.0, 5.0.0
> Environment: JDK 11.0.12
> Reporter: Olof Asbrink
> Priority: Major
> Attachments: GROOVY-10355-Assessment.pdf, screenshot-1.png,
> screenshot-2.png
>
>
> This behavior seems unexpected:
> {code:java}
> String b = "B"
> System.out.println("A" + (b) + "C")
> {code}
> Throws this exception:
> {code:java}
> org.codehaus.groovy.control.MultipleCompilationErrorsException: startup
> failed:
> /tmp/repo1.gm: 2: unable to resolve class b
> @ line 2, column 26.
> System.out.println("A" + (b) + "C")
> ^{code}
> However these examples work:
> {code:java}
> String b = "B"
> System.out.println("A" + b + "C")
> {code}
> and
> {code:java}
> String b = "B"
> System.out.println("A" + (b))
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
