Skip to content

Commit a922f22

Browse files
authored
Fix GroovyParserVisitor crash on multi-variable field declarations (#7467)
* Fix `GroovyParserVisitor` crash on multi-variable field declarations Groovy emits a separate `FieldNode` for each variable in a multi-variable field declaration (e.g., `final String a, b`). The parser was treating each one as a complete declaration and re-consuming the modifiers and type tokens, desynchronizing the cursor and eventually throwing `StringIndexOutOfBoundsException` when parsing later nodes. Detect continuation fields in `visitVariableField` and emit a `MultiVariable` marker with an empty type expression, mirroring the existing handling for local multi-variable declarations like `def a = 1, b = 2`. * Simplify multi-variable field handling by reusing `maybeMultiVariable` * Keep `maybeMultiVariable` in `RewriteGroovyVisitor` Call it through the existing `visitor` instance from `visitVariableField` to avoid displacing the `sourceBefore` javadoc. * Revert stray whitespace change in `ConstructorTest`
1 parent 8912b00 commit a922f22

2 files changed

Lines changed: 28 additions & 4 deletions

File tree

rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -586,9 +586,19 @@ private J.EnumValue visitEnumField(FieldNode field) {
586586
private void visitVariableField(FieldNode field) {
587587
RewriteGroovyVisitor visitor = new RewriteGroovyVisitor(field, this);
588588

589-
List<J.Annotation> annotations = visitAndGetAnnotations(field, this);
590-
List<J.Modifier> modifiers = getModifiers();
591-
TypeTree typeExpr = field.isDynamicTyped() ? null : visitTypeTree(field.getOriginType());
589+
// Groovy emits a separate FieldNode per variable in a multi-variable declaration (e.g. `final String a, b`);
590+
// continuation fields have no modifiers or type keyword in the source, so skip past just the comma.
591+
Optional<MultiVariable> multiVariable = visitor.maybeMultiVariable();
592+
List<J.Annotation> annotations = multiVariable.isPresent() ? emptyList() : visitAndGetAnnotations(field, this);
593+
List<J.Modifier> modifiers = multiVariable.isPresent() ? emptyList() : getModifiers();
594+
TypeTree typeExpr;
595+
if (field.isDynamicTyped()) {
596+
typeExpr = null;
597+
} else if (multiVariable.isPresent()) {
598+
typeExpr = new J.Identifier(randomId(), EMPTY, Markers.EMPTY, emptyList(), "", typeMapping.type(field.getOriginType()), null);
599+
} else {
600+
typeExpr = visitTypeTree(field.getOriginType());
601+
}
592602

593603
J.Identifier name = new J.Identifier(randomId(), sourceBefore(field.getName()), Markers.EMPTY,
594604
emptyList(), field.getName(), typeMapping.type(field.getOriginType()), typeMapping.variableType(field));
@@ -612,7 +622,7 @@ private void visitVariableField(FieldNode field) {
612622
J.VariableDeclarations variableDeclarations = new J.VariableDeclarations(
613623
randomId(),
614624
EMPTY,
615-
Markers.EMPTY,
625+
multiVariable.<Markers>map(mv -> Markers.build(singleton(mv))).orElse(Markers.EMPTY),
616626
annotations,
617627
modifiers,
618628
typeExpr,

rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/VariableDeclarationsTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,20 @@ void multipleTypeMultipleVariableDeclaration() {
125125
);
126126
}
127127

128+
@Issue("https://github.com/openrewrite/rewrite/issues/7463")
129+
@Test
130+
void multipleVariableFieldDeclaration() {
131+
rewriteRun(
132+
groovy(
133+
"""
134+
class A {
135+
final String first, second
136+
}
137+
"""
138+
)
139+
);
140+
}
141+
128142
@Test
129143
void genericVariableDeclaration() {
130144
rewriteRun(

0 commit comments

Comments
 (0)