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 2021/08/18 16:50:05 UTC

[groovy] branch GROOVY_3_0_X updated (45ec584 -> 3dca50c)

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

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


    from 45ec584  GROOVY-9984: don't use null arguments for return type generics inference
     new 6e4049e  GROOVY-10052: no re-visit for closure without changed/shared variable(s)
     new 9ecfe44  GROOVY-10056: Inferred parameter type of lambda expression for multi-dimensions array is not correct
     new 3dca50c  GROOVY-10075: STC: always re-check extension method receiver/argument(s)

The 3 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:
 .../transform/stc/StaticTypeCheckingSupport.java   | 26 +++----
 .../transform/stc/StaticTypeCheckingVisitor.java   | 40 +++++++----
 .../bugs/{Groovy9265.groovy => Groovy10056.groovy} | 33 ++++-----
 .../groovy/transform/stc/ClosuresSTCTest.groovy    | 41 ++++++++++-
 .../stc/DefaultGroovyMethodsSTCTest.groovy         | 80 +++++++++++++++-------
 .../groovy/runtime/m12n/TestStringExtension.java   | 12 +++-
 6 files changed, 158 insertions(+), 74 deletions(-)
 copy src/test/groovy/bugs/{Groovy9265.groovy => Groovy10056.groovy} (67%)

[groovy] 01/03: GROOVY-10052: no re-visit for closure without changed/shared variable(s)

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

commit 6e4049e16f7a79a173fcbbc12b46f8bf86d1c75a
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Apr 23 13:31:39 2021 -0500

    GROOVY-10052: no re-visit for closure without changed/shared variable(s)
---
 .../transform/stc/StaticTypeCheckingVisitor.java   | 30 ++++++++++------
 .../groovy/transform/stc/ClosuresSTCTest.groovy    | 41 +++++++++++++++++++++-
 2 files changed, 60 insertions(+), 11 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 876e34e..2f72981 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1923,8 +1923,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                 typeCheckingContext.controlStructureVariables.remove(forLoop.getVariable());
             }
         }
-        boolean typeChanged = isSecondPassNeededForControlStructure(varOrigType, oldTracker);
-        if (typeChanged) visitForLoop(forLoop);
+        if (isSecondPassNeededForControlStructure(varOrigType, oldTracker)) {
+            visitForLoop(forLoop);
+        }
     }
 
     /**
@@ -2338,8 +2339,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         typeCheckingContext.isInStaticContext = false;
 
         // collect every variable expression used in the closure body
-        Map<VariableExpression, ClassNode> variableTypes = new HashMap<>();
-        expression.getCode().visit(new VariableExpressionTypeMemoizer(variableTypes));
+        Map<VariableExpression, ClassNode> varTypes = new HashMap<>();
+        expression.getCode().visit(new VariableExpressionTypeMemoizer(varTypes, true));
         Map<VariableExpression, List<ClassNode>> oldTracker = pushAssignmentTracking();
         SharedVariableCollector collector = new SharedVariableCollector(getSourceUnit());
         collector.visitClosureExpression(expression);
@@ -2393,9 +2394,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         }
 
         typeCheckingContext.popEnclosingClosure();
-
-        boolean typeChanged = isSecondPassNeededForControlStructure(variableTypes, oldTracker);
-        if (typeChanged) visitClosureExpression(expression);
+        // check types of closure shared variables for change
+        if (isSecondPassNeededForControlStructure(varTypes, oldTracker)) {
+            visitClosureExpression(expression);
+        }
 
         // restore original metadata
         restoreVariableExpressionMetadata(variableMetadata);
@@ -5900,10 +5902,16 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     }
 
     protected class VariableExpressionTypeMemoizer extends ClassCodeVisitorSupport {
+        private final boolean onlySharedVariables;
         private final Map<VariableExpression, ClassNode> varOrigType;
 
         public VariableExpressionTypeMemoizer(final Map<VariableExpression, ClassNode> varOrigType) {
+            this(varOrigType, false);
+        }
+
+        public VariableExpressionTypeMemoizer(final Map<VariableExpression, ClassNode> varOrigType, final boolean onlySharedVariables) {
             this.varOrigType = varOrigType;
+            this.onlySharedVariables = onlySharedVariables;
         }
 
         @Override
@@ -5913,12 +5921,14 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
 
         @Override
         public void visitVariableExpression(final VariableExpression expression) {
-            super.visitVariableExpression(expression);
             Variable var = findTargetVariable(expression);
-            if (var instanceof VariableExpression) {
+            if ((!onlySharedVariables || var.isClosureSharedVariable()) && var instanceof VariableExpression) {
                 VariableExpression ve = (VariableExpression) var;
-                varOrigType.put(ve, ve.getNodeMetaData(INFERRED_TYPE));
+                ClassNode cn = ve.getNodeMetaData(INFERRED_TYPE);
+                if (cn == null) cn = ve.getOriginType();
+                varOrigType.put(ve, cn);
             }
+            super.visitVariableExpression(expression);
         }
     }
 }
diff --git a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
index b148665..0759138 100644
--- a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
@@ -18,6 +18,8 @@
  */
 package groovy.transform.stc
 
