groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sh...@apache.org
Subject [2/2] 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)
Date Fri, 05 Feb 2016 05:13:05 GMT
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/10632b46
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/10632b46
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/10632b46

Branch: refs/heads/GROOVY_2_4_X
Commit: 10632b46cfe13ee13f890c6c6eacd701deaf933d
Parents: 28a17b3
Author: Shil S <shil.sinha@gmail.com>
Authored: Tue Oct 13 03:03:55 2015 -0400
Committer: Shil Sinha <shils@apache.org>
Committed: Fri Feb 5 00:08:17 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/10632b46/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/10632b46/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 9e1fae2..c789e64 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1225,12 +1225,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;
 
@@ -1246,7 +1244,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);
@@ -1256,7 +1259,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) {
@@ -1281,12 +1284,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/10632b46/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"
     }


Mime
View raw message