diff --git a/rewrite-gradle/src/main/java/org/openrewrite/gradle/AddDependencyVisitor.java b/rewrite-gradle/src/main/java/org/openrewrite/gradle/AddDependencyVisitor.java index 61659730458..64e58977947 100644 --- a/rewrite-gradle/src/main/java/org/openrewrite/gradle/AddDependencyVisitor.java +++ b/rewrite-gradle/src/main/java/org/openrewrite/gradle/AddDependencyVisitor.java @@ -290,22 +290,10 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu } } else { Space originalPrefix = addDependencyInvocation.getPrefix(); - if (currentStatement instanceof J.VariableDeclarations) { - J.VariableDeclarations variableDeclarations = (J.VariableDeclarations) currentStatement; - if (variableDeclarations.getTypeExpression() != null) { - addDependencyInvocation = addDependencyInvocation.withPrefix(variableDeclarations.getTypeExpression().getPrefix()); - } - } else { - addDependencyInvocation = addDependencyInvocation.withPrefix(currentStatement.getPrefix()); - } + addDependencyInvocation = addDependencyInvocation.withPrefix(currentStatement.getPrefix()); if (addDependencyInvocation.getSimpleName().equals(beforeDependency.getSimpleName())) { - if (currentStatement instanceof J.VariableDeclarations) { - J.VariableDeclarations variableDeclarations = (J.VariableDeclarations) currentStatement; - if (variableDeclarations.getTypeExpression() != null && !variableDeclarations.getTypeExpression().getPrefix().equals(originalPrefix)) { - statements.set(i, variableDeclarations.withTypeExpression(variableDeclarations.getTypeExpression().withPrefix(originalPrefix))); - } - } else if (!currentStatement.getPrefix().equals(originalPrefix)) { + if (!currentStatement.getPrefix().equals(originalPrefix)) { statements.set(i, currentStatement.withPrefix(originalPrefix)); } } 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 e14a122df1f..abf35207e2e 100644 --- a/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java +++ b/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java @@ -39,9 +39,9 @@ import org.openrewrite.java.marker.OmitParentheses; import org.openrewrite.java.marker.Semicolon; import org.openrewrite.java.marker.TrailingComma; -import org.openrewrite.java.tree.*; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.Statement; +import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; import java.math.BigDecimal; @@ -113,16 +113,16 @@ private static boolean isOlderThanGroovy3() { } /** - * Groovy methods can be declared with "def" AND a return type - * In these cases the "def" is semantically meaningless but needs to be preserved for source code accuracy - * If there is both a def and a return type, this method returns a RedundantDef object and advances the cursor - * position past the "def" keyword, leaving the return type to be parsed as normal. - * In any other situation an empty Optional is returned and the cursor is not advanced. + * Groovy methods can be declared with "def" AND a return type + * In these cases the "def" is semantically meaningless but needs to be preserved for source code accuracy + * If there is both a def and a return type, this method returns a RedundantDef object and advances the cursor + * position past the "def" keyword, leaving the return type to be parsed as normal. + * In any other situation an empty Optional is returned and the cursor is not advanced. */ private Optional maybeRedundantDef(ClassNode type, String name) { int saveCursor = cursor; Space defPrefix = whitespace(); - if(source.startsWith("def", cursor)) { + if (source.startsWith("def", cursor)) { skip("def"); // The def is redundant only when it is followed by the method's return type // I hope no one puts an annotation between "def" and the return type @@ -182,8 +182,8 @@ public G.CompilationUnit visit(SourceUnit unit, ModuleNode ast) throws GroovyPar for (ClassNode aClass : ast.getClasses()) { if (aClass.getSuperClass() == null || !("groovy.lang.Script".equals(aClass.getSuperClass().getName()) || - "RewriteGradleProject".equals(aClass.getSuperClass().getName()) || - "RewriteSettings".equals(aClass.getSuperClass().getName()))) { + "RewriteGradleProject".equals(aClass.getSuperClass().getName()) || + "RewriteSettings".equals(aClass.getSuperClass().getName()))) { sortedByPosition.computeIfAbsent(pos(aClass), i -> new ArrayList<>()).add(aClass); } } @@ -822,8 +822,8 @@ public void visitArgumentlistExpression(ArgumentListExpression expression) { // When named parameters are in use they may appear before, after, or intermixed with any positional arguments if (unparsedArgs.size() > 1 && unparsedArgs.get(0) instanceof MapExpression && (unparsedArgs.get(0).getLastLineNumber() > unparsedArgs.get(1).getLastLineNumber() || - (unparsedArgs.get(0).getLastLineNumber() == unparsedArgs.get(1).getLastLineNumber() && - unparsedArgs.get(0).getLastColumnNumber() > unparsedArgs.get(1).getLastColumnNumber()))) { + (unparsedArgs.get(0).getLastLineNumber() == unparsedArgs.get(1).getLastLineNumber() && + unparsedArgs.get(0).getLastColumnNumber() > unparsedArgs.get(1).getLastColumnNumber()))) { // Figure out the source-code ordering of the expressions MapExpression namedArgExpressions = (MapExpression) unparsedArgs.get(0); @@ -1099,7 +1099,7 @@ public void visitBlockStatement(BlockStatement block) { J expr = visit(statement); if (i == blockStatements.size() - 1 && (expr instanceof Expression)) { if (parent instanceof ClosureExpression || (parent instanceof MethodNode && - !JavaType.Primitive.Void.equals(typeMapping.type(((MethodNode) parent).getReturnType())))) { + !JavaType.Primitive.Void.equals(typeMapping.type(((MethodNode) parent).getReturnType())))) { expr = new J.Return(randomId(), expr.getPrefix(), Markers.EMPTY, expr.withPrefix(EMPTY)); expr = expr.withMarkers(expr.getMarkers().add(new ImplicitReturn(randomId()))); @@ -1438,7 +1438,10 @@ public void visitNotExpression(NotExpression expression) { @Override public void visitDeclarationExpression(DeclarationExpression expression) { + Space prefix = whitespace(); Optional redundantDef = maybeRedundantDef(expression.getVariableExpression().getType(), expression.getVariableExpression().getName()); + Optional multiVariable = maybeMultiVariable(); + List modifiers = getModifiers(); TypeTree typeExpr = visitVariableExpressionType(expression.getVariableExpression()); J.VariableDeclarations.NamedVariable namedVariable; @@ -1466,19 +1469,60 @@ public void visitDeclarationExpression(DeclarationExpression expression) { J.VariableDeclarations variableDeclarations = new J.VariableDeclarations( randomId(), - EMPTY, - redundantDef.map(Markers.EMPTY::add).orElse(Markers.EMPTY), - emptyList(), + prefix, + Markers.EMPTY, emptyList(), + modifiers, typeExpr, null, emptyList(), singletonList(JRightPadded.build(namedVariable)) ); + if (redundantDef.isPresent()) { + variableDeclarations = variableDeclarations.withMarkers(variableDeclarations.getMarkers().add(redundantDef.get())); + } + if (multiVariable.isPresent()) { + variableDeclarations = variableDeclarations.withMarkers(variableDeclarations.getMarkers().add(multiVariable.get())); + } queue.add(variableDeclarations); } + private Optional maybeMultiVariable() { + int saveCursor = cursor; + Space commaPrefix = whitespace(); + if (source.startsWith(",", cursor)) { + skip(","); + return Optional.of(new MultiVariable(randomId(), commaPrefix)); + } + cursor = saveCursor; + return Optional.empty(); + } + + private List getModifiers() { + List modifiers = new ArrayList<>(); + int saveCursor = cursor; + Space prefix = whitespace(); + while (source.startsWith("def", cursor) || source.startsWith("var", cursor) || source.startsWith("final", cursor)) { + if (source.startsWith("var", cursor)) { + modifiers.add(new J.Modifier(randomId(), prefix, Markers.EMPTY, "var", J.Modifier.Type.LanguageExtension, emptyList())); + cursor += 3; + } else if (source.startsWith("def", cursor)) { + modifiers.add(new J.Modifier(randomId(), prefix, Markers.EMPTY, "def", J.Modifier.Type.LanguageExtension, emptyList())); + cursor += 3; + } else if (source.startsWith("final", cursor)) { + modifiers.add(new J.Modifier(randomId(), prefix, Markers.EMPTY, "final", J.Modifier.Type.LanguageExtension, emptyList())); + cursor += 5; + } else { + break; + } + saveCursor = cursor; + prefix = whitespace(); + } + cursor = saveCursor; + return modifiers; + } + @Override public void visitEmptyExpression(EmptyExpression expression) { queue.add(new J.Empty(randomId(), EMPTY, Markers.EMPTY)); @@ -1766,7 +1810,7 @@ public void visitMethodCallExpression(MethodCallExpression call) { MethodNode methodNode = (MethodNode) call.getNodeMetaData().get(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET); JavaType.Method methodType = null; if (methodNode == null && call.getObjectExpression() instanceof VariableExpression && - ((VariableExpression) call.getObjectExpression()).getAccessedVariable() != null) { + ((VariableExpression) call.getObjectExpression()).getAccessedVariable() != null) { // Groovy doesn't know what kind of object this method is being invoked on // But if this invocation is inside a Closure we may have already enriched its parameters with types from the static type checker // Use any such type information to attempt to find a matching method @@ -2097,29 +2141,18 @@ public void visitPrefixExpression(PrefixExpression unary) { public TypeTree visitVariableExpressionType(VariableExpression expression) { JavaType type = typeMapping.type(staticType(((org.codehaus.groovy.ast.expr.Expression) expression))); + Space prefix = whitespace(); + String typeName = ""; - if (expression.isDynamicTyped()) { - Space prefix = whitespace(); - String keyword; - if (source.substring(cursor).startsWith("final")) { - keyword = "final"; - } else { - keyword = source.substring(cursor, cursor + 3); - } - cursor += keyword.length(); - return new J.Identifier(randomId(), - prefix, - Markers.EMPTY, - emptyList(), - keyword, - type, null); + if (!expression.isDynamicTyped() && source.startsWith(expression.getOriginType().getUnresolvedName(), cursor)) { + typeName = expression.getOriginType().getUnresolvedName(); + cursor += typeName.length(); } - Space prefix = sourceBefore(expression.getOriginType().getUnresolvedName()); J.Identifier ident = new J.Identifier(randomId(), EMPTY, Markers.EMPTY, emptyList(), - expression.getOriginType().getUnresolvedName(), + typeName, type, null); if (expression.getOriginType().getGenericsTypes() != null) { return new J.ParameterizedType(randomId(), prefix, Markers.EMPTY, ident, visitTypeParameterizations( @@ -2671,7 +2704,7 @@ private String name() { Can contain a J.FieldAccess, as in a variable declaration with fully qualified type parameterization: List */ - private JContainer visitTypeParameterizations(GenericsType @Nullable[] genericsTypes) { + private JContainer visitTypeParameterizations(GenericsType @Nullable [] genericsTypes) { Space prefix = sourceBefore("<"); List> parameters; diff --git a/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyPrinter.java b/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyPrinter.java index 17e0a466a6c..0ab2bb60c2c 100644 --- a/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyPrinter.java +++ b/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyPrinter.java @@ -265,13 +265,17 @@ public J visitVariableDeclarations(J.VariableDeclarations multiVariable, PrintOu beforeSyntax(multiVariable, Space.Location.VARIABLE_DECLARATIONS_PREFIX, p); visitSpace(Space.EMPTY, Space.Location.ANNOTATIONS, p); visit(multiVariable.getLeadingAnnotations(), p); - for (J.Modifier m : multiVariable.getModifiers()) { - visitModifier(m, p); - } multiVariable.getMarkers().findFirst(RedundantDef.class).ifPresent(def -> { visitSpace(def.getPrefix(), Space.Location.LANGUAGE_EXTENSION, p); p.append("def"); }); + for (J.Modifier m : multiVariable.getModifiers()) { + visitModifier(m, p); + } + multiVariable.getMarkers().findFirst(MultiVariable.class).ifPresent(multiVar -> { + visitSpace(multiVar.getPrefix(), Space.Location.NAMED_VARIABLE_SUFFIX, p); + p.append(","); + }); visit(multiVariable.getTypeExpression(), p); // For backwards compatibility. for (JLeftPadded dim : multiVariable.getDimensionsBeforeName()) { diff --git a/rewrite-groovy/src/main/java/org/openrewrite/groovy/marker/MultiVariable.java b/rewrite-groovy/src/main/java/org/openrewrite/groovy/marker/MultiVariable.java new file mode 100644 index 00000000000..3554529bf6b --- /dev/null +++ b/rewrite-groovy/src/main/java/org/openrewrite/groovy/marker/MultiVariable.java @@ -0,0 +1,30 @@ +/* + * Copyright 2024 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.groovy.marker; + +import lombok.Value; +import lombok.With; +import org.openrewrite.java.tree.Space; +import org.openrewrite.marker.Marker; + +import java.util.UUID; + +@Value +@With +public class MultiVariable implements Marker { + UUID id; + Space prefix; +} 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 8a15073a41b..e1421259a9d 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 @@ -24,6 +24,41 @@ @SuppressWarnings({"GroovyUnusedAssignment", "GrUnnecessarySemicolon"}) class AssignmentTest implements RewriteTest { + @Test + void noKeyword() { + rewriteRun( + groovy( + """ + x = "s" + """ + ) + ); + } + + @Test + void simple() { + rewriteRun( + groovy( + """ + def x = "s" + """ + ) + ); + } + + @Test + void simpleWithFinal() { + rewriteRun( + groovy( + """ + final def x = "x" + def final y = "y" + final z = "z" + """ + ) + ); + } + @Test void concat() { rewriteRun( @@ -91,4 +126,52 @@ void baseNConversions() { ) ); } + + @Test + void multipleAssignmentsAtOneLine() { + rewriteRun( + groovy( + """ + def startItem = '| ', endItem = ' |' + def repeatLength = startItem.length() + output.length() + endItem.length() + println("\\n" + ("-" * repeatLength) + "\\n" + startItem + output + endItem + "\\n" + ("-" * repeatLength)) + """ + ) + ); + } + + @Test + void multipleAssignmentsAtOneLineSimple() { + rewriteRun( + groovy( + """ + def a = '1', b = '2' + """ + ) + ); + } + + @Test + void multipleAssignmentsAtMultipleLineDynamicType() { + rewriteRun( + groovy( + """ + def a = '1' , + b = '2' + """ + ) + ); + } + + @Test + void multipleAssignmentsAtMultipleLineStaticType() { + rewriteRun( + groovy( + """ + String a = '1' , + b = '2' + """ + ) + ); + } }