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/03/01 21:25:19 UTC

[groovy] branch GROOVY_2_5_X updated (25b48f5 -> 6df419f)

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

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


    from 25b48f5  GROOVY-10052: no re-visit for closure without changed/shared variable(s)
     new 2aee402  GROOVY-9885: STC: use checkCompatibleAssignmentTypes for map constructor
     new 6df419f  GROOVY-9882, GROOVY-9997, etc.: STC: check functional interface coercion

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../codehaus/groovy/ast/tools/GenericsUtils.java   |  16 ++-
 .../transform/stc/StaticTypeCheckingVisitor.java   | 109 +++++++++++++++------
 .../transform/stc/ConstructorsSTCTest.groovy       |  16 +++
 .../stc/FieldsAndPropertiesSTCTest.groovy          |  40 ++++++--
 src/test/groovy/transform/stc/Groovy8247Bug.groovy |   5 +-
 5 files changed, 141 insertions(+), 45 deletions(-)

[groovy] 02/02: GROOVY-9882, GROOVY-9997, etc.: STC: check functional interface coercion

Posted by em...@apache.org.
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

commit 6df419fa16aac2b020e924c6ba6c9f05a18ca9b6
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Jan 15 11:52:27 2021 -0600

    GROOVY-9882, GROOVY-9997, etc.: STC: check functional interface coercion
    
    Conflicts:
    	src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
---
 .../codehaus/groovy/ast/tools/GenericsUtils.java   | 16 +++--
 .../transform/stc/StaticTypeCheckingVisitor.java   | 72 ++++++++++++++++++----
 .../stc/FieldsAndPropertiesSTCTest.groovy          | 40 ++++++++++--
 src/test/groovy/transform/stc/Groovy8247Bug.groovy |  5 +-
 4 files changed, 107 insertions(+), 26 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 47d6d86..931d630 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
