Skip to content

Commit

Permalink
Merge branch 'main' into 4703-groovy-parser-does-not-correctly-handle…
Browse files Browse the repository at this point in the history
…-nested-parenthesis

# Conflicts:
#	rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java
#	rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MethodInvocationTest.java
#	rewrite-java/src/main/java/org/openrewrite/java/tree/Space.java
  • Loading branch information
jevanlingen committed Dec 6, 2024
2 parents 0a61029 + 7013518 commit e20e95c
Show file tree
Hide file tree
Showing 13 changed files with 609 additions and 176 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,35 @@ private static boolean isOlderThanGroovy3() {
return olderThanGroovy3;
}

/**
* 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<RedundantDef> maybeRedundantDef(ClassNode type, String name) {
int saveCursor = cursor;
Space defPrefix = whitespace();
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
int cursorBeforeReturnType = cursor;
int lengthOfTypePrefix = indexOfNextNonWhitespace(cursorBeforeReturnType, source) - cursorBeforeReturnType;
// Differentiate between the next token being the method/variable name and it being the return type
if (!source.startsWith(name, cursorBeforeReturnType + lengthOfTypePrefix)) {
TypeTree returnType = visitTypeTree(type);
if (source.startsWith(returnType.toString(), cursorBeforeReturnType + lengthOfTypePrefix)) {
cursor = cursorBeforeReturnType;
return Optional.of(new RedundantDef(randomId(), defPrefix));
}
}
}
cursor = saveCursor;
return Optional.empty();
}

public G.CompilationUnit visit(SourceUnit unit, ModuleNode ast) throws GroovyParsingException {
NavigableMap<LineColumn, List<ASTNode>> sortedByPosition = new TreeMap<>();
for (org.codehaus.groovy.ast.stmt.Statement s : ast.getStatementBlock().getStatements()) {
Expand Down Expand Up @@ -491,6 +520,7 @@ public void visitMethod(MethodNode method) {

List<J.Annotation> annotations = visitAndGetAnnotations(method);
List<J.Modifier> modifiers = visitModifiers(method.getModifiers());
Optional<RedundantDef> redundantDef = maybeRedundantDef(method.getReturnType(), method.getName());
TypeTree returnType = visitTypeTree(method.getReturnType());

// Method name might be in quotes
Expand Down Expand Up @@ -564,7 +594,8 @@ null, emptyList(),
bodyVisitor.visit(method.getCode());

queue.add(new J.MethodDeclaration(
randomId(), fmt, Markers.EMPTY,
randomId(), fmt,
redundantDef.map(Markers.EMPTY::add).orElse(Markers.EMPTY),
annotations,
modifiers,
null,
Expand Down Expand Up @@ -1387,12 +1418,13 @@ public void visitNotExpression(NotExpression expression) {

@Override
public void visitDeclarationExpression(DeclarationExpression expression) {
Optional<RedundantDef> redundantDef = maybeRedundantDef(expression.getVariableExpression().getType(), expression.getVariableExpression().getName());
TypeTree typeExpr = visitVariableExpressionType(expression.getVariableExpression());

J.VariableDeclarations.NamedVariable namedVariable;
if (expression.isMultipleAssignmentDeclaration()) {
// def (a, b) = [1, 2]
throw new UnsupportedOperationException("FIXME");
throw new UnsupportedOperationException("Parsing multiple assignment (e.g.: def (a, b) = [1, 2]) is not implemented");
} else {
J.Identifier name = visit(expression.getVariableExpression());
namedVariable = new J.VariableDeclarations.NamedVariable(
Expand All @@ -1415,7 +1447,7 @@ public void visitDeclarationExpression(DeclarationExpression expression) {
J.VariableDeclarations variableDeclarations = new J.VariableDeclarations(
randomId(),
EMPTY,
Markers.EMPTY,
redundantDef.map(Markers.EMPTY::add).orElse(Markers.EMPTY),
emptyList(),
emptyList(),
typeExpr,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.openrewrite.groovy.tree.GRightPadded;
import org.openrewrite.groovy.tree.GSpace;
import org.openrewrite.java.JavaPrinter;
import org.openrewrite.java.marker.CompactConstructor;
import org.openrewrite.java.tree.*;
import org.openrewrite.marker.Marker;
import org.openrewrite.marker.Markers;
Expand Down Expand Up @@ -258,6 +259,36 @@ public J visitTypeCast(J.TypeCast t, PrintOutputCapture<P> p) {
return t;
}


@Override
public J visitVariableDeclarations(J.VariableDeclarations multiVariable, PrintOutputCapture<P> p) {
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");
});
visit(multiVariable.getTypeExpression(), p);
// For backwards compatibility.
for (JLeftPadded<Space> dim : multiVariable.getDimensionsBeforeName()) {
visitSpace(dim.getBefore(), Space.Location.DIMENSION_PREFIX, p);
p.append('[');
visitSpace(dim.getElement(), Space.Location.DIMENSION, p);
p.append(']');
}
if (multiVariable.getVarargs() != null) {
visitSpace(multiVariable.getVarargs(), Space.Location.VARARGS, p);
p.append("...");
}
visitRightPadded(multiVariable.getPadding().getVariables(), JRightPadded.Location.NAMED_VARIABLE, ",", p);
afterSyntax(multiVariable, p);
return multiVariable;
}

@Override
public J visitLambda(J.Lambda lambda, PrintOutputCapture<P> p) {
beforeSyntax(lambda, Space.Location.LAMBDA_PREFIX, p);
Expand Down Expand Up @@ -327,6 +358,39 @@ public J visitForEachLoop(J.ForEachLoop forEachLoop, PrintOutputCapture<P> p) {
afterSyntax(forEachLoop, p);
return forEachLoop;
}
@Override
public J visitMethodDeclaration(J.MethodDeclaration method, PrintOutputCapture<P> p) {
beforeSyntax(method, Space.Location.METHOD_DECLARATION_PREFIX, p);
visitSpace(Space.EMPTY, Space.Location.ANNOTATIONS, p);
visit(method.getLeadingAnnotations(), p);
for (J.Modifier m : method.getModifiers()) {
visitModifier(m, p);
}
J.TypeParameters typeParameters = method.getAnnotations().getTypeParameters();
if (typeParameters != null) {
visit(typeParameters.getAnnotations(), p);
visitSpace(typeParameters.getPrefix(), Space.Location.TYPE_PARAMETERS, p);
visitMarkers(typeParameters.getMarkers(), p);
p.append('<');
visitRightPadded(typeParameters.getPadding().getTypeParameters(), JRightPadded.Location.TYPE_PARAMETER, ",", p);
p.append('>');
}
method.getMarkers().findFirst(RedundantDef.class).ifPresent(def -> {
visitSpace(def.getPrefix(), Space.Location.LANGUAGE_EXTENSION, p);
p.append("def");
});
visit(method.getReturnTypeExpression(), p);
visit(method.getAnnotations().getName().getAnnotations(), p);
visit(method.getName(), p);
if (!method.getMarkers().findFirst(CompactConstructor.class).isPresent()) {
visitContainer("(", method.getPadding().getParameters(), JContainer.Location.METHOD_DECLARATION_PARAMETERS, ",", ")", p);
}
visitContainer("throws", method.getPadding().getThrows(), JContainer.Location.THROWS, ",", null, p);
visit(method.getBody(), p);
visitLeftPadded("default", method.getPadding().getDefaultValue(), JLeftPadded.Location.METHOD_DECLARATION_DEFAULT_VALUE, p);
afterSyntax(method, p);
return method;
}

@Override
public J visitMethodInvocation(J.MethodInvocation method, PrintOutputCapture<P> p) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* 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;

/**
* In Groovy methods can be declared with a return type and also a redundant 'def' keyword.
* This captures the extra def keyword.
*/
@Value
@With
public class RedundantDef implements Marker {
UUID id;
Space prefix;
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.openrewrite.groovy.tree;

import org.junit.jupiter.api.Test;
import org.junitpioneer.jupiter.ExpectedToFail;
import org.openrewrite.Issue;
import org.openrewrite.test.RewriteTest;

Expand Down Expand Up @@ -84,6 +85,18 @@ void groovyCastAndInvokeMethod() {
);
}

