groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject [groovy] 01/05: GROOVY-9095: Default retention policy not implemented for annotation (also covers GROOVY-9096)
Date Sun, 28 Apr 2019 09:02:12 GMT
This is an automated email from the ASF dual-hosted git repository.

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

commit 728ed25d5e22ac73d4316d1781532408907e57e4
Author: Paul King <paulk@asert.com.au>
AuthorDate: Fri Apr 26 22:19:20 2019 +1000

    GROOVY-9095: Default retention policy not implemented for annotation (also covers GROOVY-9096)
---
 .../groovy/transform/AnnotationCollectorMode.java  |  1 -
 .../org/codehaus/groovy/ast/AnnotationNode.java    | 59 ++++++++++++----------
 .../codehaus/groovy/classgen/ExtendedVerifier.java | 33 ++++++++----
 .../codehaus/groovy/control/ResolveVisitor.java    |  4 +-
 src/test/gls/annotations/AnnotationTest.groovy     |  2 +-
 5 files changed, 59 insertions(+), 40 deletions(-)

diff --git a/src/main/groovy/groovy/transform/AnnotationCollectorMode.java b/src/main/groovy/groovy/transform/AnnotationCollectorMode.java
index 308d0c0..75b78bf 100644
--- a/src/main/groovy/groovy/transform/AnnotationCollectorMode.java
+++ b/src/main/groovy/groovy/transform/AnnotationCollectorMode.java
@@ -21,7 +21,6 @@
 package groovy.transform;
 
 public enum AnnotationCollectorMode {
-    // TODO should we support @Repeatable from Java 8?
     /**
      * Annotations from the annotation collection will always be inserted. After all transforms
have been run, it will
      * be an error if multiple annotations (excluding those with SOURCE retention) exist.
diff --git a/src/main/java/org/codehaus/groovy/ast/AnnotationNode.java b/src/main/java/org/codehaus/groovy/ast/AnnotationNode.java
index c493299..55543da 100644
--- a/src/main/java/org/codehaus/groovy/ast/AnnotationNode.java
+++ b/src/main/java/org/codehaus/groovy/ast/AnnotationNode.java
@@ -26,7 +26,7 @@ import java.util.LinkedHashMap;
 import java.util.Map;
 
 /**
- * Represents an annotation which can be attached to interfaces, classes, methods and fields.
+ * Represents an annotation which can be attached to interfaces, classes, methods, fields,
parameters, and other places.
  */
 public class AnnotationNode extends ASTNode {
     public static final int CONSTRUCTOR_TARGET = 1 << 1;
@@ -45,7 +45,7 @@ public class AnnotationNode extends ASTNode {
 
     private final ClassNode classNode;
     private Map<String, Expression> members;
-    private boolean runtimeRetention= false, sourceRetention= false, classRetention = false;
+    private boolean runtimeRetention = false, sourceRetention = false, /* explicit */ classRetention
= false;
     private int allowedTargets = ALL_TARGETS;
 
     public AnnotationNode(ClassNode classNode) {
@@ -62,7 +62,7 @@ public class AnnotationNode extends ASTNode {
         }
         return members;
     }
-    
+
     public Expression getMember(String name) {
         if (members == null) {
             return null;
@@ -91,62 +91,67 @@ public class AnnotationNode extends ASTNode {
         assertMembers();
         members.put(name, value);
     }
-    
+
     public boolean isBuiltIn(){
         return false;
     }
 
     /**
-     * Flag corresponding to <code>RetentionPolicy</code>.
-     * @return <tt>true</tt> if the annotation should be visible at runtime,

-     *      <tt>false</tt> otherwise
+     * Flag corresponding to <code>RetentionPolicy.RUNTIME</code>.
+     * @return <tt>true</tt> if the annotation should be visible at runtime,
+     *         <tt>false</tt> otherwise
      */
     public boolean hasRuntimeRetention() {
         return this.runtimeRetention;
     }
 
     /**
-     * Sets the internal flag of this annotation runtime retention policy.
-     * If the current annotation has 
-     * <code>RetentionPolicy.RUNTIME</code> or if <tt>false</tt>
-     * if the <code>RetentionPolicy.CLASS</code>.
+     * Sets the internal flag if the current annotation has
+     * <code>RetentionPolicy.SOURCE</code>.
+     *
      * @param flag if <tt>true</tt> then current annotation is marked as having
-     *     <code>RetentionPolicy.RUNTIME</code>. If <tt>false</tt>
then
-     *     the annotation has <code>RetentionPolicy.CLASS</code>.
+     *     <code>RetentionPolicy.RUNTIME</code>.
      */
     public void setRuntimeRetention(boolean flag) {
         this.runtimeRetention = flag;
     }
-    
+
     /**
      * Flag corresponding to <code>RetentionPolicy.SOURCE</code>.
-     * @return <tt>true</tt> if the annotation is only allowed in sources 
-     *      <tt>false</tt> otherwise
+     * @return <tt>true</tt> if the annotation is only allowed in sources
+     *         <tt>false</tt> otherwise
      */
     public boolean hasSourceRetention() {
-        if (!runtimeRetention && !classRetention) return true;
         return this.sourceRetention;
     }
 
-    /** Sets the internal flag if the current annotation has 
-     * <code>RetentionPolicy.SOURCE</code>.
-     */ 
+    /**
+     * Sets the internal flag if the current annotation has <code>RetentionPolicy.SOURCE</code>.
+     *
+     * @param flag if <tt>true</tt> then current annotation is marked as having
+     *     <code>RetentionPolicy.SOURCE</code>.
+     */
     public void setSourceRetention(boolean flag) {
         this.sourceRetention = flag;
     }
 
     /**
      * Flag corresponding to <code>RetentionPolicy.CLASS</code>.
-     * @return <tt>true</tt> if the annotation is recorded by the compiler,
-     *                       but not visible at runtime     *
-      *        <tt>false</tt> otherwise
+     * This is the default when no <code>RetentionPolicy</code> annotations are
present.
+     *
+     * @return <tt>true</tt> if the annotation is written in the bytecode, but
not visible at runtime
+     *         <tt>false</tt> otherwise
      */
     public boolean hasClassRetention() {
+        if (!runtimeRetention && !sourceRetention) return true;
         return this.classRetention;
     }
 
-    /** Sets the internal flag if the current annotation has
-     * <code>RetentionPolicy.CLASS</code>.
+    /**
+     * Sets the internal flag if the current annotation has an explicit <code>RetentionPolicy.CLASS</code>.
+     *
+     * @param flag if <tt>true</tt> then current annotation is marked as having
+     *     <code>RetentionPolicy.CLASS</code>.
      */
     public void setClassRetention(boolean flag) {
         this.classRetention = flag;
@@ -155,11 +160,11 @@ public class AnnotationNode extends ASTNode {
     public void setAllowedTargets(int bitmap) {
         this.allowedTargets = bitmap;
     }
-    
+
     public boolean isTargetAllowed(int target) {
         return (this.allowedTargets & target) == target;
     }
-    
+
     public static String targetToName(int target) {
         switch(target) {
             case TYPE_TARGET:
diff --git a/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
index 96849ac..58a5bc3 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
@@ -47,6 +47,8 @@ import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
 import org.codehaus.groovy.syntax.SyntaxException;
 import org.objectweb.asm.Opcodes;
 
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -152,17 +154,17 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
             addError("Annotations are not supported in the current runtime. " + JVM_ERROR_MESSAGE,
node);
             return;
         }
-        Map<String, List<AnnotationNode>> runtimeAnnotations = new LinkedHashMap<String,
List<AnnotationNode>>();
+        Map<String, List<AnnotationNode>> nonSourceAnnotations = new LinkedHashMap<String,
List<AnnotationNode>>();
         for (AnnotationNode unvisited : node.getAnnotations()) {
             AnnotationNode visited = visitAnnotation(unvisited);
             String name = visited.getClassNode().getName();
-            if (visited.hasRuntimeRetention()) {
-                List<AnnotationNode> seen = runtimeAnnotations.get(name);
+            if (!visited.hasSourceRetention()) {
+                List<AnnotationNode> seen = nonSourceAnnotations.get(name);
                 if (seen == null) {
                     seen = new ArrayList<AnnotationNode>();
                 }
                 seen.add(visited);
-                runtimeAnnotations.put(name, seen);
+                nonSourceAnnotations.put(name, seen);
             }
             boolean isTargetAnnotation = name.equals("java.lang.annotation.Target");
 
@@ -175,7 +177,7 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
             visitDeprecation(node, visited);
             visitOverride(node, visited);
         }
-        checkForDuplicateAnnotations(node, runtimeAnnotations);
+        checkForDuplicateAnnotations(node, nonSourceAnnotations);
     }
 
     private void checkForDuplicateAnnotations(AnnotatedNode node, Map<String, List<AnnotationNode>>
runtimeAnnotations) {
@@ -199,7 +201,20 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
                 }
                 if (repeatable != null) {
                     AnnotationNode collector = new AnnotationNode(repeatable);
-                    collector.setRuntimeRetention(true); // checked earlier
+                    if (repeatable.isResolved()) {
+                        Class repeatableType = repeatable.getTypeClass();
+                        Retention retAnn = (Retention) repeatableType.getAnnotation(Retention.class);
+                        collector.setRuntimeRetention(retAnn != null && retAnn.value().equals(RetentionPolicy.RUNTIME));
+                    } else if (repeatable.redirect() != null) {
+                        for (AnnotationNode annotationNode : repeatable.redirect().getAnnotations())
{
+                            if (!annotationNode.getClassNode().getName().equals("java.lang.annotation.Retention"))
+                                continue;
+                            String value = annotationNode.getMember("value").getText();
+                            collector.setRuntimeRetention(value.equals(RetentionPolicy.RUNTIME.name())
||
+                                    value.equals(RetentionPolicy.class.getName() + "." +
RetentionPolicy.RUNTIME.name()));
+                        }
+                    }
+
                     List<Expression> annos = new ArrayList<Expression>();
                     for (AnnotationNode an : next.getValue()) {
                         annos.add(new AnnotationConstantExpression(an));
@@ -281,7 +296,7 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
             }
             ClassNode superClass = next.getUnresolvedSuperClass();
             if (superClass != null) {
-                next =  correctToGenericsSpecRecurse(updatedGenericsSpec, superClass);
+                next = correctToGenericsSpecRecurse(updatedGenericsSpec, superClass);
             } else {
                 next = null;
             }
@@ -290,7 +305,7 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
     }
 
     private static MethodNode getDeclaredMethodCorrected(Map genericsSpec, MethodNode mn,
ClassNode correctedNext) {
-        for (MethodNode orig :  correctedNext.getDeclaredMethods(mn.getName())) {
+        for (MethodNode orig : correctedNext.getDeclaredMethods(mn.getName())) {
             MethodNode method = correctToGenericsSpec(genericsSpec, orig);
             if (ParameterUtils.parametersEqual(method.getParameters(), mn.getParameters()))
{
                 return method;
@@ -338,4 +353,4 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
     public void visitGenericType(GenericsType genericsType) {
 
     }
-}
+}
\ No newline at end of file
diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index c7fd0e6..b6ec7ec 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -1336,8 +1336,8 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
             if (annType.isResolved()) {
                 Class annTypeClass = annType.getTypeClass();
                 Retention retAnn = (Retention) annTypeClass.getAnnotation(Retention.class);
-                if (retAnn != null && retAnn.value().equals(RetentionPolicy.RUNTIME)
&& !isRepeatable(annTypeClass)) {
-                    // remember runtime/non-repeatable annos (auto collecting of Repeatable
annotations is handled elsewhere)
+                if (retAnn != null && !retAnn.value().equals(RetentionPolicy.SOURCE)
&& !isRepeatable(annTypeClass)) {
+                    // remember non-source/non-repeatable annos (auto collecting of Repeatable
annotations is handled elsewhere)
                     AnnotationNode anyPrevAnnNode = tmpAnnotations.put(annTypeClass.getName(),
an);
                     if (anyPrevAnnNode != null) {
                         addError("Cannot specify duplicate annotation on the same member
: " + annType.getName(), an);
diff --git a/src/test/gls/annotations/AnnotationTest.groovy b/src/test/gls/annotations/AnnotationTest.groovy
index d5a5ea6..6ec88be 100644
--- a/src/test/gls/annotations/AnnotationTest.groovy
+++ b/src/test/gls/annotations/AnnotationTest.groovy
@@ -278,7 +278,7 @@ class AnnotationTest extends CompilableTestSupport {
         assertScript """
             import java.lang.annotation.*
 
-            // a random annnotation type
+            // a random annotation type
             @Retention(RetentionPolicy.RUNTIME)
             @interface MyAnnotation {
                 String stringValue()


Mime
View raw message