You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2019/01/22 04:43:28 UTC
[groovy] branch GROOVY_2_5_X updated (f71ddf6 -> 15c6834)
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a change to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from f71ddf6 GROOVY-8873: Fails at runtime with @CompileStatic and two nested with statements with ClassCastException (closes #854)
new 8ab86fe GROOVY-7996: Using with method with a closure that references a protected property produces ClassCastException (closes #857)
new 15c6834 GROOVY-8073/GROOVY-7687: add test cases for additional cases fixed by GROOVY-7996
The 2 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:
.../VariableExpressionTransformer.java | 9 ++-
.../transform/stc/StaticTypeCheckingVisitor.java | 26 ++++++-
src/test/groovy/bugs/Groovy7996Bug.groovy | 88 ++++++++++++++++++++++
3 files changed, 118 insertions(+), 5 deletions(-)
create mode 100644 src/test/groovy/bugs/Groovy7996Bug.groovy
[groovy] 01/02: GROOVY-7996: Using with method with a closure that
references a protected property produces ClassCastException (closes #857)
Posted by pa...@apache.org.
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()
+ '''
+ }
+}
[groovy] 02/02: GROOVY-8073/GROOVY-7687: add test cases for
additional cases fixed by GROOVY-7996
Posted by pa...@apache.org.
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 15c6834edac59d86f81fcb09b1053beb0b2d1afb
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Jan 22 11:56:18 2019 +1000
GROOVY-8073/GROOVY-7687: add test cases for additional cases fixed by GROOVY-7996
---
src/test/groovy/bugs/Groovy7996Bug.groovy | 39 +++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/src/test/groovy/bugs/Groovy7996Bug.groovy b/src/test/groovy/bugs/Groovy7996Bug.groovy
index d8ba632..cc6a3d0 100644
--- a/src/test/groovy/bugs/Groovy7996Bug.groovy
+++ b/src/test/groovy/bugs/Groovy7996Bug.groovy
@@ -46,4 +46,43 @@ class Groovy7996Bug extends GroovyTestCase {
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
+ }
+ }
+ }
+ '''
+ }
}