groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From emil...@apache.org
Subject [groovy] branch master updated: refactor addTransformsToClassNode and supporting methods
Date Tue, 19 Nov 2019 21:11:27 GMT
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/master by this push:
     new b551e2b  refactor addTransformsToClassNode and supporting methods
b551e2b is described below

commit b551e2b2def997463145cd995145fa10385d7389
Author: Eric Milles <eric.milles@thomsonreuters.com>
AuthorDate: Tue Nov 19 15:00:15 2019 -0600

    refactor addTransformsToClassNode and supporting methods
---
 .../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'
     }
 }


Mime
View raw message