You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/06/24 00:25:07 UTC

[groovy] branch GROOVY_3_0_X updated (bb09ad4 -> fc5de1c)

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

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


    from bb09ad4  Trivial refactoring: extract constants
     new 5944dae  GROOVY-8274: allow user-supplied methodMissing in non-static inner class
     new 1cde739  GROOVY-9604: STC: add support for accessing additional closure metadata
     new fc5de1c  GROOVY-9598: account for static modifier when checking for outer members

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:
 .../classgen/InnerClassCompletionVisitor.java      | 357 ++++++++-------------
 .../groovy/classgen/VariableScopeVisitor.java      |  31 +-
 .../transform/stc/StaticTypeCheckingVisitor.java   |  78 ++---
 src/test/gls/innerClass/InnerClassTest.groovy      |  62 ++++
 src/test/groovy/transform/stc/BugsSTCTest.groovy   | 113 ++++---
 .../groovy/transform/stc/DelegatesToSTCTest.groovy |  34 +-
 .../stc/FieldsAndPropertiesSTCTest.groovy          |  47 +++
 7 files changed, 386 insertions(+), 336 deletions(-)


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

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 1cde73987941874e469660d315186b728e979a43
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
    
    (cherry picked from commit f60285ec072821502f33d7a999b5574bee53b6e2)
---
 .../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 24adb19..e9dc5a2 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -546,34 +546,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;
 
