openrewrite / openrewrite/rewrite
GroovyParser - errors on lambda arg without parens
Open
Nobody has claimed this yet.
bug
- Dominant language
- Java
- Stars
- 3.7k
- Forks
- 570
- Avg merge
- 13h 12m
- Merged PRs (30d)
- 261
Description
See unit test(s) below, with analysis from Claude.
/*
import org.junit.jupiter.api.Test
import org.junitpioneer.jupiter.ExpectedToFail
import org.openrewrite.groovy.Assertions.groovy
import org.openrewrite.test.RewriteTest
Description: `org.openrewrite.groovy.GroovyParser` mis-parses a Groovy command expression whose sole
argument is a lambda with a non-empty parameter list — `reqBoxes.forEach (String box) -> { ... }`,
i.e. a Java-style lambda passed without parentheses around the argument list. Groovy accepts this
(it is an ordinary command expression, and `groovyc` produces exactly the same AST as the fully
parenthesised `reqBoxes.forEach((String box) -> { ... })`), but the parser mistakes the lambda's
own parameter parentheses for the method call's argument-list parentheses.
Root cause (rewrite-groovy `GroovyParserVisitor.visitArgumentlistExpression`): whether the argument
list is parenthesised is decided purely by looking at the next non-whitespace character —
int saveCursor = cursor;
Space beforeOpenParen = whitespace();
boolean hasParentheses = true;
if (source.charAt(cursor) == '(') {
skip("(");
} else {
hasParentheses = false;
beforeOpenParen = EMPTY;
cursor = saveCursor;
}
For `forEach (String box) -> { ... }` that character is the `(` of the *lambda's parameter list*.
It gets consumed as the argument list's opening paren, so the container is recorded as parenthesised
and the cursor is left one paren out of step with the source for the remainder of the enclosing
construct.
Two symptoms follow, depending on what comes next:
(1) A single occurrence produces an LST that is not print-idempotent — printing emits a closing `)`
that does not exist in the source (`}` becomes `})`). `requirePrintEqualsInput` catches this and
downgrades the file to a `ParseError`, so recipes silently skip the file rather than corrupt it.
(2) Two or more occurrences in the same file compound the drift until it reaches
`visitMethodCallExpression` in a state where neither the invoked method name is at the cursor nor
`select` is a `J.Identifier`, at which point it hits
`throw new IllegalArgumentException("Unable to parse method call")`. That is the reported failure.
Only the combination breaks. A paren-less *closure* argument (`forEach { String box -> ... }`, the
idiomatic Groovy spelling) parses correctly, as does a paren-less lambda with an EMPTY parameter list
(`submit () -> { ... }`) and the fully parenthesised lambda. The trigger is specifically
"command expression + lambda + non-empty parameter list"; the parameter may be typed or untyped.
Expected behavior: `receiver.method (Type p) -> { ... }` should parse and round-trip unchanged,
producing the same LST as `receiver.method((Type p) -> { ... })` with `OmitParentheses` marked on the
argument container, exactly as already happens for a paren-less closure argument.
Actual behavior: the whole source file fails to parse and is dropped from the source set, so every
recipe in the run silently skips it. Reproduced verbatim (2026-08-27) against rewrite-groovy 8.88.0
— note the cursor context is byte-for-byte the reported one:
[WARNING] There were problems parsing src/test/groovy/.../IntegrationTestBase.groovy
org.openrewrite.groovy.GroovyParsingException: Failed to parse ... at cursor position 16009.
The surrounding characters in the original source are:
} else {
finalNoParents.add(box)
}
}
return ~cursor~>LockboxResponse.builder()
.parentLockboxes(finalLockboxList)
.noParentLockboxes(finalNoParents)
.build()
at org.openrewrite.groovy.GroovyParserVisitor.visit (GroovyParserVisitor.java:249)
Caused by: java.lang.IllegalArgumentException: Unable to parse method call
at ...RewriteGroovyVisitor.lambda$visitMethodCallExpression$17 (GroovyParserVisitor.java:2739)
at ...RewriteGroovyVisitor.insideParentheses (GroovyParserVisitor.java:1293)
at ...RewriteGroovyVisitor.visitMethodCallExpression (GroovyParserVisitor.java:2668)
Note the exception is misleading in two ways, which is why this took bisection to localise: the
reported cursor sits on a perfectly valid `LockboxResponse.builder()` chain that is NOT the problem,
and the stack names `visitMethodCallExpression` rather than the `visitArgumentlistExpression` call
that actually desynced the cursor several statements earlier.
Version first seen: NOT a regression and not BOM-specific. Reproduced identically — same exception,
same cause, same cursor offset — on org.openrewrite:rewrite-groovy 8.81.6, 8.85.0, 8.86.0, 8.87.0 and
8.88.0, i.e. on both io.moderne.recipe:moderne-recipe-bom 0.40.0 (→ rewrite-recipe-bom 3.35.0 →
rewrite-bom 8.87.0, the version this repo pins) and 0.41.0 (→ 3.36.0 → 8.88.0). The
`visitArgumentlistExpression` paren detection is unchanged on openrewrite/rewrite `main` as of commit
d512b673d, so the latest code is still affected.
External references:
- Root cause: rewrite-groovy `GroovyParserVisitor.visitArgumentlistExpression`, the
`if (source.charAt(cursor) == '(')` paren detection (line 1383 on openrewrite/rewrite main).
- Throw site for symptom (2): `GroovyParserVisitor.visitMethodCallExpression`, the
`throw new IllegalArgumentException("Unable to parse method call")` branch reached when `select` is
not a `J.Identifier` (line 2739 in rewrite-groovy 8.88.0 — the captured stack's line numbers match
that release exactly, including `insideParentheses` at 1293 and `labeled` at 1315).
- Groovy command expressions: https://docs.groovy-lang.org/latest/html/documentation/#_command_chain_expressions
- Not previously reported upstream. Searched openrewrite/rewrite issues and PRs on 2026-08-27 via the
- GitHub search API: `"Unable to parse method call"` returns only #7870, #4116 and #5730 (all closed,
all unrelated root causes) and zero PRs; `groovy lambda parenthes*`, `groovy "command expression"`
and `groovy OmitParentheses` return no matching issue. `moderneinc/customer-requests` is not
readable with this token, so it could not be checked.
Proposed fix (upstream): in `visitArgumentlistExpression`, do not treat the next `(` as the argument
list's opening paren when it is actually the parameter list of a lambda argument. The single
expression in the `ArgumentListExpression` is a `LambdaExpression` whose recorded start position can
be compared against the candidate paren's offset — if the paren lies at or after the lambda's start,
it belongs to the lambda and the argument list should be marked `OmitParentheses`, the same treatment
a paren-less closure argument already gets.
*/
class GroovyParenlessLambdaArgumentTest : RewriteTest {
/**
* Faithful reduction of the reported failure, trimmed from the affected `IntegrationTestBase.groovy`
* to the two stubbing blocks that matter. Verified to reproduce the reported cursor context
* byte-for-byte: `GroovyParsingException ... return ~cursor~>LockboxResponse.builder()` caused by
* `IllegalArgumentException: Unable to parse method call`. Single-argument `groovy(...)` asserts the
* source parses and round-trips unchanged. `@ExpectedToFail` because it throws instead.
*/
@Test
@ExpectedToFail("Two paren-less lambda arguments in one file desync the parser until it throws 'Unable to parse method call'. Needs upstream OpenRewrite fix")
fun reportedShapeWithTwoParenlessLambdaArgumentsShouldParse() {
rewriteRun(
groovy(
"""
import static org.mockito.ArgumentMatchers.anyInt
import static org.mockito.ArgumentMatchers.anyList
import static org.mockito.Mockito.when
abstract class IntegrationTestBase {
void mockRestCallMultipleV2(Map<String, Lockbox> lockboxIdToParentLockbox) {
when(preferenceClient.retrieveParentLockboxes(anyList(), anyInt())).thenAnswer {
Map<String, Lockbox> finalMap = new HashMap<>()
def finalNoParents = new ArrayList<String>()
List<String> reqBoxes = it.getArgument(0)
reqBoxes.forEach (String box) -> {
if (lockboxIdToParentLockbox.containsKey(box)) {
finalMap.put(box, lockboxIdToParentLockbox.get(box))
} else {
finalNoParents.add(box)
}
}
return ParentLockboxRequest.builder()
.lockboxIdToParentLockbox(finalMap)
.boxesWithoutParents(finalNoParents)
.build()
}
when(lockboxDAO.findLockboxesByLockBoxNumbers(anyList(), anyInt())).thenAnswer {
List<Lockbox> finalLockboxList = new ArrayList<Lockbox>()
def finalNoParents = new ArrayList<String>()
List<String> reqBoxes = it.getArgument(0)
reqBoxes.forEach (String box) -> {
if (lockboxIdToParentLockbox.containsKey(box)) {
finalLockboxList.add(lockboxIdToParentLockbox.get(box))
} else {
finalNoParents.add(box)
}
}
return LockboxResponse.builder()
.parentLockboxes(finalLockboxList)
.noParentLockboxes(finalNoParents)
.build()
}
}
}
""".trimIndent(),
),
)
}
/**
* The defect stripped to its minimum: one command expression with a lambda argument, nothing else.
* Nothing throws here — the LST is simply not print-idempotent, because the parser emits a closing
* `)` that is not in the source (`}` prints as `})`). `requirePrintEqualsInput` turns that into a
* `ParseError`, so the file is skipped rather than mangled on disk. This is the same desync that,
* repeated, escalates to the hard failure above. `@ExpectedToFail` because the source does not
* round-trip.
*/
@Test
@ExpectedToFail("A paren-less lambda argument prints a spurious closing paren, so the LST is not print-idempotent. Needs upstream OpenRewrite fix")
fun singleParenlessLambdaArgumentShouldRoundTrip() {
rewriteRun(
groovy(
"""
class A {
def f(List<String> reqBoxes) {
reqBoxes.forEach (String box) -> {
println box
}
}
}
""".trimIndent(),
),
)
}
/**
* Confirms the parameter's declared type is irrelevant — an untyped lambda parameter desyncs
* identically, because the trigger is the parameter list's parentheses, not what is inside them.
* `@ExpectedToFail` for the same reason as the typed case.
*/
@Test
@ExpectedToFail("An untyped paren-less lambda argument desyncs the parser the same way as a typed one. Needs upstream OpenRewrite fix")
fun parenlessLambdaArgumentWithUntypedParameterShouldRoundTrip() {
rewriteRun(
groovy(
"""
class A {
def f(List<String> reqBoxes) {
reqBoxes.forEach (box) -> {
println box
}
}
}
""".trimIndent(),
),
)
}
/**
* Baseline isolating the missing parentheses as the trigger: the identical lambda passed with an
* explicit argument list parses and round-trips cleanly. Passing.
*/
@Test
fun baselineParenthesisedLambdaArgumentParses() {
rewriteRun(
groovy(
"""
class A {
def f(List<String> reqBoxes) {
reqBoxes.forEach((String box) -> {
println box
})
}
}
""".trimIndent(),
),
)
}
/**
* Baseline isolating the *lambda* as the trigger: the idiomatic Groovy spelling of the same
* intent — a paren-less closure argument — parses and round-trips cleanly, so the parser already
* handles `OmitParentheses` on a command expression correctly. Passing. This is also the
* workaround to hand the affected team.
*/
@Test
fun baselineIdiomaticParenlessClosureArgumentParses() {
rewriteRun(
groovy(
"""
class A {
def f(List<String> reqBoxes) {
reqBoxes.forEach { String box ->
println box
}
}
}
""".trimIndent(),
),
)
}
/**
* Baseline isolating the *non-empty* parameter list as the trigger: a paren-less lambda whose
* parameter list is empty parses fine, because consuming its `()` as the argument list's parens
* happens to leave the cursor in the right place. Passing; together with the two baselines above
* this pins the defect to "command expression + lambda + non-empty parameter list".
*/
@Test
fun baselineParenlessLambdaWithEmptyParameterListParses() {
rewriteRun(
groovy(
"""
class A {
def f() {
submit () -> {
println "x"
}
}
}
""".trimIndent(),
),
)
}
}
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in GroovyParserVisitor.visitArgumentlistExpression, focusing on how command expressions distinguish the argument-list parentheses from a lambda's parameter parentheses. Run the three GroovyParenlessLambdaArgumentTest cases, including reportedShapeWithTwoParenlessLambdaArgumentsShouldParse and singleParenlessLambdaArgumentShouldRoundTrip. Done means typed and untyped paren-less lambda arguments parse, round-trip unchanged, and produce the same LST as the parenthesized form.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- groovy, java, kotlin
- Domain
- compilers, tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 75/100