@Test
@ExpectedToFail("Parentheses with method invocation is not yet supported")
void groovyCastAndInvokeMethodWithParentheses() {
rewriteRun(
groovy(
"""
(((((( "" as String ))))).toString())
"""
)
);
}

@Test
void javaCastAndInvokeMethod() {
rewriteRun(
Expand All @@ -99,4 +112,16 @@ void javaCastAndInvokeMethod() {
)
);
}

@ExpectedToFail("Parentheses with method invocation is not yet supported")
@Test
void javaCastAndInvokeMethodWithParentheses() {
rewriteRun(
groovy(
"""
(((((( (String) "" )).toString()))))
"""
)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package org.openrewrite.groovy.tree;

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.openrewrite.Issue;
import org.openrewrite.java.JavaIsoVisitor;
Expand Down Expand Up @@ -182,16 +181,16 @@ void escapedMethodNameWithSpacesTest() {
}

@Issue("https://github.com/openrewrite/rewrite/issues/4705")
@Disabled
@Test
void functionWithDefAndExplicitReturnType() {
rewriteRun(
groovy(
"""
class A {
def int one() { 1 }
}
"""
class A {
def /*int*/ int one() { 1 }
def /*Object*/ Object two() { 2 }
}
"""
)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.openrewrite.groovy.tree;

import org.junit.jupiter.api.Test;
import org.junitpioneer.jupiter.ExpectedToFail;
import org.openrewrite.test.RewriteTest;

import static org.openrewrite.groovy.Assertions.groovy;
Expand Down Expand Up @@ -53,4 +54,18 @@ void parenthesized() {
)
);
}

@ExpectedToFail("Parentheses with method invocation is not yet supported")
@Test
void parenthesizedAndInvokeMethodWithParentheses() {
rewriteRun(
groovy(
"""
(((( 8..19 ))).each { majorVersion ->
if (majorVersion == 9) return
})
"""
)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.openrewrite.groovy.Assertions.groovy;

@SuppressWarnings({"GroovyUnusedAssignment", "DataFlowIssue"})
@SuppressWarnings({"GroovyUnusedAssignment", "DataFlowIssue", "GrUnnecessaryDefModifier"})
class VariableDeclarationsTest implements RewriteTest {

@Test
Expand Down Expand Up @@ -203,4 +203,17 @@ class A {
)
);
}

@Issue("https://github.com/openrewrite/rewrite/issues/4705")
@Test
void defAndExplicitReturnType() {
rewriteRun(
groovy(
"""
def /*int*/ int one = 1
def /*Object*/ Object two = 2
"""
)
);
}
}
Loading

0 comments on commit e20e95c

Please sign in to comment.