+import groovy.test.NotYetImplemented
+
 /**
  * Unit tests for static type checking : closures.
  */
@@ -32,8 +34,8 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
         '''
     }
 
+    // GROOVY-9079: no params to statically type check but shouldn't get NPE
     void testClosureWithoutArgumentsExplicit() {
-        // GROOVY-9079: no params to statically type check but shouldn't get NPE
         assertScript '''
             import java.util.concurrent.Callable
 
@@ -287,6 +289,43 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
         ''', 'Cannot find matching method A#m()'
     }
 
+    // GROOVY-10052
+    void testClosureSharedVariable4() {
+        assertScript '''
+            String x
+            def f = { ->
+                x = Optional.of('x').orElseThrow{ new Exception() }
+            }
+            assert f() == 'x'
+            assert x == 'x'
+        '''
+    }
+
+    @NotYetImplemented // GROOVY-10052
+    void testClosureSharedVariable5() {
+        assertScript '''
+            def x
+            def f = { ->
+                x = Optional.of('x').orElseThrow{ new Exception() }
+            }
+            assert f() == 'x'
+            assert x == 'x'
+        '''
+    }
+
+    // GROOVY-10052
+    void testNotClosureSharedVariable() {
+        assertScript '''
+            String x = Optional.of('x').orElseThrow{ new Exception() }
+            def f = { ->
+                String y = Optional.of('y').orElseThrow{ new Exception() }
+            }
+
+            assert x == 'x'
+            assert f() == 'y'
+        '''
+    }
+
     void testClosureCallAsAMethod() {
         assertScript '''
             Closure cl = { 'foo' }

[groovy] 02/03: GROOVY-10056: Inferred parameter type of lambda expression for multi-dimensions array is not correct

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

commit 9ecfe44a98bdabb6cd7c840f2326a4efcb6389f4
Author: Daniel.Sun <su...@apache.org>
AuthorDate: Mon Apr 26 20:18:56 2021 +0800

    GROOVY-10056: Inferred parameter type of lambda expression for multi-dimensions array is not correct
---
 .../transform/stc/StaticTypeCheckingVisitor.java   |  5 +--
 src/test/groovy/bugs/Groovy10056.groovy            | 44 ++++++++++++++++++++++
 2 files changed, 45 insertions(+), 4 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 2f72981..7b2b1be 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -5234,10 +5234,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                                             applyGenericsContextToParameterClass(resolvedPlaceholders, paramType));
                         }
                     }