@@ -3392,18 +3401,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()) {
@@ -3436,11 +3438,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 6a89299..da59f91 100644
--- a/src/test/groovy/transform/stc/DelegatesToSTCTest.groovy
+++ b/src/test/groovy/transform/stc/DelegatesToSTCTest.groovy
@@ -442,42 +442,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
         '''
     }
 


[groovy] 01/03: GROOVY-8274: allow user-supplied methodMissing in non-static inner class

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 5944dae2609598446541111323a3e85c12ca0a16
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Jun 21 22:31:29 2020 -0500

    GROOVY-8274: allow user-supplied methodMissing in non-static inner class
    
    (cherry picked from commit bcce7bee5e69fa3ae945bfec0283e8fdb03a5cda)
---
 .../classgen/InnerClassCompletionVisitor.java      | 357 ++++++++-------------
 src/test/gls/innerClass/InnerClassTest.groovy      |  24 ++
 2 files changed, 164 insertions(+), 217 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/InnerClassCompletionVisitor.java b/src/main/java/org/codehaus/groovy/classgen/InnerClassCompletionVisitor.java
index 077d84a..231dcae 100644
--- a/src/main/java/org/codehaus/groovy/classgen/InnerClassCompletionVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/InnerClassCompletionVisitor.java
@@ -18,14 +18,12 @@
  */
 package org.codehaus.groovy.classgen;
 
-import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.ConstructorNode;
 import org.codehaus.groovy.ast.FieldNode;
 import org.codehaus.groovy.ast.InnerClassNode;
 import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.ast.Parameter;
-import org.codehaus.groovy.ast.expr.ClassExpression;
 import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
 import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.ast.expr.TupleExpression;
@@ -38,21 +36,28 @@ import org.objectweb.asm.MethodVisitor;
 import org.objectweb.asm.Opcodes;
 
 import java.util.List;
+import java.util.function.BiConsumer;
 
 import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedConstructor;
-import static org.apache.groovy.ast.tools.MethodNodeUtils.getCodeAsBlock;
 import static org.apache.groovy.ast.tools.ConstructorNodeUtils.getFirstIfSpecialConstructorCall;
+import static org.apache.groovy.ast.tools.MethodNodeUtils.getCodeAsBlock;
 import static org.codehaus.groovy.ast.ClassHelper.CLOSURE_TYPE;
+import static org.codehaus.groovy.ast.ClassHelper.OBJECT_TYPE;
+import static org.codehaus.groovy.ast.ClassHelper.STRING_TYPE;
+import static org.codehaus.groovy.ast.ClassHelper.VOID_TYPE;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.block;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorSuperX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.param;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.params;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
 
 public class InnerClassCompletionVisitor extends InnerClassVisitorHelper implements Opcodes {
 
-    private final SourceUnit sourceUnit;
     private ClassNode classNode;
-    private FieldNode thisField = null;
+    private FieldNode thisField;
+    private final SourceUnit sourceUnit;
 
     private static final String
             CLOSURE_INTERNAL_NAME   = BytecodeHelper.getClassInternalName(CLOSURE_TYPE),
@@ -69,7 +74,7 @@ public class InnerClassCompletionVisitor extends InnerClassVisitorHelper impleme
 
     @Override
     public void visitClass(ClassNode node) {
-        this.classNode = node;
+        classNode = node;
         thisField = null;
         InnerClassNode innerClass = null;
         if (!node.isEnum() && !node.isInterface() && node instanceof InnerClassNode) {
@@ -85,7 +90,7 @@ public class InnerClassCompletionVisitor extends InnerClassVisitorHelper impleme
         if (node.getInnerClasses().hasNext()) addDispatcherMethods(node);
         if (innerClass == null) return;
         super.visitClass(node);
-        addDefaultMethods(innerClass);
+        addMopMethods(innerClass);
     }
 
     @Override
@@ -109,57 +114,40 @@ public class InnerClassCompletionVisitor extends InnerClassVisitorHelper impleme
         // add the dispatcher methods
 
         // add method dispatcher
-        Parameter[] parameters = new Parameter[]{
-                new Parameter(ClassHelper.STRING_TYPE, "name"),
-                new Parameter(ClassHelper.OBJECT_TYPE, "args")
-        };
+        BlockStatement block = new BlockStatement();
         MethodNode method = classNode.addSyntheticMethod(
                 "this$dist$invoke$" + objectDistance,
-                ACC_PUBLIC + ACC_SYNTHETIC,
-                ClassHelper.OBJECT_TYPE,
-                parameters,
+                ACC_PUBLIC,
+                OBJECT_TYPE,
+                params(param(STRING_TYPE, "name"), param(OBJECT_TYPE, "args")),
                 ClassNode.EMPTY_ARRAY,
-                null
+                block
         );
-
-        BlockStatement block = new BlockStatement();
-        setMethodDispatcherCode(block, VariableExpression.THIS_EXPRESSION, parameters);
-        method.setCode(block);
+        setMethodDispatcherCode(block, VariableExpression.THIS_EXPRESSION, method.getParameters());
 
         // add property setter
-        parameters = new Parameter[]{
-                new Parameter(ClassHelper.STRING_TYPE, "name"),
-                new Parameter(ClassHelper.OBJECT_TYPE, "value")
-        };
+        block = new BlockStatement();
         method = classNode.addSyntheticMethod(
                 "this$dist$set$" + objectDistance,
-                ACC_PUBLIC + ACC_SYNTHETIC,
-                ClassHelper.VOID_TYPE,
-                parameters,
+                ACC_PUBLIC,
+                VOID_TYPE,
+                params(param(STRING_TYPE, "name"), param(OBJECT_TYPE, "value")),
                 ClassNode.EMPTY_ARRAY,
-                null
+                block
         );
-
-        block = new BlockStatement();
-        setPropertySetterDispatcher(block, VariableExpression.THIS_EXPRESSION, parameters);
-        method.setCode(block);
+        setPropertySetterDispatcher(block, VariableExpression.THIS_EXPRESSION, method.getParameters());
 
         // add property getter
-        parameters = new Parameter[]{
-                new Parameter(ClassHelper.STRING_TYPE, "name")
-        };
+        block = new BlockStatement();
         method = classNode.addSyntheticMethod(
                 "this$dist$get$" + objectDistance,
-                ACC_PUBLIC + ACC_SYNTHETIC,
-                ClassHelper.OBJECT_TYPE,
-                parameters,
+                ACC_PUBLIC,
+                OBJECT_TYPE,
+                params(param(STRING_TYPE, "name")),
                 ClassNode.EMPTY_ARRAY,
-                null
+                block
         );
-
-        block = new BlockStatement();
-        setPropertyGetterDispatcher(block, VariableExpression.THIS_EXPRESSION, parameters);
-        method.setCode(block);
+        setPropertyGetterDispatcher(block, VariableExpression.THIS_EXPRESSION, method.getParameters());
     }
 
     private void getThis(MethodVisitor mv, String classInternalName, String outerClassDescriptor, String innerClassInternalName) {
@@ -173,201 +161,137 @@ public class InnerClassCompletionVisitor extends InnerClassVisitorHelper impleme
         }
     }
 
-    private void addDefaultMethods(InnerClassNode node) {
+    private void addMopMethods(final InnerClassNode node) {
         final boolean isStatic = isStatic(node);
-
-        ClassNode outerClass = node.getOuterClass();
+        final ClassNode outerClass = node.getOuterClass();
+        final int outerClassDistance = getObjectDistance(outerClass);
         final String classInternalName = BytecodeHelper.getClassInternalName(node);
         final String outerClassInternalName = getInternalName(outerClass, isStatic);
         final String outerClassDescriptor = getTypeDescriptor(outerClass, isStatic);
-        final int objectDistance = getObjectDistance(outerClass);
-
-        // add missing method dispatcher
-        Parameter[] parameters = new Parameter[]{
-                new Parameter(ClassHelper.STRING_TYPE, "name"),
-                new Parameter(ClassHelper.OBJECT_TYPE, "args")
-        };
-
-        String methodName = "methodMissing";
-        if (isStatic)
-            addCompilationErrorOnCustomMethodNode(node, methodName, parameters);
-
-        MethodNode method = node.addSyntheticMethod(
-                methodName,
-                Opcodes.ACC_PUBLIC,
-                ClassHelper.OBJECT_TYPE,
-                parameters,
-                ClassNode.EMPTY_ARRAY,
-                null
-        );
 
-        BlockStatement block = new BlockStatement();
-        if (isStatic) {
-            setMethodDispatcherCode(block, new ClassExpression(outerClass), parameters);
-        } else {
-            block.addStatement(
-                    new BytecodeSequence(new BytecodeInstruction() {
-                        public void visit(MethodVisitor mv) {
-                            getThis(mv,classInternalName,outerClassDescriptor,outerClassInternalName);
-                            mv.visitVarInsn(ALOAD, 1);
-                            mv.visitVarInsn(ALOAD, 2);
-                            mv.visitMethodInsn(INVOKEVIRTUAL, outerClassInternalName, "this$dist$invoke$" + objectDistance, "(Ljava/lang/String;Ljava/lang/Object;)Ljava/lang/Object;", false);
-                            mv.visitInsn(ARETURN);
-                        }
-                    })
-            );
-        }
-        method.setCode(block);
-
-        // add static missing method dispatcher
-        methodName = "$static_methodMissing";
-        if (isStatic)
-            addCompilationErrorOnCustomMethodNode(node, methodName, parameters);
-
-        method = node.addSyntheticMethod(
-                methodName,
-                Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC,
-                ClassHelper.OBJECT_TYPE,
-                parameters,
-                ClassNode.EMPTY_ARRAY,
-                null
+        addSyntheticMethod(node,
+                "methodMissing",
+                ACC_PUBLIC,
+                OBJECT_TYPE,
+                params(param(STRING_TYPE, "name"), param(OBJECT_TYPE, "args")),
+                (methodBody, parameters) -> {
+                    if (isStatic) {
+                        setMethodDispatcherCode(methodBody, classX(outerClass), parameters);
+                    } else {
+                        methodBody.addStatement(
+                                new BytecodeSequence(new BytecodeInstruction() {
+                                    @Override
+                                    public void visit(final MethodVisitor mv) {
+                                        getThis(mv, classInternalName, outerClassDescriptor, outerClassInternalName);
+                                        mv.visitVarInsn(ALOAD, 1);
+                                        mv.visitVarInsn(ALOAD, 2);
+                                        mv.visitMethodInsn(INVOKEVIRTUAL, outerClassInternalName, "this$dist$invoke$" + outerClassDistance, "(Ljava/lang/String;Ljava/lang/Object;)Ljava/lang/Object;", false);
+                                        mv.visitInsn(ARETURN);
+                                    }
+                                })
+                        );
+                    }
+                }
         );
 
-        block = new BlockStatement();
-        setMethodDispatcherCode(block, new ClassExpression(outerClass), parameters);
-        method.setCode(block);
-
-        // add property setter dispatcher
-        parameters = new Parameter[]{
-                new Parameter(ClassHelper.STRING_TYPE, "name"),
-                new Parameter(ClassHelper.OBJECT_TYPE, "val")
-        };
-
-        methodName = "propertyMissing";
-        if (isStatic)
-            addCompilationErrorOnCustomMethodNode(node, methodName, parameters);
-
-        method = node.addSyntheticMethod(
-                methodName,
-                Opcodes.ACC_PUBLIC,
-                ClassHelper.VOID_TYPE,
-                parameters,
-                ClassNode.EMPTY_ARRAY,
-                null
+        addSyntheticMethod(node,
+                "$static_methodMissing",
+                ACC_PUBLIC | ACC_STATIC,
+                OBJECT_TYPE,
+                params(param(STRING_TYPE, "name"), param(OBJECT_TYPE, "args")),
+                (methodBody, parameters) -> {
+                    setMethodDispatcherCode(methodBody, classX(outerClass), parameters);
+                }
         );
 
-        block = new BlockStatement();
-        if (isStatic) {
-            setPropertySetterDispatcher(block, new ClassExpression(outerClass), parameters);
-        } else {
-            block.addStatement(
-                    new BytecodeSequence(new BytecodeInstruction() {
-                        public void visit(MethodVisitor mv) {
-                            getThis(mv,classInternalName,outerClassDescriptor,outerClassInternalName);
-                            mv.visitVarInsn(ALOAD, 1);
-                            mv.visitVarInsn(ALOAD, 2);
-                            mv.visitMethodInsn(INVOKEVIRTUAL, outerClassInternalName, "this$dist$set$" + objectDistance, "(Ljava/lang/String;Ljava/lang/Object;)V", false);
-                            mv.visitInsn(RETURN);
-                        }
-                    })
-            );
-        }
-        method.setCode(block);
-
-        // add static property missing setter dispatcher
-        methodName = "$static_propertyMissing";
-        if (isStatic)
-            addCompilationErrorOnCustomMethodNode(node, methodName, parameters);
-
-        method = node.addSyntheticMethod(
-                methodName,
-                Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC,
-                ClassHelper.VOID_TYPE,
-                parameters,
-                ClassNode.EMPTY_ARRAY,
-                null
+        addSyntheticMethod(node,
+                "propertyMissing",
+                ACC_PUBLIC,
+                VOID_TYPE,
+                params(param(STRING_TYPE, "name"), param(OBJECT_TYPE, "value")),
+                (methodBody, parameters) -> {
+                    if (isStatic) {
+                        setPropertySetterDispatcher(methodBody, classX(outerClass), parameters);
+                    } else {
+                        methodBody.addStatement(
+                                new BytecodeSequence(new BytecodeInstruction() {
+                                    @Override
+                                    public void visit(final MethodVisitor mv) {
+                                        getThis(mv, classInternalName, outerClassDescriptor, outerClassInternalName);
+                                        mv.visitVarInsn(ALOAD, 1);
+                                        mv.visitVarInsn(ALOAD, 2);
+                                        mv.visitMethodInsn(INVOKEVIRTUAL, outerClassInternalName, "this$dist$set$" + outerClassDistance, "(Ljava/lang/String;Ljava/lang/Object;)V", false);
+                                        mv.visitInsn(RETURN);
+                                    }
+                                })
+                        );
+                    }
+                }
         );
 
-        block = new BlockStatement();
-        setPropertySetterDispatcher(block, new ClassExpression(outerClass), parameters);
-        method.setCode(block);
-
-        // add property getter dispatcher
-        parameters = new Parameter[]{
-                new Parameter(ClassHelper.STRING_TYPE, "name")
-        };
-
-        methodName = "propertyMissing";
-        if (isStatic)
-            addCompilationErrorOnCustomMethodNode(node, methodName, parameters);
-
-        method = node.addSyntheticMethod(
-                methodName,
-                Opcodes.ACC_PUBLIC,
-                ClassHelper.OBJECT_TYPE,
-                parameters,
-                ClassNode.EMPTY_ARRAY,
-                null
+        addSyntheticMethod(node,
+                "$static_propertyMissing",
+                ACC_PUBLIC | ACC_STATIC,
+                VOID_TYPE,
+                params(param(STRING_TYPE, "name"), param(OBJECT_TYPE, "value")),
+                (methodBody, parameters) -> {
+                    setPropertySetterDispatcher(methodBody, classX(outerClass), parameters);
+                }
         );
 
-        block = new BlockStatement();
-        if (isStatic) {
-            setPropertyGetterDispatcher(block, new ClassExpression(outerClass), parameters);
-        } else {
-            block.addStatement(
-                    new BytecodeSequence(new BytecodeInstruction() {
-                        public void visit(MethodVisitor mv) {
-                            getThis(mv,classInternalName,outerClassDescriptor,outerClassInternalName);
-                            mv.visitVarInsn(ALOAD, 1);
-                            mv.visitMethodInsn(INVOKEVIRTUAL, outerClassInternalName, "this$dist$get$" + objectDistance, "(Ljava/lang/String;)Ljava/lang/Object;", false);
-                            mv.visitInsn(ARETURN);
-                        }
-                    })
-            );
-        }
-        method.setCode(block);
-
-        // add static property missing getter dispatcher
-        methodName = "$static_propertyMissing";
-        if (isStatic)
-            addCompilationErrorOnCustomMethodNode(node, methodName, parameters);
-
-        method = node.addSyntheticMethod(
-                methodName,
-                Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC,
-                ClassHelper.OBJECT_TYPE,
-                parameters,
-                ClassNode.EMPTY_ARRAY,
-                null
+        addSyntheticMethod(node,
+                "propertyMissing",
+                ACC_PUBLIC,
+                OBJECT_TYPE,
+                params(param(STRING_TYPE, "name")),
+                (methodBody, parameters) -> {
+                    if (isStatic) {
+                        setPropertyGetterDispatcher(methodBody, classX(outerClass), parameters);
+                    } else {
+                        methodBody.addStatement(
+                                new BytecodeSequence(new BytecodeInstruction() {
+                                    @Override
+                                    public void visit(final MethodVisitor mv) {
+                                        getThis(mv, classInternalName, outerClassDescriptor, outerClassInternalName);
+                                        mv.visitVarInsn(ALOAD, 1);
+                                        mv.visitMethodInsn(INVOKEVIRTUAL, outerClassInternalName, "this$dist$get$" + outerClassDistance, "(Ljava/lang/String;)Ljava/lang/Object;", false);
+                                        mv.visitInsn(ARETURN);
+                                    }
+                                })
+                        );
+                    }
+                }
         );
 
-        block = new BlockStatement();
-        setPropertyGetterDispatcher(block, new ClassExpression(outerClass), parameters);
-        method.setCode(block);
+        addSyntheticMethod(node,
+                "$static_propertyMissing",
+                ACC_PUBLIC | ACC_STATIC,
+                OBJECT_TYPE,
+                params(param(STRING_TYPE, "name")),
+                (methodBody, parameters) -> {
+                    setPropertyGetterDispatcher(methodBody, classX(outerClass), parameters);
+                }
+        );
     }
 
-    /**
-     * Adds a compilation error if a {@link MethodNode} with the given <tt>methodName</tt> and
-     * <tt>parameters</tt> exists in the {@link InnerClassNode}.
-     */
-    private void addCompilationErrorOnCustomMethodNode(InnerClassNode node, String methodName, Parameter[] parameters) {
-        MethodNode existingMethodNode = node.getMethod(methodName, parameters);
-        // if there is a user-defined methodNode, add compiler error msg and continue
-        if (existingMethodNode != null && !isSynthetic(existingMethodNode))  {
-            addError("\"" +methodName + "\" implementations are not supported on static inner classes as " +
+    private void addSyntheticMethod(final InnerClassNode node, final String methodName, final int modifiers,
+            final ClassNode returnType, final Parameter[] parameters, final BiConsumer<BlockStatement, Parameter[]> consumer) {
+        MethodNode method = node.getMethod(methodName, parameters);
+        if (method != null) {
+            // GROOVY-8914: pre-compiled classes lose synthetic boolean - TODO fix earlier as per GROOVY-4346 then remove extra check here
+            if (isStatic(node) && !method.isSynthetic() && (method.getModifiers() & ACC_SYNTHETIC) == 0) {
+                // if there is a user-defined methodNode, add compiler error and continue
+                addError("\"" + methodName + "\" implementations are not supported on static inner classes as " +
                     "a synthetic version of \"" + methodName + "\" is added during compilation for the purpose " +
                     "of outer class delegation.",
-                    existingMethodNode);
+                    method);
+            }
+            return;
         }
-    }
 
-    // GROOVY-8914: pre-compiled classes lose synthetic boolean - TODO fix earlier as per GROOVY-4346 then remove extra check here
-    private boolean isSynthetic(MethodNode existingMethodNode) {
-        return existingMethodNode.isSynthetic() || hasSyntheticModifier(existingMethodNode);
-    }
-
-    private boolean hasSyntheticModifier(MethodNode existingMethodNode) {
-        return (existingMethodNode.getModifiers() & Opcodes.ACC_SYNTHETIC) != 0;
+        BlockStatement methodBody = block();
+        consumer.accept(methodBody, parameters);
+        node.addSyntheticMethod(methodName, modifiers, returnType, parameters, ClassNode.EMPTY_ARRAY, methodBody);
     }
 
     private void addThisReference(ConstructorNode node) {
@@ -440,5 +364,4 @@ public class InnerClassCompletionVisitor extends InnerClassVisitorHelper impleme
         addError("unable to find a unique prefix name for synthetic this reference in inner class constructor", node);
         return namePrefix;
     }
-
 }
diff --git a/src/test/gls/innerClass/InnerClassTest.groovy b/src/test/gls/innerClass/InnerClassTest.groovy
index a9ba77a..e82496d 100644
--- a/src/test/gls/innerClass/InnerClassTest.groovy
+++ b/src/test/gls/innerClass/InnerClassTest.groovy
@@ -1207,6 +1207,30 @@ final class InnerClassTest {
         '''
     }
 
