You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2022/03/29 00:01:12 UTC

[groovy] branch GROOVY_4_0_X updated: GROOVY-10553: prevent duplicate type annotations

This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/GROOVY_4_0_X by this push:
     new 87a907d  GROOVY-10553: prevent duplicate type annotations
87a907d is described below

commit 87a907d563bb027498df62978734b374a78dcf97
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Mar 28 17:34:36 2022 -0500

    GROOVY-10553: prevent duplicate type annotations
---
 .../codehaus/groovy/classgen/ExtendedVerifier.java |  6 ++--
 .../transform/trait/TraitASTTransformation.java    | 30 ++++++++---------
 .../traitx/TraitASTTransformationTest.groovy       | 39 ++++++++++++++++++++--
 3 files changed, 54 insertions(+), 21 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
index a83e6b8..069cf6e 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
@@ -213,9 +213,11 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
             }
         }
         if (!typeUseAnnos.isEmpty()) {
-            targetType.addTypeAnnotations(typeUseAnnos);
-            targetType.setAnnotated(true);
             for (AnnotationNode anno : typeUseAnnos) {
+                if (!targetType.getTypeAnnotations().contains(anno)) {
+                    targetType.addTypeAnnotation(anno);
+                    targetType.setAnnotated(true);
+                }
                 if (keepTarget != null && !anno.isTargetAllowed(keepTarget)) {
                     mixed.remove(anno);
                 }
diff --git a/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
index 4f89097..353f18a 100644
--- a/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
@@ -56,7 +56,6 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Iterator;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
 
@@ -225,7 +224,7 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
 
         // add methods
         List<MethodNode> methods = new ArrayList<>(cNode.getMethods());
-        List<MethodNode> nonPublicAPIMethods = new LinkedList<>();
+        List<MethodNode> nonPublicAPIMethods = new ArrayList<>();
         List<Statement> staticInitStatements = null;
         for (final MethodNode methodNode : methods) {
             boolean declared = methodNode.getDeclaringClass() == cNode;
@@ -362,16 +361,17 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
     }
 
     /**
-     * Copies annotation from the trait to the helper, excluding the trait annotation itself.
+     * Copies annotations from the trait to the helper, excluding non-applicable
+     * items such as {@code @Trait} and {@code @Sealed}.
      *
      * @param cNode the trait class node
      * @param helper the helper class node
      */
     private static void copyClassAnnotations(final ClassNode cNode, final ClassNode helper) {
-        List<AnnotationNode> annotations = cNode.getAnnotations();
-        for (AnnotationNode annotation : annotations) {
-            if (!annotation.getClassNode().equals(Traits.TRAIT_CLASSNODE)
-                    && !annotation.getClassNode().equals(SEALED_TYPE)) {
+        for (AnnotationNode annotation : cNode.getAnnotations()) {
+            ClassNode annotationType = annotation.getClassNode();
+            if (!annotationType.equals(Traits.TRAIT_CLASSNODE)
+                    && !annotationType.equals(SEALED_TYPE)) {
                 helper.addAnnotation(annotation);
             }
         }
@@ -541,11 +541,12 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
                 fieldHelper,
                 null
         );
+        dummyField.setSynthetic(true);
         // copy annotations from field to dummy field
-        List<AnnotationNode> copied = new LinkedList<>();
-        List<AnnotationNode> notCopied = new LinkedList<>();
-        GeneralUtils.copyAnnotatedNodeAnnotations(field, copied, notCopied);
-        dummyField.addAnnotations(copied);
+        List<AnnotationNode> copy = new ArrayList<>();
+        List<AnnotationNode> skip = new ArrayList<>();
+        GeneralUtils.copyAnnotatedNodeAnnotations(field, copy, skip);
+        dummyField.addAnnotations(copy);
         fieldHelper.addField(dummyField);
 
         // retain legacy field (will be given lower precedence than above)
@@ -559,11 +560,8 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
                 fieldHelper,
                 null
         );
-        // copy annotations from field to legacy dummy field
-        copied = new LinkedList<>();
-        notCopied = new LinkedList<>();
-        GeneralUtils.copyAnnotatedNodeAnnotations(field, copied, notCopied);
-        dummyField.addAnnotations(copied);
+        dummyField.setSynthetic(true);
+        dummyField.addAnnotations(copy);
         fieldHelper.addField(dummyField);
     }
 
diff --git a/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy b/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
index 3f429db..d77065e 100644
--- a/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
@@ -1888,15 +1888,48 @@ final class TraitASTTransformationTest {
             trait Foo {
                 @Deprecated void foo() { 'ok' }
             }
-            @ASTTest(phase=CANONICALIZATION,value={
-                assert node.getDeclaredMethod('foo').annotations.any { it.classNode.nameWithoutPackage == 'Deprecated'}
+            @ASTTest(phase=CANONICALIZATION, value={
+                assert node.getDeclaredMethod('foo').annotations.any {
+                    it.classNode.nameWithoutPackage == 'Deprecated'
+                }
             })
-            class Bar implements Foo {}
+            class Bar implements Foo {
+            }
             def b = new Bar()
             b.foo()
         '''
     }
 
+    @Test // GROOVY-10553
+    void testAnnotationShouldBeCarriedOver2() {
+        assertScript '''
+            import groovy.transform.*
+            import java.lang.annotation.*
+            @Retention(RetentionPolicy.RUNTIME)
+            @Target([ElementType.FIELD,ElementType.TYPE_USE])
+            @interface Foo {
+            }
+
+            trait Bar {
+                @Foo String string
+            }
+            class Baz implements Bar {
+            }
+
+            @ASTTest(phase=CLASS_GENERATION, value={
+                def type = node.rightExpression.type
+
+                assert type.name == 'Baz'
+                def field = type.getField('Bar__string')
+                assert field.type.typeAnnotations.size() == 1
+
+                field = type.interfaces[1].getField('$ins$1Bar__string')
+                assert field.type.typeAnnotations.size() == 1 // no duplicate
+            })
+            def baz = new Baz(string:'foobar')
+        '''
+    }
+
     @Test
     void testShouldCompileTraitMethodStatically() {
         def err = shouldFail '''