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/19 20:27:47 UTC

[groovy] branch GROOVY_2_5_X updated: GROOVY-9059: use redirect in generics spec if redirect is placeholder

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

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


The following commit(s) were added to refs/heads/GROOVY_2_5_X by this push:
     new f75c967df2 GROOVY-9059: use redirect in generics spec if redirect is placeholder
f75c967df2 is described below

commit f75c967df2a235174bd366a546b970a3e3f7a276
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Jul 19 15:17:38 2022 -0500

    GROOVY-9059: use redirect in generics spec if redirect is placeholder
---
 .../codehaus/groovy/ast/tools/GenericsUtils.java   | 174 ++++++++++++---------
 src/test/groovy/bugs/Groovy9059.groovy             |  68 ++++++++
 2 files changed, 169 insertions(+), 73 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java b/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
index 3bf6b32ad7..379ccf559b 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
@@ -144,69 +144,82 @@ public class GenericsUtils {
         return gt;
     }
 
-    public static Map<GenericsType.GenericsTypeName, GenericsType> extractPlaceholders(ClassNode cn) {
-        Map<GenericsType.GenericsTypeName, GenericsType> ret = new HashMap<>();
-        extractPlaceholders(cn, ret);
-        return ret;
+    /**
+     * Returns the type parameter/argument relationships of the specified type.
+     *
+     * @param type the class node to check
+     */
+    public static Map<GenericsType.GenericsTypeName, GenericsType> extractPlaceholders(final ClassNode type) {
+        Map<GenericsType.GenericsTypeName, GenericsType> placeholders = new HashMap<>();
+        extractPlaceholders(type, placeholders);
+        return placeholders;
     }
 
     /**
-     * For a given classnode, fills in the supplied map with the parameterized
-     * types it defines.
+     * Populates the supplied map with the type parameter/argument relationships
+     * of the specified type.
      *
-     * @param node the class node to check
-     * @param map the generics type information collector
+     * @param type the class node to check
+     * @param placeholders the generics type information collector
      */
-    public static void extractPlaceholders(ClassNode node, Map<GenericsType.GenericsTypeName, GenericsType> map) {
-        if (node == null) return;
+    public static void extractPlaceholders(final ClassNode type, final Map<GenericsType.GenericsTypeName, GenericsType> placeholders) {
+        if (type == null) return;
 
-        if (node.isArray()) {
-            extractPlaceholders(node.getComponentType(), map);
+        if (type.isArray()) {
+            extractPlaceholders(type.getComponentType(), placeholders);
             return;
         }
 
-        if (!node.isUsingGenerics() || !node.isRedirectNode()) return;
-        GenericsType[] parameterized = node.getGenericsTypes();
-        if (parameterized == null || parameterized.length == 0) return;
-        GenericsType[] redirectGenericsTypes = node.redirect().getGenericsTypes();
-        if (redirectGenericsTypes == null ||
-                (node.isGenericsPlaceHolder() && redirectGenericsTypes.length != parameterized.length) /* GROOVY-8609 */) {
-            redirectGenericsTypes = parameterized;
+        if (!type.isUsingGenerics() || !type.isRedirectNode()) return;
+        GenericsType[] parameterized = type.getGenericsTypes(); int n;
+        if (parameterized == null || (n = parameterized.length) == 0) return;
+
+        // GROOVY-8609, GROOVY-10067, etc.
+        if (type.isGenericsPlaceHolder()) {
+            GenericsType gt = parameterized[0];
+            GenericsType.GenericsTypeName name = new GenericsType.GenericsTypeName(gt.getName());
+            if (!placeholders.containsKey(name)) placeholders.put(name, gt);
+            return;
         }
-        if (redirectGenericsTypes.length != parameterized.length) {
+
+        GenericsType[] redirectGenericsTypes = type.redirect().getGenericsTypes();
+        if (redirectGenericsTypes == null) {
+            redirectGenericsTypes = parameterized;
+        } else if (redirectGenericsTypes.length != n) {
             throw new GroovyBugError("Expected earlier checking to detect generics parameter arity mismatch" +
-                    "\nExpected: " + node.getName() + toGenericTypesString(redirectGenericsTypes) +
-                    "\nSupplied: " + node.getName() + toGenericTypesString(parameterized));
+                    "\nExpected: " + type.getName() + toGenericTypesString(redirectGenericsTypes) +
+                    "\nSupplied: " + type.getName() + toGenericTypesString(parameterized));
         }
 
-        List<GenericsType> valueList = new LinkedList<>();
-        for (int i = 0; i < redirectGenericsTypes.length; i++) {
-            GenericsType redirectType = redirectGenericsTypes[i];
-            if (redirectType.isPlaceholder()) {
-                GenericsType.GenericsTypeName name = new GenericsType.GenericsTypeName(redirectType.getName());
-                if (!map.containsKey(name)) {
-                    GenericsType value = parameterized[i];
-                    map.put(name, value);
-
-                    valueList.add(value);
+        List<GenericsType> typeArguments = new ArrayList<>(n);
+        for (int i = 0; i < n; i += 1) {
+            GenericsType rgt = redirectGenericsTypes[i];
+            if (rgt.isPlaceholder()) {
+                GenericsType.GenericsTypeName name = new GenericsType.GenericsTypeName(rgt.getName());
+                if (!placeholders.containsKey(name)) {
+                    GenericsType typeArgument = parameterized[i];
+                    placeholders.put(name, typeArgument);
+                    typeArguments.add(typeArgument);
                 }
             }
         }
 
-        for (GenericsType value : valueList) {
-            if (value.isWildcard()) {
-                ClassNode lowerBound = value.getLowerBound();
+        // examine non-placeholder type args
+        for (GenericsType gt : typeArguments) {
+            if (gt.isWildcard()) {
+                ClassNode lowerBound = gt.getLowerBound();
                 if (lowerBound != null) {
-                    extractPlaceholders(lowerBound, map);
-                }
-                ClassNode[] upperBounds = value.getUpperBounds();
-                if (upperBounds != null) {
-                    for (ClassNode upperBound : upperBounds) {
-                        extractPlaceholders(upperBound, map);
+                    extractPlaceholders(lowerBound, placeholders);
+                } else {
+                    ClassNode[] upperBounds = gt.getUpperBounds();
+                    if (upperBounds != null) {
+                        for (ClassNode upperBound : upperBounds) {
+                            extractPlaceholders(upperBound, placeholders);
+                        }
                     }
                 }
-            } else if (!value.isPlaceholder()) {
-                extractPlaceholders(value.getType(), map);
+            } else if (!gt.isPlaceholder()) {
+                extractPlaceholders(gt.getType(), placeholders);
             }
         }
     }
@@ -320,12 +333,15 @@ public class GenericsUtils {
         if (type.isArray()) {
             return makeClassSafeWithGenerics(type.getComponentType(), genericTypes).makeArray();
         }
-        GenericsType[] gtypes = EMPTY_GENERICS_ARRAY;
-        if (genericTypes != null) {
-            gtypes = new GenericsType[genericTypes.length];
-            System.arraycopy(genericTypes, 0, gtypes, 0, gtypes.length);
+        int nTypes = (genericTypes == null ? 0 : genericTypes.length);
+        GenericsType[] gTypes;
+        if (nTypes == 0) {
+            gTypes = EMPTY_GENERICS_ARRAY;
+        } else {
+            gTypes = new GenericsType[nTypes];
+            System.arraycopy(genericTypes, 0, gTypes, 0, nTypes);
         }
-        return makeClassSafe0(type, gtypes);
+        return makeClassSafe0(type, gTypes);
     }
 
     public static MethodNode correctToGenericsSpec(Map<String, ClassNode> genericsSpec, MethodNode mn) {
@@ -344,7 +360,7 @@ public class GenericsUtils {
     }
 
     public static ClassNode correctToGenericsSpecRecurse(Map<String, ClassNode> genericsSpec, ClassNode type) {
-        return correctToGenericsSpecRecurse(genericsSpec, type, new ArrayList<String>());
+        return correctToGenericsSpecRecurse(genericsSpec, type, Collections.<String>emptyList());
     }
 
     /**
@@ -355,7 +371,7 @@ public class GenericsUtils {
         ClassNode[] newTypes = new ClassNode[types.length];
         boolean modified = false;
         for (int i = 0; i < types.length; i++) {
-            newTypes[i] = correctToGenericsSpecRecurse(genericsSpec, types[i], new ArrayList<String>());
+            newTypes[i] = correctToGenericsSpecRecurse(genericsSpec, types[i], Collections.<String>emptyList());
             modified = modified || (types[i] != newTypes[i]);
         }
         if (!modified) return types;
@@ -370,10 +386,14 @@ public class GenericsUtils {
             String name = type.getGenericsTypes()[0].getName();
             exclusions = plus(exclusions, name); // GROOVY-7722
             type = genericsSpec.get(name);
-            if (type != null && type.isGenericsPlaceHolder() && type.getGenericsTypes() == null) {
-                ClassNode placeholder = ClassHelper.makeWithoutCaching(type.getUnresolvedName());
-                placeholder.setGenericsPlaceHolder(true);
-                type = makeClassSafeWithGenerics(type, new GenericsType(placeholder));
+            if (type != null && type.isGenericsPlaceHolder()) {
+                if (type.getGenericsTypes() == null) {
+                    ClassNode placeholder = ClassHelper.makeWithoutCaching(type.getUnresolvedName());
+                    placeholder.setGenericsPlaceHolder(true);
+                    return makeClassSafeWithGenerics(type, new GenericsType(placeholder));
+                } else if (!name.equals(type.getUnresolvedName())) {
+                    return correctToGenericsSpecRecurse(genericsSpec, type, exclusions);
+                }
             }
         }
         if (type == null) type = ClassHelper.OBJECT_TYPE;
@@ -431,9 +451,13 @@ public class GenericsUtils {
         if (type.isArray()) {
             return correctToGenericsSpec(genericsSpec, type.getComponentType()).makeArray();
         }
-        if (type.isGenericsPlaceHolder()) {
+        if (type.isGenericsPlaceHolder() && type.getGenericsTypes() != null) {
             String name = type.getGenericsTypes()[0].getName();
             type = genericsSpec.get(name);
+            if (type != null && type.isGenericsPlaceHolder()
+                    && !name.equals(type.getUnresolvedName())) {
+                return correctToGenericsSpec(genericsSpec, type);
+            }
         }
         if (type == null) type = ClassHelper.OBJECT_TYPE;
         return type;
@@ -472,31 +496,35 @@ public class GenericsUtils {
     }
 
     public static Map<String, ClassNode> addMethodGenerics(MethodNode current, Map<String, ClassNode> oldSpec) {
-        Map<String, ClassNode> ret = new HashMap<String, ClassNode>(oldSpec);
-        // ret starts with the original type specs, now add gts for the current method if any
-        GenericsType[] sgts = current.getGenericsTypes();
-        if (sgts != null) {
-            for (GenericsType sgt : sgts) {
-                String name = sgt.getName();
-                if (sgt.isPlaceholder()) {
+        Map<String, ClassNode> newSpec = new HashMap<String, ClassNode>(oldSpec);
+        GenericsType[] gts = current.getGenericsTypes();
+        if (gts != null) {
+            for (GenericsType gt : gts) {
+                String name = gt.getName();
+                ClassNode type = gt.getType();
+                if (gt.isPlaceholder()) {
                     ClassNode redirect;
-                    if (sgt.getUpperBounds() != null) {
-                        redirect = sgt.getUpperBounds()[0];
-                    } else if (sgt.getLowerBound() != null) {
-                        redirect = sgt.getLowerBound();
+                    if (gt.getUpperBounds() != null) {
+                        redirect = gt.getUpperBounds()[0];
+                    } else if (gt.getLowerBound() != null) {
+                        redirect = gt.getLowerBound();
                     } else {
                         redirect = ClassHelper.OBJECT_TYPE;
                     }
-                    ClassNode type = ClassHelper.makeWithoutCaching(name);
-                    type.setGenericsPlaceHolder(true);
-                    type.setRedirect(redirect);
-                    ret.put(name, type);
-                } else {
-                    ret.put(name, sgt.getType());
+                    if (redirect.isGenericsPlaceHolder()) {
+                        // "T extends U" or "T super U"
+                        type = redirect; // GROOVY-9059
+                    } else {
+                        // "T" or "T extends Type" or "T super Type"
+                        type = ClassHelper.makeWithoutCaching(name);
+                        type.setGenericsPlaceHolder(true);
+                        type.setRedirect(redirect);
+                    }
                 }
+                newSpec.put(name, type);
             }
         }
-        return ret;
+        return newSpec;
     }
 
     public static void extractSuperClassGenerics(ClassNode type, ClassNode target, Map<String, ClassNode> spec) {
diff --git a/src/test/groovy/bugs/Groovy9059.groovy b/src/test/groovy/bugs/Groovy9059.groovy
new file mode 100644
index 0000000000..4c0ea6326b
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy9059.groovy
@@ -0,0 +1,68 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.bugs
+
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+final class Groovy9059 {
+
+    @Test
+    void testTypeParameterWithExtends1() {
+        assertScript '''
+            interface Face<T> {
+                def <O extends T> O process(O o)
+            }
+
+            def impl = new Face<CharSequence>() {
+                @Override
+                def <Chars extends CharSequence> Chars process(Chars chars) { chars }
+            }
+        '''
+    }
+
+    @Test
+    void testTypeParameterWithExtends2() {
+        assertScript '''
+            interface Face<T> {
+                def <O extends T> O process(O o)
+            }
+
+            def impl = new Face<CharSequence>() {
+                @Override @SuppressWarnings('unchecked')
+                CharSequence process(CharSequence chars) { chars }
+            }
+        '''
+    }
+
+    @Test
+    void testTypeParameterWithExtends3() {
+        assertScript '''
+            interface Face<T> {
+                def <O extends T> O process(O o)
+            }
+
+            def impl = new Face<String>() {
+                @Override @SuppressWarnings('unchecked')
+                String process(String string) { string }
+            }
+        '''
+    }
+}