@@ -974,13 +974,19 @@ public class GenericsUtils {
      */
     public static ClassNode findActualTypeByGenericsPlaceholderName(String placeholderName, Map<GenericsType, GenericsType> genericsPlaceholderAndTypeMap) {
         for (Map.Entry<GenericsType, GenericsType> entry : genericsPlaceholderAndTypeMap.entrySet()) {
-            GenericsType declaringGenericsType = entry.getKey();
-
-            if (placeholderName.equals(declaringGenericsType.getName())) {
-                return entry.getValue().getType().redirect();
+            if (placeholderName.equals(entry.getKey().getName())) {
+                GenericsType gt = entry.getValue();
+                if (gt.isWildcard()) {
+                    if (gt.getLowerBound() != null) {
+                        return gt.getLowerBound();
+                    }
+                    if (gt.getUpperBounds() != null) {
+                        return gt.getUpperBounds()[0];
+                    }
+                }
+                return gt.getType();
             }
         }
-
         return null;
     }
 
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 19b0ff6..5da7e75 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -174,6 +174,7 @@ import static org.codehaus.groovy.ast.ClassHelper.make;
 import static org.codehaus.groovy.ast.ClassHelper.short_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.void_WRAPPER_TYPE;
 import static org.codehaus.groovy.ast.tools.ClosureUtils.getParametersSafe;
+import static org.codehaus.groovy.ast.tools.ClosureUtils.hasImplicitParameter;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.binX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
@@ -798,9 +799,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                     return;
                 }
             } else {
-                if (op == ASSIGN) { // GROOVY-9971
+                if (op == ASSIGN) {
                     ClassNode lType = getOriginalDeclarationType(leftExpression);
-                    if (isClosureWithType(lType) && rightExpression instanceof ClosureExpression) {
+                    if (isFunctionalInterface(lType)) {
+                        processFunctionalInterfaceAssignment(lType, rightExpression);
+                    } else if (isClosureWithType(lType) && rightExpression instanceof ClosureExpression) {
                         storeInferredReturnType(rightExpression, getCombinedBoundType(lType.getGenericsTypes()[0]));
                     }
                 }
@@ -1007,6 +1010,44 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         return type.equals(CLOSURE_TYPE) && type.getGenericsTypes() != null && type.getGenericsTypes().length == 1;
     }
 
+    private static boolean isFunctionalInterface(final ClassNode type) {
+        return type.isInterface() && isSAMType(type);
+    }
+
+    private void processFunctionalInterfaceAssignment(final ClassNode lhsType, final Expression rhsExpression) {
+        if (rhsExpression instanceof ClosureExpression) {
+            MethodNode abstractMethod = findSAM(lhsType);
+            Map<GenericsType, GenericsType> mappings = GenericsUtils.makeDeclaringAndActualGenericsTypeMapOfExactType(abstractMethod.getDeclaringClass(), lhsType);
+
+            ClassNode[] samParameterTypes = extractTypesFromParameters(abstractMethod.getParameters());
+            for (int i = 0; i < samParameterTypes.length; i += 1) {
+                if (samParameterTypes[i].isGenericsPlaceHolder()) {
+                    samParameterTypes[i] = GenericsUtils.findActualTypeByGenericsPlaceholderName(samParameterTypes[i].getUnresolvedName(), mappings);
+                }
+            }
+
+            Parameter[] closureParameters = getParametersSafe((ClosureExpression) rhsExpression);
+            if (closureParameters.length == samParameterTypes.length || (1 == samParameterTypes.length && hasImplicitParameter((ClosureExpression) rhsExpression))) {
+                for (int i = 0; i < closureParameters.length; i += 1) {
+                    Parameter parameter = closureParameters[i];
+                    if (parameter.isDynamicTyped()) {
+                        parameter.setType(samParameterTypes[i]);
+                        parameter.setOriginType(samParameterTypes[i]);
+                    }
+                }
+            } else {
+                String descriptor = toMethodParametersString(findSAM(lhsType).getName(), samParameterTypes);
+                addStaticTypeError("Wrong number of parameters for method target " + descriptor, rhsExpression);
+            }
+
+            ClassNode returnType = abstractMethod.getReturnType();
+            if (returnType.isGenericsPlaceHolder()) {
+                returnType = GenericsUtils.findActualTypeByGenericsPlaceholderName(returnType.getUnresolvedName(), mappings);
+            }
+            storeInferredReturnType(rhsExpression, returnType);
+        }
+    }
+
     private static boolean isCompoundAssignment(final Expression exp) {
         if (!(exp instanceof BinaryExpression)) return false;
         int type = ((BinaryExpression) exp).getOperation().getType();
@@ -1876,7 +1917,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     private void visitInitialExpression(final Expression value, final Expression target, final ASTNode position) {
         if (value != null) {
             ClassNode lType = target.getType();
-            if (isClosureWithType(lType) && value instanceof ClosureExpression) {
+            if (isFunctionalInterface(lType)) {
+                processFunctionalInterfaceAssignment(lType, value);
+            } else if (isClosureWithType(lType) && value instanceof ClosureExpression) {
                 storeInferredReturnType(value, getCombinedBoundType(lType.getGenericsTypes()[0]));
             }
 
@@ -2780,7 +2823,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                         inferClosureParameterTypes(receiver, arguments, (ClosureExpression) expression, param, selectedMethod);
                     }
                     ClassNode targetType = param.getType();
-                    if (isClosureWithType(targetType)) { // GROOVY-9971
+                    if (isFunctionalInterface(targetType)) {
+                        processFunctionalInterfaceAssignment(targetType, expression);
+                    } else if (isClosureWithType(targetType)) { // GROOVY-9971
                         storeInferredReturnType(expression, getCombinedBoundType(targetType.getGenericsTypes()[0]));
                     }
                 }
@@ -4008,16 +4053,17 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
 
     @Override
     public void visitCastExpression(final CastExpression expression) {
-        super.visitCastExpression(expression);
-        if (!expression.isCoerce()) {
-            ClassNode targetType = expression.getType();
-            Expression source = expression.getExpression();
-            ClassNode expressionType = getType(source);
-            if (!checkCast(targetType, source) && !isDelegateOrOwnerInClosure(source)) {
-                addStaticTypeError("Inconvertible types: cannot cast " + expressionType.toString(false) + " to " + targetType.toString(false), expression);
-            }
+        ClassNode type = expression.getType();
+        Expression target = expression.getExpression();
+        if (isFunctionalInterface(type)) { // GROOVY-9997
+            processFunctionalInterfaceAssignment(type, target);
+        }
+
+        target.visit(this);
+
+        if (!expression.isCoerce() && !checkCast(type, target) && !isDelegateOrOwnerInClosure(target)) {
+            addStaticTypeError("Inconvertible types: cannot cast " + prettyPrintType(getType(target)) + " to " + prettyPrintType(type), expression);
         }
-        storeType(expression, expression.getType());
     }
 
     private boolean isDelegateOrOwnerInClosure(Expression exp) {
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index 0e55f07..5258261 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -293,9 +293,9 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
     void testFieldInitShouldPass() {
         assertScript '''
             class Foo {
-                int x = 1
+                int bar = 1
             }
-            1
+            new Foo()
         '''
     }
 
@@ -303,9 +303,9 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
     void testFieldInitShouldNotPassBecauseOfIncompatibleTypes() {
         shouldFailWithMessages '''
             class Foo {
-                int x = new Date()
+                int bar = new Date()
             }
-            1
+            new Foo()
         ''', 'Cannot assign value of type java.util.Date to variable of type int'
     }
 
@@ -313,12 +313,40 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
     void testFieldInitShouldNotPassBecauseOfIncompatibleTypesWithClosure() {
         shouldFailWithMessages '''
             class Foo {
-                Closure<List> cls = { Date aDate ->  aDate.getTime() }
+                Closure<List> bar = { Date date -> date.getTime() }
             }
-            1
+            new Foo()
         ''', 'Incompatible generic argument types. Cannot assign groovy.lang.Closure <java.lang.Long> to: groovy.lang.Closure <List>'
     }
 
+    void testFieldInitShouldNotPassBecauseOfIncompatibleTypesWithClosure2() {
+        shouldFailWithMessages '''
+            class Foo {
+                java.util.concurrent.Callable<String> bar = { 123 }
+            }
+            new Foo()
+        ''', 'Incompatible generic argument types. Cannot assign java.util.concurrent.Callable <java.lang.Integer> to: java.util.concurrent.Callable <String>'
+    }
+
+    // GROOVY-9882
+    void testFieldInitShouldPassForCcompatibleTypesWithClosure() {
+        assertScript '''
+            class Foo {
+                java.util.concurrent.Callable<String> bar = { 'abc' }
+            }
+            assert new Foo().bar.call() == 'abc'
+        '''
+    }
+
+    void testFieldInitClosureParameterMismatch() {
+        shouldFailWithMessages '''
+            class Foo {
+                java.util.concurrent.Callable<String> bar = { a -> '' }
+            }
+            new Foo()
+        ''', 'Wrong number of parameters'
+    }
+
     // GROOVY-5517
     void testShouldFindStaticPropertyEvenIfObjectImplementsMap() {
         assertScript '''
diff --git a/src/test/groovy/transform/stc/Groovy8247Bug.groovy b/src/test/groovy/transform/stc/Groovy8247Bug.groovy
index 7488352..3533a58 100644
--- a/src/test/groovy/transform/stc/Groovy8247Bug.groovy
+++ b/src/test/groovy/transform/stc/Groovy8247Bug.groovy
@@ -21,7 +21,7 @@ package groovy.transform.stc
 
 class Groovy8247Bug extends StaticTypeCheckingTestCase {
     void testClosureWithExplicitParamNoInferrableArguments() {
-        assertScript '''
+        shouldFailWithMessages '''
             def runnable(Runnable r) {
                 r.run()
             }
@@ -31,6 +31,7 @@ class Groovy8247Bug extends StaticTypeCheckingTestCase {
                 }
             }
             foo()
-        '''
+        ''',
+        'Wrong number of parameters for method target run()'
     }
 }

[groovy] 01/02: GROOVY-9885: STC: use checkCompatibleAssignmentTypes for map constructor

Posted by em...@apache.org.
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

commit 2aee4021546fd8b89870e03a1cb7f7f5debf4b00
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Jan 16 11:41:30 2021 -0600

    GROOVY-9885: STC: use checkCompatibleAssignmentTypes for map constructor
    
    Conflicts:
    	src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
---
 .../transform/stc/StaticTypeCheckingVisitor.java   | 37 +++++++++++-----------
 .../transform/stc/ConstructorsSTCTest.groovy       | 16 ++++++++++
 2 files changed, 34 insertions(+), 19 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 9d21672..19b0ff6 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1257,10 +1257,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         ClassNode wrappedRHS = adjustTypeForSpreading(inferredRightExpressionType, leftExpression);
 
         // check types are compatible for assignment
-        boolean compatible = checkCompatibleAssignmentTypes(leftRedirect, wrappedRHS, rightExpression);
-
-
-        if (!compatible) {
+        if (!checkCompatibleAssignmentTypes(leftRedirect, wrappedRHS, rightExpression)) {
             if (!extension.handleIncompatibleAssignment(leftExpressionType, inferredRightExpressionType, assignmentExpression)) {
                 addAssignmentError(leftExpressionType, inferredRightExpressionType, assignmentExpression.getRightExpression());
             }
@@ -1277,23 +1274,25 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         // workaround for map-style checks putting setter info on wrong AST nodes
         typeCheckingContext.pushEnclosingBinaryExpression(null);
         for (MapEntryExpression entryExpression : mapExpression.getMapEntryExpressions()) {
-            Expression keyExpr = entryExpression.getKeyExpression();
-            if (!(keyExpr instanceof ConstantExpression)) {
-                addStaticTypeError("Dynamic keys in map-style constructors are unsupported in static type checking", keyExpr);
+            Expression keyExpression = entryExpression.getKeyExpression();
+            if (!(keyExpression instanceof ConstantExpression)) {
+                addStaticTypeError("Dynamic keys in map-style constructors are unsupported in static type checking", keyExpression);
             } else {
-                AtomicReference<ClassNode> lookup = new AtomicReference<ClassNode>();
-                PropertyExpression pexp = new PropertyExpression(varX("_", receiverType), keyExpr.getText());
-                boolean hasProperty = existsProperty(pexp, false, new PropertyLookupVisitor(lookup));
-                if (!hasProperty) {
-                    addStaticTypeError("No such property: " + keyExpr.getText() +
-                            " for class: " + receiverType.getName(), receiver);
+                String pName = keyExpression.getText();
+                AtomicReference<ClassNode> pType = new AtomicReference<>();
+                if (!existsProperty(new PropertyExpression(varX("_", receiverType), pName), false, new PropertyLookupVisitor(pType))) {
+                    addStaticTypeError("No such property: " + pName + " for class: " + receiverType.getText(), receiver);
                 } else {
-                    ClassNode valueType = getType(entryExpression.getValueExpression());
-                    MethodNode setter = receiverType.getSetterMethod("set" + MetaClassHelper.capitalize(pexp.getPropertyAsString()), false);
-                    ClassNode toBeAssignedTo = setter == null ? lookup.get() : setter.getParameters()[0].getType();
-                    if (!isAssignableTo(valueType, toBeAssignedTo)
-                            && !extension.handleIncompatibleAssignment(toBeAssignedTo, valueType, entryExpression)) {
-                        addAssignmentError(toBeAssignedTo, valueType, entryExpression);
+                    MethodNode setter = receiverType.getSetterMethod("set" + MetaClassHelper.capitalize(pName), false);
+                    ClassNode targetType = setter != null ? setter.getParameters()[0].getType() : pType.get();
+                    Expression valueExpression = entryExpression.getValueExpression();
+                    ClassNode valueType = getType(valueExpression);
+
+                    ClassNode resultType = getResultType(targetType, ASSIGN, valueType,
+                                assignX(keyExpression, valueExpression, entryExpression));
+                    if (!checkCompatibleAssignmentTypes(targetType, resultType, valueExpression)
+                            && !extension.handleIncompatibleAssignment(targetType, valueType, entryExpression)) {
+                        addAssignmentError(targetType, valueType, entryExpression);
                     }
                 }
             }
diff --git a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
index f4d98ef..734fd48 100644
--- a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
@@ -318,6 +318,22 @@ class ConstructorsSTCTest extends StaticTypeCheckingTestCase {
         '''
     }
 
+    // GROOVY-9885
+    void testUseGStringTernaryInNamedParameter() {
+        assertScript '''
+            @groovy.transform.ToString
+            class Pogo {
+                String value
+            }
+            def make(String string, whatever) {
+                new Pogo(value: string.trim() ?: "$whatever")
+            }
+            assert make('x','y').toString() == 'Pogo(x)'
+            assert make(' ','y').toString() == 'Pogo(y)'
+            assert make(' ',123).toString() == 'Pogo(123)'
+        '''
+    }
+
     // GROOVY-5578
     void testConstructJavaBeanFromMap() {
         assertScript '''import groovy.transform.stc.MyBean