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/09/26 15:38:22 UTC

[groovy] branch master updated: GROOVY-10763: `setSuperClass`/`setInterfaces` updates generics indicator

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 c5b3a4f7ba GROOVY-10763: `setSuperClass`/`setInterfaces` updates generics indicator
c5b3a4f7ba is described below

commit c5b3a4f7badb6cef592c935bb05f60ad9243f95f
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Sep 26 09:28:29 2022 -0500

    GROOVY-10763: `setSuperClass`/`setInterfaces` updates generics indicator
---
 .../java/org/codehaus/groovy/ast/ClassNode.java    | 160 +++++++++++----------
 .../groovy/ast/tools/WideningCategories.java       |  48 +++----
 .../groovy/transform/stc/UnionTypeClassNode.java   |  13 +-
 .../org/codehaus/groovy/ast/ClassNodeTest.java     |  63 +++++---
 4 files changed, 160 insertions(+), 124 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/ClassNode.java b/src/main/java/org/codehaus/groovy/ast/ClassNode.java
index b729d74681..7ca9619bdc 100644
--- a/src/main/java/org/codehaus/groovy/ast/ClassNode.java
+++ b/src/main/java/org/codehaus/groovy/ast/ClassNode.java
@@ -48,7 +48,6 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 
-import static java.util.Arrays.stream;
 import static java.util.stream.Collectors.toList;
 import static org.apache.groovy.ast.tools.MethodNodeUtils.getCodeAsBlock;
 import static org.codehaus.groovy.ast.ClassHelper.SEALED_TYPE;
