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 2020/06/21 16:06:28 UTC

[groovy] 01/01: GROOVY-9604: STC: add support for accessing additional closure metadata

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

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

commit f460f369ad556408b007d49068f8d8dd5aa881ae
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Jun 21 10:59:00 2020 -0500

    GROOVY-9604: STC: add support for accessing additional closure metadata
---
 .../transform/stc/StaticTypeCheckingVisitor.java   |  78 +++++++-------
 src/test/groovy/transform/stc/BugsSTCTest.groovy   | 113 ++++++++++++---------
 .../groovy/transform/stc/DelegatesToSTCTest.groovy |  34 +++----
 3 files changed, 122 insertions(+), 103 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 397594e..4581c40 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -547,34 +547,43 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         super.visitVariableExpression(vexp);
         if (storeTypeForSuper(vexp)) return;
         if (storeTypeForThis(vexp)) return;
-        String name = vexp.getName();
 
-        TypeCheckingContext.EnclosingClosure enclosingClosure = typeCheckingContext.getEnclosingClosure();
-        if (enclosingClosure != null) {
-            switch (name) {
-                case "delegate":
-                    DelegationMetadata dm = getDelegationMetadata(enclosingClosure.getClosureExpression());
-                    if (dm != null) {
-                        storeType(vexp, dm.getType());
-                        return;
-                    }
-                    // falls through
-                case "owner":
-                    if (typeCheckingContext.getEnclosingClosureStack().size() > 1) {
-                        storeType(vexp, CLOSURE_TYPE);
-                        return;
-                    }
-                    // falls through
-                case "thisObject":
-                    storeType(vexp, typeCheckingContext.getEnclosingClassNode());
-                    return;
-            }
-        }
+        final String name = vexp.getName();
+        final Variable accessedVariable = vexp.getAccessedVariable();
+        final TypeCheckingContext.EnclosingClosure enclosingClosure = typeCheckingContext.getEnclosingClosure();
 
-        Variable accessedVariable = vexp.getAccessedVariable();
         VariableExpression localVariableExpression = null;
         if (accessedVariable instanceof DynamicVariable) {
-            // a dynamic variable is either a class member used in a 'with' or an undeclared variable
+            // a dynamic variable is either a closure property, a class member referenced from a closure, or an undeclared variable
+
+            if (enclosingClosure != null) {
+                switch (name) {
+                    case "delegate":
+                        DelegationMetadata dm = getDelegationMetadata(enclosingClosure.getClosureExpression());
+                        if (dm != null) {
+                            storeType(vexp, dm.getType());
+                            return;
+                        }
+                        // falls through
+                    case "owner":
+                        if (typeCheckingContext.getEnclosingClosureStack().size() > 1) {
+                            storeType(vexp, CLOSURE_TYPE);
+                            return;
+                        }
+                        // falls through
+                    case "thisObject":
+                        storeType(vexp, typeCheckingContext.getEnclosingClassNode());
+                        return;
+                    case "parameterTypes":
+                        storeType(vexp, CLASS_Type.makeArray());
+                        return;
+                    case "maximumNumberOfParameters":
+                    case "resolveStrategy":
+                    case "directive":
+                        storeType(vexp, int_TYPE);
+                        return;
+                }
+            }
 
             if (tryVariableExpressionAsProperty(vexp, name)) return;
 
@@ -3393,18 +3402,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                         break;
                     }
                 }
