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