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

commit 8ab86fe9c6dd61767d90425d103d33d741b82fe2
Author: Paul King <pa...@asert.com.au>
AuthorDate: Mon Jan 21 13:16:05 2019 +1000

    GROOVY-7996: Using with method with a closure that references a protected 
property produces ClassCastException (closes #857)
---
 .../VariableExpressionTransformer.java             |  9 ++--
 .../transform/stc/StaticTypeCheckingVisitor.java   | 26 +++++++++++-
 src/test/groovy/bugs/Groovy7996Bug.groovy          | 49 ++++++++++++++++++++++
 3 files changed, 79 insertions(+), 5 deletions(-)

diff --git 
a/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
 
b/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
index 548d9b2..e17f429 100644
--- 
a/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
+++ 
b/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
@@ -52,15 +52,16 @@ public class VariableExpressionTransformer {
         // handle it
         Object val = expr.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
         if (val == null) return null;
-        VariableExpression implicitThis = new VariableExpression("this");
-        PropertyExpression pexp = new PropertyExpression(implicitThis, 
expr.getName());
+        // TODO handle the owner and delegate cases better for nested 
scenarios and potentially remove the need for the implicit this case
+        VariableExpression receiver = new 
VariableExpression("owner".equals(val) ? (String) val : "delegate".equals(val) 
? (String) val : "this");
+        PropertyExpression pexp = new PropertyExpression(receiver, 
expr.getName());
         pexp.copyNodeMetaData(expr);
         pexp.setImplicitThis(true);
         pexp.getProperty().setSourcePosition(expr);
         ClassNode owner = 
expr.getNodeMetaData(StaticCompilationMetadataKeys.PROPERTY_OWNER);
         if (owner != null) {
-            implicitThis.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, 
owner);
-            implicitThis.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, 
val);
+            receiver.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, owner);
+            receiver.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, val);
         }
         return pexp;
     }
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 497f9a2..4a12aa5 100644
--- 
a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ 
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -488,9 +488,10 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
      * Given a field node, checks if we are accessing or setting a private 
field from an inner class.
      */
     private void checkOrMarkPrivateAccess(Expression source, FieldNode fn, 
boolean lhsOfAssignment) {
+        if (fn == null) return;
         ClassNode enclosingClassNode = 
typeCheckingContext.getEnclosingClassNode();
         ClassNode declaringClass = fn.getDeclaringClass();
-        if (fn != null && Modifier.isPrivate(fn.getModifiers()) &&
+        if (fn.isPrivate() &&
                 (declaringClass != enclosingClassNode || 
typeCheckingContext.getEnclosingClosure() != null) &&
                 declaringClass.getModule() == enclosingClassNode.getModule()) {
             if (!lhsOfAssignment && 
enclosingClassNode.isDerivedFrom(declaringClass)) {
@@ -512,6 +513,28 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
         }
     }
 
+    /**
+     * Given a field node, checks if we are accessing or setting a public or 
protected field from an inner class.
+     */
+    private String checkOrMarkInnerFieldAccess(Expression source, FieldNode 
fn, boolean lhsOfAssignment, String delegationData) {
+        if (fn == null || fn.isStatic()) return delegationData;
+        ClassNode enclosingClassNode = 
typeCheckingContext.getEnclosingClassNode();
+        ClassNode declaringClass = fn.getDeclaringClass();
+        // private handled elsewhere
+        if ((fn.isPublic() || fn.isProtected()) &&
+                (declaringClass != enclosingClassNode || 
typeCheckingContext.getEnclosingClosure() != null) &&
+                declaringClass.getModule() == enclosingClassNode.getModule() 
&& !lhsOfAssignment && enclosingClassNode.isDerivedFrom(declaringClass)) {
+            if (source instanceof PropertyExpression) {
+                PropertyExpression pe = (PropertyExpression) source;
+                // this and attributes handled elsewhere
+                if ("this".equals(pe.getPropertyAsString()) || source 
instanceof AttributeExpression) return delegationData;
+                
pe.getObjectExpression().putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, 
"owner");
+            }
+            return "owner";
+        }
+        return delegationData;
+    }
+
     private MethodNode findValidGetter(ClassNode classNode, String name) {
         MethodNode getterMethod = classNode.getGetterMethod(name);
         if (getterMethod != null && (getterMethod.isPublic() || 
getterMethod.isProtected())) {
@@ -1714,6 +1737,7 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
         if (visitor != null) visitor.visitField(field);
         storeWithResolve(field.getOriginType(), receiver, 
field.getDeclaringClass(), field.isStatic(), expressionToStoreOn);
         checkOrMarkPrivateAccess(expressionToStoreOn, field, lhsOfAssignment);
+        delegationData = checkOrMarkInnerFieldAccess(expressionToStoreOn, 
field, lhsOfAssignment, delegationData);
         if (delegationData != null) {
             
expressionToStoreOn.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, 
delegationData);
         }
diff --git a/src/test/groovy/bugs/Groovy7996Bug.groovy 
b/src/test/groovy/bugs/Groovy7996Bug.groovy
new file mode 100644
index 0000000..d8ba632
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy7996Bug.groovy
@@ -0,0 +1,49 @@
+/*
+ *  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()
+        '''
+    }
+}

Reply via email to