groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From emil...@apache.org
Subject [groovy] 01/01: GROOVY-9799: avoid using ClassNode#getTypeClass
Date Tue, 10 Nov 2020 17:27:45 GMT
This is an automated email from the ASF dual-hosted git repository.

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

commit 560ad180d7a1cf7818c0ff19bf86d7d537888b3f
Author: Eric Milles <eric.milles@thomsonreuters.com>
AuthorDate: Tue Nov 10 09:55:10 2020 -0600

    GROOVY-9799: avoid using ClassNode#getTypeClass
---
 .../codehaus/groovy/ast/tools/ParameterUtils.java  | 32 +++++------
 ...StaticTypesMethodReferenceExpressionWriter.java | 67 ++++++++++------------
 .../transform/stc/StaticTypeCheckingSupport.java   |  2 +-
 .../transform/stc/MethodReferenceTest.groovy       | 54 ++++++++++++-----
 4 files changed, 82 insertions(+), 73 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/tools/ParameterUtils.java b/src/main/java/org/codehaus/groovy/ast/tools/ParameterUtils.java
index b3b7756..372c4aa 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/ParameterUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/ParameterUtils.java
@@ -21,37 +21,33 @@ package org.codehaus.groovy.ast.tools;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.Parameter;
+import org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport;
 
 import java.util.function.BiPredicate;
 
 public class ParameterUtils {
 
-    public static boolean parametersEqual(Parameter[] a, Parameter[] b) {
+    public static boolean parametersEqual(final Parameter[] a, final Parameter[] b) {
         return parametersEqual(a, b, false);
     }
 
-    public static boolean parametersEqualWithWrapperType(Parameter[] a, Parameter[] b) {
+    public static boolean parametersEqualWithWrapperType(final Parameter[] a, final Parameter[]
b) {
         return parametersEqual(a, b, true);
     }
 
     /**
-     * Checks if two parameter arrays are type-compatible.
+     * Checks compatibility of parameter arrays. Each parameter should match the
+     * following condition: {@code sourceType.isAssignableTo(targetType)}
      *
-     * each parameter should match the following condition:
-     * {@code targetParameter.getType().getTypeClass().isAssignableFrom(sourceParameter.getType().getTypeClass())}
-     *
-     * @param source source parameters
-     * @param target target parameters
-     * @return the check result
      * @since 3.0.0
      */
-    public static boolean parametersCompatible(Parameter[] source, Parameter[] target) {
-        return parametersMatch(source, target, (sourceType, targetType) ->
-            ClassHelper.getWrapper(targetType).getTypeClass().isAssignableFrom(ClassHelper.getWrapper(sourceType).getTypeClass())
-        );
+    public static boolean parametersCompatible(final Parameter[] source, final Parameter[]
target) {
+        return parametersMatch(source, target, StaticTypeCheckingSupport::isAssignableTo);
     }
 
-    private static boolean parametersEqual(Parameter[] a, Parameter[] b, boolean wrapType)
{
+    //--------------------------------------------------------------------------
+
+    private static boolean parametersEqual(final Parameter[] a, final Parameter[] b, final
boolean wrapType) {
         return parametersMatch(a, b, (aType, bType) -> {
             if (wrapType) {
                 aType = ClassHelper.getWrapper(aType);
@@ -61,19 +57,17 @@ public class ParameterUtils {
         });
     }
 
-    private static boolean parametersMatch(Parameter[] a, Parameter[] b, BiPredicate<ClassNode,
ClassNode> typeChecker) {
+    private static boolean parametersMatch(final Parameter[] a, final Parameter[] b, final
BiPredicate<ClassNode, ClassNode> typeChecker) {
         if (a.length == b.length) {
-            boolean answer = true;
             for (int i = 0, n = a.length; i < n; i += 1) {
                 ClassNode aType = a[i].getType();
                 ClassNode bType = b[i].getType();
 
                 if (!typeChecker.test(aType, bType)) {
-                    answer = false;
-                    break;
+                    return false;
                 }
             }
-            return answer;
+            return true;
         }
         return false;
     }
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java
b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java
index 1e7ecb2..6fead88 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java
@@ -35,7 +35,6 @@ import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.ast.expr.MethodReferenceExpression;
 import org.codehaus.groovy.ast.tools.GeneralUtils;
-import org.codehaus.groovy.ast.tools.ParameterUtils;
 import org.codehaus.groovy.classgen.asm.BytecodeHelper;
 import org.codehaus.groovy.classgen.asm.MethodReferenceExpressionWriter;
 import org.codehaus.groovy.classgen.asm.WriterController;
@@ -55,8 +54,10 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.returnS;
+import static org.codehaus.groovy.ast.tools.ParameterUtils.parametersCompatible;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.filterMethodsByVisibility;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.findDGMMethodsForClassNode;
+import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.isAssignableTo;
 import static org.codehaus.groovy.transform.stc.StaticTypesMarker.CLOSURE_ARGUMENTS;
 
 /**
@@ -128,10 +129,8 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
             if (isConstructorReference) {
                 // TODO: move the checking code to the parser
                 addFatalError("Constructor reference must be className::new", methodReferenceExpression);
-            }
-
-            if (methodRefMethod.isStatic()) {
-                ClassExpression classExpression = new ClassExpression(typeOrTargetRefType);
+            } else if (methodRefMethod.isStatic()) {
+                ClassExpression classExpression = classX(typeOrTargetRefType);
                 classExpression.setSourcePosition(typeOrTargetRef);
                 typeOrTargetRef = classExpression;
                 isClassExpression = true;
@@ -165,17 +164,13 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
             addFatalError("Failed to find the expected method["
                     + methodRefName + "("
                     + Arrays.stream(parametersWithExactType)
-                            .map(e -> e.getType().getName())
+                            .map(e -> e.getType().getText())
                             .collect(Collectors.joining(","))
                     + ")] in the type[" + typeOrTargetRefType.getName() + "]", methodReferenceExpression);
-        }
-
-        if (parametersWithExactType.length > 0 && isTypeReferingInstanceMethod(typeOrTargetRef,
methodRefMethod)) {
-            Parameter firstParameter = parametersWithExactType[0];
-            Class<?> typeOrTargetClass = typeOrTargetRef.getType().getTypeClass();
-            Class<?> firstParameterClass = firstParameter.getType().getTypeClass();
-            if (!typeOrTargetClass.isAssignableFrom(firstParameterClass)) {
-                throw new RuntimeParserException("Invalid receiver type: " + firstParameterClass
+ " is not compatible with " + typeOrTargetClass, typeOrTargetRef);
+        } else if (parametersWithExactType.length > 0 && isTypeReferingInstanceMethod(typeOrTargetRef,
methodRefMethod)) {
+            ClassNode firstParameterType = parametersWithExactType[0].getType();
+            if (!isAssignableTo(firstParameterType, typeOrTargetRefType)) {
+                throw new RuntimeParserException("Invalid receiver type: " + firstParameterType.getText()
+ " is not compatible with " + typeOrTargetRefType.getText(), typeOrTargetRef);
             }
         }
     }
@@ -272,35 +267,31 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
         return parameters;
     }
 
-    private MethodNode findMethodRefMethod(final String methodRefName, final Parameter[]
abstractMethodParameters, final Expression typeOrTargetRef, final ClassNode typeOrTargetRefType)
{
-        List<MethodNode> methods = typeOrTargetRefType.getMethods(methodRefName);
-        methods.addAll(findDGMMethodsForClassNode(controller.getSourceUnit().getClassLoader(),
typeOrTargetRefType, methodRefName));
-        methods = filterMethodsByVisibility(methods, controller.getClassNode());
-
-        List<MethodNode> candidates = new ArrayList<>();
-        for (MethodNode mn : methods) {
-            Parameter[] parameters = abstractMethodParameters;
-            if (isTypeReferingInstanceMethod(typeOrTargetRef, mn)) {
-                if (abstractMethodParameters.length == 0) {
-                    continue;
-                }
+    private MethodNode findMethodRefMethod(final String methodName, final Parameter[] samParameters,
final Expression typeOrTargetRef, final ClassNode typeOrTargetRefType) {
+        List<MethodNode> methods = findVisibleMethods(methodName, typeOrTargetRefType);
 
-                parameters = removeFirstParameter(abstractMethodParameters);
-            }
+        return chooseMethodRefMethodCandidate(typeOrTargetRef, methods.stream().filter(method
-> {
+            Parameter[] parameters = method.getParameters();
+            if (isTypeReferingInstanceMethod(typeOrTargetRef, method)) {
+                // there is an implicit parameter for "String::length"
+                ClassNode firstParamType = method.getDeclaringClass();
 
-            Parameter[] methodParameters;
-            if (isExtensionMethod(mn)) {
-                methodParameters = removeFirstParameter(((ExtensionMethodNode) mn).getExtensionMethodNode().getParameters());
-            } else {
-                methodParameters = mn.getParameters();
-            }
+                int n = parameters.length;
+                Parameter[] plusOne = new Parameter[n + 1];
+                plusOne[0] = new Parameter(firstParamType, "");
+                System.arraycopy(parameters, 0, plusOne, 1, n);
 
-            if (ParameterUtils.parametersCompatible(parameters, methodParameters)) {
-                candidates.add(mn);
+                parameters = plusOne;
             }
-        }
+            return parametersCompatible(samParameters, parameters);
+        }).collect(Collectors.toList()));
+    }
 
-        return chooseMethodRefMethodCandidate(typeOrTargetRef, candidates);
+    private List<MethodNode> findVisibleMethods(final String name, final ClassNode
type) {
+        List<MethodNode> methods = type.getMethods(name);
+        methods.addAll(findDGMMethodsForClassNode(controller.getSourceUnit().getClassLoader(),
type, name));
+        methods = filterMethodsByVisibility(methods, controller.getClassNode());
+        return methods;
     }
 
     private void addFatalError(final String msg, final ASTNode node) {
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
index da08043..b6482f5 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -439,7 +439,7 @@ public abstract class StaticTypeCheckingSupport {
      *
      * @return true if the class node is assignable to the other class node, false otherwise
      */
-    static boolean isAssignableTo(ClassNode type, ClassNode toBeAssignedTo) {
+    public static boolean isAssignableTo(ClassNode type, ClassNode toBeAssignedTo) {
         if (type == toBeAssignedTo || type == UNKNOWN_PARAMETER_TYPE) return true;
         if (isPrimitiveType(type)) type = getWrapper(type);
         if (isPrimitiveType(toBeAssignedTo)) toBeAssignedTo = getWrapper(toBeAssignedTo);
diff --git a/src/test/groovy/transform/stc/MethodReferenceTest.groovy b/src/test/groovy/transform/stc/MethodReferenceTest.groovy
index 43eb77d..ffa0d92 100644
--- a/src/test/groovy/transform/stc/MethodReferenceTest.groovy
+++ b/src/test/groovy/transform/stc/MethodReferenceTest.groovy
@@ -26,7 +26,7 @@ import static groovy.test.GroovyAssert.assertScript
 import static groovy.test.GroovyAssert.shouldFail
 
 final class MethodReferenceTest {
- 
+
     private final GroovyShell shell = new GroovyShell(new CompilerConfiguration().tap {
         addCompilationCustomizers(new ImportCustomizer().addStarImports('java.util.function')
                 .addImports('java.util.stream.Collectors', 'groovy.transform.CompileStatic'))
@@ -73,19 +73,6 @@ final class MethodReferenceTest {
         '''
     }
 
-    @Test // class::staticMethod
-    void testFunctionCS() {
-        assertScript shell, '''
-            @CompileStatic
-            void p() {
-                def result = [1, -2, 3].stream().map(Math::abs).collect(Collectors.toList())
-                assert [1, 2, 3] == result
-            }
-
-            p()
-        '''
-    }
-
     @Test // instance::instanceMethod
     void testBinaryOperatorII() {
         assertScript shell, '''
@@ -255,6 +242,43 @@ final class MethodReferenceTest {
     }
 
     @Test // class::staticMethod
+    void testFunctionCS() {
+        assertScript shell, '''
+            @CompileStatic
+            void p() {
+                def result = [1, -2, 3].stream().map(Math::abs).collect(Collectors.toList())
+                assert [1, 2, 3] == result
+            }
+
+            p()
+        '''
+    }
+
+    @Test // class::staticMethod -- GROOVY-9799
+    void testFunctionCS2() {
+        assertScript shell, '''
+            class C {
+                String x
+            }
+
+            class D {
+                String x
+                static D from(C c) {
+                    new D(x: c.x)
+                }
+            }
+
+            @CompileStatic
+            def test(C c) {
+                Optional.of(c).map(D::from).get()
+            }
+
+            def d = test(new C(x: 'x'))
+            assert d.x == 'x'
+        '''
+    }
+
+    @Test // class::staticMethod
     void testFunctionCS_RHS() {
         assertScript shell, '''
             @CompileStatic
@@ -362,7 +386,7 @@ final class MethodReferenceTest {
             p()
         '''
 
-        assert err =~ 'Invalid receiver type: class java.lang.Integer is not compatible with
class java.lang.String'
+        assert err =~ 'Invalid receiver type: java.lang.Integer is not compatible with java.lang.String'
     }
 
     @Test // class::instanceMethod, actually class::staticMethod


Mime
View raw message