Skip to content

Commit

Permalink
Skip parsing groovy generated transform methods (#4848)
Browse files Browse the repository at this point in the history 
* Skip parsing groovy generated transform methods

* Skip parsing groovy generated transform methods

* Skip parsing groovy generated transform methods

* Skip parsing groovy generated transform methods

* improvement

* improvement

* improvement

* improvement
  • Loading branch information
@jevanlingen
jevanlingen authored Jan 6, 2025
1 parent c455436 commit a3249f2
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 18 deletions.
60 changes: 42 additions & 18 deletions rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
package org.openrewrite.groovy;

import groovy.lang.GroovySystem;
import groovy.transform.Generated;
import groovy.transform.Immutable;
import groovyjarjarasm.asm.Opcodes;
import lombok.Getter;
import lombok.RequiredArgsConstructor;
Expand Down Expand Up @@ -305,7 +307,7 @@ J.Block visitClassBlock(ClassNode clazz) {
org.codehaus.groovy.ast.stmt.Statement statement = ((BlockStatement) method.getCode()).getStatements().get(0);
sortedByPosition.computeIfAbsent(pos(statement), i -> new ArrayList<>()).add(statement);
}
} else {
} else if (method.getAnnotations(new ClassNode(Generated.class)).isEmpty()) {
sortedByPosition.computeIfAbsent(pos(method), i -> new ArrayList<>()).add(method);
}
}
Expand Down Expand Up @@ -448,7 +450,6 @@ private void visitVariableField(FieldNode field) {
@Override
protected void visitAnnotation(AnnotationNode annotation) {
RewriteGroovyVisitor bodyVisitor = new RewriteGroovyVisitor(annotation, this);

String lastArgKey = annotation.getMembers().keySet().stream().reduce("", (k1, k2) -> k2);
Space prefix = sourceBefore("@");
NameTree annotationType = visitTypeTree(annotation.getClassNode());
Expand All @@ -457,8 +458,10 @@ protected void visitAnnotation(AnnotationNode annotation) {
arguments = JContainer.build(
sourceBefore("("),
annotation.getMembers().entrySet().stream()
// Non-value implicit properties should not be represented in our LST.
.filter(it -> sourceStartsWith(it.getKey()) || "value".equals(it.getKey()))
.map(arg -> {
boolean isImplicitValue = "value".equals(arg.getKey()) && !source.startsWith("value", indexOfNextNonWhitespace(cursor, source));
boolean isImplicitValue = "value".equals(arg.getKey()) && !sourceStartsWith("value");
Space argPrefix = isImplicitValue ? whitespace() : sourceBefore(arg.getKey());
Space isSign = isImplicitValue ? null : sourceBefore("=");
Expression expression;
Expand All @@ -478,8 +481,12 @@ protected void visitAnnotation(AnnotationNode annotation) {
.collect(toList()),
Markers.EMPTY
);
} else if (source.startsWith("(", indexOfNextNonWhitespace(cursor, source))) {
// An annotation with empty arguments like @Foo()
// Rare scenario where annotation does only have non-value implicit properties
if (arguments.getElements().isEmpty()) {
arguments = null;
}
} else if (sourceStartsWith("(")) {
// Annotation with empty arguments like @Foo()
arguments = JContainer.build(sourceBefore("("),
singletonList(JRightPadded.build(new J.Empty(randomId(), sourceBefore(")"), Markers.EMPTY))),
Markers.EMPTY);
Expand Down Expand Up @@ -551,7 +558,7 @@ class B { class B {
}

Space varargs = null;
if (paramType instanceof J.ArrayType && hasVarargs()) {
if (paramType instanceof J.ArrayType && sourceStartsWith("...")) {
int varargStart = indexOfNextNonWhitespace(cursor, source);
varargs = format(source, cursor, varargStart);
cursor = varargStart + 3;
Expand Down Expand Up @@ -642,8 +649,17 @@ public List<J.Annotation> visitAndGetAnnotations(AnnotatedNode node) {

List<J.Annotation> paramAnnotations = new ArrayList<>(node.getAnnotations().size());
for (AnnotationNode annotationNode : node.getAnnotations()) {
visitAnnotation(annotationNode);
paramAnnotations.add(pollQueue());
// The groovy compiler can add or remove annotations for AST transformations.
// Because @groovy.transform.Immutable is discarded in favour of other transform annotations, the removed annotation must be parsed by hand.
if (sourceStartsWith("@" + Immutable.class.getSimpleName()) || sourceStartsWith("@" + Immutable.class.getCanonicalName()) ) {
visitAnnotation(new AnnotationNode(new ClassNode(Immutable.class)));
paramAnnotations.add(pollQueue());
}

if (appearsInSource(annotationNode)) {
visitAnnotation(annotationNode);
paramAnnotations.add(pollQueue());
}
}
return paramAnnotations;
}
Expand Down Expand Up @@ -747,7 +763,7 @@ public void visitArgumentlistExpression(ArgumentListExpression expression) {
}

List<org.codehaus.groovy.ast.expr.Expression> unparsedArgs = expression.getExpressions().stream()
.filter(GroovyParserVisitor::appearsInSource)
.filter(GroovyParserVisitor.this::appearsInSource)
.collect(toList());
// If the first parameter to a function is a Map, then groovy allows "named parameters" style invocations, see:
// https://docs.groovy-lang.org/latest/html/documentation/#_named_parameters_2
Expand Down Expand Up @@ -1017,7 +1033,7 @@ public void visitBinaryExpression(BinaryExpression binary) {
public void visitBlockStatement(BlockStatement block) {
Space fmt = EMPTY;
Space staticInitPadding = EMPTY;
boolean isStaticInit = source.substring(indexOfNextNonWhitespace(cursor, source)).startsWith("static");
boolean isStaticInit = sourceStartsWith("static");
Object parent = nodeCursor.getParentOrThrow().getValue();
if (isStaticInit) {
fmt = sourceBefore("static");
Expand Down Expand Up @@ -2367,7 +2383,7 @@ private TypeTree arrayType(ClassNode classNode) {
}
Space prefix = whitespace();
TypeTree elemType = typeTree(typeTree);
JLeftPadded<Space> dimension = hasVarargs() ? null : padLeft(sourceBefore("["), sourceBefore("]"));
JLeftPadded<Space> dimension = sourceStartsWith("...") ? null : padLeft(sourceBefore("["), sourceBefore("]"));
return new J.ArrayType(randomId(), prefix, Markers.EMPTY,
count == 1 ? elemType : mapDimensions(elemType, classNode.getComponentType()),
null,
Expand All @@ -2392,10 +2408,6 @@ private TypeTree mapDimensions(TypeTree baseType, ClassNode classNode) {
return baseType;
}

private boolean hasVarargs() {
return source.startsWith("...", indexOfNextNonWhitespace(cursor, source));
}

/**
* Get all characters of the source file between the cursor and the given delimiter.
* The cursor will be moved past the delimiter.
Expand All @@ -2416,6 +2428,14 @@ private Space sourceBefore(String untilDelim) {
return space;
}

/**
* Tests if the source beginning at the current cursor starts with the specified delimiter.
* Whitespace characters are excluded, the cursor will not be moved.
*/
private boolean sourceStartsWith(String delimiter) {
return source.startsWith(delimiter, indexOfNextNonWhitespace(cursor, source));
}

/**
* Returns a string that is a part of this source. The substring begins at the specified beginIndex and extends until delimiter.
* The cursor will not be moved.
Expand Down Expand Up @@ -2708,13 +2728,17 @@ private J.TypeParameter visitTypeParameter(GenericsType genericType) {

/**
* Sometimes the groovy compiler inserts phantom elements into argument lists and class bodies,
* presumably to pass type information around. These elements do not appear in source code and should not
* be represented in our AST.
* presumably to pass type information around. Other times the groovy compiler adds extra transform annotations.
* These elements do not appear in source code and should not be represented in our LST.
*
* @param node possible phantom node
* @return true if the node reports that it does have a position within the source code
*/
private static boolean appearsInSource(ASTNode node) {
private boolean appearsInSource(ASTNode node) {
if (node instanceof AnnotationNode) {
return sourceStartsWith("@" + ((AnnotationNode) node).getClassNode().getUnresolvedName());
}

return node.getColumnNumber() >= 0 && node.getLineNumber() >= 0 && node.getLastColumnNumber() >= 0 && node.getLastLineNumber() >= 0;
}

Expand Down
79 changes: 79 additions & 0 deletions rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/AnnotationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,21 @@ void simple() {
groovy(
"""
@Foo
class Test implements Runnable {
@java.lang.Override
void run() {}
}
"""
)
);
}

@Test
void simpleFQN() {
rewriteRun(
groovy(
"""
@org.springframework.stereotype.Service
class Test {}
"""
)
Expand All @@ -48,6 +63,19 @@ class Test {}
);
}

@Test
void inline() {
rewriteRun(
groovy(
"""
@Foo class Test implements Runnable {
@Override void run() {}
}
"""
)
);
}

@Test
void withProperties() {
rewriteRun(
Expand Down Expand Up @@ -84,4 +112,55 @@ class Test {}
)
);
}

@Issue("https://github.com/openrewrite/rewrite/issues/4254")
@Test
void groovyTransformAnnotation() {
rewriteRun(
groovy(
"""
import groovy.transform.EqualsAndHashCode
import groovy.transform.ToString
@Foo
@ToString
@EqualsAndHashCode
@Bar
class Test {}
"""
)
);
}

@Issue("https://github.com/openrewrite/rewrite/issues/4254")
@Test
void groovyTransformImmutableAnnotation() {
rewriteRun(
groovy(
"""
import groovy.transform.Immutable
import groovy.transform.TupleConstructor
@Foo
@TupleConstructor
@Immutable
@Bar
class Test {}
"""
)
);
}

@Issue("https://github.com/openrewrite/rewrite/issues/4254")
@Test
void groovyTransformImmutableFQNAnnotation() {
rewriteRun(
groovy(
"""
@groovy.transform.Immutable
class Test {}
"""
)
);
}
}

0 comments on commit a3249f2

Please sign in to comment.