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

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


The following commit(s) were added to refs/heads/GROOVY_3_0_X by this push:
     new 55ba573d7e GROOVY-10049, GROOVY-10648: don't mix type param contexts 
when resolving
55ba573d7e is described below

commit 55ba573d7eb3a6b152390f18badd24255a341650
Author: Eric Milles <eric.mil...@thomsonreuters.com>
AuthorDate: Sat Jun 4 09:14:13 2022 -0500

    GROOVY-10049, GROOVY-10648: don't mix type param contexts when resolving
    
      [1].stream().reduce(0, (r,e) -> r + e)
          ^ Stream<Integer>   ^ ^ both Integer -- not int or unknown
    
      def <T extends Number> List<T> f(Class<T> t) {
        [t.newInstance(1)].stream().filter(n -> n.intValue() > 0).toList()
                           ^ Stream<T ext Num>  ^ T or Number -- not Object
    
    3_0_X backport -- see also GROOVY-8247, GROOVY-8917, GROOVY-9790
---
 .../transform/stc/StaticTypeCheckingVisitor.java   | 238 ++++++++-------------
 .../groovy/transform/stc/GenericsSTCTest.groovy    |  22 +-
 2 files changed, 113 insertions(+), 147 deletions(-)

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 ba83a7144e..23f95c673c 100644
--- 
a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ 
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -254,9 +254,9 @@ import static 
org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.findDG
 import static 
org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.findDGMMethodsForClassNode;
 import static 
org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.findSetters;
 import static 
org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.findTargetVariable;
+import static 
org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.fullyResolve;
 import static 
org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.fullyResolveType;
 import static 
org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.getCombinedBoundType;
-import static 
org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.getCorrectedClassNode;
 import static 
org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.getGenericsWithoutArray;
 import static 
org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.getOperationName;
 import static 
org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf;
@@ -2881,149 +2881,99 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
     }
 
     /**
-     * This method is responsible for performing type inference on closure 
argument types whenever code like this is
-     * found: <code>foo.collect { it.toUpperCase() }</code>.
-     * In this case, the type checker tries to find if the 
<code>collect</code> method has its {@link Closure} argument
-     * annotated with {@link groovy.transform.stc.ClosureParams}. If yes, then 
additional type inference can be performed
-     * and the type of <code>it</code> may be inferred.
+     * Performs type inference on closure argument types whenever code like 
this
+     * is found: <code>foo.collect { it.toUpperCase() }</code>.
+     * <p>
+     * In this case the type checker tries to find if the {@code collect} 
method
+     * has its {@link Closure} argument annotated with {@link ClosureParams}. 
If
+     * so, then additional type inference can be performed and the type of
+     * {@code it} may be inferred.
      *
      * @param receiver
      * @param arguments
-     * @param expression     a closure expression for which the argument types 
should be inferred
-     * @param param          the parameter where to look for a {@link 
groovy.transform.stc.ClosureParams} annotation.
-     * @param selectedMethod the method accepting a closure
+     * @param expression closure or lambda expression for which the argument 
types should be inferred
+     * @param target     parameter which may provide {@link ClosureParams} 
annotation or SAM type
+     * @param method     method that declares {@code target}
      */
