You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by sh...@apache.org on 2016/01/26 05:42:38 UTC
groovy git commit: GROOVY-7627 Property calls are not correctly type
checked if the setter parameter type or getter return type are different from
the underlying field type (closes #139)
Repository: groovy
Updated Branches:
refs/heads/master 00ddfdf40 -> 13c0bda2a
GROOVY-7627 Property calls are not correctly type checked if the setter parameter type or getter return type are different from the underlying field type
(closes #139)
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/13c0bda2
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/13c0bda2
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/13c0bda2
Branch: refs/heads/master
Commit: 13c0bda2aa076a429b854912496e90cf427927ab
Parents: 00ddfdf
Author: Shil S <sh...@gmail.com>
Authored: Tue Oct 13 03:03:55 2015 -0400
Committer: Shil Sinha <sh...@apache.org>
Committed: Mon Jan 25 23:42:01 2016 -0500
----------------------------------------------------------------------
.../asm/sc/StaticTypesCallSiteWriter.java | 2 +-
.../stc/StaticTypeCheckingVisitor.java | 21 +++---
.../stc/FieldsAndPropertiesSTCTest.groovy | 74 ++++++++++++++++++++
3 files changed, 88 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/13c0bda2/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
index 526d001..8d54036 100644
--- a/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
+++ b/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
@@ -471,7 +471,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
// and that one references the other, the getters for properties have not been
// generated by the compiler yet (generated by the Verifier)
PropertyNode propertyNode = receiverType.getProperty(methodName);
- if (propertyNode!=null) {
+ if (getterNode == null && propertyNode != null) {
// it is possible to use a getter
String prefix = "get";
if (boolean_TYPE.equals(propertyNode.getOriginType())) {
http://git-wip-us.apache.org/repos/asf/groovy/blob/13c0bda2/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index ef2e61f..cd06ae7 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1226,12 +1226,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
field = allowStaticAccessToMember(field, staticOnly);
if (storeField(field, isAttributeExpression, pexp, current, visitor, receiver.getData(), !readMode)) return true;
- PropertyNode propertyNode = current.getProperty(propertyName);
- propertyNode = allowStaticAccessToMember(propertyNode, staticOnly);
- if (storeProperty(propertyNode, pexp, current, visitor, receiver.getData())) return true;
+ boolean isThisExpression = objectExpression instanceof VariableExpression
+ && ((VariableExpression) objectExpression).isThisExpression()
+ && objectExpressionType.equals(current);
- boolean isThisExpression = objectExpression instanceof VariableExpression &&
- ((VariableExpression) objectExpression).isThisExpression() && objectExpressionType.equals(current);
if (storeField(field, isThisExpression, pexp, receiver.getType(), visitor, receiver.getData(), !readMode))
return true;
@@ -1247,7 +1245,12 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
// need to visit even if we only look for a setters for compatibility
if (visitor != null && getter != null) visitor.visitMethod(getter);
- if (readMode) {
+ PropertyNode propertyNode = current.getProperty(propertyName);
+ propertyNode = allowStaticAccessToMember(propertyNode, staticOnly);
+ //prefer explicit getter or setter over property if receiver is not 'this'
+ boolean checkGetterOrSetter = !isThisExpression || propertyNode == null;
+
+ if (readMode && checkGetterOrSetter) {
if (getter != null) {
ClassNode cn = inferReturnTypeGenerics(current, getter, ArgumentListExpression.EMPTY_ARGUMENTS);
storeInferredTypeForPropertyExpression(pexp, cn);
@@ -1257,7 +1260,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
pexp.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
return true;
}
- } else {
+ } else if (!readMode && checkGetterOrSetter) {
if (!setters.isEmpty()) {
if (visitor != null) {
if (field != null) {
@@ -1282,12 +1285,14 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
pexp.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
}
return true;
- } else if (getter != null) {
+ } else if (getter != null && propertyNode == null) {
pexp.putNodeMetaData(StaticTypesMarker.READONLY_PROPERTY, true);
}
}
foundGetterOrSetter = foundGetterOrSetter || !setters.isEmpty() || getter != null;
+ if (storeProperty(propertyNode, pexp, current, visitor, receiver.getData())) return true;
+
if (storeField(field, true, pexp, current, visitor, receiver.getData(), !readMode)) return true;
// if the property expression is an attribute expression (o.@attr), then
// we stop now, otherwise we must check the parent class
http://git-wip-us.apache.org/repos/asf/groovy/blob/13c0bda2/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index 90ce986..a99b27a 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -739,6 +739,80 @@ import org.codehaus.groovy.ast.stmt.AssertStatement
'''
}
+ void testPropertyStyleSetterArgShouldBeCheckedAgainstParamType() {
+ shouldFailWithMessages '''
+ class Foo {
+ Bar bar;
+
+ void setBar(int x) {
+ this.bar = new Bar(x: x)
+ }
+ }
+
+ class Bar {
+ int x
+ }
+
+ Foo foo = new Foo()
+ foo.bar = new Bar()
+ ''', 'Cannot assign value of type Bar to variable of type int'
+
+ assertScript '''
+ class Foo {
+ Bar bar;
+
+ void setBar(int x) {
+ this.bar = new Bar(x: x)
+ }
+ }
+
+ class Bar {
+ int x
+ }
+
+ Foo foo = new Foo()
+ foo.bar = 1
+ assert foo.bar.x == 1
+ '''
+ }
+
+ void testPropertyStyleGetterUsageShouldBeCheckedAgainstReturnType() {
+ shouldFailWithMessages '''
+ class Foo {
+ Bar bar;
+
+ int getBar() {
+ bar.x
+ }
+ }
+
+ class Bar {
+ int x
+ }
+
+ Foo foo = new Foo(bar: new Bar(x: 1))
+ Bar bar = foo.bar
+ ''', 'Cannot assign value of type int to variable of type Bar'
+
+ assertScript '''
+ class Foo {
+ Bar bar;
+
+ int getBar() {
+ bar.x
+ }
+ }
+
+ class Bar {
+ int x
+ }
+
+ Foo foo = new Foo(bar: new Bar(x: 1))
+ int x = foo.bar
+ assert x == 1
+ '''
+ }
+
public static interface InterfaceWithField {
String boo = "I don't fancy fields in interfaces"
}