-                if (mn.isEmpty() && typeCheckingContext.getEnclosingClosure() != null && args.length == 0) {
-                    // add special handling of "delegate", "owner", and "this" in a closure
-                    switch (name) {
-                        case "getDelegate":
-                            mn = Collections.singletonList(GET_DELEGATE);
-                            break;
-                        case "getOwner":
-                            mn = Collections.singletonList(GET_OWNER);
-                            break;
-                        case "getThisObject":
-                            mn = Collections.singletonList(GET_THISOBJECT);
-                            break;
+                if (mn.isEmpty() && call.isImplicitThis() && isThisExpression(objectExpression) && typeCheckingContext.getEnclosingClosure() != null) {
+                    mn = CLOSURE_TYPE.getDeclaredMethods(name);
+                    if (!mn.isEmpty()) {
+                        chosenReceiver = Receiver.make(CLOSURE_TYPE);
+                        objectExpression.removeNodeMetaData(INFERRED_TYPE);
                     }
                 }
                 if (mn.isEmpty()) {
@@ -3437,11 +3439,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                             returnType = (irtg != null && implementsInterfaceOrIsSubclassOf(irtg, returnType) ? irtg : returnType);
                             callArgsVisited = true;
                         }
+                        // GROOVY-6091: use of "delegate" or "getDelegate()" does not make use of @DelegatesTo metadata
                         if (directMethodCallCandidate == GET_DELEGATE && typeCheckingContext.getEnclosingClosure() != null) {
                             DelegationMetadata md = getDelegationMetadata(typeCheckingContext.getEnclosingClosure().getClosureExpression());
-                            returnType = typeCheckingContext.getEnclosingClassNode();
                             if (md != null) {
                                 returnType = md.getType();
+                            } else {
+                                returnType = typeCheckingContext.getEnclosingClassNode();
                             }
                         }
                         if (typeCheckMethodsWithGenericsOrFail(chosenReceiver.getType(), args, mn.get(0), call)) {
diff --git a/src/test/groovy/transform/stc/BugsSTCTest.groovy b/src/test/groovy/transform/stc/BugsSTCTest.groovy
index 28b1d77..a0f21f9 100644
--- a/src/test/groovy/transform/stc/BugsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/BugsSTCTest.groovy
@@ -190,70 +190,87 @@ class BugsSTCTest extends StaticTypeCheckingTestCase {
         new StaticGroovy2()'''
     }
 
-    void testClosureDelegateThisOwner() {
+    void testClosureThisObjectDelegateOwnerProperty() {
         assertScript '''
-            class A {
-                A that = this
+            class C {
                 void m() {
-                    def cl = {
-                        @ASTTest(phase=INSTRUCTION_SELECTION, value= {
-                            assert node.getNodeMetaData(INFERRED_TYPE)?.name == 'A'
+                    C that = this;
+
+                    { ->
+                        @ASTTest(phase=INSTRUCTION_SELECTION, value={
+                            assert node.getNodeMetaData(INFERRED_TYPE)?.name == 'C'
                         })
-                        def foo = this
-                        assert this == that
-                    }
-                    cl()
-                    cl = {
-                        @ASTTest(phase=INSTRUCTION_SELECTION, value= {
-                            assert node.getNodeMetaData(INFERRED_TYPE)?.name == 'A'
+                        def ref = thisObject
+                        assert ref == that
+                    }();
+
+                    { ->
+                        @ASTTest(phase=INSTRUCTION_SELECTION, value={
+                            assert node.getNodeMetaData(INFERRED_TYPE)?.name == 'C'
                         })
-                        def foo = delegate
-                        assert delegate == that
-                    }
-                    cl()
-                    cl = {
-                        @ASTTest(phase=INSTRUCTION_SELECTION, value= {
-                            assert node.getNodeMetaData(INFERRED_TYPE)?.name == 'A'
+                        def ref = delegate
+                        assert ref == that
+                    }();
+
+                    { ->
+                        @ASTTest(phase=INSTRUCTION_SELECTION, value={
+                            assert node.getNodeMetaData(INFERRED_TYPE)?.name == 'C'
                         })
-                        def foo = owner
-                        assert owner == that
-                    }
+                        def ref = owner
+                        assert ref == that
+                    }();
                 }
             }
-            new A().m()
+            new C().m()
         '''
     }
-    void testClosureDelegateThisOwnerUsingGetters() {
+
+    void testClosureThisObjectDelegateOwnerAccessor() {
         assertScript '''
-            class A {
-                A that = this
+            class C {
                 void m() {
-                    def cl = {
-                        @ASTTest(phase=INSTRUCTION_SELECTION, value= {
-                            assert node.getNodeMetaData(INFERRED_TYPE)?.name == 'A'
+                    C that = this;
+
+                    { ->
+                        @ASTTest(phase=INSTRUCTION_SELECTION, value={
+                            assert node.getNodeMetaData(INFERRED_TYPE)?.name == 'C'
                         })
-                        def foo = getThisObject()
-                        assert getThisObject() == that
-                    }
-                    cl()
-                    cl = {
-                        @ASTTest(phase=INSTRUCTION_SELECTION, value= {
-                            assert node.getNodeMetaData(INFERRED_TYPE)?.name == 'A'
+                        def ref = getThisObject()
+                        assert ref == that
+                    }();
+
+                    { ->
+                        @ASTTest(phase=INSTRUCTION_SELECTION, value={
+                            assert node.getNodeMetaData(INFERRED_TYPE)?.name == 'C'
                         })
-                        def foo = getDelegate()
-                        assert getDelegate() == that
-                    }
-                    cl()
-                    cl = {
-                        @ASTTest(phase=INSTRUCTION_SELECTION, value= {
-                            assert node.getNodeMetaData(INFERRED_TYPE)?.name == 'A'
+                        def ref = getDelegate()
+                        assert ref == that
+                    }();
+
+                    { ->
+                        @ASTTest(phase=INSTRUCTION_SELECTION, value={
+                            assert node.getNodeMetaData(INFERRED_TYPE)?.name == 'C'
                         })
-                        def foo = getOwner()
-                        assert getOwner() == that
-                    }
+                        def ref = getOwner()
+                        assert ref == that
+                    }();
+                }
+            }
+            new C().m()
+        '''
+    }
+
+    // GROOVY-9604
+    void testClosureResolveStrategy() {
+        assertScript '''
+            class C {
+                def m() {
+                    return { ->
+                        resolveStrategy + getResolveStrategy()
+                    }();
                 }
             }
-            new A().m()
+            assert new C().m() == 0
         '''
     }
 
diff --git a/src/test/groovy/transform/stc/DelegatesToSTCTest.groovy b/src/test/groovy/transform/stc/DelegatesToSTCTest.groovy
index 5d11147..cb155f0 100644
--- a/src/test/groovy/transform/stc/DelegatesToSTCTest.groovy
+++ b/src/test/groovy/transform/stc/DelegatesToSTCTest.groovy
@@ -440,42 +440,40 @@ class DelegatesToSTCTest extends StaticTypeCheckingTestCase {
     // GROOVY-6091
     void testExplicitUseOfDelegateProperty() {
         assertScript '''
-            def with(@DelegatesTo.Target Object target, @DelegatesTo(strategy = Closure.DELEGATE_FIRST) Closure arg) {
-                arg.delegate = target
-                arg.setResolveStrategy(Closure.DELEGATE_FIRST)
-                arg()
+            def with(@DelegatesTo.Target Object target, @DelegatesTo(strategy = Closure.DELEGATE_FIRST) Closure block) {
+                block.resolveStrategy = Closure.DELEGATE_FIRST
+                block.delegate = target
+                block.call()
             }
 
             def test() {
-                def obj = [1, 2]
-                with(obj) {
-                    print(delegate.last()) //error is here
+                def list = [1, 2]
+                with(list) {
+                    delegate.last() // error is here
                 }
             }
 
-            test()
-
+            assert test() == 2
         '''
     }
 
     // GROOVY-6091
     void testExplicitUseOfDelegateMethod() {
         assertScript '''
-            def with(@DelegatesTo.Target Object target, @DelegatesTo(strategy = Closure.DELEGATE_FIRST) Closure arg) {
-                arg.delegate = target
-                arg.setResolveStrategy(Closure.DELEGATE_FIRST)
-                arg()
+            def with(@DelegatesTo.Target Object target, @DelegatesTo(strategy = Closure.DELEGATE_FIRST) Closure block) {
+                block.resolveStrategy = Closure.DELEGATE_FIRST
+                block.delegate = target
+                block.call()
             }
 
             def test() {
-                def obj = [1, 2]
-                with(obj) {
-                    print(getDelegate().last()) //error is here
+                def list = [1, 2]
+                with(list) { ->
+                    getDelegate().last() // error is here
                 }
             }
 
-            test()
-
+            assert test() == 2
         '''
     }