groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject [groovy] branch GROOVY_2_5_X updated: GROOVY-7996: check for get(String)/set(String, Object) or propertyMissing
Date Sat, 16 Nov 2019 13:34:55 GMT
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/GROOVY_2_5_X by this push:
     new cd0ca19  GROOVY-7996: check for get(String)/set(String,Object) or propertyMissing
cd0ca19 is described below

commit cd0ca19b4b99cd859bc28cf48944a8aa69d69fa4
Author: Eric Milles <eric.milles@thomsonreuters.com>
AuthorDate: Fri Nov 15 14:03:13 2019 -0600

    GROOVY-7996: check for get(String)/set(String,Object) or propertyMissing
    
    roll back initial changes
    
    (Groovy 2.5 backport)
---
 .../transform/stc/StaticTypeCheckingVisitor.java   |  50 +++---
 src/test/groovy/bugs/Groovy7996.groovy             | 187 +++++++++++++++++++++
 src/test/groovy/bugs/Groovy7996Bug.groovy          |  88 ----------
 3 files changed, 213 insertions(+), 112 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 1bfc997..eab3714 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -513,21 +513,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
         }
     }
 
-    /**
-     * Checks valid cases for accessing a field from an inner class.
-     */
-    private String checkOrMarkInnerPropertyOwnerAccess(PropertyExpression source, boolean
lhsOfAssignment, String delegationData) {
-        // check for reference to method, closure, for loop, try with, or catch block parameter
from a non-nested closure
-        if (typeCheckingContext.getEnclosingClosureStack().size() == 1 && !"this".equals(source.getPropertyAsString()))
{
-            if (!(source.getObjectExpression() instanceof VariableExpression &&
-                    ((VariableExpression) source.getObjectExpression()).getAccessedVariable()
instanceof Parameter)) {
-                delegationData = "owner";
-                source.getObjectExpression().putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER,
delegationData);
-            }
-        }
-        return delegationData;
-    }
-
     private MethodNode findValidGetter(ClassNode classNode, String name) {
         MethodNode getterMethod = classNode.getGetterMethod(name);
         if (getterMethod != null && (getterMethod.isPublic() || getterMethod.isProtected()))
{
@@ -1571,7 +1556,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
                     queue.add(current.getUnresolvedSuperClass());
                 }
             }
-            // GROOVY-5568, the property may be defined by DGM
+
+            // GROOVY-5568: the property may be defined by DGM
             List<ClassNode> dgmReceivers = new ArrayList<ClassNode>(2);
             dgmReceivers.add(testClass);
             if (isPrimitiveType(testClass)) dgmReceivers.add(getWrapper(testClass));
@@ -1594,6 +1580,25 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
                     }
                 }
             }
