[
https://issues.apache.org/jira/browse/GROOVY-10355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18106475#comment-18106475
]
Paul King edited comment on GROOVY-10355 at 8/21/26 4:35 AM:
-------------------------------------------------------------
Thanks for the careful review [~sunlan] - you were right on essentially
everything, and the {{in}} grouping catch was an important one. All points are
now addressed on the branch. Point by point:
*{{in}} grouping.* Confirmed, and worse than a missing feature: on master those
forms don't compile, so the sketch was turning a compile error into a silently
inverted answer ({{\(x) in list && true}} evaluated to {{false}}, and the
{{if}} form took the wrong branch; a chained {{\(x) in a in b}} inverted the
same way). Your {{return}} observation is also right - {{return}} parses a
plain expression, so it grouped correctly all along. Fixed along the lines you
suggested, generalized: the repair now splices the binary reading in at
relational precedence, descending the argument's left spine through any
operator binding no tighter than relational (equality, regex find/match,
bitwise, logical, ternary/elvis, a further relational or coercion) and
repairing at the leaf:
{code:groovy}
def x = 1, list = [1]
assert ((x) in list && true) // (x in list) && true
assert ((x) in list == true) // (x in list) == true
assert (x) in [1] in [true] // left-associative, like the
unparenthesized form
assert ((x) in list ? 'y' : 'n') == 'y'
assert ((x) in [] ?: 'none') == 'none'
{code}
Tighter-binding right-hand sides such as {{\(x) in 1..5}} correctly stay whole.
To answer your scope question directly: the intent is the full relational
meaning, not just a simple name on the right - it is now implemented and tested
that way.
*{{as}}.* All three confirmed and fixed. {{List<String>}} arrives as a
{{ClassExpression}} as you said, so the repair now uses {{right.getType()}}
(generics preserved). {{String[]}} turns out to arrive as an empty subscript on
the element type, so that shape is read as an array type (recursively, so
{{String[][]}} works too). {{\(x) as Long ?: 0}} falls out of the same splice
as {{(\(x) as Long) ?: 0}}. Anything that still can't be read as a type bails
out to a compile error rather than guessing - and those bail-outs now carry an
explanatory hint as well (see below).
*Source positions.* Confirmed and fixed - every node {{combineRebalancing}}
(and the new splice) creates or rotates now gets {{configureAST}} spanning
left-operand start to right-operand end, and an AST-walking test asserts no
{{-1}} positions remain. While in there I also tightened the unary/cast wrapper
spans ({{-(a)}} in {{-(a) - 3}} previously spanned the whole expression).
*{{CAST_RESOLVE_HINT}}.* Now annotated {{@Internal}}.
*Comments/wording.* You read the check correctly - the behaviour is the
intended one (the convention encoded is "class names start with an uppercase
letter", so {{_foo}} and {{$foo}} read as values), but the comments said
otherwise. I renamed the method to {{hasValueConventionFinalSegment}}, fixed
the comments, and added {{(_foo) + 1}} / {{($bar) - 1}} tests.
*Tests.* Your exact {{assert (\(x) in list && true)}} is in the suite now (I
verified it failed before the fix), along with the grouping-equivalence family
and the three {{as}} shapes as statements. One correction though: {{(\(x) as
Long)}} does not work on master. I checked released 5.0.6, 5.1.0, 6.0.0-alpha-1
and 6.0.0-beta-2, in both dynamic and {{@CompileStatic}} mode, and all fail
with {{unable to resolve class x}}. The outer parentheses don't rescue it
because {{expressionInPar}} accepts a command expression, so the inner {{\(x)
as Long}} takes the same mis-parse path. That assertion is therefore genuine
repair coverage and I've kept it. Agreed on {{(p)++}} - it passes on master
unchanged and stays purely as a regression guard.
While verifying I found two further families that failed with a bare {{unable
to resolve class x}} and no hint: shapes where the repair bails out (e.g.
{{\(x) as 3}}), which now get a shape-specific hint explaining how the
expression was read, and parenthesized right-hand sides like {{\(x) in
(list)}}, which parse as a method call of the keyword identifier on the cast -
now covered by adding {{MethodCallExpression}} to the ambiguous-operand set,
and for those the {{(\(x))}} workaround the hint suggests genuinely works.
({{\(x) in [1, 2] && true}} is fine, incidentally - the cast only captures the
unary-level {{in [1, 2]}}, so the trailing operator lands on the repaired
binary.)
The suite is now 18 compile-and-run tests asserting evaluated results, and the
full antlr4 parser test group is green, including {{SyntaxErrorTest}}, which
asserts error-message content and so exercises the hint changes.
was (Author: paulk):
Thanks for the careful review [~sunlan] - you were right on essentially
everything, and the {{in}} grouping catch was an important one. All points are
now addressed on the branch. Point by point:
*{{in}} grouping.* Confirmed, and worse than a missing feature: on master those
forms don't compile, so the sketch was turning a compile error into a silently
inverted answer ({{(x) in list && true}} evaluated to {{false}}, and the {{if}}
form took the wrong branch; a chained {{(x) in a in b}} inverted the same way).
Your {{return}} observation is also right - {{return}} parses a plain
expression, so it grouped correctly all along. Fixed along the lines you
suggested, generalized: the repair now splices the binary reading in at
relational precedence, descending the argument's left spine through any
operator binding no tighter than relational (equality, regex find/match,
bitwise, logical, ternary/elvis, a further relational or coercion) and
repairing at the leaf:
{code:groovy}
def x = 1, list = [1]
assert ((x) in list && true) // (x in list) && true
assert ((x) in list == true) // (x in list) == true
assert (x) in [1] in [true] // left-associative, like the
unparenthesized form
assert ((x) in list ? 'y' : 'n') == 'y'
assert ((x) in [] ?: 'none') == 'none'
{code}
Tighter-binding right-hand sides such as {{(x) in 1..5}} correctly stay whole.
To answer your scope question directly: the intent is the full relational
meaning, not just a simple name on the right - it is now implemented and tested
that way.
*{{as}}.* All three confirmed and fixed. {{List<String>}} arrives as a
{{ClassExpression}} as you said, so the repair now uses {{right.getType()}}
(generics preserved). {{String[]}} turns out to arrive as an empty subscript on
the element type, so that shape is read as an array type (recursively, so
{{String[][]}} works too). {{(x) as Long ?: 0}} falls out of the same splice as
{{((x) as Long) ?: 0}}. Anything that still can't be read as a type bails out
to a compile error rather than guessing - and those bail-outs now carry an
explanatory hint as well (see below).
*Source positions.* Confirmed and fixed - every node {{combineRebalancing}}
(and the new splice) creates or rotates now gets {{configureAST}} spanning
left-operand start to right-operand end, and an AST-walking test asserts no
{{-1}} positions remain. While in there I also tightened the unary/cast wrapper
spans ({{-(a)}} in {{-(a) - 3}} previously spanned the whole expression).
*{{CAST_RESOLVE_HINT}}.* Now annotated {{@Internal}}.
*Comments/wording.* You read the check correctly - the behaviour is the
intended one (the convention encoded is "class names start with an uppercase
letter", so {{_foo}} and {{$foo}} read as values), but the comments said
otherwise. I renamed the method to {{hasValueConventionFinalSegment}}, fixed
the comments, and added {{(_foo) + 1}} / {{($bar) - 1}} tests.
*Tests.* Your exact {{assert ((x) in list && true)}} is in the suite now (I
verified it failed before the fix), along with the grouping-equivalence family
and the three {{as}} shapes as statements. One correction though: {{((x) as
Long)}} does not work on master. I checked released 5.0.6, 5.1.0, 6.0.0-alpha-1
and 6.0.0-beta-2, in both dynamic and {{@CompileStatic}} mode, and all fail
with {{unable to resolve class x}}. The outer parentheses don't rescue it
because {{expressionInPar}} accepts a command expression, so the inner {{(x) as
Long}} takes the same mis-parse path. That assertion is therefore genuine
repair coverage and I've kept it. Agreed on {{(p)++}} - it passes on master
unchanged and stays purely as a regression guard.
While verifying I found two further families that failed with a bare {{unable
to resolve class x}} and no hint: shapes where the repair bails out (e.g. {{(x)
as 3}}), which now get a shape-specific hint explaining how the expression was
read, and parenthesized right-hand sides like {{(x) in (list)}}, which parse as
a method call of the keyword identifier on the cast - now covered by adding
{{MethodCallExpression}} to the ambiguous-operand set, and for those the
{{((x))}} workaround the hint suggests genuinely works. ({{(x) in [1, 2] &&
true}} is fine, incidentally - the cast only captures the unary-level {{in [1,
2]}}, so the trailing operator lands on the repaired binary.)
The suite is now 18 compile-and-run tests asserting evaluated results, and the
full antlr4 parser test group is green, including {{SyntaxErrorTest}}, which
asserts error-message content and so exercises the hint changes.
> 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)