+    @Test // GROOVY-8274
+    void testMissingMethodHandling() {
+        assertScript '''
+            class A {
+                class B {
+                    def methodMissing(String name, args) {
+                        return name
+                    }
+                }
+
+                def test(Closure c) {
+                    c.resolveStrategy = Closure.DELEGATE_ONLY
+                    c.delegate = new B()
+                    c.call()
+                }
+            }
+
+            def x = new A().test { ->
+                hello() // missing
+            }
+            assert x == 'hello'
+        '''
+    }
+
     @Test // GROOVY-6831
     void testNestedPropertyHandling() {
         assertScript '''


[groovy] 03/03: GROOVY-9598: account for static modifier when checking for outer members

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit fc5de1cf3ce6fc703b81ecee76357ceea01091bb
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Jun 22 19:56:48 2020 -0500

    GROOVY-9598: account for static modifier when checking for outer members
    
    (cherry picked from commit a960547e9a560d5d7216912c5fe2e1ea7116d20e)
---
 .../groovy/classgen/VariableScopeVisitor.java      | 31 +++++++-------
 src/test/gls/innerClass/InnerClassTest.groovy      | 38 +++++++++++++++++
 .../stc/FieldsAndPropertiesSTCTest.groovy          | 47 ++++++++++++++++++++++
 3 files changed, 100 insertions(+), 16 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
index 12edb39..ef22321 100644
--- a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
@@ -56,6 +56,7 @@ import java.util.LinkedList;
 import java.util.function.BiConsumer;
 
 import static java.lang.reflect.Modifier.isFinal;
+import static java.lang.reflect.Modifier.isStatic;
 import static org.apache.groovy.ast.tools.MethodNodeUtils.getPropertyName;
 
 /**
@@ -191,10 +192,12 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
             if (pn.getName().equals(name)) return pn;
         }
 
-        Variable ret = findClassMember(cn.getSuperClass(), name);
-        if (ret != null) return ret;
-        if (isAnonymous(cn)) return null;
-        return findClassMember(cn.getOuterClass(), name);
+        for (ClassNode face : cn.getInterfaces()) {
+            FieldNode fn = face.getDeclaredField(name);
+            if (fn != null) return fn;
+        }
+
+        return findClassMember(cn.getSuperClass(), name);
     }
 
     private Variable findVariableDeclaration(final String name) {
@@ -225,9 +228,13 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
                 break;
             }
 
-            ClassNode classScope = scope.getClassScope();
-            if (classScope != null) {
-                Variable member = findClassMember(classScope, name);
+            ClassNode node = scope.getClassScope();
+            if (node != null) {
+                Variable member = findClassMember(node, name);
+                while (member == null && node.getOuterClass() != null && !isAnonymous(node)) {
+                    crossingStaticContext = (crossingStaticContext || isStatic(node.getModifiers()));
+                    member = findClassMember((node = node.getOuterClass()), name);
+                }
                 if (member != null) {
                     boolean staticScope = (crossingStaticContext || inSpecialConstructorCall), staticMember = member.isInStaticContext();
                     // prevent a static context (e.g. a static method) from accessing a non-static variable (e.g. a non-static field)
@@ -236,7 +243,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
                     }
                 }
                 // GROOVY-5961
-                if (!isAnonymous(classScope)) break;
+                if (!isAnonymous(scope.getClassScope())) break;
             }
             scope = scope.getParent();
         }
@@ -460,14 +467,6 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
         popState();
     }
 
-    /**
-     * Sets the current class node context.
-     */
-    public void prepareVisit(ClassNode node) {
-        currentClass = node;
-        currentScope.setClassScope(node);
-    }
-
     @Override
     public void visitConstructorCallExpression(final ConstructorCallExpression expression) {
         boolean oldInSpecialCtorFlag = inSpecialConstructorCall;
diff --git a/src/test/gls/innerClass/InnerClassTest.groovy b/src/test/gls/innerClass/InnerClassTest.groovy
index e82496d..3f4393e 100644
--- a/src/test/gls/innerClass/InnerClassTest.groovy
+++ b/src/test/gls/innerClass/InnerClassTest.groovy
@@ -600,6 +600,26 @@ final class InnerClassTest {
     }
 
     @Test
+    void testUsageOfOuterField12() {
+        def err = shouldFail '''
+            class C {
+                int count
+                static def m() {
+                    new LinkedList() {
+                        def get(int i) {
+                            count += 1
+                            super.get(i)
+                        }
+                    }
+                }
+            }
+            C.m()
+        '''
+
+        assert err =~ /Apparent variable 'count' was found in a static scope but doesn't refer to a local variable, static field or class./
+    }
+
+    @Test
     void testUsageOfOuterSuperField() {
         assertScript '''
             class InnerBase {
@@ -635,6 +655,24 @@ final class InnerClassTest {
     }
 
     @Test
+    void testUsageOfOuterSuperField2() {
+        assertScript '''
+            interface I {
+                String CONST = 'value'
+            }
+            class A implements I {
+                static class B {
+                    def test() {
+                        CONST
+                    }
+                }
+            }
+            def x = new A.B().test()
+            assert x == 'value'
+        '''
+    }
+
+    @Test
     void testUsageOfOuterField_WrongCallToSuper() {
         shouldFail '''
             class Outer {
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index 545cde5..3b39eff 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -387,6 +387,53 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
         '''
     }
 
+    void testOuterPropertyAccess1() {
+        assertScript '''
+            class Outer {
+                class Inner {
+                    def m() {
+                        p
+                    }
+                }
+                def p = 1
+            }
+            def i = new Outer.Inner(new Outer())
+            def x = i.m()
+            assert x == 1
+        '''
+    }
+
+    // GROOVY-9598
+    void testOuterPropertyAccess2() {
+        shouldFailWithMessages '''
+            class Outer {
+                static class Inner {
+                    def m() {
+                        p
+                    }
+                }
+                def p = 1
+            }
+            def i = new Outer.Inner()
+            def x = i.m()
+        ''', "Apparent variable 'p' was found in a static scope but doesn't refer to a local variable, static field or class."
+    }
+
+    void testOuterPropertyAccess3() {
+        shouldFailWithMessages '''
+            class Outer {
+                static class Inner {
+                    def m() {
+                        this.p
+                    }
+                }
+                def p = 1
+            }
+            def i = new Outer.Inner()
+            def x = i.m()
+        ''', 'No such property: p for class: Outer$Inner'
+    }
+
     void testPrivateFieldAccessInClosure() {
         assertScript '''
             class A {