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/07/06 18:27:34 UTC

[groovy] branch GROOVY_4_0_X updated (06af450a85 -> 8215ed3c38)

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

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


    from 06af450a85 GROOVY-9980: visit annotation expression before values
     new 515cde9df3 minor refactor
     new 9472fd0289 refactor duplication
     new 8215ed3c38 GROOVY-10068: retain field type during constant inlining

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/groovy/ast/tools/ExpressionUtils.java   | 262 ++++-----
 .../codehaus/groovy/control/ResolveVisitor.java    |  54 +-
 src/test/gls/annotations/AnnotationTest.groovy     | 644 +++++++++++----------
 3 files changed, 466 insertions(+), 494 deletions(-)


[groovy] 01/03: minor refactor

Posted by em...@apache.org.
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

commit 515cde9df3713131535c95286b4c3ad1d8e30a54
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Jul 6 10:34:14 2022 -0500

    minor refactor
---
 .../apache/groovy/ast/tools/ExpressionUtils.java   | 257 ++++-----
 src/test/gls/annotations/AnnotationTest.groovy     | 629 ++++++++++-----------
 2 files changed, 430 insertions(+), 456 deletions(-)

diff --git a/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java b/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java
index a5f8813463..373f8a283a 100644
--- a/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java
+++ b/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java
@@ -34,8 +34,9 @@ import org.codehaus.groovy.runtime.typehandling.NumberMath;
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
 import java.util.Arrays;
+import java.util.List;
+import java.util.ListIterator;
 
-import static org.codehaus.groovy.ast.ClassHelper.isStringType;
 import static org.codehaus.groovy.ast.ClassHelper.isWrapperByte;
 import static org.codehaus.groovy.ast.ClassHelper.isWrapperCharacter;
 import static org.codehaus.groovy.ast.ClassHelper.isWrapperDouble;