@@ -151,8 +150,9 @@ public class ClassNode extends AnnotatedNode {
     private MixinNode[] mixins;
     private List<Statement> objectInitializers;
     private List<ConstructorNode> constructors;
-    private final MapOfLists methods;
-    private List<MethodNode> methodsList;
+    // TODO: initialize for primary nodes only!
+    private final MapOfLists methods = new MapOfLists();
+    private List<MethodNode> methodsList = Collections.emptyList();
     private List<FieldNode> fields;
     private List<PropertyNode> properties;
     private Map<String, FieldNode> fieldIndex;
@@ -164,6 +164,7 @@ public class ClassNode extends AnnotatedNode {
     private ClassNode superClass;
     protected boolean isPrimaryNode;
     protected List<InnerClassNode> innerClasses;
+    // TODO: initialize for primary nodes only!!
     private List<ClassNode> permittedSubclasses = new ArrayList<>(4);
     private List<AnnotationNode> typeAnnotations = Collections.emptyList();
     private List<RecordComponentNode> recordComponents = Collections.emptyList();
@@ -175,11 +176,11 @@ public class ClassNode extends AnnotatedNode {
 
     // use this to synchronize access for the lazy init
     protected final Object lazyInitLock = new Object();
-
-    // clazz!=null when resolved
-    protected Class clazz;
     // only false when this classNode is constructed from a class
     private volatile boolean lazyInitDone = true;
+
+    // clazz!=null when resolved
+    protected Class<?> clazz;
     // not null if if the ClassNode is an array
     private ClassNode componentType;
     // if not null this instance is handled as proxy
@@ -196,6 +197,63 @@ public class ClassNode extends AnnotatedNode {
     // of 1 element describing the name of the placeholder
     private boolean placeholder;
 
+    //--------------------------------------------------------------------------
+
+    /**
+     * @param name       the fully-qualified name of the class
+     * @param modifiers  the modifiers; see {@link java.lang.reflect.Modifier Modifier} or {@link org.objectweb.asm.Opcodes Opcodes}
+     * @param superClass the base class; use "java.lang.Object" if no direct base class
+     * @param interfaces the interfaces
+     * @param mixins     the mixins
+     */
+    public ClassNode(final String name, final int modifiers, final ClassNode superClass, final ClassNode[] interfaces, final MixinNode[] mixins) {
+        this.name = name;
+        this.modifiers = modifiers;
+
+        this.isPrimaryNode = true;
+        setSuperClass(superClass);
+        setInterfaces(interfaces);
+        setMixins(mixins);
+    }
+
+    /**
+     * @param name       the fully-qualified name of the class
+     * @param modifiers  the modifiers; see {@link java.lang.reflect.Modifier Modifier} or {@link org.objectweb.asm.Opcodes Opcodes}
+     * @param superClass the base class; use "java.lang.Object" if no direct base class
+     */
+    public ClassNode(final String name, final int modifiers, final ClassNode superClass) {
+        this(name, modifiers, superClass, ClassNode.EMPTY_ARRAY, MixinNode.EMPTY_ARRAY);
+    }
+
+    /**
+     * Creates a non-primary {@code ClassNode} from a real class.
+     */
+    public ClassNode(final Class<?> c) {
+        this(c.getName(), c.getModifiers(), null, null, MixinNode.EMPTY_ARRAY);
+        this.clazz = c;
+        this.lazyInitDone = false;
+        this.isPrimaryNode = false;
+    }
+
+    /**
+     * Constructor used by {@code makeArray()} if a real class is available.
+     */
+    private ClassNode(final Class<?> c, final ClassNode componentType) {
+        this(c);
+        this.componentType = componentType;
+    }
+
+    /**
+     * Constructor used by {@code makeArray()} if no real class is available.
+     */
+    private ClassNode(final ClassNode componentType) {
+        this(componentType.getName() + "[]", ACC_PUBLIC, ClassHelper.OBJECT_TYPE);
+        this.componentType = componentType.redirect();
+        this.isPrimaryNode = false;
+    }
+
+    //--------------------------------------------------------------------------
+
     /**
      * Returns the {@code ClassNode} this node is a proxy for or the node itself.
      */
@@ -245,33 +303,6 @@ public class ClassNode extends AnnotatedNode {
         return redirect().isPrimaryNode || (componentType != null && componentType.isPrimaryClassNode());
     }
 
-    /**
-     * Constructor used by {@code makeArray()} if no real class is available.
-     */
-    private ClassNode(ClassNode componentType) {
-        this(componentType.getName() + "[]", ACC_PUBLIC, ClassHelper.OBJECT_TYPE);
-        this.componentType = componentType.redirect();
-        isPrimaryNode = false;
-    }
-
-    /**
-     * Constructor used by {@code makeArray()} if a real class is available.
-     */
-    private ClassNode(Class<?> c, ClassNode componentType) {
-        this(c);
-        this.componentType = componentType;
-    }
-
-    /**
-     * Creates a non-primary {@code ClassNode} from a real class.
-     */
-    public ClassNode(Class<?> c) {
-        this(c.getName(), c.getModifiers(), null, null, MixinNode.EMPTY_ARRAY);
-        clazz = c;
-        lazyInitDone = false;
-        isPrimaryNode = false;
-    }
-
     /**
      * The complete class structure will be initialized only when really needed
      * to avoid having too many objects during compilation.
@@ -318,45 +349,16 @@ public class ClassNode extends AnnotatedNode {
         this.syntheticPublic = syntheticPublic;
     }
 
-    /**
-     * @param name       the fully-qualified name of the class
-     * @param modifiers  the modifiers; see {@link org.objectweb.asm.Opcodes}
-     * @param superClass the base class; use "java.lang.Object" if no direct base class
-     */
-    public ClassNode(String name, int modifiers, ClassNode superClass) {
-        this(name, modifiers, superClass, EMPTY_ARRAY, MixinNode.EMPTY_ARRAY);
-    }
-
-    /**
-     * @param name       the fully-qualified name of the class
-     * @param modifiers  the modifiers; see {@link org.objectweb.asm.Opcodes}
-     * @param superClass the base class; use "java.lang.Object" if no direct base class
-     * @param interfaces the interfaces for this class
-     * @param mixins     the mixins for this class
-     */
-    public ClassNode(String name, int modifiers, ClassNode superClass, ClassNode[] interfaces, MixinNode[] mixins) {
-        this.name = name;
-        this.modifiers = modifiers;
-        this.superClass = superClass;
-        this.interfaces = interfaces;
-        this.mixins = mixins;
-
-        isPrimaryNode = true;
-        if (superClass != null) {
-            usesGenerics = superClass.isUsingGenerics();
-        }
-        if (!usesGenerics && interfaces != null) {
-            usesGenerics = stream(interfaces).anyMatch(ClassNode::isUsingGenerics);
+    public void setSuperClass(final ClassNode superClass) {
+        if (redirect != null) {
+            redirect.setSuperClass(superClass);
+        } else {
+            this.superClass = superClass;
+            // GROOVY-10763: update generics indicator
+            if (superClass != null && !usesGenerics && isPrimaryNode) {
+                usesGenerics = superClass.isUsingGenerics();
+            }
         }
-        methods = new MapOfLists();
-        methodsList = Collections.emptyList();
-    }
-
-    /**
-     * Sets the superclass of this {@code ClassNode}.
-     */
-    public void setSuperClass(ClassNode superClass) {
-        redirect().superClass = superClass;
     }
 
     /**
@@ -381,11 +383,17 @@ public class ClassNode extends AnnotatedNode {
         return interfaces;
     }
 
-    public void setInterfaces(ClassNode[] interfaces) {
+    public void setInterfaces(final ClassNode[] interfaces) {
         if (redirect != null) {
             redirect.setInterfaces(interfaces);
         } else {
             this.interfaces = interfaces;
+            // GROOVY-10763: update generics indicator
+            if (interfaces != null && !usesGenerics && isPrimaryNode) {
+                for (int i = 0, n = interfaces.length; i < n; i += 1) {
+                    usesGenerics |= interfaces[i].isUsingGenerics();
+                }
+            }
         }
     }
 
@@ -416,8 +424,12 @@ public class ClassNode extends AnnotatedNode {
         return redirect().mixins;
     }
 
-    public void setMixins(MixinNode[] mixins) {
-        redirect().mixins = mixins;
+    public void setMixins(final MixinNode[] mixins) {
+        if (redirect != null) {
+            redirect.setMixins(mixins);
+        } else {
+            this.mixins = mixins;
+        }
     }
 
     /**
diff --git a/src/main/java/org/codehaus/groovy/ast/tools/WideningCategories.java b/src/main/java/org/codehaus/groovy/ast/tools/WideningCategories.java
index d68bc3cf5e..de4ab86bee 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/WideningCategories.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/WideningCategories.java
@@ -220,7 +220,7 @@ public class WideningCategories {
                 }
             }
 
-            return new LowestUpperBoundClassNode(((LowestUpperBoundClassNode)lub).name, superClass, interfaces);
+            return new LowestUpperBoundClassNode(lub.getUnresolvedName(), superClass, interfaces);
         } else {
             return parameterizeLowestUpperBound(lub, a, b, lub);
         }
@@ -524,7 +524,6 @@ public class WideningCategories {
      *
      */
     public static class LowestUpperBoundClassNode extends ClassNode {
-        private final String name;
         private final String text;
         private final ClassNode upper;
         private final ClassNode[] interfaces;
@@ -532,42 +531,34 @@ public class WideningCategories {
 
         public LowestUpperBoundClassNode(final String name, final ClassNode upper, final ClassNode... interfaces) {
             super(name, ACC_PUBLIC | ACC_FINAL, upper, interfaces, null);
-            this.name = name;
             this.upper = upper;
             this.interfaces = interfaces;
             Arrays.sort(interfaces, (cn1, cn2) -> {
-                String n1 = cn1 instanceof LowestUpperBoundClassNode ? ((LowestUpperBoundClassNode) cn1).name : cn1.getName();
-                String n2 = cn2 instanceof LowestUpperBoundClassNode ? ((LowestUpperBoundClassNode) cn2).name : cn2.getName();
-                return n1.compareTo(n2);
+                String name1 = cn1 instanceof LowestUpperBoundClassNode ? cn1.getUnresolvedName() : cn1.getName();
+                String name2 = cn2 instanceof LowestUpperBoundClassNode ? cn2.getUnresolvedName() : cn2.getName();
+                return name1.compareTo(name2);
             });
             compileTimeClassNode = isObjectType(upper) && interfaces.length > 0 ? interfaces[0] : upper;
 
             StringJoiner sj = new StringJoiner(" or ", "(", ")");
             if (!isObjectType(upper)) sj.add(upper.getText());
             for (ClassNode i: interfaces) sj.add(i.getText());
-            this.text = sj.toString();
-
-            boolean usesGenerics = upper.isUsingGenerics();
-            List<GenericsType[]> genericsTypesList = new ArrayList<>();
-            genericsTypesList.add(upper.getGenericsTypes());
-            for (ClassNode anInterface : interfaces) {
-                usesGenerics |= anInterface.isUsingGenerics();
-                genericsTypesList.add(anInterface.getGenericsTypes());
-            }
-            setUsingGenerics(usesGenerics);
-            if (usesGenerics) {
-                List<GenericsType> flatList = new ArrayList<>();
-                for (GenericsType[] gts : genericsTypesList) {
-                    if (gts != null) {
-                        Collections.addAll(flatList, gts);
-                    }
+            text = sj.toString();
+
+            if (isUsingGenerics()) {
+                List<GenericsType> generics = new ArrayList<>();
+                if (upper.getGenericsTypes() != null)
+                    Collections.addAll(generics, upper.getGenericsTypes());
+                for (ClassNode i : interfaces) {
+                    if (i.getGenericsTypes() != null)
+                        Collections.addAll(generics, i.getGenericsTypes());
                 }
-                setGenericsTypes(flatList.toArray(GenericsType.EMPTY_ARRAY));
+                setGenericsTypes(generics.toArray(GenericsType.EMPTY_ARRAY));
             }
         }
 
         public String getLubName() {
-            return name;
+            return getUnresolvedName();
         }
 
         @Override
@@ -585,13 +576,6 @@ public class WideningCategories {
             return compileTimeClassNode.getTypeClass();
         }
 
-        @Override
-        public int hashCode() {
-            int result = super.hashCode();
-            result = 31 * result + (name != null ? name.hashCode() : 0);
-            return result;
-        }
-
         @Override
         public String toString(boolean x) {
             return text;
@@ -617,7 +601,7 @@ public class WideningCategories {
             for (int i = 0; i < interfaces.length; i += 1) {
                 faces[i] = interfaces[i].getPlainNodeReference();
             }
-            return new LowestUpperBoundClassNode(name, upper.getPlainNodeReference(), faces);
+            return new LowestUpperBoundClassNode(getUnresolvedName(), upper.getPlainNodeReference(), faces);
         }
     }
 
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/UnionTypeClassNode.java b/src/main/java/org/codehaus/groovy/transform/stc/UnionTypeClassNode.java
index 7d79463d8d..1a54d1132c 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/UnionTypeClassNode.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/UnionTypeClassNode.java
@@ -61,6 +61,7 @@ class UnionTypeClassNode extends ClassNode {
     UnionTypeClassNode(final ClassNode... classNodes) {
         super(makeName(classNodes), 0, ClassHelper.OBJECT_TYPE);
         delegates = classNodes == null ? ClassNode.EMPTY_ARRAY : classNodes;
+        isPrimaryNode = false;
     }
 
     private static String makeName(final ClassNode[] nodes) {
@@ -423,7 +424,11 @@ class UnionTypeClassNode extends ClassNode {
 
     @Override
     public void setInterfaces(final ClassNode[] interfaces) {
-        throw new UnsupportedOperationException();
+        if (isPrimaryNode) {
+            super.setInterfaces(interfaces);
+        } else {
+            throw new UnsupportedOperationException();
+        }
     }
 
     @Override
@@ -458,7 +463,11 @@ class UnionTypeClassNode extends ClassNode {
 
     @Override
     public void setSuperClass(final ClassNode superClass) {
-        throw new UnsupportedOperationException();
+        if (isPrimaryNode) {
+            super.setSuperClass(superClass);
+        } else {
+            throw new UnsupportedOperationException();
+        }
     }
 
     @Override
diff --git a/src/test/org/codehaus/groovy/ast/ClassNodeTest.java b/src/test/org/codehaus/groovy/ast/ClassNodeTest.java
index 371ce1241e..51b9c7dd70 100644
--- a/src/test/org/codehaus/groovy/ast/ClassNodeTest.java
+++ b/src/test/org/codehaus/groovy/ast/ClassNodeTest.java
@@ -18,33 +18,63 @@
  */
 package org.codehaus.groovy.ast;
 
-import junit.framework.TestCase;
-import org.objectweb.asm.Opcodes;
+import org.codehaus.groovy.ast.tools.GenericsUtils;
+import org.junit.Before;
+import org.junit.Test;
 
-import static groovy.test.GroovyAssert.isAtLeastJdk;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
+import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
 
-/**
- * Tests the ClassNode
- */
-public class ClassNodeTest extends TestCase implements Opcodes {
+public final class ClassNodeTest {
 
-    ClassNode classNode = new ClassNode("Foo", ACC_PUBLIC, ClassHelper.OBJECT_TYPE);
-    ClassNode innerClassNode = new InnerClassNode(classNode, "Foo$1", ACC_PUBLIC, ClassHelper.OBJECT_TYPE);
+    private final ClassNode classNode = new ClassNode("Foo", ACC_PUBLIC, ClassHelper.OBJECT_TYPE);
+    private final ClassNode innerClassNode = new InnerClassNode(classNode, "Foo$1", ACC_PUBLIC, ClassHelper.OBJECT_TYPE);
 
-    protected void setUp() throws Exception {
+    @Before
+    public void setUp() {
         classNode.addField("field", ACC_PUBLIC, ClassHelper.STRING_TYPE, null);
     }
 
+    @Test
     public void testOuterClass() {
         assertNull(classNode.getOuterClass());
         assertNotNull(innerClassNode.getOuterClass());
     }
 
+    @Test
     public void testOuterField() {
         assertNull(classNode.getOuterField("field"));
         assertNotNull(innerClassNode.getOuterField("field"));
     }
 
+    @Test // GROOVY-10763
+    public void testSuperClass() {
+        assertTrue(classNode.isPrimaryClassNode());
+        assertFalse(classNode.isUsingGenerics());
+
+        ClassNode superClass = GenericsUtils.makeClassSafe0(ClassHelper.REFERENCE_TYPE, ClassHelper.STRING_TYPE.asGenericsType());
+        classNode.setSuperClass(superClass);
+
+        assertTrue("'using generics' not updated", classNode.isUsingGenerics());
+    }
+
+    @Test // GROOVY-10763
+    public void testInterfaces() {
+        assertTrue(classNode.isPrimaryClassNode());
+        assertFalse(classNode.isUsingGenerics());
+
+        ClassNode[] interfaces = {GenericsUtils.makeClassSafe0(ClassHelper.ITERABLE_TYPE, ClassHelper.STRING_TYPE.asGenericsType())};
+        classNode.setInterfaces(interfaces);
+
+        assertTrue("'using generics' not updated", classNode.isUsingGenerics());
+    }
+
+    @Test
     public void testPackageName() {
         assertEquals("Package", null, classNode.getPackageName());
 
@@ -52,19 +82,20 @@ public class ClassNodeTest extends TestCase implements Opcodes {
         assertEquals("Package", "com.acme", packageNode.getPackageName());
     }
 
-    public void testPermittedSubclasses() throws ClassNotFoundException {
-        if (!isAtLeastJdk("17.0")) return;
+    @Test
+    public void testPermittedSubclasses() throws Exception {
+        assumeTrue(groovy.test.GroovyAssert.isAtLeastJdk("17.0"));
 
-        Class<?> clazz = Class.forName("java.lang.constant.ConstantDesc");
-        ClassNode cn = new ClassNode(clazz);
+        Class<?> c = Class.forName("java.lang.constant.ConstantDesc");
+        ClassNode cn = new ClassNode(c);
         assertTrue(!cn.getPermittedSubclasses().isEmpty());
         assertTrue(cn.isSealed());
 
-        cn = ClassHelper.make(clazz);
+        cn = ClassHelper.make(c);
         assertTrue(!cn.getPermittedSubclasses().isEmpty());
         assertTrue(cn.isSealed());
 
-        // some constructors of `ClassNode` will not trigger the lazy initialization
+        // some constructors of ClassNode do not trigger the lazy initialization
 //        cn = ClassHelper.make("java.lang.constant.ConstantDesc");
 //        assertTrue(!cn.getPermittedSubclasses().isEmpty());
 //        assertTrue(cn.isSealed());