-                    if (isVargs && lastArg && argumentType.isArray()) {
-                        argumentType = argumentType.getComponentType();
-                    }
-                    if (isVargs && lastArg && paramType.isArray()) {
+                    if (isVargs && lastArg && paramType.isArray() && !argumentType.isArray()) {
                         paramType = paramType.getComponentType();
                     }
                     argumentType = wrapTypeIfNecessary(argumentType);
diff --git a/src/test/groovy/bugs/Groovy10056.groovy b/src/test/groovy/bugs/Groovy10056.groovy
new file mode 100644
index 0000000..aa8cc89
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy10056.groovy
@@ -0,0 +1,44 @@
+/*
+ *  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 bugs
+
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+final class Groovy10056  {
+    @Test
+    void test() {
+        assertScript '''\
+            @groovy.transform.CompileStatic
+            def test() {
+                String[][] strsArray = new String[][] {
+                    new String[] {'a', 'b', 'c'},
+                    new String[] {'d', 'e', 'f'}
+                }
+                Arrays.stream(strsArray)
+                            .limit(1)
+                            .forEach(strs -> {
+                                assert ['a', 'b', 'c'] == Arrays.asList(strs)
+                            })
+            }
+            test()
+        '''
+    }
+}

[groovy] 03/03: GROOVY-10075: STC: always re-check extension method receiver/argument(s)

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

commit 3dca50c2024d75abed700acd079100c5f05f40d0
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri May 7 15:22:53 2021 -0500

    GROOVY-10075: STC: always re-check extension method receiver/argument(s)
---
 .../transform/stc/StaticTypeCheckingSupport.java   | 26 +++----
 .../transform/stc/StaticTypeCheckingVisitor.java   |  5 ++
 .../stc/DefaultGroovyMethodsSTCTest.groovy         | 80 +++++++++++++++-------
 .../groovy/runtime/m12n/TestStringExtension.java   | 12 +++-
 4 files changed, 83 insertions(+), 40 deletions(-)

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 2692f16..2a5fd0d 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -1364,25 +1364,21 @@ public abstract class StaticTypeCheckingSupport {
     }
 
     protected static boolean typeCheckMethodsWithGenerics(final ClassNode receiver, final ClassNode[] argumentTypes, final MethodNode candidateMethod) {
-        boolean isExtensionMethod = candidateMethod instanceof ExtensionMethodNode;
-        if (!isExtensionMethod
-                && receiver.isUsingGenerics()
+        if (candidateMethod instanceof ExtensionMethodNode) {
+            ClassNode[] realTypes = new ClassNode[argumentTypes.length + 1];
+            realTypes[0] = receiver; // object expression is implicit argument
+            System.arraycopy(argumentTypes, 0, realTypes, 1, argumentTypes.length);
+            MethodNode realMethod = ((ExtensionMethodNode) candidateMethod).getExtensionMethodNode();
+            return typeCheckMethodsWithGenerics(realMethod.getDeclaringClass(), realTypes, realMethod, true);
+        }
+
+        if (receiver.isUsingGenerics()
                 && receiver.equals(CLASS_Type)
                 && !candidateMethod.getDeclaringClass().equals(CLASS_Type)) {
             return typeCheckMethodsWithGenerics(receiver.getGenericsTypes()[0].getType(), argumentTypes, candidateMethod);
         }
-        // both candidate method and receiver have generic information so a check is possible
-        GenericsType[] genericsTypes = candidateMethod.getGenericsTypes();
-        boolean methodUsesGenerics = (genericsTypes != null && genericsTypes.length > 0);
-        if (isExtensionMethod && methodUsesGenerics) {
-            ClassNode[] dgmArgs = new ClassNode[argumentTypes.length + 1];
-            dgmArgs[0] = receiver;
-            System.arraycopy(argumentTypes, 0, dgmArgs, 1, argumentTypes.length);
-            MethodNode extensionMethodNode = ((ExtensionMethodNode) candidateMethod).getExtensionMethodNode();
-            return typeCheckMethodsWithGenerics(extensionMethodNode.getDeclaringClass(), dgmArgs, extensionMethodNode, true);
-        } else {
-            return typeCheckMethodsWithGenerics(receiver, argumentTypes, candidateMethod, false);
-        }
+
+        return typeCheckMethodsWithGenerics(receiver, argumentTypes, candidateMethod, false);
     }
 
     private static boolean typeCheckMethodsWithGenerics(final ClassNode receiver, final ClassNode[] argumentTypes, final MethodNode candidateMethod, final boolean isExtensionMethod) {
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 7b2b1be..440aebd 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1605,6 +1605,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                 for (MethodNode method : findDGMMethodsByNameAndArguments(getSourceUnit().getClassLoader(), dgmReceiver, "is" + capName, ClassNode.EMPTY_ARRAY)) {
                     if (Boolean_TYPE.equals(getWrapper(method.getReturnType()))) methods.add(method);
                 }
+                if (isUsingGenericsOrIsArrayUsingGenerics(dgmReceiver)) {
+                    methods.removeIf(method -> // GROOVY-10075: "List<Integer>" vs "List<String>"
+                        !typeCheckMethodsWithGenerics(dgmReceiver, ClassNode.EMPTY_ARRAY, method)
+                    );
+                }
                 if (!methods.isEmpty()) {
                     List<MethodNode> bestMethods = chooseBestMethod(dgmReceiver, methods, ClassNode.EMPTY_ARRAY);
                     if (bestMethods.size() == 1) {
diff --git a/src/test/groovy/transform/stc/DefaultGroovyMethodsSTCTest.groovy b/src/test/groovy/transform/stc/DefaultGroovyMethodsSTCTest.groovy
index cf4c8f6..23e3aef 100644
--- a/src/test/groovy/transform/stc/DefaultGroovyMethodsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/DefaultGroovyMethodsSTCTest.groovy
@@ -24,37 +24,37 @@ package groovy.transform.stc
 class DefaultGroovyMethodsSTCTest extends StaticTypeCheckingTestCase {
 
     void testEach() {
-        assertScript """
+        assertScript '''
             ['a','b'].each { // DGM#each(Object, Closure)
                 println it // DGM#println(Object,Object)
             }
-        """
+        '''
 
-        assertScript """
+        assertScript '''
             ['a','b'].eachWithIndex { it, i ->// DGM#eachWithIndex(Object, Closure)
                 println it // DGM#println(Object,Object)
             }
-        """
+        '''
     }
 
     void testStringToInteger() {
-        assertScript """
-        String name = "123"
-        name.toInteger() // toInteger() is defined by DGM
-        """
+        assertScript '''
+            String name = "123"
+            name.toInteger() // toInteger() is defined by DGM
+        '''
     }
 
     void testVariousAssignmentsThenToInteger() {
-        assertScript """
-         class A {
-          void foo() {}
-         }
-        def name = new A()
-        name.foo()
-        name = 1
-        name = '123'
-        name.toInteger() // toInteger() is defined by DGM
-        """
+        assertScript '''
+            class A {
+                void foo() {}
+            }
+            def name = new A()
+            name.foo()
+            name = 1
+            name = '123'
+            name.toInteger() // toInteger() is defined by DGM
+        '''
     }
 
     void testMethodsOnPrimitiveTypes() {
@@ -76,14 +76,48 @@ class DefaultGroovyMethodsSTCTest extends StaticTypeCheckingTestCase {
     }
 
     // GROOVY-5568
-    void testDGMMethodAsProperty() {
+    void testPropertySemantics1() {
         assertScript '''
-            String foo(InputStream input) {
-                input.text
+            String test(InputStream input) {
+                input.text // IOGroovyMethods#getText(InputStream)
             }
-            def text = new ByteArrayInputStream('foo'.getBytes())
-            assert foo(text) == 'foo'
+            assert test(new ByteArrayInputStream('foo'.bytes)) == 'foo'
         '''
+
+        assertScript '''
+            def chars = new StringBuilder('foo').chars // StringGroovyMethods#getChars(CharSequence)
+            assert chars == new char[] {'f','o','o'}
+        '''
+
+        assertScript '''
+            def a = Character.valueOf((char) 'a')
+            assert a.letter // DefaultGroovyMethods#isLetter(Character)
+        '''
+    }
+
+    // GROOVY-10075
+    void testPropertySemantics2() {
+        // see org.codehaus.groovy.runtime.m12n.TestStringExtension
+
+        assertScript '''
+            List<String> strings = ['x','y','z']
+            assert strings.getSequence() == 'x'
+            assert strings.getString() == 'x'
+          //assert strings.sequence == 'x'
+          //assert strings.string == 'x'
+        '''
+
+        shouldFailWithMessages '''
+            List<Number> numbers = [(Number)1, 2, 3]
+            numbers.getSequence()
+            numbers.getString()
+            numbers.sequence
+            numbers.string
+        ''',
+        'Cannot call <CS extends java.lang.CharSequence> java.util.List <java.lang.Number>#getSequence() with arguments []',
+        'Cannot call java.util.List <java.lang.Number>#getString() with arguments []',
+        'No such property: sequence for class: java.util.List <java.lang.Number>',
+        'No such property: string for class: java.util.List <java.lang.Number>'
     }
 
     // GROOVY-5584
diff --git a/src/test/org/codehaus/groovy/runtime/m12n/TestStringExtension.java b/src/test/org/codehaus/groovy/runtime/m12n/TestStringExtension.java
index fdd5e9f..fbbe8c2 100644
--- a/src/test/org/codehaus/groovy/runtime/m12n/TestStringExtension.java
+++ b/src/test/org/codehaus/groovy/runtime/m12n/TestStringExtension.java
@@ -19,8 +19,16 @@
 package org.codehaus.groovy.runtime.m12n;
 
 public class TestStringExtension {
+
     public static String reverseToUpperCase(String self) {
-        StringBuilder sb = new StringBuilder(self.toUpperCase());
-        return sb.reverse().toString();
+        return new StringBuilder(self.toUpperCase()).reverse().toString();
+    }
+
+    public static String getString(java.util.List<String> self) {
+        return self.get(0);
+    }
+
+    public static <CS extends CharSequence> CharSequence getSequence(java.util.List<CS> self) {
+        return self.get(0);
     }
 }