@@ -57,20 +58,62 @@ import static org.codehaus.groovy.syntax.Types.RIGHT_SHIFT_UNSIGNED;
 
 public final class ExpressionUtils {
 
-    // NOTE: values are sorted in ascending order
+    public static boolean isNullConstant(final Expression expression) {
+        return expression instanceof ConstantExpression && ((ConstantExpression) expression).isNullExpression();
+    }
+
+    public static boolean isThisExpression(final Expression expression) {
+        return expression instanceof VariableExpression && ((VariableExpression) expression).isThisExpression();
+    }
+
+    public static boolean isSuperExpression(final Expression expression) {
+        return expression instanceof VariableExpression && ((VariableExpression) expression).isSuperExpression();
+    }
+
+    public static boolean isThisOrSuper(final Expression expression) {
+        return isThisExpression(expression) || isSuperExpression(expression);
+    }
+
+    /**
+     * Determines if a type matches another type (or array thereof).
+     *
+     * @param targetType the candidate type
+     * @param type the type we are checking against
+     * @param recurse true if we can have multi-dimension arrays; should be false for annotation member types
+     * @return true if the type equals the targetType or array thereof
+     */
+    public static boolean isTypeOrArrayOfType(final ClassNode targetType, final ClassNode type, final boolean recurse) {
+        if (targetType == null) return false;
+        if (type.equals(targetType)) return true;
+        return (targetType.isArray() && recurse ? isTypeOrArrayOfType(targetType.getComponentType(), type, recurse) : type.equals(targetType.getComponentType()));
+    }
+
+    /**
+     * Determines if a type is derived from Number (or array thereof).
+     *
+     * @param targetType the candidate type
+     * @param recurse true if we can have multi-dimension arrays; should be false for annotation member types
+     * @return true if the type equals the targetType or array thereof
+     */
+    public static boolean isNumberOrArrayOfNumber(final ClassNode targetType, final boolean recurse) {
+        if (targetType == null) return false;
+        if (targetType.isDerivedFrom(ClassHelper.Number_TYPE)) return true;
+        return (targetType.isArray() && recurse ? isNumberOrArrayOfNumber(targetType.getComponentType(), recurse) : targetType.isArray() && targetType.getComponentType().isDerivedFrom(ClassHelper.Number_TYPE));
+    }
+
+    //--------------------------------------------------------------------------
+
     private static final int[] HANDLED_TYPES = {
             PLUS, MINUS, MULTIPLY, DIVIDE, POWER,
-            LEFT_SHIFT, RIGHT_SHIFT, RIGHT_SHIFT_UNSIGNED,
-            BITWISE_OR, BITWISE_AND, BITWISE_XOR
+            BITWISE_OR, BITWISE_AND, BITWISE_XOR,
+            LEFT_SHIFT, RIGHT_SHIFT, RIGHT_SHIFT_UNSIGNED
     };
     static {
         Arrays.sort(HANDLED_TYPES);
     }
 
-    private ExpressionUtils() { }
-
     /**
-     * Turns expressions of the form ConstantExpression(40) + ConstantExpression(2)
+     * Converts expressions like ConstantExpression(40) + ConstantExpression(2)
      * into the simplified ConstantExpression(42) at compile time.
      *
      * @param be the binary expression
@@ -98,7 +141,7 @@ public final class ExpressionUtils {
                 Expression leftX = transformInlineConstants(be.getLeftExpression(), targetType);
                 Expression rightX = transformInlineConstants(be.getRightExpression(), isShift ? ClassHelper.int_TYPE : targetType);
                 if (leftX instanceof ConstantExpression && rightX instanceof ConstantExpression) {
-                    Number left = safeNumber((ConstantExpression) leftX);
+                    Number left  = safeNumber((ConstantExpression) leftX);
                     Number right = safeNumber((ConstantExpression) rightX);
                     if (left == null || right == null) return null;
                     Number result = null;
@@ -164,46 +207,53 @@ public final class ExpressionUtils {
         return null;
     }
 
-    private static Number safeNumber(final ConstantExpression constX) {
-        Object value = constX.getValue();
-        if (value instanceof Number) return (Number) value;
-        return null;
-    }
-
-    private static ConstantExpression configure(final Expression origX, final ConstantExpression newX) {
-        newX.setSourcePosition(origX);
-        return newX;
-    }
-
     /**
-     * Determine if a type matches another type (or array thereof).
+     * Transforms constants that would appear in annotations so they aren't lost.
+     * Subsequent processing determines whether they are valid, this retains the
+     * constant value as a constant expression.
+     * <p>
+     * The attribute values of annotations must be primitive, string, annotation
+     * or enumeration constants. In various places such constants can be seen
+     * during type resolution but won't be readily accessible in later phases,
+     * e.g. they might be embedded into constructor code.
      *
-     * @param targetType the candidate type
-     * @param type the type we are checking against
-     * @param recurse true if we can have multi-dimension arrays; should be false for annotation member types
-     * @return true if the type equals the targetType or array thereof
+     * @param exp the original expression
+     * @return original or transformed expression
      */
-    public static boolean isTypeOrArrayOfType(final ClassNode targetType, final ClassNode type, final boolean recurse) {
-        if (targetType == null) return false;
-        return type.equals(targetType) ||
-                (targetType.isArray() && recurse
-                ? isTypeOrArrayOfType(targetType.getComponentType(), type, recurse)
-                : type.equals(targetType.getComponentType()));
-    }
+    public static Expression transformInlineConstants(final Expression exp) {
+        if (exp instanceof PropertyExpression) {
+            PropertyExpression pe = (PropertyExpression) exp;
+            if (pe.getObjectExpression() instanceof ClassExpression) {
+                ClassNode clazz = pe.getObjectExpression().getType();
+                FieldNode field = ClassNodeUtils.getField(clazz, pe.getPropertyAsString());
+                if (field != null && field.isStatic() && field.isFinal() && !field.isEnum()
+                        && field.getInitialValueExpression() instanceof ConstantExpression) {
+                    ConstantExpression value = (ConstantExpression) field.getInitialValueExpression();
+                    value = new ConstantExpression(value.getValue());
+                    return configure(exp, value);
+                }
+            }
+        } else if (exp instanceof BinaryExpression) {
+            BinaryExpression be = (BinaryExpression) exp;
+            Expression lhs = transformInlineConstants(be.getLeftExpression());
+            Expression rhs = transformInlineConstants(be.getRightExpression());
+            if (be.getOperation().getType() == PLUS // GROOVY-9855: inline string concat
+                    && lhs instanceof ConstantExpression && rhs instanceof ConstantExpression
+                    && ClassHelper.isStringType(lhs.getType()) && ClassHelper.isStringType(rhs.getType())) {
+                return configure(exp, new ConstantExpression(lhs.getText() + rhs.getText()));
+            }
+            be.setLeftExpression(lhs);
+            be.setRightExpression(rhs);
 
-    /**
-     * Determine if a type is derived from Number (or array thereof).
-     *
-     * @param targetType the candidate type
-     * @param recurse true if we can have multi-dimension arrays; should be false for annotation member types
-     * @return true if the type equals the targetType or array thereof
-     */
-    public static boolean isNumberOrArrayOfNumber(final ClassNode targetType, final boolean recurse) {
-        if (targetType == null) return false;
-        return targetType.isDerivedFrom(ClassHelper.Number_TYPE) ||
-                (targetType.isArray() && recurse
-                ? isNumberOrArrayOfNumber(targetType.getComponentType(), recurse)
-                : targetType.isArray() && targetType.getComponentType().isDerivedFrom(ClassHelper.Number_TYPE));
+        } else if (exp instanceof ListExpression) {
+            List<Expression> list = ((ListExpression) exp).getExpressions();
+            for (ListIterator<Expression> it = list.listIterator(); it.hasNext();) {
+                Expression e = transformInlineConstants(it.next());
+                it.set(e);
+            }
+        }
+
+        return exp;
     }
 
     /**
@@ -222,49 +272,45 @@ public final class ExpressionUtils {
     public static Expression transformInlineConstants(final Expression exp, final ClassNode attrType) {
         if (exp instanceof PropertyExpression) {
             PropertyExpression pe = (PropertyExpression) exp;
-            if (pe.getObjectExpression() instanceof ClassExpression) {
-                ClassExpression ce = (ClassExpression) pe.getObjectExpression();
-                ClassNode type = ce.getType();
-                if (type.isEnum() || !(type.isResolved() || type.isPrimaryClassNode()))
-                    return exp;
-
+            ClassNode type = pe.getObjectExpression().getType();
+            if (pe.getObjectExpression() instanceof ClassExpression && !type.isEnum()) {
                 if (type.isPrimaryClassNode()) {
-                    FieldNode fn = type.redirect().getField(pe.getPropertyAsString());
+                    FieldNode fn = type.getField(pe.getPropertyAsString());
                     if (fn != null && fn.isStatic() && fn.isFinal()) {
-                        Expression ce2 = transformInlineConstants(fn.getInitialValueExpression(), attrType);
-                        if (ce2 != null) {
-                            return ce2;
+                        Expression e = transformInlineConstants(fn.getInitialValueExpression(), attrType);
+                        if (e != null) {
+                            return e;
                         }
                     }
-                } else {
+                } else if (type.isResolved()) {
                     try {
                         Field field = type.redirect().getTypeClass().getField(pe.getPropertyAsString());
                         if (field != null && Modifier.isStatic(field.getModifiers()) && Modifier.isFinal(field.getModifiers())) {
-                            ConstantExpression ce3 = new ConstantExpression(field.get(null), true);
-                            configure(exp, ce3);
-                            return ce3;
+                            ConstantExpression ce = new ConstantExpression(field.get(null), true);
+                            configure(exp, ce);
+                            return ce;
                         }
                     } catch (Exception | LinkageError e) {
                         // ignore, leave property expression in place and we'll report later
                     }
                 }
             }
-        } else if (exp instanceof BinaryExpression) {
-            ConstantExpression ce = transformBinaryConstantExpression((BinaryExpression) exp, attrType);
-            if (ce != null) {
-                return ce;
-            }
         } else if (exp instanceof VariableExpression) {
             VariableExpression ve = (VariableExpression) exp;
             if (ve.getAccessedVariable() instanceof FieldNode) {
                 FieldNode fn = (FieldNode) ve.getAccessedVariable();
                 if (fn.isStatic() && fn.isFinal()) {
-                    Expression ce = transformInlineConstants(fn.getInitialValueExpression(), attrType);
-                    if (ce != null) {
-                        return ce;
+                    Expression e = transformInlineConstants(fn.getInitialValueExpression(), attrType);
+                    if (e != null) {
+                        return e;
                     }
                 }
             }
+        } else if (exp instanceof BinaryExpression) {
+            ConstantExpression ce = transformBinaryConstantExpression((BinaryExpression) exp, attrType);
+            if (ce != null) {
+                return ce;
+            }
         } else if (exp instanceof ListExpression) {
             return transformListOfConstants((ListExpression) exp, attrType);
         }
@@ -297,80 +343,19 @@ public final class ExpressionUtils {
         return origList;
     }
 
-    /**
-     * The attribute values of annotations must be primitive, String or Enum constants.
-     * In various places, such constants can be seen during type resolution but won't be
-     * readily accessible in later phases, e.g. they might be embedded into constructor code.
-     * This method transforms constants that would appear in annotations early so they aren't lost.
-     * Subsequent processing determines whether they are valid, this method simply retains
-     * the constant value as a constant expression.
-     *
-     * @param exp the original expression
-     * @return the converted expression
-     */
-    public static Expression transformInlineConstants(final Expression exp) {
-        if (exp instanceof PropertyExpression) {
-            PropertyExpression pe = (PropertyExpression) exp;
-            if (pe.getObjectExpression() instanceof ClassExpression) {
-                ClassExpression ce = (ClassExpression) pe.getObjectExpression();
-                ClassNode type = ce.getType();
-                FieldNode field = ClassNodeUtils.getField(type, pe.getPropertyAsString());
-                if (type.isEnum() && field != null && field.isEnum()) return exp;
-                Expression constant = findConstant(field);
-                if (constant != null) return constant;
-            }
-        } else if (exp instanceof BinaryExpression) {
-            BinaryExpression be = (BinaryExpression) exp;
-            Expression lhs = transformInlineConstants(be.getLeftExpression());
-            Expression rhs = transformInlineConstants(be.getRightExpression());
-            if (be.getOperation().getType() == PLUS && lhs instanceof ConstantExpression && rhs instanceof ConstantExpression &&
-                    isStringType(lhs.getType()) && isStringType(rhs.getType())) {
-                // GROOVY-9855: inline string concat
-                return configure(be, new ConstantExpression(lhs.getText() + rhs.getText()));
-            }
-            be.setLeftExpression(lhs);
-            be.setRightExpression(rhs);
-            return be;
-        } else if (exp instanceof ListExpression) {
-            ListExpression origList = (ListExpression) exp;
-            ListExpression newList = new ListExpression();
-            boolean changed = false;
-            for (Expression e : origList.getExpressions()) {
-                Expression transformed = transformInlineConstants(e);
-                newList.addExpression(transformed);
-                if (transformed != e) changed = true;
-            }
-            if (changed) {
-                newList.setSourcePosition(origList);
-                return newList;
-            }
-            return origList;
-        }
-
-        return exp;
-    }
-
-    private static Expression findConstant(final FieldNode fn) {
-        if (fn != null && !fn.isEnum() && fn.isStatic() && fn.isFinal()
-                && fn.getInitialValueExpression() instanceof ConstantExpression) {
-            return fn.getInitialValueExpression();
-        }
-        return null;
-    }
+    //--------------------------------------------------------------------------
 
-    public static boolean isNullConstant(final Expression expression) {
-        return expression instanceof ConstantExpression && ((ConstantExpression) expression).isNullExpression();
-    }
-
-    public static boolean isThisExpression(final Expression expression) {
-        return expression instanceof VariableExpression && ((VariableExpression) expression).isThisExpression();
+    private static ConstantExpression configure(final Expression origX, final ConstantExpression newX) {
+        newX.setSourcePosition(origX);
+        return newX;
     }
 
-    public static boolean isSuperExpression(final Expression expression) {
-        return expression instanceof VariableExpression && ((VariableExpression) expression).isSuperExpression();
+    private static Number safeNumber(final ConstantExpression constX) {
+        Object value = constX.getValue();
+        return value instanceof Number ? (Number) value : null;
     }
 
-    public static boolean isThisOrSuper(final Expression expression) {
-        return isThisExpression(expression) || isSuperExpression(expression);
+    private ExpressionUtils() {
+        assert false;
     }
 }
diff --git a/src/test/gls/annotations/AnnotationTest.groovy b/src/test/gls/annotations/AnnotationTest.groovy
index 6597b25d78..b8885c8d46 100644
--- a/src/test/gls/annotations/AnnotationTest.groovy
+++ b/src/test/gls/annotations/AnnotationTest.groovy
@@ -18,51 +18,61 @@
  */
 package gls.annotations
 
-import gls.CompilableTestSupport
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
 
 /**
  * Tests various properties of annotation definitions.
  */
-final class AnnotationTest extends CompilableTestSupport {
+final class AnnotationTest {
+
+    private final GroovyShell shell = GroovyShell.withConfig {
+        imports {
+            star 'java.lang.annotation'
+            staticStar 'java.lang.annotation.ElementType'
+            staticStar 'java.lang.annotation.RetentionPolicy'
+        }
+    }
 
     /**
-     * Check that it is possible to annotate an annotation definition with field and method target elements.
+     * Checks that it is possible to annotate an annotation definition with field and method target elements.
      */
+    @Test
     void testAnnotateAnnotationDefinitionWithMethodAndFieldTargetElementTypes() {
-        shouldCompile """
-            import java.lang.annotation.*
-            import static java.lang.annotation.RetentionPolicy.*
-            import static java.lang.annotation.ElementType.*
-
+        shell.parse '''
             @Retention(RUNTIME)
             @Target([METHOD, FIELD])
-            @interface MyAnnotation { }
-        """
+            @interface MyAnnotation {
+            }
+        '''
     }
 
+    @Test
     void testCannotAnnotateAnnotationDefinitionIfTargetIsNotOfTypeOrAnnotationType() {
-        shouldNotCompile """
-            import java.lang.annotation.*
-            import static java.lang.annotation.ElementType.*
-
-            // all target elements except ANNOTATION_TYPE and TYPE
+        shouldFail shell, '''
+            // all targets except ANNOTATION_TYPE, TYPE, TYPE_PARAMETER, TYPE_USE and MODULE
             @Target([CONSTRUCTOR, METHOD, FIELD, LOCAL_VARIABLE, PACKAGE, PARAMETER])
-            @interface MyAnnotation { }
+            @interface AnAnnotation {
+            }
 
-            @MyAnnotation
-            @interface AnotherAnnotation {}
-        """
+            @AnAnnotation
+            @interface AnotherAnnotation {
+            }
+        '''
     }
 
     /**
-     * The @OneToMany cascade parameter takes an array of CascadeType.
+     * The {@code @OneToMany} cascade parameter takes an array of CascadeType.
      * To use this annotation in Java with this parameter, you do <code>@OneToMany(cascade = { CascadeType.ALL })</code>
      * In Groovy, you do <code>@OneToMany(cascade = [ CascadeType.ALL ])</code> (brackets instead of braces)
      * But when there's just one value in the array, the curly braces or brackets can be omitted:
      * <code>@OneToMany(cascade = [ CascadeType.ALL ])</code>
      */
+    @Test
     void testOmittingBracketsForSingleValueArrayParameter() {
-        shouldCompile """
+        shell.parse '''
             import gls.annotations.*
 
             class Book {}
@@ -76,28 +86,30 @@ final class AnnotationTest extends CompilableTestSupport {
 
             assert annotation instanceof OneToMany
             assert annotation.cascade() == [CascadeType.ALL]
-        """
+        '''
     }
 
+    @Test
     void testPrimitiveDefault() {
         // NOTE: for int anything else than a plain number will fail.
         // For example 1l will fail too, even if it could be converted.
         // If this should be changed, then further discussion is needed.
-        shouldNotCompile """
+        shouldFail shell, '''
             @interface X {
                 int x() default "1" // must be integer
             }
-        """
+        '''
 
-        shouldCompile """
+        shell.parse '''
             @interface X {
                 int x() default 1
             }
-        """
+        '''
     }
 
+    @Test
     void testConstant() {
-        assertScript """
+        assertScript shell, '''
             class Baz {
                 // static final int OTHER = 5
                 // below we would like to but can't use:
@@ -116,14 +128,12 @@ final class AnnotationTest extends CompilableTestSupport {
             }
 
             new Baz().run()
-        """
+        '''
     }
 
+    @Test // GROOVY-4811
     void testArrayDefault() {
-        // GROOVY-4811
-        assertScript '''
-            import java.lang.annotation.*
-
+        assertScript shell, '''
             @Retention(RetentionPolicy.RUNTIME)
             @Target(ElementType.TYPE)
             @interface Temp {
@@ -136,95 +146,94 @@ final class AnnotationTest extends CompilableTestSupport {
             assert Bar.getAnnotation(Temp).bar() == ['1']
         '''
 
-        shouldNotCompile """
+        shouldFail shell, '''
             @interface X {
                 String[] x() default ["1",2] // list must contain elements of correct type
             }
-        """
+        '''
 
-        shouldCompile """
+        shell.parse '''
             @interface X {
                 String[] x() default ["a","b"]
             }
-        """
+        '''
     }
 
+    @Test
     void testClassDefault() {
-        shouldNotCompile """
+        shouldFail shell, '''
             @interface X {
                 Class x() default "1" // must be list
             }
-        """
+        '''
 
-        shouldCompile """
+        shell.parse '''
             @interface X {
                 Class x() default Number.class // class with .class
             }
-        """
+        '''
 
-        shouldCompile """
+        shell.parse '''
             @interface X {
                 Class x() default Number
             }
-        """
+        '''
     }
 
+    @Test
     void testEnumDefault() {
-        shouldNotCompile """
+        shouldFail shell, '''
             @interface X {
                 ElementType x() default "1" // must be Enum
             }
-        """
+        '''
 
-        shouldNotCompile """
+        shouldFail shell, '''
             @interface X {
                 ElementType x() default Target.TYPE // must be Enum of correct type
             }
-        """
-
-        shouldCompile """
-            import java.lang.annotation.*
+        '''
 
+        shell.parse '''
             @interface X {
                 ElementType x() default ElementType.METHOD
             }
-        """
+        '''
     }
 
+    @Test
     void testAnnotationDefault() {
-        shouldNotCompile """
-            import java.lang.annotation.*
+        shouldFail shell, '''
             @interface X {
                 Target x() default "1" // must be Annotation
             }
-        """
-
-        shouldNotCompile """
-            import java.lang.annotation.*
+        '''
 
+        shouldFail shell, '''
             @interface X {
                 Target x() default @Retentention // must be correct type
             }
-        """
+        '''
 
-        shouldCompile """
-            import java.lang.annotation.*
+        shell.parse '''
             @interface X {
                 Target x() default @Target([ElementType.TYPE])
             }
-        """
+        '''
     }
 
+    @Test
     void testSelfReference() {
-        shouldNotCompile """
+        shouldFail shell, '''
             @interface X {
                 X x() default @X // self reference must not work
             }
-        """
+        '''
     }
 
+    @Test
     void testCyclicReference() {
-        shouldNotCompile """
+        shouldFail shell, '''
             @interface X {
                 Y x() default @Y
             }
@@ -232,52 +241,57 @@ final class AnnotationTest extends CompilableTestSupport {
             @interface Y {
                 X x() default @X
             }
-        """
+        '''
     }
 
+    @Test
     void testParameter() {
-        shouldNotCompile """
+        shouldFail shell, '''
             @interface X {
                 String x(int x) default "" // annotation members can't have parameters
             }
-        """
+        '''
     }
 
+    @Test
     void testThrowsClause() {
-        shouldNotCompile """
+        shouldFail shell, '''
             @interface X {
                 String x() throws IOException // annotation members can't have exceptions
             }
-        """
+        '''
     }
 
+    @Test
     void testExtends() {
-        shouldNotCompile """
-            @interface X extends Serializable{}
+        shouldFail shell, '''
             // annotation members can't extend
-        """
+            @interface X extends Serializable {
+            }
+        '''
     }
 
+    @Test
     void testInvalidMemberType() {
-        shouldNotCompile """
+        shouldFail shell, '''
             @interface X {
                 Object x() // Object is no type for constants
             }
-        """
+        '''
     }
 
+    @Test
     void testNull() {
-        shouldNotCompile """
+        shouldFail shell, '''
             @interface X {
                 String x() default null // null is no constant for annotations
             }
-        """
+        '''
     }
 
+    @Test
     void testUsage() {
-        assertScript """
-            import java.lang.annotation.*
-
+        assertScript shell, '''
             // a random annotation type
             @Retention(RetentionPolicy.RUNTIME)
             @interface MyAnnotation {
@@ -290,7 +304,6 @@ final class AnnotationTest extends CompilableTestSupport {
                 Target defaultAnnotation() default @Target([ElementType.TYPE])
             }
 
-
             @MyAnnotation(stringValue = "for class", intValue = 100)
             class Foo {}
 
@@ -305,13 +318,14 @@ final class AnnotationTest extends CompilableTestSupport {
 
             assert my.defaultEnum() == ElementType.TYPE
             assert my.defaultAnnotation() instanceof Target
-        """
+        '''
     }
 
+    @Test
     void testJavaAnnotationUsageWithGroovyKeyword() {
-        assertScript """
+        assertScript shell, '''
             package gls.annotations
-            import java.lang.annotation.*
+
             @JavaAnnotation(in = 3)
             class Foo {}
 
@@ -319,24 +333,23 @@ final class AnnotationTest extends CompilableTestSupport {
             assert annotations.size() == 1
             JavaAnnotation my = annotations[0]
             assert my.in() == 3
-        """
+        '''
     }
 
+    @Test
     void testUsageOnClass() {
-        assertScript """
-            import java.lang.annotation.*
-
+        assertScript shell, '''
             @Deprecated
             class Foo{}
 
             assert Foo.class.annotations.size() == 1
-        """
+        '''
     }
 
+    @Test
     void testFieldAndPropertyRuntimeRetention() {
-        assertScript """
+        assertScript shell, '''
             import org.codehaus.groovy.ast.ClassNode
-            import java.lang.annotation.*
 
             @Retention(RetentionPolicy.RUNTIME)
             @interface Annotation1 {}
@@ -359,13 +372,12 @@ final class AnnotationTest extends CompilableTestSupport {
                     assert annotations: "Annotation on 'property1' not found"
                 }
             }
-        """
+        '''
     }
 
+    @Test
     void testSingletonArrayUsage() {
-        assertScript """
-            import java.lang.annotation.*
-
+        assertScript shell, '''
             // a random annnotation type
             @Retention(RetentionPolicy.RUNTIME)
             @interface MyAnnotation {
@@ -380,29 +392,31 @@ final class AnnotationTest extends CompilableTestSupport {
             MyAnnotation my = annotations[0]
             assert my.things().size() == 1
             assert my.things()[0] == "x"
-        """
+        '''
     }
 
+    @Test
     void testSettingAnnotationMemberTwice() {
-        shouldNotCompile """
-        package gls.annotations
+        shouldFail shell, '''
+            package gls.annotations
 
-        @JavaAnnotation(in = 1, in = 2)
-        class Foo {}
-    """
+            @JavaAnnotation(in = 1, in = 2)
+            class Foo {}
+        '''
     }
 
+    @Test
     void testGetterCallWithSingletonAnnotation() {
-        assertScript """
+        assertScript shell, '''
             @Singleton class MyService3410{}
             assert MyService3410.instance != null
             assert MyService3410.getInstance() != null
-      """
+        '''
     }
 
-    void testAttributePropertyConstants() {
-        assertScript """
-            import java.lang.annotation.*
+    @Test
+    void testAttributeValueConstants() {
+        assertScript shell, '''
             import static Constants.*
 
             @Retention(RetentionPolicy.RUNTIME)
@@ -434,12 +448,12 @@ final class AnnotationTest extends CompilableTestSupport {
             assert ClassWithAnnotationUsingConstant.getDeclaredField('annotatedStrings').annotations[0].array() == ['foo', 'bar', "GROOVY"]
             assert ClassWithAnnotationUsingConstant.getDeclaredField('annotatedMath1').annotations[0].value() == Math.PI
             assert ClassWithAnnotationUsingConstant.getDeclaredField('annotatedMath2').annotations[0].value() == Constants.APPROX_PI
-        """
+        '''
     }
 
-    void testNestedAttributePropertyConstants() {
-        assertScript """
-            import java.lang.annotation.*
+    @Test
+    void testAttributeValueConstants2() {
+        assertScript shell, '''
             import static Constants.*
 
             @Retention(RetentionPolicy.RUNTIME)
@@ -473,13 +487,126 @@ final class AnnotationTest extends CompilableTestSupport {
 
             assert ClassWithNestedAnnotationUsingConstant.getDeclaredField('outer').annotations[0].array() == ['foo', 'GROOVY', 'baz']
             assert ClassWithNestedAnnotationUsingConstant.getDeclaredField('outer').annotations[0].value().value() == ['foobar', 'bar', 'GROOVY']
-        """
+        '''
     }
 
-    void testRuntimeRetentionAtAllLevels() {
-        assertScript """
-            import java.lang.annotation.*
+    @Test // GROOVY-3278
+    void testAttributeValueConstants3() {
+        assertScript shell, '''
+            import gls.annotations.*
+
+            @ConstAnnotation(ints = 42)
+            class Child1 extends Base3278 {}
+
+            class OtherConstants {
+                static final Integer CONST3 = 3278
+            }
+
+            @ConstAnnotation(ints = [-1, Base3278.CONST, Base3278.CONST1, Base3278.CONST2, OtherConstants.CONST3, Integer.MIN_VALUE],
+                          strings = ['foo', Base3278.CONST4, Base3278.CONST5, Base3278.CONST5 + 'bing'])
+            class Child3 extends Base3278 {}
+
+            assert new Child1().ints() == [42]
+            assert new Child2().ints() == [2147483647]
+            new Child3().with {
+                assert ints() == [-1, 3278, 2048, 2070, 3278, -2147483648]
+                assert strings() == ['foo', 'foobar', 'foobarbaz', 'foobarbazbing']
+            }
+        '''
+    }
+
+    @Test // GROOVY-8898
+    void testAttributeValueConstants4() {
+        assertScript shell, '''
+            @Retention(RetentionPolicy.RUNTIME)
+            @Target(ElementType.TYPE)
+            @interface MyAnnotation {
+                String[] groups() default []
+                MyEnum alt() default MyEnum.ALT1
+            }
+
+            class Base {
+                static final String CONST = 'bar'
+                def groups() { getClass().annotations[0].groups() }
+                def alt() { getClass().annotations[0].alt() }
+            }
+
+            enum MyEnum {
+                ALT1, ALT2;
+                public static final String CONST = 'baz'
+            }
+
+            @MyAnnotation(groups = ['foo', Base.CONST, MyEnum.CONST], alt = MyEnum.ALT2)
+            class Child extends Base {}
+
+            new Child().with {
+                assert groups() == ['foo', 'bar', 'baz']
+                assert alt() == MyEnum.ALT2
+            }
+        '''
+    }
+
+    @Test
+    void testAttributeValueConstants5() {
+        assertScript shell, '''
+            @Retention(RetentionPolicy.RUNTIME)
+            @Target(ElementType.FIELD)
+            @interface Pattern {
+                String regexp()
+            }
+
+            @Retention(RetentionPolicy.RUNTIME)
+            @Target(ElementType.FIELD)
+            @interface LimitedDouble {
+                double max() default Double.MAX_VALUE
+                double min() default Double.MIN_VALUE
+                double zero() default (1.0 - 1.0d) * 1L
+            }
+
+            @Retention(RetentionPolicy.RUNTIME)
+            @Target(ElementType.FIELD)
+            @interface IntegerConst {
+                int kilo() default 0b1 << 10
+                int two() default 64 >> 5
+                int nine() default 0b1100 ^ 0b101
+                int answer() default 42
+            }
+
+            class MyClass {
+                private static interface Constants {
+                    static final String CONST = 'foo' + 'bar'
+                }
+
+                @Pattern(regexp=Constants.CONST)
+                String myString
+
+                @LimitedDouble(min=0.0d, max=50.0d * 2)
+                double myDouble
+
+                @IntegerConst(answer=(5 ** 2) * (2 << 1))
+                int myInteger
+            }
+
+            assert MyClass.getDeclaredField('myString').annotations[0].regexp() == 'foobar'
+
+            MyClass.getDeclaredField('myDouble').annotations[0].with {
+                assert max() == 100.0d
+                assert min() == 0.0d
+                assert zero() == 0.0d
+            }
 
+            MyClass.getDeclaredField('myInteger').annotations[0].with {
+                assert kilo() == 1024
+                assert two() == 2
+                assert nine() == 9
+                assert answer() == 100
+            }
+        '''
+    }
+
+    @Test
+    void testRuntimeRetentionAtAllLevels() {
+        assertScript shell, '''
             @Retention(RetentionPolicy.RUNTIME)
             @Target([ElementType.TYPE, ElementType.METHOD, ElementType.PARAMETER, ElementType.CONSTRUCTOR, ElementType.FIELD])
             @interface MyAnnotation {
@@ -511,15 +638,13 @@ final class AnnotationTest extends CompilableTestSupport {
             def constructor = c1.class.constructors[0]
             assert constructor.annotations[0].value() == 'constructor'
             assert constructor.parameterAnnotations[0][0].value() == 'constructor param'
-        """
+        '''
     }
 
+    @Test
     void testAnnotationWithValuesNotGivenForAttributesWithoutDefaults() {
-        def scriptStr
-        scriptStr = """
-            import java.lang.annotation.Retention
-            import java.lang.annotation.RetentionPolicy
-
+        // compiler should not allow this, because there is no default value for x
+        shouldFail shell, '''
             @Retention(RetentionPolicy.RUNTIME)
             @interface Annot3454V1 {
                 String x()
@@ -527,15 +652,10 @@ final class AnnotationTest extends CompilableTestSupport {
 
             @Annot3454V1
             class Bar {}
-        """
-
-        // compiler should not allow this, because there is no default value for x
-        shouldNotCompile(scriptStr)
-
-        scriptStr = """
-            import java.lang.annotation.Retention
-            import java.lang.annotation.RetentionPolicy
+        '''
 
+        // compiler should not allow this, because there is no default value for y
+        shouldFail shell, '''
             @Retention(RetentionPolicy.RUNTIME)
             @interface Annot3454V2 {
                 String x() default 'xxx'
@@ -544,15 +664,10 @@ final class AnnotationTest extends CompilableTestSupport {
 
             @Annot3454V2
             class Bar {}
-        """
-
-        // compiler should not allow this, because there is no default value for y
-        shouldNotCompile(scriptStr)
-
-        scriptStr = """
-            import java.lang.annotation.Retention
-            import java.lang.annotation.RetentionPolicy
+        '''
 
+        // compiler should allow this, because there is default value for x and value provided for y
+        assertScript shell, '''
             @Retention(RetentionPolicy.RUNTIME)
             @interface Annot3454V3 {
                 String x() default 'xxx'
@@ -565,55 +680,53 @@ final class AnnotationTest extends CompilableTestSupport {
             def anno = Bar.class.getAnnotation(Annot3454V3)
             assert anno.x() == 'xxx'
             assert anno.y() == 'yyy'
-        """
-        // compiler should allow this, because there is default value for x and value provided for y
-        assertScript(scriptStr)
+        '''
     }
 
+    @Test
     void testAllowedTargetsCheckAlsoWorksForAnnotationTypesDefinedOutsideThisCompilationUnit() {
-        shouldCompile """
-            @java.lang.annotation.Documented
-            @interface Foo {}
-        """
+        shell.parse '''
+            @Documented
+            @interface Foo {
+            }
+        '''
 
-        shouldNotCompile """
-            @java.lang.annotation.Documented
+        shouldFail shell, '''
+            @Documented
             class Foo {
                 def bar
             }
-        """
+        '''
 
-        shouldNotCompile """
+        shouldFail shell, '''
             class Foo {
-                @java.lang.annotation.Documented
+                @Documented
                 def bar
             }
-        """
+        '''
 
-        shouldCompile """
+        shell.parse '''
             import org.junit.*
 
             class Foo {
                 @Test
                 void foo() {}
             }
-        """
+        '''
 
-        shouldNotCompile """
+        shouldFail shell, '''
             import org.junit.*
 
             @Test
             class Foo {
                 void foo() {}
             }
-        """
+        '''
     }
 
-    // GROOVY-6025
+    @Test // GROOVY-6025
     void testAnnotationDefinitionDefaultValues() {
-        assertScript '''
-            import java.lang.annotation.*
-
+        assertScript shell, '''
             @Retention(RetentionPolicy.RUNTIME)
             @Target([ElementType.TYPE, ElementType.METHOD])
             public @interface Foo {
@@ -644,49 +757,62 @@ final class AnnotationTest extends CompilableTestSupport {
                    string == "b=6|c1='A'|c2='B'|c3='C'|c4='D'|d1=2.0|d2=2.1|d3=2.2|d4=2.3|f1=1.0f|f2=1.1f|f3=1.2f|f4=1.3f|i1=0|i2=1|s1=2|s2=3|s3=4|s4=5" ||
                    // changed in some jdk14 versions
                    string == "b=(byte)0x06|c1='A'|c2='B'|c3='C'|c4='D'|d1=2.0|d2=2.1|d3=2.2|d4=2.3|f1=1.0f|f2=1.1f|f3=1.2f|f4=1.3f|i1=0|i2=1|s1=2|s2=3|s3=4|s4=5"
-
         '''
     }
 
-    // GROOVY-6093
+    @Test // GROOVY-6093
     void testAnnotationOnEnumConstant() {
-        assertScript '''import gls.annotations.XmlEnum
+        assertScript shell, '''
+            import gls.annotations.XmlEnum
             import gls.annotations.XmlEnumValue
 
             @XmlEnum
             enum GroovyEnum {
-                @XmlEnumValue("good")
-                BAD
+                @XmlEnumValue('good') BAD
             }
+
             assert GroovyEnum.class.getField('BAD').isAnnotationPresent(XmlEnumValue)
         '''
     }
 
-    // GROOVY-7151
+    @Test // GROOVY-7151
     void testAnnotateAnnotationDefinitionWithAnnotationWithTypeTarget() {
-        shouldCompile codeWithMetaAnnotationWithTarget("TYPE")
+        shell.parse codeWithMetaAnnotationWithTarget('TYPE')
     }
 
     void testAnnotateAnnotationDefinitionWithAnnotationWithAnnotationTypeTarget() {
-        shouldCompile codeWithMetaAnnotationWithTarget("ANNOTATION_TYPE")
+        shell.parse codeWithMetaAnnotationWithTarget('ANNOTATION_TYPE')
+    }
+
+    private static String codeWithMetaAnnotationWithTarget(targetElementTypeName) {
+        return """
+            import java.lang.annotation.*
+            import static java.lang.annotation.RetentionPolicy.*
+            import static java.lang.annotation.ElementType.*
+
+            @Retention(RUNTIME)
+            @Target(${targetElementTypeName})
+            @interface Import {}
+
+            @Retention(RUNTIME)
+            @Target([FIELD])
+            @Import
+            @interface EnableFeature { }
+        """
     }
 
-    // GROOVY-7806
+    @Test // GROOVY-7806
     void testNewlinesAllowedBeforeBlock() {
-        shouldCompile '''
+        shell.parse '''
             @interface ANNOTATION_A
             {
             }
         '''
     }
 
-    // GROOVY-8226
+    @Test // GROOVY-8226
     void testAnnotationOnParameterType() {
-        assertScript '''
-            import java.lang.annotation.*
-            import static java.lang.annotation.ElementType.*
-            import static java.lang.annotation.RetentionPolicy.*
-
+        assertScript shell, '''
             @Target([PARAMETER, FIELD, METHOD, ANNOTATION_TYPE, TYPE_USE])
             @Retention(RUNTIME)
             public @interface NonNull { }
@@ -706,11 +832,9 @@ final class AnnotationTest extends CompilableTestSupport {
         '''
     }
 
-    // GROOVY-8234
+    @Test // GROOVY-8234
     void testAnnotationWithRepeatableSupported() {
-        assertScript '''
-            import java.lang.annotation.*
-
+        assertScript shell, '''
             class MyClass {
                 // TODO confirm the JDK9 behavior is what we expect
                 private static final List<String> expected = [
@@ -766,11 +890,9 @@ final class AnnotationTest extends CompilableTestSupport {
         '''
     }
 
-    // GROOVY-9452
+    @Test // GROOVY-9452
     void testDuplicationAnnotationOnClassWithParams() {
-        def err = shouldFail '''
-            import java.lang.annotation.*
-
+        def err = shouldFail shell, '''
             @Target(ElementType.TYPE)
             @Retention(RetentionPolicy.RUNTIME)
             @Repeatable(As)
@@ -792,9 +914,9 @@ final class AnnotationTest extends CompilableTestSupport {
         assert err =~ /Cannot specify duplicate annotation/
     }
 
-    // GROOVY-7033
+    @Test // GROOVY-7033
     void testClassLiteralsRecognizedForAnonymousInnerClassAnnotationUsage() {
-        shouldCompile '''
+        shell.parse '''
             @interface A {
                 Class value()
             }
@@ -809,8 +931,9 @@ final class AnnotationTest extends CompilableTestSupport {
         '''
     }
 
+    @Test
     void testVariableExpressionsReferencingConstantsSeenForAnnotationAttributes() {
-        shouldCompile '''
+        shell.parse '''
             class C {
                 public static final String VALUE = 'rawtypes'
                 @SuppressWarnings(VALUE)
@@ -819,125 +942,9 @@ final class AnnotationTest extends CompilableTestSupport {
         '''
     }
 
-    void testSimpleBinaryExpressions() {
-        assertScript '''
-            import java.lang.annotation.*
-
-            @Retention(RetentionPolicy.RUNTIME)
-            @Target(ElementType.FIELD)
-            @interface Pattern {
-                String regexp()
-            }
-
-            @Retention(RetentionPolicy.RUNTIME)
-            @Target(ElementType.FIELD)
-            @interface LimitedDouble {
-                double max() default Double.MAX_VALUE
-                double min() default Double.MIN_VALUE
-                double zero() default (1.0 - 1.0d) * 1L
-            }
-
-            @Retention(RetentionPolicy.RUNTIME)
-            @Target(ElementType.FIELD)
-            @interface IntegerConst {
-                int kilo() default 0b1 << 10
-                int two() default 64 >> 5
-                int nine() default 0b1100 ^ 0b101
-                int answer() default 42
-            }
-
-            class MyClass {
-                private static interface Constants {
-                    static final String CONST = 'foo' + 'bar'
-                }
-
-                @Pattern(regexp=Constants.CONST)
-                String myString
-
-                @LimitedDouble(min=0.0d, max=50.0d * 2)
-                double myDouble
-
-                @IntegerConst(answer=(5 ** 2) * (2 << 1))
-                int myInteger
-            }
-
-            assert MyClass.getDeclaredField('myString').annotations[0].regexp() == 'foobar'
-
-            MyClass.getDeclaredField('myDouble').annotations[0].with {
-                assert max() == 100.0d
-                assert min() == 0.0d
-                assert zero() == 0.0d
-            }
-
-            MyClass.getDeclaredField('myInteger').annotations[0].with {
-                assert kilo() == 1024
-                assert two() == 2
-                assert nine() == 9
-                assert answer() == 100
-            }
-        '''
-    }
-
-    void testAnnotationAttributeConstantFromPrecompiledGroovyClass() {
-        // GROOVY-3278
-        assertScript '''
-            import gls.annotations.*
-
-            @ConstAnnotation(ints = 42)
-            class Child1 extends Base3278 {}
-
-            class OtherConstants {
-                static final Integer CONST3 = 3278
-            }
-
-            @ConstAnnotation(ints = [-1, Base3278.CONST, Base3278.CONST1, Base3278.CONST2, OtherConstants.CONST3, Integer.MIN_VALUE],
-                          strings = ['foo', Base3278.CONST4, Base3278.CONST5, Base3278.CONST5 + 'bing'])
-            class Child3 extends Base3278 {}
-
-            assert new Child1().ints() == [42]
-            assert new Child2().ints() == [2147483647]
-            new Child3().with {
-                assert ints() == [-1, 3278, 2048, 2070, 3278, -2147483648]
-                assert strings() == ['foo', 'foobar', 'foobarbaz', 'foobarbazbing']
-            }
-        '''
-    }
-
-    void testAnnotationAttributeConstantFromEnumConstantField() {
-        // GROOVY-8898
-        assertScript '''
-            import java.lang.annotation.*
-
-            @Retention(RetentionPolicy.RUNTIME)
-            @Target(ElementType.TYPE)
-            @interface MyAnnotation {
-                String[] groups() default []
-                MyEnum alt() default MyEnum.ALT1
-            }
-
-            class Base {
-                static final String CONST = 'bar'
-                def groups() { getClass().annotations[0].groups() }
-                def alt() { getClass().annotations[0].alt() }
-            }
-
-            enum MyEnum {
-                ALT1, ALT2;
-                public static final String CONST = 'baz'
-            }
-
-            @MyAnnotation(groups = ['foo', Base.CONST, MyEnum.CONST], alt = MyEnum.ALT2)
-            class Child extends Base {}
-
-            new Child().with {
-                assert groups() == ['foo', 'bar', 'baz']
-                assert alt() == MyEnum.ALT2
-            }
-        '''
-    }
-
+    @Test
     void testAnnotationRetentionMirrorsJava() {
-        assertScript '''
+        assertScript shell, '''
             for (retention in ['', '@Retention(SOURCE)', '@Retention(CLASS)', '@Retention(RUNTIME)']) {
                 def src = """
                     import java.lang.annotation.Retention;
@@ -952,9 +959,9 @@ final class AnnotationTest extends CompilableTestSupport {
         '''
     }
 
+    @Test
     void testAnnotationWithRepeatableSupportedPrecompiledJava() {
-        assertScript '''
-            import java.lang.annotation.*
+        assertScript shell, '''
             import gls.annotations.*
 
             class MyClass {
@@ -991,22 +998,4 @@ final class AnnotationTest extends CompilableTestSupport {
             }
         '''
     }
-
-    //Parametrized tests in Spock would allow to make it much more readable
-    private static String codeWithMetaAnnotationWithTarget(String targetElementTypeName) {
-        """
-            import java.lang.annotation.*
-            import static java.lang.annotation.RetentionPolicy.*
-            import static java.lang.annotation.ElementType.*
-
-            @Retention(RUNTIME)
-            @Target(${targetElementTypeName})
-            @interface Import {}
-
-            @Retention(RUNTIME)
-            @Target([FIELD])
-            @Import
-            @interface EnableFeature { }
-        """
-    }
 }


[groovy] 03/03: GROOVY-10068: retain field type during constant inlining

Posted by em...@apache.org.
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

commit 8215ed3c389b565dfe8cd01eb4727f018b11a9a7
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Jul 6 11:45:20 2022 -0500

    GROOVY-10068: retain field type during constant inlining
---
 .../java/org/apache/groovy/ast/tools/ExpressionUtils.java |  5 +++++
 src/test/gls/annotations/AnnotationTest.groovy            | 15 +++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java b/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java
index 373f8a283a..aaf4d3e7a0 100644
--- a/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java
+++ b/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java
@@ -230,6 +230,11 @@ public final class ExpressionUtils {
                         && field.getInitialValueExpression() instanceof ConstantExpression) {
                     ConstantExpression value = (ConstantExpression) field.getInitialValueExpression();
                     value = new ConstantExpression(value.getValue());
+                    // GROOVY-10068, et al.: retain field's type
+                    if (!value.getType().equals(field.getType())
+                            && ClassHelper.isNumberType(field.getType()))
+                        value.setType(field.getType());
+                    // TODO: Boolean, Character, String
                     return configure(exp, value);
                 }
             }
diff --git a/src/test/gls/annotations/AnnotationTest.groovy b/src/test/gls/annotations/AnnotationTest.groovy
index b8885c8d46..291ec12af9 100644
--- a/src/test/gls/annotations/AnnotationTest.groovy
+++ b/src/test/gls/annotations/AnnotationTest.groovy
@@ -604,6 +604,21 @@ final class AnnotationTest {
         '''
     }
 
+    @Test // GROOVY-10068
+    void testAttributeValueConstants6() {
+        assertScript shell, '''
+            @interface A {
+                short value()
+            }
+            class C {
+                public static final short ONE = 1
+            }
+
+            @A(C.ONE)
+            def local
+        '''
+    }
+
     @Test
     void testRuntimeRetentionAtAllLevels() {
         assertScript shell, '''


[groovy] 02/03: refactor duplication

Posted by em...@apache.org.
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

commit 9472fd02894201de1bcbf45cfd48c1ee45d2282e
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Jul 6 12:57:02 2022 -0500

    refactor duplication
---
 .../codehaus/groovy/control/ResolveVisitor.java    | 54 +++++++---------------
 1 file changed, 16 insertions(+), 38 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index 79e7c4fbec..31ab094b0f 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -19,7 +19,6 @@
 package org.codehaus.groovy.control;
 
 import groovy.lang.Tuple2;
-import org.apache.groovy.ast.tools.ExpressionUtils;
 import org.codehaus.groovy.GroovyBugError;
 import org.codehaus.groovy.ast.ASTNode;
 import org.codehaus.groovy.ast.AnnotationNode;
@@ -44,7 +43,6 @@ import org.codehaus.groovy.ast.expr.BinaryExpression;
 import org.codehaus.groovy.ast.expr.CastExpression;
 import org.codehaus.groovy.ast.expr.ClassExpression;
 import org.codehaus.groovy.ast.expr.ClosureExpression;
-import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
 import org.codehaus.groovy.ast.expr.DeclarationExpression;
 import org.codehaus.groovy.ast.expr.Expression;
@@ -81,7 +79,7 @@ import java.util.Set;
 import java.util.function.Predicate;
 
 import static groovy.lang.Tuple.tuple;
-import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
+import static org.apache.groovy.ast.tools.ExpressionUtils.transformInlineConstants;
 import static org.codehaus.groovy.ast.tools.ClosureUtils.getParametersSafe;
 
 /**
@@ -428,7 +426,7 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
         ClassNode cn = currentClass;
         Set<ClassNode> cycleCheck = new HashSet<>();
         // GROOVY-4043: for type "X", try "A$X" with each type in the class hierarchy (except for Object)
-        for (; cn != null && cycleCheck.add(cn) && !isObjectType(cn); cn = cn.getSuperClass()) {
+        for (; cn != null && cycleCheck.add(cn) && !ClassHelper.isObjectType(cn); cn = cn.getSuperClass()) {
             if (setRedirect(type, cn)) return true;
         }
 
@@ -1196,12 +1194,7 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
     }
 
     protected Expression transformAnnotationConstantExpression(final AnnotationConstantExpression ace) {
-        AnnotationNode an = (AnnotationNode) ace.getValue();
-        ClassNode type = an.getClassNode();
-        resolveOrFail(type, " for annotation", an);
-        for (Map.Entry<String, Expression> member : an.getMembers().entrySet()) {
-            member.setValue(transform(member.getValue()));
-        }
+        visitAnnotation((AnnotationNode) ace.getValue());
         return ace;
     }
 
@@ -1233,40 +1226,25 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
     @Override
     protected void visitAnnotation(final AnnotationNode node) {
         resolveOrFail(node.getClassNode(), " for annotation", node, true);
-
-        for (Map.Entry<String, Expression> member : node.getMembers().entrySet()) {
-            Expression value = transformInlineConstants(transform(member.getValue()));
+        // duplicates part of AnnotationVisitor because we cannot wait until later
+        for (Map.Entry<String, Expression> entry : node.getMembers().entrySet()) {
+            // resolve constant-looking expressions statically
+            // do it now since they get transformed away later
+            Expression value = transform(entry.getValue());
+            value = transformInlineConstants(value);
             checkAnnotationMemberValue(value);
-            member.setValue(value);
-        }
-    }
-
-    // resolve constant-looking expressions statically (do here as they get transformed away later)
-    private static Expression transformInlineConstants(final Expression exp) {
-        if (exp instanceof AnnotationConstantExpression) {
-            ConstantExpression ce = (ConstantExpression) exp;
-            if (ce.getValue() instanceof AnnotationNode) {
-                // replicate a little bit of AnnotationVisitor here
-                // because we can't wait until later to do this
-                AnnotationNode an = (AnnotationNode) ce.getValue();
-                for (Map.Entry<String, Expression> member : an.getMembers().entrySet()) {
-                    member.setValue(transformInlineConstants(member.getValue()));
-                }
-            }
-        } else {
-            return ExpressionUtils.transformInlineConstants(exp);
+            entry.setValue(value);
         }
-        return exp;
     }
 
-    private void checkAnnotationMemberValue(final Expression newValue) {
-        if (newValue instanceof PropertyExpression) {
-            PropertyExpression pe = (PropertyExpression) newValue;
+    private void checkAnnotationMemberValue(final Expression value) {
+        if (value instanceof PropertyExpression) {
+            PropertyExpression pe = (PropertyExpression) value;
             if (!(pe.getObjectExpression() instanceof ClassExpression)) {
                 addError("unable to find class '" + pe.getText() + "' for annotation attribute constant", pe.getObjectExpression());
             }
-        } else if (newValue instanceof ListExpression) {
-            ListExpression le = (ListExpression) newValue;
+        } else if (value instanceof ListExpression) {
+            ListExpression le = (ListExpression) value;
             for (Expression e : le.getExpressions()) {
                 checkAnnotationMemberValue(e);
             }
@@ -1547,7 +1525,7 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
             GenericsType[] parameters = type.redirect().getGenericsTypes();
             if (parameters != null && i < parameters.length) {
                 ClassNode implicitBound = parameters[i].getType();
-                if (!isObjectType(implicitBound))
+                if (!ClassHelper.isObjectType(implicitBound))
                     argument.getType().setRedirect(implicitBound);
             }
         }