diff --git a/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java b/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java index a0a1252d63..a4b4428153 100644 --- a/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java +++ b/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java @@ -2110,34 +2110,10 @@ public void visitDeclarationExpression(DeclarationExpression expression) { VariableExpression firstVar = (VariableExpression) tupleExpressions.get(0); TypeTree typeExpr = visitVariableExpressionType(firstVar); - Space beforeOpenParen = sourceBefore("("); - - List> tupleVars = new ArrayList<>(tupleExpressions.size()); - for (int i = 0; i < tupleExpressions.size(); i++) { - VariableExpression varExpr = (VariableExpression) tupleExpressions.get(i); - TypeTree innerType = visitVariableExpressionType(varExpr); - J.Identifier name = doVisit(varExpr); - J.VariableDeclarations.NamedVariable nv = new J.VariableDeclarations.NamedVariable( - randomId(), - name.getPrefix(), - Markers.EMPTY, - name.withPrefix(EMPTY), - emptyList(), - null, - typeMapping.variableType(name.getSimpleName(), innerType.getType())); - J.VariableDeclarations innerDecl = new J.VariableDeclarations( - randomId(), EMPTY, Markers.EMPTY, - emptyList(), emptyList(), - innerType, null, - singletonList(JRightPadded.build(nv))); - Space after = i < tupleExpressions.size() - 1 ? sourceBefore(",") : sourceBefore(")"); - tupleVars.add(JRightPadded.build(innerDecl).withAfter(after)); - } - - G.TupleExpression tupleDeclarator = new G.TupleExpression( - randomId(), EMPTY, Markers.EMPTY, - JContainer.build(beforeOpenParen, tupleVars, Markers.EMPTY), - null); + List tupleVarExprs = tupleExpressions.stream() + .map(e -> (VariableExpression) e) + .collect(toList()); + G.TupleExpression tupleDeclarator = parseTupleExpression(tupleVarExprs); J.VariableDeclarations.NamedVariable namedVariable = new J.VariableDeclarations.NamedVariable( randomId(), EMPTY, Markers.EMPTY, @@ -2799,11 +2775,28 @@ public void visitThrowStatement(ThrowStatement statement) { queue.add(new J.Throw(randomId(), fmt, Markers.EMPTY, doVisit(statement.getExpression()))); } - // the current understanding is that TupleExpression only exist as method invocation arguments. - // this is the reason behind the simplifying assumption that there is one expression, and it is - // a NamedArgumentListExpression. @Override public void visitTupleExpression(TupleExpression tuple) { + List expressions = tuple.getExpressions(); + + // A TupleExpression whose visible elements are all VariableExpression is the LHS of a + // destructuring assignment without `def`: (a, b) = expr. + // (Synthetic elements like the implicit outer-`this` are VariableExpression too but + // don't appear in source and must not influence this check.) + List visibleExpressions = expressions.stream() + .filter(GroovyParserVisitor.this::appearsInSource) + .collect(toList()); + boolean isDestructuringLhs = !visibleExpressions.isEmpty() && + visibleExpressions.stream().allMatch(e -> e instanceof VariableExpression); + if (isDestructuringLhs) { + List varExprs = visibleExpressions.stream() + .map(e -> (VariableExpression) e) + .collect(toList()); + queue.add(parseTupleExpression(varExprs)); + return; + } + + // TupleExpression as method invocation arguments: each element is a NamedArgumentListExpression int saveCursor = cursor; Space beforeOpenParen = whitespace(); @@ -2816,8 +2809,8 @@ public void visitTupleExpression(TupleExpression tuple) { cursor = saveCursor; } - List> args = new ArrayList<>(tuple.getExpressions().size()); - for (org.codehaus.groovy.ast.expr.Expression expression : tuple.getExpressions()) { + List> args = new ArrayList<>(expressions.size()); + for (org.codehaus.groovy.ast.expr.Expression expression : expressions) { // Skip synthetic args (e.g. the implicit outer-`this` Groovy adds when // constructing a non-static inner class from within the enclosing class) if (!appearsInSource(expression)) { @@ -2861,6 +2854,27 @@ public void visitTupleExpression(TupleExpression tuple) { queue.add(JContainer.build(beforeOpenParen, args, Markers.EMPTY)); } + private G.TupleExpression parseTupleExpression(List varExprs) { + Space beforeOpenParen = sourceBefore("("); + List> tupleVars = new ArrayList<>(varExprs.size()); + for (int i = 0; i < varExprs.size(); i++) { + VariableExpression varExpr = varExprs.get(i); + TypeTree innerType = visitVariableExpressionType(varExpr); + J.Identifier name = doVisit(varExpr); + J.VariableDeclarations.NamedVariable nv = new J.VariableDeclarations.NamedVariable( + randomId(), name.getPrefix(), Markers.EMPTY, + name.withPrefix(EMPTY), emptyList(), null, + typeMapping.variableType(name.getSimpleName(), innerType.getType())); + J.VariableDeclarations innerDecl = new J.VariableDeclarations( + randomId(), EMPTY, Markers.EMPTY, emptyList(), emptyList(), + innerType, null, singletonList(JRightPadded.build(nv))); + Space after = i < varExprs.size() - 1 ? sourceBefore(",") : sourceBefore(")"); + tupleVars.add(JRightPadded.build(innerDecl).withAfter(after)); + } + return new G.TupleExpression(randomId(), EMPTY, Markers.EMPTY, + JContainer.build(beforeOpenParen, tupleVars, Markers.EMPTY), null); + } + @Override public void visitTryCatchFinally(TryCatchStatement node) { Space prefix = sourceBefore("try"); diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/AssignmentTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/AssignmentTest.java index 064455c2d3..c220a5673c 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/AssignmentTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/AssignmentTest.java @@ -343,4 +343,17 @@ void destructuringAssignmentNoSpacesAroundEquals() { ); } + @Test + void destructuringAssignmentToExistingVariables() { + rewriteRun( + groovy( + """ + def key = 'k' + def elemAttrs = [:] + (key, elemAttrs) = ['x', [:]] + """ + ) + ); + } + }