Fix groovy multivariable declaration #4757
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| /* | ||
| * Copyright 2024 the original author or authors. | ||
| * <p> | ||
| * 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 | ||
| * <p> | ||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||
| * <p> | ||
| * 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; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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" | ||
Laurens-W marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| final z = "z" | ||
| """ | ||
|
Comment on lines
+53
to
+57
There was a problem hiding this comment. Perhaps good to call out here that we now model Note that I don't expect this to occur often, if at all, and it still prints OK; but figured comment it here for a closer look by others reviewing as well. There was a problem hiding this comment. While this isn't perfect either, I think I would opt for mapping all of There was a problem hiding this comment. Implemented this. I'm not sure if we can actually mark the As we discussed we may want to find a way to combine consecutive There was a problem hiding this comment. At least marking the __ There was a problem hiding this comment. @sambsnyd Since you recently added that |
||
| ) | ||
| ); | ||
| } | ||
|
|
||
| @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' | ||
| """ | ||
| ) | ||
| ); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is very unclear to me what these changes here have to do with the parsing issue. Is a comment required here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to the way
DeclarationExpressions were parsed intoJ.VariableDeclarationswe needed a specific check here. After my changes the parsing is more in-line with other types and so we no longer need that exceptional case here!