-    protected void inferClosureParameterTypes(final ClassNode receiver, final 
Expression arguments, final ClosureExpression expression, final Parameter 
param, final MethodNode selectedMethod) {
-        List<AnnotationNode> annotations = 
param.getAnnotations(CLOSUREPARAMS_CLASSNODE);
+    protected void inferClosureParameterTypes(final ClassNode receiver, final 
Expression arguments, final ClosureExpression expression, final Parameter 
target, final MethodNode method) {
+        List<AnnotationNode> annotations = 
target.getAnnotations(CLOSUREPARAMS_CLASSNODE);
         if (annotations != null && !annotations.isEmpty()) {
             for (AnnotationNode annotation : annotations) {
                 Expression hintClass = annotation.getMember("value");
-                Expression options = annotation.getMember("options");
-                Expression resolverClass = 
annotation.getMember("conflictResolutionStrategy");
                 if (hintClass instanceof ClassExpression) {
-                    doInferClosureParameterTypes(receiver, arguments, 
expression, selectedMethod, hintClass, resolverClass, options);
-                }
-            }
-        } else if (isSAMType(param.getOriginType())) {
-            // SAM coercion
-            inferSAMType(param, receiver, selectedMethod, 
InvocationWriter.makeArgumentList(arguments), expression);
-        }
-    }
-
-    /**
-     * In a method call with SAM coercion the inference is to be understood as 
a
-     * two phase process.  We have the normal method call to the target method
-     * with the closure argument and we have the SAM method that will be called
-     * inside the normal target method.  To infer correctly we have to 
"simulate"
-     * this process. We know the call to the closure will be done through the 
SAM
-     * type, so the SAM type generics deliver information about the Closure.  
At
-     * the same time the SAM class is used in the target method parameter,
-     * providing a connection from the SAM type and the target method's class.
-     */
-    private void inferSAMType(final Parameter param, final ClassNode receiver, 
final MethodNode methodWithSAMParameter, final ArgumentListExpression 
originalMethodCallArguments, final ClosureExpression openBlock) {
-        // first we try to get as much information about the declaration class 
through the receiver
-        Map<GenericsTypeName, GenericsType> targetMethodConnections = new 
HashMap<>();
-        for (ClassNode face : receiver.getAllInterfaces()) {
-            extractGenericsConnections(targetMethodConnections, 
getCorrectedClassNode(receiver, face, true), face.redirect());
-        }
-        if (!receiver.isInterface()) {
-            extractGenericsConnections(targetMethodConnections, receiver, 
receiver.redirect());
-        }
-
-        // then we use the method with the SAM-type parameter to get more 
information about the declaration
-        Parameter[] parametersOfMethodContainingSAM = 
methodWithSAMParameter.getParameters();
-        for (int i = 0, n = parametersOfMethodContainingSAM.length; i < n; i 
+= 1) {
-            ClassNode parameterType = 
parametersOfMethodContainingSAM[i].getType();
-            // potentially skip empty varargs
-            if (i == (n - 1) && i == 
originalMethodCallArguments.getExpressions().size() && parameterType.isArray()) 
{
-                continue;
-            }
-            Expression callArg = originalMethodCallArguments.getExpression(i);
-            // we look at the closure later in detail, so skip it here
-            if (callArg == openBlock) {
-                continue;
-            }
-            extractGenericsConnections(targetMethodConnections, 
getType(callArg), parameterType);
-        }
-
-        // To make a connection to the SAM class we use that new information
-        // to replace the generics in the SAM type parameter of the target
-        // method and than that to make the connections to the SAM type 
generics
-        ClassNode paramTypeWithReceiverInformation = 
applyGenericsContext(targetMethodConnections, param.getOriginType());
-        Map<GenericsTypeName, GenericsType> samTypeConnections = new 
HashMap<>();
-        ClassNode samTypeRedirect = 
paramTypeWithReceiverInformation.redirect();
-        extractGenericsConnections(samTypeConnections, 
paramTypeWithReceiverInformation, samTypeRedirect);
-
-        // should the open block provide final information we apply that
-        // to the corresponding parameters of the SAM type method
-        MethodNode abstractMethod = findSAM(samTypeRedirect);
-        ClassNode[] abstractMethodParamTypes = 
extractTypesFromParameters(abstractMethod.getParameters());
-        ClassNode[] blockParamTypes = 
openBlock.getNodeMetaData(CLOSURE_ARGUMENTS);
-        if (blockParamTypes == null) {
-            Parameter[] p = openBlock.getParameters();
-            if (p == null) {
-                // zero parameter closure e.g. { -> println 'no args' }
-                blockParamTypes = ClassNode.EMPTY_ARRAY;
-            } else if (p.length == 0 && abstractMethodParamTypes.length != 0) {
-                // implicit it
-                blockParamTypes = abstractMethodParamTypes;
-            } else {
-                blockParamTypes = new ClassNode[p.length];
-                for (int i = 0, n = p.length; i < n; i += 1) {
-                    if (p[i] != null && !p[i].isDynamicTyped()) {
-                        blockParamTypes[i] = p[i].getType();
-                    } else {
-                        blockParamTypes[i] = 
typeOrNull(abstractMethodParamTypes, i);
+                    Expression options = annotation.getMember("options");
+                    Expression resolverClass = 
annotation.getMember("conflictResolutionStrategy");
+                    doInferClosureParameterTypes(receiver, arguments, 
expression, method, hintClass, resolverClass, options);
+                }
+            }
+        } else if (isSAMType(target.getOriginType())) { // SAM-type coercion
+            Map<GenericsTypeName, GenericsType> context = method.isStatic() ? 
new HashMap<>() : extractPlaceHolders(null, receiver, getDeclaringClass(method, 
arguments));
+            GenericsType[] typeParameters = method instanceof ConstructorNode 
? method.getDeclaringClass().getGenericsTypes() : applyGenericsContext(context, 
method.getGenericsTypes());
+
+            if (typeParameters != null) {
+                boolean typeParametersResolved = false;
+                // first check for explicit type arguments
+                Expression emc = typeCheckingContext.getEnclosingMethodCall();
+                if (emc instanceof MethodCallExpression) {
+                    MethodCallExpression mce = (MethodCallExpression) emc;
+                    if (mce.getArguments() == arguments) {
+                        GenericsType[] typeArguments = mce.getGenericsTypes();
+                        if (typeArguments != null) {
+                            int n = typeParameters.length;
+                            if (n == typeArguments.length) {
+                                typeParametersResolved = true;
+                                for (int i = 0; i < n; i += 1) {
+                                    context.put(new 
GenericsTypeName(typeParameters[i].getName()), typeArguments[i]);
+                                }
+                            }
+                        }
                     }
                 }
-            }
-        }
-        for (int i = 0, n = blockParamTypes.length; i < n; i += 1) {
-            extractGenericsConnections(samTypeConnections, blockParamTypes[i], 
typeOrNull(abstractMethodParamTypes, i));
-        }
+                if (!typeParametersResolved) {
+                    // check for implicit type arguments
+                    int i = -1; Parameter[] p = method.getParameters();
+                    for (Expression argument : (ArgumentListExpression) 
arguments) { i += 1;
+                        if (argument instanceof ClosureExpression || 
isNullConstant(argument)) continue;
 
-        // finally apply the generics information to the parameters and
-        // store the type of parameter and block type as meta information
-        for (int i = 0, n = blockParamTypes.length; i < n; i += 1) {
-            blockParamTypes[i] = applyGenericsContext(samTypeConnections, 
typeOrNull(abstractMethodParamTypes, i));
-        }
-
-        
tryToInferUnresolvedBlockParameterType(paramTypeWithReceiverInformation, 
abstractMethod, blockParamTypes);
+                        ClassNode pType = p[Math.min(i, p.length - 
1)].getType();
+                        Map<GenericsTypeName, GenericsType> gc = new 
HashMap<>();
+                        extractGenericsConnections(gc, 
wrapTypeIfNecessary(getType(argument)), pType);
 
-        openBlock.putNodeMetaData(CLOSURE_ARGUMENTS, blockParamTypes);
-    }
+                        gc.forEach((key, gt) -> {
+                            for (GenericsType tp : typeParameters) {
+                                if (tp.getName().equals(key.getName())) {
+                                    context.putIfAbsent(key, gt); // TODO: 
merge
+                                    break;
+                                }
+                            }
+                        });
+                    }
 
-    private void tryToInferUnresolvedBlockParameterType(final ClassNode 
paramTypeWithReceiverInformation, final MethodNode methodForSAM, final 
ClassNode[] blockParameterTypes) {
-        List<Integer> indexList = new LinkedList<>();
-        for (int i = 0, n = blockParameterTypes.length; i < n; i += 1) {
-            ClassNode blockParameterType = blockParameterTypes[i];
-            if (blockParameterType != null && 
blockParameterType.isGenericsPlaceHolder()) {
-                indexList.add(i);
+                    for (GenericsType tp : typeParameters) {
+                        context.computeIfAbsent(new 
GenericsTypeName(tp.getName()), x -> fullyResolve(tp, context));
+                    }
+                }
             }
-        }
 
-        if (!indexList.isEmpty()) {
-            // If the parameter type failed to resolve, try to find the 
parameter type through the class hierarchy
-            Map<GenericsType, GenericsType> genericsTypeMap = 
GenericsUtils.makeDeclaringAndActualGenericsTypeMapOfExactType(methodForSAM.getDeclaringClass(),
 paramTypeWithReceiverInformation);
+            ClassNode[] samParamTypes = 
GenericsUtils.parameterizeSAM(applyGenericsContext(context, 
target.getType())).getV1();
 
-            for (Integer index : indexList) {
-                for (Map.Entry<GenericsType, GenericsType> entry : 
genericsTypeMap.entrySet()) {
-                    if 
(entry.getKey().getName().equals(blockParameterTypes[index].getUnresolvedName()))
 {
-                        ClassNode type = entry.getValue().getType();
-                        if (type != null && !type.isGenericsPlaceHolder()) {
-                            blockParameterTypes[index] = type;
-                        }
-                        break;
-                    }
+            ClassNode[] paramTypes = 
expression.getNodeMetaData(CLOSURE_ARGUMENTS);
+            if (paramTypes == null) {
+                int n; Parameter[] p = expression.getParameters();
+                if (p == null) {
+                    // zero parameters
+                    paramTypes = ClassNode.EMPTY_ARRAY;
+                } else if ((n = p.length) == 0) {
+                    // implicit parameter(s)
+                    paramTypes = samParamTypes;
+                } else { // TODO: error for length mismatch
+                    paramTypes = Arrays.copyOf(samParamTypes, n);
                 }
+                expression.putNodeMetaData(CLOSURE_ARGUMENTS, paramTypes);
             }
         }
     }
 
-    private static ClassNode typeOrNull(final ClassNode[] 
parameterTypesForSAM, final int i) {
-        return i < parameterTypesForSAM.length ? parameterTypesForSAM[i] : 
null;
-    }
-
     private List<ClassNode[]> getSignaturesFromHint(final ClosureExpression 
expression, final MethodNode selectedMethod, final Expression hintClass, final 
Expression options) {
         // initialize hints
         List<ClassNode[]> closureSignatures;
@@ -3089,25 +3039,24 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
             }
         }
         if (candidates.size() > 1) {
-            Iterator<ClassNode[]> candIt = candidates.iterator();
-            while (candIt.hasNext()) {
+            for (Iterator<ClassNode[]> candIt = candidates.iterator(); 
candIt.hasNext(); ) {
                 ClassNode[] inferred = candIt.next();
                 for (int i = 0, n = closureParams.length; i < n; i += 1) {
                     Parameter closureParam = closureParams[i];
-                    ClassNode originType = closureParam.getOriginType();
+                    ClassNode declaredType = closureParam.getOriginType();
                     ClassNode inferredType;
-                    if (i < inferred.length - 1 || inferred.length == 
closureParams.length) {
+                    if (i < inferred.length - 1 || inferred.length == n) {
                         inferredType = inferred[i];
-                    } else { // vargs?
-                        ClassNode lastArgInferred = inferred[inferred.length - 
1];
-                        if (lastArgInferred.isArray()) {
-                            inferredType = lastArgInferred.getComponentType();
+                    } else {
+                        ClassNode lastInferred = inferred[inferred.length - 1];
+                        if (lastInferred.isArray()) {
+                            inferredType = lastInferred.getComponentType();
                         } else {
                             candIt.remove();
                             continue;
                         }
                     }
-                    if (!typeCheckMethodArgumentWithGenerics(originType, 
inferredType, i == (n - 1))) {
+                    if (!typeCheckMethodArgumentWithGenerics(declaredType, 
inferredType, i == (n - 1))) {
                         candIt.remove();
                     }
                 }
@@ -3126,24 +3075,21 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
             } else {
                 for (int i = 0, n = closureParams.length; i < n; i += 1) {
                     Parameter closureParam = closureParams[i];
-                    ClassNode originType = closureParam.getOriginType();
+                    ClassNode declaredType = closureParam.getOriginType();
                     ClassNode inferredType = OBJECT_TYPE;
-                    if (i < inferred.length - 1 || inferred.length == 
closureParams.length) {
+                    if (i < inferred.length - 1 || inferred.length == n) {
                         inferredType = inferred[i];
-                    } else { // vargs?
-                        ClassNode lastArgInferred = inferred[inferred.length - 
1];
-                        if (lastArgInferred.isArray()) {
-                            inferredType = lastArgInferred.getComponentType();
+                    } else {
+                        ClassNode lastInferred = inferred[inferred.length - 1];
+                        if (lastInferred.isArray()) {
+                            inferredType = lastInferred.getComponentType();
                         } else {
-                            addError("Incorrect number of parameters. Expected 
" + inferred.length + " but found " + closureParams.length, expression);
+                            addError("Incorrect number of parameters. Expected 
" + inferred.length + " but found " + n, expression);
                         }
                     }
-                    boolean lastArg = i == (n - 1);
-
-                    if (!typeCheckMethodArgumentWithGenerics(originType, 
inferredType, lastArg)) {
-                        addError("Expected parameter of type " + 
inferredType.toString(false) + " but got " + originType.toString(false), 
closureParam.getType());
+                    if (!typeCheckMethodArgumentWithGenerics(declaredType, 
inferredType, i == n-1)) {
+                        addError("Expected parameter of type " + 
prettyPrintType(inferredType) + " but got " + prettyPrintType(declaredType), 
closureParam.getType());
                     }
-
                     
typeCheckingContext.controlStructureVariables.put(closureParam, inferredType);
                 }
             }
@@ -5291,8 +5237,8 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
             ArgumentListExpression args = new ArgumentListExpression();
             VariableExpression vexp = varX("$self", receiver);
             args.addExpression(vexp);
-            if (arguments instanceof ArgumentListExpression) {
-                for (Expression argument : (ArgumentListExpression) arguments) 
{
+            if (arguments instanceof TupleExpression) {
+                for (Expression argument : (TupleExpression) arguments) {
                     args.addExpression(argument);
                 }
             } else {
diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy 
b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
index 6b8b2d2e48..f021659414 100644
--- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -289,7 +289,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
         '''
     }
 
-    @NotYetImplemented // GROOVY-10049, GROOVY-10053
+    // GROOVY-10049
     void testReturnTypeInferenceWithMethodGenerics9() {
         assertScript '''
             def <X> Set<X> f(Class<X> x) {
@@ -302,7 +302,10 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
             def result = g(Integer)
             assert result == [ 42 ]
         '''
+    }
 
+    @NotYetImplemented // GROOVY-10053
+    void testReturnTypeInferenceWithMethodGenericsA() {
         ['t::cast', 'n -> t.cast(n)', 'n -> (N) n', '{ n -> (N) n }'].each { 
cast ->
             assertScript """
                 Set<Number> f() {
@@ -319,7 +322,10 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
                 assert result == [42] as Set
             """
         }
+    }
 
+    @NotYetImplemented
+    void testReturnTypeInferenceWithMethodGenericsB() {
         assertScript '''
             def <T> String test(Iterable<T> iterable) {
                 Iterator<T> it = iterable.iterator()
@@ -1975,6 +1981,20 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase 
{
         '''
     }
 
+    // GROOVY-10648
+    void testShouldUseMethodGenericType16() {
+        assertScript '''
+            def <T extends Number> Set<T> test(Iterable<T> iterable) {
+                final Set<T> result = new HashSet<>()
+                iterable.forEach { result.add(it) }
+                return result
+            }
+            def set = test([1,2.3])
+            assert set.size() == 2
+            assert 1 in set
+        '''
+    }
+
     // GROOVY-5516
     void testAddAllWithCollectionShouldBeAllowed() {
         assertScript '''import 
org.codehaus.groovy.transform.stc.ExtensionMethodNode

Reply via email to