groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sh...@apache.org
Subject 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 Tue, 26 Jan 2016 04:42:38 GMT
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 <shil.sinha@gmail.com>
Authored: Tue Oct 13 03:03:55 2015 -0400
Committer: Shil Sinha <shils@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"
     }


Mime
View raw message