This is an automated email from the ASF dual-hosted git repository. paulk pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push: new c416e12 GROOVY-7996: Using with method with a closure that references a protected property produces ClassCastException (closes #857) c416e12 is described below commit c416e12930d4c62894936668399ea7ed6b49f7a5 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 fdae2cc..14116cd 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -495,9 +495,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)) { @@ -519,6 +520,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())) { @@ -1796,6 +1819,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() + ''' + } +}