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/11/16 13:34:55 UTC
[groovy] branch GROOVY_2_5_X updated: GROOVY-7996: check for
get(String)/set(String, Object) or propertyMissing
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 <er...@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
- }
- }
- }
- '''
- }
-}