Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip parsing groovy generated transform methods #4848

Merged
merged 9 commits into from
Jan 6, 2025
Merged

Skip parsing groovy generated transform methods #4848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
dd40843
Skip parsing groovy generated transform methods
jevanlingen Jan 6, 2025
a5e9862
Skip parsing groovy generated transform methods
jevanlingen Jan 6, 2025
c8082fb
Skip parsing groovy generated transform methods
jevanlingen Jan 6, 2025
c22959d
Skip parsing groovy generated transform methods
jevanlingen Jan 6, 2025
9c31255
Merge branch 'main' into 4254-groovy-generated-transform-methods-brea…
jevanlingen Jan 6, 2025
bb25ac7
improvement
jevanlingen Jan 6, 2025
7e0cda6
improvement
jevanlingen Jan 6, 2025
9822576
improvement
jevanlingen Jan 6, 2025
85f5d1d
improvement
jevanlingen Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 42 additions & 18 deletions rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java
View file
Open in desktop
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
Open in desktop
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 {}
"""
)
);
}
}
Loading