You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/02/02 16:39:04 UTC

[groovy] branch GROOVY_2_5_X updated: Backport fix for GROOVY-9204(closes #1157)

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

sunlan 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 5183a7b  Backport fix for GROOVY-9204(closes #1157)
5183a7b is described below

commit 5183a7b2548e58a2ad9191bfff7c727166fc30df
Author: Bo Zhang <zh...@gmail.com>
AuthorDate: Mon Feb 3 00:38:16 2020 +0800

    Backport fix for GROOVY-9204(closes #1157)
---
 .../org/apache/groovy/internal/util/Predicate.java |  31 ++++
 .../codehaus/groovy/ast/tools/GenericsUtils.java   | 165 +++++++++++++++++++--
 .../transform/stc/StaticTypeCheckingVisitor.java   |   3 +-
 src/test/groovy/bugs/groovy9204/Four.java          |  24 +++
 src/test/groovy/bugs/groovy9204/Groovy9204.groovy  |  47 ++++++
 src/test/groovy/bugs/groovy9204/One.java           |  25 ++++
 src/test/groovy/bugs/groovy9204/Three.java         |  24 +++
 src/test/groovy/bugs/groovy9204/Two.java           |  24 +++
 .../groovy/transform/stc/GenericsSTCTest.groovy    |   2 +-
 9 files changed, 334 insertions(+), 11 deletions(-)

diff --git a/src/main/java/org/apache/groovy/internal/util/Predicate.java b/src/main/java/org/apache/groovy/internal/util/Predicate.java
new file mode 100644
index 0000000..527acfb
--- /dev/null
+++ b/src/main/java/org/apache/groovy/internal/util/Predicate.java
@@ -0,0 +1,31 @@
+/*
+ *  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 org.apache.groovy.internal.util;
+
+import org.apache.groovy.lang.annotation.Incubating;
+
+/**
+ * Backport of Java8 Function.
+ * INTERNAL USE ONLY.
+ */
+@Incubating
+public interface Predicate<T> {
+    boolean test(T t);
+}
\ No newline at end of file
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 adc1fc4..38049ae 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
@@ -20,7 +20,9 @@ package org.codehaus.groovy.ast.tools;
 
 import antlr.RecognitionException;
 import antlr.TokenStreamException;
+import groovy.lang.Tuple2;
 import groovy.transform.stc.IncorrectTypeHintException;
+import org.apache.groovy.internal.util.Predicate;
 import org.apache.groovy.util.SystemUtil;
 import org.codehaus.groovy.GroovyBugError;
 import org.codehaus.groovy.antlr.AntlrParserPlugin;
@@ -168,7 +170,7 @@ public class GenericsUtils {
         if (parameterized == null || parameterized.length == 0) return;
         GenericsType[] redirectGenericsTypes = node.redirect().getGenericsTypes();
         if (redirectGenericsTypes == null ||
-                (node.isGenericsPlaceHolder() && redirectGenericsTypes.length != parameterized.length) /* GROOVY-8609 */ ) {
+                (node.isGenericsPlaceHolder() && redirectGenericsTypes.length != parameterized.length) /* GROOVY-8609 */) {
             redirectGenericsTypes = parameterized;
         }
         if (redirectGenericsTypes.length != parameterized.length) {
@@ -690,14 +692,19 @@ public class GenericsUtils {
      * If no cached item found, cache and return the result of {@link #findParameterizedType(ClassNode, ClassNode)}
      */
     public static ClassNode findParameterizedTypeFromCache(final ClassNode genericsClass, final ClassNode actualType) {
+        return findParameterizedType(genericsClass, actualType, false);
+    }
+
+    // Backported from 3.0.0
+    private static ClassNode findParameterizedTypeFromCache(final ClassNode genericsClass, final ClassNode actualType, final boolean tryToFindExactType) {
         if (!PARAMETERIZED_TYPE_CACHE_ENABLED) {
-            return findParameterizedType(genericsClass, actualType);
+            return findParameterizedType(genericsClass, actualType, tryToFindExactType);
         }
 
         SoftReference<ClassNode> sr = PARAMETERIZED_TYPE_CACHE.getAndPut(new ParameterizedTypeCacheKey(genericsClass, actualType), new EvictableCache.ValueProvider<ParameterizedTypeCacheKey, SoftReference<ClassNode>>() {
             @Override
             public SoftReference<ClassNode> provide(ParameterizedTypeCacheKey key) {
-                return new SoftReference<>(findParameterizedType(key.getGenericsClass(), key.getActualType()));
+                return new SoftReference<>(findParameterizedType(key.getGenericsClass(), key.getActualType(), tryToFindExactType));
             }
         });
 
@@ -713,6 +720,11 @@ public class GenericsUtils {
      * @return the parameterized type
      */
     public static ClassNode findParameterizedType(ClassNode genericsClass, ClassNode actualType) {
+        return findParameterizedType(genericsClass, actualType, false);
+    }
+
+    // Backported from 3.0.0
+    private static ClassNode findParameterizedType(ClassNode genericsClass, ClassNode actualType, boolean tryToFindExactType) {
         ClassNode parameterizedType = null;
 
         if (null == genericsClass.getGenericsTypes()) {
@@ -724,12 +736,18 @@ public class GenericsUtils {
         List<ClassNode> classNodeList = new LinkedList<>(getAllSuperClassesAndInterfaces(actualType));
         classNodeList.add(0, actualType);
 
+        LinkedList<ClassNode> parameterizedTypeCandidateList = new LinkedList<>();
+
         for (ClassNode cn : classNodeList) {
             if (cn == genericsClass) {
                 continue;
             }
 
-            if (!genericsClass.equals(cn.redirect())) {
+            if (tryToFindExactType && null != cn.getGenericsTypes() && hasNonPlaceHolders(cn)) {
+                parameterizedTypeCandidateList.add(cn);
+            }
+
+            if (!(genericsClass.equals(cn.redirect()))) {
                 continue;
             }
 
@@ -739,9 +757,33 @@ public class GenericsUtils {
             }
         }
 
+        if (null == parameterizedType) {
+            if (!parameterizedTypeCandidateList.isEmpty()) {
+                parameterizedType = parameterizedTypeCandidateList.getLast();
+            }
+        }
+
         return parameterizedType;
     }
 
+    /**
+     * Backported from 3.0.0.
+     *
+     * Check whether the ClassNode has non generics placeholders, aka not placeholder
+     *
+     * @param parameterizedType the class node
+     * @return the result
+     * @since 2.5.9
+     */
+    private static boolean hasNonPlaceHolders(ClassNode parameterizedType) {
+        return checkPlaceHolders(parameterizedType, new Predicate<GenericsType>() {
+            @Override
+            public boolean test(GenericsType genericsType) {
+                return !genericsType.isPlaceholder();
+            }
+        });
+    }
+
     private static boolean isGenericsTypeArraysLengthEqual(GenericsType[] declaringGenericsTypes, GenericsType[] actualGenericsTypes) {
         return null != actualGenericsTypes && declaringGenericsTypes.length == actualGenericsTypes.length;
     }
@@ -793,18 +835,123 @@ public class GenericsUtils {
      * so we need actual types:  T: String, S: Long
      */
     public static Map<GenericsType, GenericsType> makeDeclaringAndActualGenericsTypeMap(ClassNode declaringClass, ClassNode actualReceiver) {
-        ClassNode parameterizedType = findParameterizedTypeFromCache(declaringClass, actualReceiver);
+        return doMakeDeclaringAndActualGenericsTypeMap(declaringClass, actualReceiver, false).getFirst();
+    }
+
+    /**
+     * Backported from 3.0.0
+     *
+     * The method is similar with {@link GenericsUtils#makeDeclaringAndActualGenericsTypeMap(ClassNode, ClassNode)},
+     * The main difference is that the method will try to map all placeholders found to the relevant exact types,
+     * but the other will not try even if the parameterized type has placeholders
+     *
+     * @param declaringClass the generics class node declaring the generics types
+     * @param actualReceiver the sub-class class node
+     * @return the placeholder-to-actualtype mapping
+     * @since 2.5.9
+     */
+    public static Map<GenericsType, GenericsType> makeDeclaringAndActualGenericsTypeMapOfExactType(ClassNode declaringClass, ClassNode actualReceiver) {
+        List<ClassNode> parameterizedTypeList = new LinkedList<>();
+
+        Map<GenericsType, GenericsType> result = makeDeclaringAndActualGenericsTypeMapOfExactType(declaringClass, actualReceiver, parameterizedTypeList);
+
+        return connectGenericsTypes(result);
+    }
+
+    private static Map<GenericsType, GenericsType> makeDeclaringAndActualGenericsTypeMapOfExactType(ClassNode declaringClass, ClassNode actualReceiver, List<ClassNode> parameterizedTypeList) {
+        Tuple2<Map<GenericsType, GenericsType>, ClassNode> resultAndParameterizedTypeTuple = doMakeDeclaringAndActualGenericsTypeMap(declaringClass, actualReceiver, true);
+        ClassNode parameterizedType = resultAndParameterizedTypeTuple.getSecond();
+        Map<GenericsType, GenericsType> result = resultAndParameterizedTypeTuple.getFirst();
+
+        if (hasPlaceHolders(parameterizedType) && !parameterizedTypeList.contains(parameterizedType)) {
+            parameterizedTypeList.add(parameterizedType);
+            result.putAll(makeDeclaringAndActualGenericsTypeMapOfExactType(parameterizedType, actualReceiver, parameterizedTypeList));
+        }
+
+        return connectGenericsTypes(result);
+    }
+
+    private static Tuple2<Map<GenericsType, GenericsType>, ClassNode> doMakeDeclaringAndActualGenericsTypeMap(ClassNode declaringClass, ClassNode actualReceiver, boolean tryToFindExactType) {
+        ClassNode parameterizedType = findParameterizedTypeFromCache(declaringClass, actualReceiver, tryToFindExactType);
 
         if (null == parameterizedType) {
+            return new Tuple2<>(Collections.<GenericsType,GenericsType>emptyMap(), parameterizedType);
+        }
+
+        Map<GenericsType, GenericsType> result = new LinkedHashMap<>();
+
+        result.putAll(makePlaceholderAndParameterizedTypeMap(declaringClass));
+        result.putAll(makePlaceholderAndParameterizedTypeMap(parameterizedType));
+
+        result = connectGenericsTypes(result);
+
+        return new Tuple2<>(result, parameterizedType);
+    }
+
+    private static Map<GenericsType, GenericsType> connectGenericsTypes(Map<GenericsType, GenericsType> genericsTypeMap) {
+        Map<GenericsType, GenericsType> result = new LinkedHashMap<>();
+
+        outer:
+        for (Map.Entry<GenericsType, GenericsType> entry : genericsTypeMap.entrySet()) {
+            GenericsType key = entry.getKey();
+            GenericsType value = entry.getValue();
+
+            if (value.isPlaceholder()) {
+                for (Map.Entry<GenericsType, GenericsType> genericsTypeMapEntry : genericsTypeMap.entrySet()) {
+                    GenericsType genericsTypeMapEntryValue = genericsTypeMapEntry.getValue();
+                    if (!genericsTypeMapEntryValue.isPlaceholder() && (genericsTypeMapEntry.getKey().getName().equals(value.getName()))) {
+                        result.put(key, genericsTypeMapEntryValue); // connected to actual type
+                        continue outer;
+                    }
+                }
+            }
+
+            result.put(key, value);
+        }
+
+        return result;
+    }
+
+    private static boolean hasPlaceHolders(ClassNode parameterizedType) {
+        return checkPlaceHolders(parameterizedType, new Predicate<GenericsType>() {
+            @Override
+            public boolean test(GenericsType genericsType) {
+                return genericsType.isPlaceholder();
+            }
+        });
+    }
+
+    private static boolean checkPlaceHolders(ClassNode parameterizedType, Predicate<GenericsType> p) {
+        if (null == parameterizedType) return false;
+
+        GenericsType[] genericsTypes = parameterizedType.getGenericsTypes();
+
+        if (null == genericsTypes) return false;
+
+        for (GenericsType genericsType : genericsTypes) {
+            if (p.test(genericsType)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    private static Map<GenericsType, GenericsType> makePlaceholderAndParameterizedTypeMap(ClassNode declaringClass) {
+        if (null == declaringClass) {
             return Collections.emptyMap();
         }
 
+        Map<GenericsType, GenericsType> result = new LinkedHashMap<>();
+
+        ClassNode redirectDeclaringClass = declaringClass.redirect();
         GenericsType[] declaringGenericsTypes = declaringClass.getGenericsTypes();
-        GenericsType[] actualGenericsTypes = parameterizedType.getGenericsTypes();
+        GenericsType[] redirectDeclaringGenericsTypes = redirectDeclaringClass.getGenericsTypes();
 
-        Map<GenericsType, GenericsType> result = new LinkedHashMap<>();
-        for (int i = 0, n = declaringGenericsTypes.length; i < n; i++) {
-            result.put(declaringGenericsTypes[i], actualGenericsTypes[i]);
+        if (null != declaringGenericsTypes && null != redirectDeclaringGenericsTypes) {
+            for (int i = 0, n = declaringGenericsTypes.length; i < n; i++) {
+                result.put(redirectDeclaringGenericsTypes[i], declaringGenericsTypes[i]);
+            }
         }
 
         return result;
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index eab3714..3248cd0 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -179,6 +179,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
 import static org.codehaus.groovy.ast.tools.GenericsUtils.findActualTypeByGenericsPlaceholderName;
 import static org.codehaus.groovy.ast.tools.GenericsUtils.makeDeclaringAndActualGenericsTypeMap;
+import static org.codehaus.groovy.ast.tools.GenericsUtils.makeDeclaringAndActualGenericsTypeMapOfExactType;
 import static org.codehaus.groovy.ast.tools.GenericsUtils.toGenericTypesString;
 import static org.codehaus.groovy.ast.tools.WideningCategories.LowestUpperBoundClassNode;
 import static org.codehaus.groovy.ast.tools.WideningCategories.isBigDecCategory;
@@ -654,7 +655,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
             ClassNode actualType =
                     findActualTypeByGenericsPlaceholderName(
                         fieldNode.getOriginType().getUnresolvedName(),
-                        makeDeclaringAndActualGenericsTypeMap(fieldNode.getDeclaringClass(), typeCheckingContext.getEnclosingClassNode())
+                        makeDeclaringAndActualGenericsTypeMapOfExactType(fieldNode.getDeclaringClass(), typeCheckingContext.getEnclosingClassNode())
                     );
 
             if (null != actualType) {
diff --git a/src/test/groovy/bugs/groovy9204/Four.java b/src/test/groovy/bugs/groovy9204/Four.java
new file mode 100644
index 0000000..58cd797
--- /dev/null
+++ b/src/test/groovy/bugs/groovy9204/Four.java
@@ -0,0 +1,24 @@
+/*
+ *  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.groovy9204;
+
+import java.util.LinkedList;
+
+public class Four extends Two<LinkedList> {
+}
diff --git a/src/test/groovy/bugs/groovy9204/Groovy9204.groovy b/src/test/groovy/bugs/groovy9204/Groovy9204.groovy
new file mode 100644
index 0000000..cc78322
--- /dev/null
+++ b/src/test/groovy/bugs/groovy9204/Groovy9204.groovy
@@ -0,0 +1,47 @@
+/*
+ *  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.groovy9204
+
+class Groovy9204 extends GroovyTestCase {
+    void testGenerics() {
+        assertScript '''
+            package groovy.bugs.groovy9204
+            @groovy.transform.CompileStatic
+            class ArrayListTest extends Three {
+                def test() {
+                    field = new ArrayList()
+                    field.add("hello")
+                    field[0]
+                }
+            }
+
+            @groovy.transform.CompileStatic
+            class LinkedListTest extends Four {
+                def test() {
+                    field = new LinkedList()
+                    field.addFirst("hello")
+                    field[0]
+                }
+            }
+            
+            assert new ArrayListTest().test() == 'hello'
+            assert new LinkedListTest().test() == 'hello'
+        '''
+    }
+}
diff --git a/src/test/groovy/bugs/groovy9204/One.java b/src/test/groovy/bugs/groovy9204/One.java
new file mode 100644
index 0000000..d079490
--- /dev/null
+++ b/src/test/groovy/bugs/groovy9204/One.java
@@ -0,0 +1,25 @@
+/*
+ *  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.groovy9204;
+
+import java.util.List;
+
+public class One<T extends List> {
+    protected T field;
+}
diff --git a/src/test/groovy/bugs/groovy9204/Three.java b/src/test/groovy/bugs/groovy9204/Three.java
new file mode 100644
index 0000000..640afcd
--- /dev/null
+++ b/src/test/groovy/bugs/groovy9204/Three.java
@@ -0,0 +1,24 @@
+/*
+ *  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.groovy9204;
+
+import java.util.List;
+
+public class Three extends Two<List> {
+}
\ No newline at end of file
diff --git a/src/test/groovy/bugs/groovy9204/Two.java b/src/test/groovy/bugs/groovy9204/Two.java
new file mode 100644
index 0000000..c4b0142
--- /dev/null
+++ b/src/test/groovy/bugs/groovy9204/Two.java
@@ -0,0 +1,24 @@
+/*
+ *  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.groovy9204;
+
+import java.util.List;
+
+public class Two<T extends List> extends One<T> {
+}
diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
index 940d7fb..93c4a5a 100644
--- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -73,7 +73,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
         shouldFailWithMessages '''
             List<String> list = []
             list << 1
-        ''', '[Static type checking] - Cannot find matching method java.util.List#leftShift(int)'
+        ''', '[Static type checking] - Cannot call <T> java.util.List <String>#leftShift(T) with arguments [int]'
     }
 
     void testAddOnList2UsingLeftShift() {