+
+            // GROOVY-7996: check if receiver implements get(String)/set(String,Object) or
propertyMissing(String)
+            if (!testClass.isArray() && !isPrimitiveType(getUnwrapper(testClass))
+                    && pexp.isImplicitThis() && typeCheckingContext.getEnclosingClosure()
!= null) {
+                MethodNode mopMethod;
+                if (readMode) {
+                    mopMethod = testClass.getMethod("get", new Parameter[]{new Parameter(STRING_TYPE,
"name")});
+                } else {
+                    mopMethod = testClass.getMethod("set", new Parameter[]{new Parameter(STRING_TYPE,
"name"), new Parameter(OBJECT_TYPE, "value")});
+                }
+                if (mopMethod == null) mopMethod = testClass.getMethod("propertyMissing",
new Parameter[]{new Parameter(STRING_TYPE, "propertyName")});
+
+                if (mopMethod != null) {
+                    pexp.putNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION, Boolean.TRUE);
+                    pexp.removeNodeMetaData(StaticTypesMarker.DECLARATION_INFERRED_TYPE);
+                    pexp.removeNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
+                    return true;
+                }
+            }
         }
 
         for (Receiver<String> receiver : receivers) {
@@ -1738,21 +1743,18 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
     private boolean storeField(FieldNode field, boolean returnTrueIfFieldExists, PropertyExpression
expressionToStoreOn, ClassNode receiver, ClassCodeVisitorSupport visitor, String delegationData,
boolean lhsOfAssignment) {
         if (field == null || !returnTrueIfFieldExists) return false;
         if (visitor != null) visitor.visitField(field);
-        storeWithResolve(field.getOriginType(), receiver, field.getDeclaringClass(), field.isStatic(),
expressionToStoreOn);
         checkOrMarkPrivateAccess(expressionToStoreOn, field, lhsOfAssignment);
-        if (field != null && !field.isStatic() && !field.isPrivate() &&
!"delegate".equals(delegationData)) {
-            delegationData = checkOrMarkInnerPropertyOwnerAccess(expressionToStoreOn, lhsOfAssignment,
delegationData);
-        }
+        storeWithResolve(field.getOriginType(), receiver, field.getDeclaringClass(), field.isStatic(),
expressionToStoreOn);
         if (delegationData != null) {
             expressionToStoreOn.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
         }
         return true;
     }
 
-    private boolean storeProperty(PropertyNode propertyNode, PropertyExpression expressionToStoreOn,
ClassNode receiver, ClassCodeVisitorSupport visitor, String delegationData) {
-        if (propertyNode == null) return false;
-        if (visitor != null) visitor.visitProperty(propertyNode);
-        storeWithResolve(propertyNode.getOriginType(), receiver, propertyNode.getDeclaringClass(),
propertyNode.isStatic(), expressionToStoreOn);
+    private boolean storeProperty(PropertyNode property, PropertyExpression expressionToStoreOn,
ClassNode receiver, ClassCodeVisitorSupport visitor, String delegationData) {
+        if (property == null) return false;
+        if (visitor != null) visitor.visitProperty(property);
+        storeWithResolve(property.getOriginType(), receiver, property.getDeclaringClass(),
property.isStatic(), expressionToStoreOn);
         if (delegationData != null) {
             expressionToStoreOn.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
         }
diff --git a/src/test/groovy/bugs/Groovy7996.groovy b/src/test/groovy/bugs/Groovy7996.groovy
new file mode 100644
index 0000000..39de261
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy7996.groovy
@@ -0,0 +1,187 @@
+/*
+ *  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 groovy.bugs
+
+import groovy.transform.CompileStatic
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+
+@CompileStatic
+final class Groovy7996 {
+
+    @Test
+    void testFieldAccessFromClosure1() {
+        def err = shouldFail '''
+            class Foo {
+                def build(@DelegatesTo(value=Foo, strategy=Closure.DELEGATE_FIRST) Closure
block) {
+                    this.with(block)
+                }
+
+                def propertyMissing(String name) {
+                    'whatever'
+                }
+            }
+
+            @groovy.transform.CompileStatic
+            class Bar {
+                protected List bar = []
+
+                boolean doStuff() {
+                    Foo foo = new Foo()
+                    foo.build {
+                        bar.isEmpty() // ClassCastException: java.lang.String cannot be cast
to java.util.List
+                    }
+                }
+            }
+
+            new Bar().doStuff()
+        '''
+
+        assert err =~ /Cannot find matching method java.lang.Object#isEmpty\(\)/
+    }
+
+    @Test
+    void testFieldAccessFromClosure2() {
+        assertScript '''
+            class Foo {
+                def build(@DelegatesTo(value=Foo, strategy=Closure.DELEGATE_FIRST) Closure
block) {
+                    this.with(block)
+                }
+
+                def propertyMissing(String name) {
+                    'whatever'
+                }
+            }
+
+            @groovy.transform.CompileStatic
+            class Bar {
+                protected List bar = []
+
+                boolean doStuff() {
+                    Foo foo = new Foo()
+                    foo.build {
+                        owner.bar.isEmpty()
+                    }
+                }
+            }
+
+            assert new Bar().doStuff()
+        '''
+    }
+
+    @Test
+    void testFieldAccessFromClosure3() {
+        assertScript '''
+            class Foo {
+                def build(@DelegatesTo(value=Foo, strategy=Closure.DELEGATE_FIRST) Closure
block) {
+                    this.with(block)
+                }
+
+                def propertyMissing(String name) {
+                    'whatever'
+                }
+            }
+
+            @groovy.transform.CompileStatic
+            class Bar {
+                protected List bar = []
+
+                boolean doStuff() {
+                    Foo foo = new Foo()
+                    foo.build {
+                        thisObject.bar.isEmpty()
+                    }
+                }
+            }
+
+            assert new Bar().doStuff()
+        '''
+    }
+
+    @Test
+    void testFieldAccessFromClosure4() {
+        assertScript '''
+            class Foo {
+                def build(@DelegatesTo(value=Foo, strategy=Closure.OWNER_FIRST) Closure block)
{
+                    block.delegate = this
+                    return block.call()
+                }
+
+                def propertyMissing(String name) {
+                    'whatever'
+                }
+            }
+
+            @groovy.transform.CompileStatic
+            class Bar {
+                protected List bar = []
+
+                boolean doStuff() {
+                    Foo foo = new Foo()
+                    foo.build {
+                        bar.isEmpty()
+                    }
+                }
+            }
+
+            assert new Bar().doStuff()
+        '''
+    }
+
+    @Test // GROOVY-7687
+    void testFieldAccessFromNestedClosure1() {
+        assertScript '''
+            @groovy.transform.CompileStatic
+            class BugTest {
+                static class Foo {
+                    public List<String> messages = ['hello', 'world']
+                }
+
+                void interactions(Foo foo, @DelegatesTo(Foo) Closure closure) {
+                    closure.delegate = foo
+                    closure()
+                }
+
+                void execute() {
+                    interactions(new Foo()) {
+                        messages.each { it.contains('o') }
+                    }
+                }
+            }
+            new BugTest().execute()
+        '''
+    }
+
+    @Test // GROOVY-8073
+    void testDelegatePropertyAccessFromClosure() {
+        assertScript '''
+            @groovy.transform.CompileStatic
+            class Main {
+                static main(args) {
+                    def map = [a: 1, b: 2]
+                    map.with {
+                        assert a == 1
+                    }
+                }
+            }
+        '''
+    }
+}
diff --git a/src/test/groovy/bugs/Groovy7996Bug.groovy b/src/test/groovy/bugs/Groovy7996Bug.groovy
deleted file mode 100644
index cc6a3d0..0000000
--- a/src/test/groovy/bugs/Groovy7996Bug.groovy
+++ /dev/null
@@ -1,88 +0,0 @@
-/*
- *  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 groovy.bugs
-
-class Groovy7996Bug extends GroovyTestCase {
-    void testPropertyAccessFromInnerClass() {
-        assertScript '''
-            class Foo7996 {
-                Object propertyMissing(String name) {
-                    return "stuff"
-                }
-
-                def build(Closure callable) {
-                    this.with(callable)
-                }
-            }
-
-            @groovy.transform.CompileStatic
-            class Bar7996 {
-                protected List bar = []
-
-                boolean doStuff() {
-                    Foo7996 foo = new Foo7996()
-                    foo.build {
-                        bar.isEmpty()
-                    }
-                }
-            }
-
-            assert new Bar7996().doStuff()
-        '''
-    }
-
-    // GROOVY-7687
-    void testCompileStaticWithNestedClosuresBug() {
-        assertScript '''
-        @groovy.transform.CompileStatic
-        class BugTest {
-            static class Foo {
-                public List<String> messages = Arrays.asList("hello", "world")
-            }
-
-            void interactions(Foo foo, @DelegatesTo(Foo) Closure closure) {
-                closure.delegate = foo
-                closure()
-            }
-
-            void execute() {
-                interactions(new Foo()) {
-                    messages.each{ it.contains('o') }
-                }
-            }
-        }
-        new BugTest().execute()
-        '''
-    }
-
-    // GROOVY-8073
-    void testCompileStaticMapInsideWithBug() {
-        assertScript '''
-            @groovy.transform.CompileStatic
-            class Main {
-                static void main(String[] args) {
-                    def map = [a: 1, b: 2]
-                    map.with {
-                        assert a == 1
-                    }
-                }
-            }
-        '''
-    }
-}


Mime
View raw message