You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2019/11/22 15:11:21 UTC
[groovy] 05/18: refactor addTransformsToClassNode and supporting
methods
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 5160c9359ef310c37f11de7ed17c8ffe01076b56
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Nov 19 15:00:15 2019 -0600
refactor addTransformsToClassNode and supporting methods
(cherry picked from commit b551e2b2def997463145cd995145fa10385d7389)
---
.../ASTTransformationCollectorCodeVisitor.java | 148 ++++++++++-----------
.../{Groovy3839Bug.groovy => Groovy3839.groovy} | 68 +++++-----
2 files changed, 103 insertions(+), 113 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java b/src/main/java/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java
index 5369e31..5fb0418 100644
--- a/src/main/java/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java
@@ -36,13 +36,13 @@ import org.codehaus.groovy.transform.trait.Traits;
import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.stream.Stream;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.evaluateExpression;
@@ -51,7 +51,7 @@ import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.evalua
* themselves by {@link GroovyASTTransformation}. Each such annotation is added.
* <p>
* This visitor is only intended to be executed once, during the
- * {@code SEMANTIC_ANALYSIS} phase of compilation.
+ * {@link CompilePhase#SEMANTIC_ANALYSIS} phase of compilation.
*/
public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSupport {
@@ -104,10 +104,7 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
nodeAnnotations.addAll(mergedList);
for (AnnotationNode annotation : nodeAnnotations) {
- Annotation transformClassAnnotation = getTransformClassAnnotation(annotation.getClassNode());
- if (transformClassAnnotation != null) {
- addTransformsToClassNode(annotation, transformClassAnnotation);
- }
+ addTransformsToClassNode(annotation);
}
}
@@ -194,7 +191,7 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
if (klass != null) {
try {
act = (AnnotationCollectorTransform) klass.getDeclaredConstructor().newInstance();
- } catch (ReflectiveOperationException e) {
+ } catch (ReflectiveOperationException | RuntimeException e) {
source.getErrorCollector().addErrorAndContinue(new ExceptionMessage(e, true, source));
}
}
@@ -202,7 +199,8 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
act = new AnnotationCollectorTransform();
}
if (act != null) {
- replacements.put(index, act.visit(annotation, alias, origin, source));
+ List<AnnotationNode> result = act.visit(annotation, alias, origin, source);
+ replacements.put(index, result);
return;
}
}
@@ -212,22 +210,6 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
}
}
- private void addTransformsToClassNode(final AnnotationNode annotation, final Annotation transformClassAnnotation) {
- List<String> transformClassNames = getTransformClassNames(annotation, transformClassAnnotation);
-
- if (transformClassNames.isEmpty()) {
- String error = "@GroovyASTTransformationClass in " + annotation.getClassNode().getName() + " does not specify any transform class names/classes";
- source.getErrorCollector().addError(new SimpleMessage(error, source));
- }
-
- for (String transformClass : transformClassNames) {
- Class<?> klass = loadTransformClass(transformClass, annotation);
- if (klass != null) {
- verifyAndAddTransform(annotation, klass);
- }
- }
- }
-
private Class<?> loadTransformClass(final String transformClass, final AnnotationNode annotation) {
try {
return transformLoader.loadClass(transformClass, false, true, false);
@@ -238,76 +220,82 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
return null;
}
- private void verifyAndAddTransform(final AnnotationNode annotation, final Class<?> klass) {
- verifyClass(annotation, klass);
- verifyCompilePhase(annotation, klass);
- addTransform(annotation, klass);
- }
+ //--------------------------------------------------------------------------
+
+ /**
+ * Determines if given annotation specifies an AST transformation and if so,
+ * adds it to the current class.
+ */
+ private void addTransformsToClassNode(final AnnotationNode annotation) {
+ Annotation transformClassAnnotation = getTransformClassAnnotation(annotation.getClassNode());
+ if (transformClassAnnotation != null) {
+ try {
+ Method valueMethod = transformClassAnnotation.getClass().getMethod("value");
+ String[] transformClassNames = (String[]) valueMethod.invoke(transformClassAnnotation);
+ if (transformClassNames == null) transformClassNames = new String[0];
+
+ Method classesMethod = transformClassAnnotation.getClass().getMethod("classes");
+ Class<?>[] transformClasses = (Class[]) classesMethod.invoke(transformClassAnnotation);
+ if (transformClasses == null) transformClasses = new Class[0];
+
+ if (transformClassNames.length == 0 && transformClasses.length == 0) {
+ String error = "@GroovyASTTransformationClass in " + annotation.getClassNode().getName() + " does not specify any transform class names or types";
+ source.getErrorCollector().addError(new SimpleMessage(error, source));
+ }
- private void verifyClass(final AnnotationNode annotation, final Class<?> klass) {
- if (!ASTTransformation.class.isAssignableFrom(klass)) {
- String error = "Not an ASTTransformation: " + klass.getName() + " declared by " + annotation.getClassNode().getName();
- source.getErrorCollector().addError(new SimpleMessage(error, source));
- }
- }
+ if (transformClassNames.length > 0 && transformClasses.length > 0) {
+ String error = "@GroovyASTTransformationClass in " + annotation.getClassNode().getName() + " should specify transforms by name or by type, not by both";
+ source.getErrorCollector().addError(new SimpleMessage(error, source));
+ }
- private void verifyCompilePhase(final AnnotationNode annotation, final Class<?> klass) {
- GroovyASTTransformation transformationClass = klass.getAnnotation(GroovyASTTransformation.class);
- if (transformationClass != null) {
- CompilePhase specifiedCompilePhase = transformationClass.phase();
- if (specifiedCompilePhase.getPhaseNumber() < CompilePhase.SEMANTIC_ANALYSIS.getPhaseNumber()) {
- String error = annotation.getClassNode().getName() + " is defined to be run in compile phase " + specifiedCompilePhase + ". Local AST transformations must run in " + CompilePhase.SEMANTIC_ANALYSIS + " or later!";
- source.getErrorCollector().addError(new SimpleMessage(error, source));
+ Stream.concat(Stream.of(transformClassNames), Stream.of(transformClasses).map(Class::getName)).forEach(transformClassName -> {
+ Class<?> transformClass = loadTransformClass(transformClassName, annotation);
+ if (transformClass != null) {
+ verifyAndAddTransform(annotation, transformClass);
+ }
+ });
+ } catch (ReflectiveOperationException | RuntimeException e) {
+ source.getErrorCollector().addError(new ExceptionMessage(e, true, source));
}
- } else {
- String error = "AST transformation implementation classes must be annotated with " + GroovyASTTransformation.class.getName() + ". " + klass.getName() + " lacks this annotation.";
- source.getErrorCollector().addError(new SimpleMessage(error, source));
}
}
- @SuppressWarnings("unchecked")
- private void addTransform(final AnnotationNode annotation, final Class<?> klass) {
- boolean apply = !Traits.isTrait(classNode) || klass == TraitASTTransformation.class;
- if (apply) {
- classNode.addTransform((Class<? extends ASTTransformation>) klass, annotation);
- }
- }
-
- private static Annotation getTransformClassAnnotation(final ClassNode annotatedType) {
- if (!annotatedType.isResolved()) return null;
+ private static Annotation getTransformClassAnnotation(final ClassNode annotationType) {
+ if (!annotationType.isResolved()) return null;
- for (Annotation ann : annotatedType.getTypeClass().getAnnotations()) {
- // because compiler clients are free to choose any GroovyClassLoader for
- // resolving ClassNodes such as annotatedType, we have to compare by name,
- // and cannot cast the return value to GroovyASTTransformationClass
- if (ann.annotationType().getName().equals(GroovyASTTransformationClass.class.getName())) {
- return ann;
+ for (Annotation a : annotationType.getTypeClass().getAnnotations()) {
+ // clients are free to choose any GroovyClassLoader for resolving a
+ // ClassNode such as annotationType; we have to compare by name and
+ // cannot cast the return value to our GroovyASTTransformationClass
+ if (a.annotationType().getName().equals(GroovyASTTransformationClass.class.getName())) {
+ return a;
}
}
return null;
}
- private List<String> getTransformClassNames(final AnnotationNode annotation, final Annotation transformClassAnnotation) {
- List<String> result = new ArrayList<>();
- try {
- Method valueMethod = transformClassAnnotation.getClass().getMethod("value");
- String[] names = (String[]) valueMethod.invoke(transformClassAnnotation);
- result.addAll(Arrays.asList(names));
-
- Method classesMethod = transformClassAnnotation.getClass().getMethod("classes");
- Class<?>[] classes = (Class[]) classesMethod.invoke(transformClassAnnotation);
- for (Class<?> klass : classes) {
- result.add(klass.getName());
- }
+ @SuppressWarnings("unchecked")
+ private void verifyAndAddTransform(final AnnotationNode annotation, final Class<?> transformClass) {
+ if (!ASTTransformation.class.isAssignableFrom(transformClass)) {
+ String error = "Not an ASTTransformation: " + transformClass.getName() + " declared by " + annotation.getClassNode().getName();
+ source.getErrorCollector().addError(new SimpleMessage(error, source));
+ }
- if (names.length > 0 && classes.length > 0) {
- String error = "@GroovyASTTransformationClass in " + annotation.getClassNode().getName() + " should specify transforms only by class names or by classes and not by both";
- source.getErrorCollector().addError(new SimpleMessage(error, source));
- }
- } catch (Exception e) {
- source.addException(e);
+ GroovyASTTransformation transformationClass = transformClass.getAnnotation(GroovyASTTransformation.class);
+ if (transformationClass == null) {
+ String error = "AST transformation implementation classes must be annotated with " + GroovyASTTransformation.class.getName() + ". " + transformClass.getName() + " lacks this annotation.";
+ source.getErrorCollector().addError(new SimpleMessage(error, source));
+ }
+
+ CompilePhase specifiedCompilePhase = transformationClass.phase();
+ if (specifiedCompilePhase.getPhaseNumber() < CompilePhase.SEMANTIC_ANALYSIS.getPhaseNumber()) {
+ String error = annotation.getClassNode().getName() + " is defined to be run in compile phase " + specifiedCompilePhase + ". Local AST transformations must run in SEMANTIC_ANALYSIS or later!";
+ source.getErrorCollector().addError(new SimpleMessage(error, source));
+ }
+
+ if (!Traits.isTrait(classNode) || transformClass == TraitASTTransformation.class) {
+ classNode.addTransform((Class<? extends ASTTransformation>) transformClass, annotation);
}
- return result;
}
}
diff --git a/src/test/groovy/bugs/Groovy3839Bug.groovy b/src/test/groovy/bugs/Groovy3839.groovy
similarity index 56%
rename from src/test/groovy/bugs/Groovy3839Bug.groovy
rename to src/test/groovy/bugs/Groovy3839.groovy
index 1fd10f1..dac7d45 100644
--- a/src/test/groovy/bugs/Groovy3839Bug.groovy
+++ b/src/test/groovy/bugs/Groovy3839.groovy
@@ -18,59 +18,61 @@
*/
package groovy.bugs
-import groovy.test.GroovyTestCase
+import groovy.transform.CompileStatic
+import org.junit.Test
-class Groovy3839Bug extends GroovyTestCase {
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+
+@CompileStatic
+final class Groovy3839 {
+
+ @Test
void testGroovyASTTransformationWithOneClass() {
- assertScript """
+ assertScript '''
import groovy.bugs.*
-
+
@G3839A1
class G3839V1 {}
// verify if the ast transform added field 1
assert G3839V1.class.fields.find{it.name == 'f1'} != null
- """
+ '''
}
+ @Test
void testGroovyASTTransformationWithMultipleClass() {
- assertScript """
+ assertScript '''
import groovy.bugs.*
-
+
@G3839A2
class G3839V2 {}
// verify if the ast transforms added field f2 and f3
assert G3839V2.class.fields.find{it.name == 'f2'} != null
assert G3839V2.class.fields.find{it.name == 'f3'} != null
- """
+ '''
}
-
+
+ @Test
void testGroovyASTTransformationWithNeitherTransClassNamesNorClasses() {
- try {
- assertScript """
- import groovy.bugs.*
-
- @G3839A3
- class G3839V3 {}
- new G3839V3()
- """
- fail('The script should have failed as @GroovyASTTransformationClass in GroovyASTTransformationClass does not specify transform class names or classes')
- }catch(ex) {
- assert ex.message.contains('@GroovyASTTransformationClass in groovy.bugs.G3839A3 does not specify any transform class names/classes')
- }
+ def err = shouldFail '''
+ import groovy.bugs.*
+
+ @G3839A3
+ class G3839V3 {}
+ new G3839V3()
+ '''
+ assert err =~ '@GroovyASTTransformationClass in groovy.bugs.G3839A3 does not specify any transform class names or types'
}
+ @Test
void testGroovyASTTransformationWithBothTransClassNamesAndClasses() {
- try {
- assertScript """
- import groovy.bugs.*
-
- @G3839A4
- class G3839V4 {}
- new G3839V4()
- """
- fail('The script should have failed as @GroovyASTTransformationClass in GroovyASTTransformationClass does specifies both transform class names and classes')
- }catch(ex) {
- assert ex.message.contains('@GroovyASTTransformationClass in groovy.bugs.G3839A4 should specify transforms only by class names or by classes and not by both')
- }
+ def err = shouldFail '''
+ import groovy.bugs.*
+
+ @G3839A4
+ class G3839V4 {}
+ new G3839V4()
+ '''
+ assert err =~ '@GroovyASTTransformationClass in groovy.bugs.G3839A4 should specify transforms by name or by type, not by both'
}
}