[1/2] groovy git commit: Refine the fix of GROOVY-8474: check getter and setter

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[1/2] groovy git commit: Refine the fix of GROOVY-8474: check getter and setter

Daniel.Sun
Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_6_X 145082417 -> 2d60daf6f


Refine the fix of GROOVY-8474: check getter and setter

(cherry picked from commit 221c139)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/6541df37
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/6541df37
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/6541df37

Branch: refs/heads/GROOVY_2_6_X
Commit: 6541df37f5df68f876753190e382c95cc97f330a
Parents: 1450824
Author: sunlan <[hidden email]>
Authored: Fri Feb 9 08:34:47 2018 +0800
Committer: sunlan <[hidden email]>
Committed: Fri Feb 9 09:05:06 2018 +0800

----------------------------------------------------------------------
 .../groovy/classgen/AsmClassGenerator.java      | 31 +++----
 src/test/groovy/bugs/Groovy8474Bug.groovy       | 94 ++++++++++++++++++++
 2 files changed, 110 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/6541df37/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 0977934..e7cb2fe 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -1010,8 +1010,7 @@ public class AsmClassGenerator extends ClassGenerator {
                 if (isSuperExpression(objectExpression)) {
                     String prefix;
                     if (controller.getCompileStack().isLHS()) {
-                        //throw new GroovyBugError("Unexpected super property set for:" + expression.getText());
-                        setSuperProperty(classNode, expression, mv);
+                        setPropertyOfSuperClass(classNode, expression, mv);
 
                         return;
                     } else {
@@ -1085,7 +1084,7 @@ public class AsmClassGenerator extends ClassGenerator {
         }
     }
 
-    private void setSuperProperty(ClassNode classNode, PropertyExpression expression, MethodVisitor mv) {
+    private void setPropertyOfSuperClass(ClassNode classNode, PropertyExpression expression, MethodVisitor mv) {
         String fieldName = expression.getPropertyAsString();
         FieldNode fieldNode = classNode.getSuperClass().getField(fieldName);
 
@@ -1097,9 +1096,10 @@ public class AsmClassGenerator extends ClassGenerator {
             throw new RuntimeParserException("Cannot modify final field[" + fieldName + "] of " + classNode.getName() + "'s super class", expression);
         }
 
-        MethodNode setter = findSetter(classNode, fieldNode);
+        MethodNode setter = findSetterOfSuperClass(classNode, fieldNode);
+        MethodNode getter = findGetterOfSuperClass(classNode, fieldNode);
 
-        if (fieldNode.isPrivate() && null == setter) {
+        if (fieldNode.isPrivate() && !getterAndSetterExists(setter, getter)) {
             throw new RuntimeParserException("Cannot access private field[" + fieldName + "] of " + classNode.getName() + "'s super class", expression);
         }
 
@@ -1115,19 +1115,20 @@ public class AsmClassGenerator extends ClassGenerator {
         }
     }
 
-    private MethodNode findSetter(ClassNode classNode, FieldNode fieldNode) {
-        String setMethodName = "set" + MetaClassHelper.capitalize(fieldNode.getName());
-        MethodNode mn = classNode.getSuperClass().getMethod(setMethodName, new Parameter[] { new Parameter(fieldNode.getType(), "") });
+    private boolean getterAndSetterExists(MethodNode setter, MethodNode getter) {
+        return null != setter && null != getter && setter.getDeclaringClass().equals(getter.getDeclaringClass());
+    }
 
-        if (null == mn) {
-            return null;
-        }
+    private MethodNode findSetterOfSuperClass(ClassNode classNode, FieldNode fieldNode) {
+        String setterMethodName = "set" + MetaClassHelper.capitalize(fieldNode.getName());
 
-        if (!ClassHelper.VOID_TYPE.equals(mn.getReturnType())) {
-            return null;
-        }
+        return classNode.getSuperClass().getSetterMethod(setterMethodName);
+    }
+
+    private MethodNode findGetterOfSuperClass(ClassNode classNode, FieldNode fieldNode) {
+        String getterMethodName = "get" + MetaClassHelper.capitalize(fieldNode.getName());
 
-        return mn;
+        return classNode.getSuperClass().getGetterMethod(getterMethodName);
     }
 
     private boolean isThisOrSuperInStaticContext(Expression objectExpression) {

http://git-wip-us.apache.org/repos/asf/groovy/blob/6541df37/src/test/groovy/bugs/Groovy8474Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy8474Bug.groovy b/src/test/groovy/bugs/Groovy8474Bug.groovy
index ea21a4b..7a6f397 100644
--- a/src/test/groovy/bugs/Groovy8474Bug.groovy
+++ b/src/test/groovy/bugs/Groovy8474Bug.groovy
@@ -75,6 +75,35 @@ class Groovy8474Bug extends GroovyTestCase {
         '''
     }
 
+    void testSettingSuperProperty4() {
+        assertScript '''
+            class K {
+              private String name
+              public String getName() {
+                name
+              }
+              public void setName(String name) {
+                this.name = name
+              }
+            }
+            class T extends K {
+              String group
+            }
+            class S extends T {
+              S() {
+                super.group = 'Hello'
+                super.name = 'World'
+              }
+              
+              public String helloWorld() {
+                "$group, $name"
+              }
+            }
+            
+            assert 'Hello, World' == new S().helloWorld()
+        '''
+    }
+
     void testSettingSuperProtectedField() {
         assertScript '''
             class T {
@@ -204,6 +233,71 @@ class Groovy8474Bug extends GroovyTestCase {
         assert errMsg.contains('Cannot access private field')
     }
 
+    void testSettingSuperPrivateProperty2() {
+        def errMsg = shouldFail '''
+            class T {
+              private String group
+              
+              public String getGroup() {
+                return group
+              }
+            }
+            
+            class S extends T {
+              S() {
+                super.group = 'Hello'
+              }
+            }
+        '''
+
+        assert errMsg.contains('Cannot access private field')
+    }
+
+    void testSettingSuperPrivateProperty3() {
+        def errMsg = shouldFail '''
+            class T {
+              private String group
+              
+              public void setGroup(String group) {
+                this.group = group
+              }
+            }
+            
+            class S extends T {
+              S() {
+                super.group = 'Hello'
+              }
+            }
+        '''
+
+        assert errMsg.contains('Cannot access private field')
+    }
+
+    void testSettingSuperPrivateProperty4() {
+        def errMsg = shouldFail '''
+            class K {
+              private String group
+              
+              public void setGroup(String group) {
+                this.group = group
+              }
+            }
+            class T extends K {
+              public String getGroup() {
+                return group
+              }
+            }
+            
+            class S extends T {
+              S() {
+                super.group = 'Hello'
+              }
+            }
+        '''
+
+        assert errMsg.contains('Cannot access private field')
+    }
+
     void testSettingSuperFinalProperty() {
         shouldFail '''
             class T {

Reply | Threaded
Open this post in threaded view
|

[2/2] groovy git commit: Refine the fix of GROOVY-8474: support primitive type

Daniel.Sun
Refine the fix of GROOVY-8474: support primitive type

(cherry picked from commit 05866b3)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/2d60daf6
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/2d60daf6
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/2d60daf6

Branch: refs/heads/GROOVY_2_6_X
Commit: 2d60daf6f91fdc4a855e9d043c8a401b4c456abc
Parents: 6541df3
Author: sunlan <[hidden email]>
Authored: Fri Feb 9 09:01:32 2018 +0800
Committer: sunlan <[hidden email]>
Committed: Fri Feb 9 09:05:14 2018 +0800

----------------------------------------------------------------------
 .../groovy/classgen/AsmClassGenerator.java      |   7 +-
 src/test/groovy/bugs/Groovy8474Bug.groovy       | 128 +++++++++++++++++++
 2 files changed, 134 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/2d60daf6/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index e7cb2fe..8123e83 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -1103,8 +1103,13 @@ public class AsmClassGenerator extends ClassGenerator {
             throw new RuntimeParserException("Cannot access private field[" + fieldName + "] of " + classNode.getName() + "'s super class", expression);
         }
 
+        OperandStack operandStack = controller.getOperandStack();
+        operandStack.doAsType(fieldNode.getType());
+
         mv.visitVarInsn(ALOAD, 0);
-        mv.visitInsn(SWAP);
+        operandStack.push(classNode);
+
+        operandStack.swap();
 
         String owner = BytecodeHelper.getClassInternalName(classNode.getSuperClass().getName());
         String desc = BytecodeHelper.getTypeDescription(fieldNode.getType());

http://git-wip-us.apache.org/repos/asf/groovy/blob/2d60daf6/src/test/groovy/bugs/Groovy8474Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy8474Bug.groovy b/src/test/groovy/bugs/Groovy8474Bug.groovy
index 7a6f397..ab64c75 100644
--- a/src/test/groovy/bugs/Groovy8474Bug.groovy
+++ b/src/test/groovy/bugs/Groovy8474Bug.groovy
@@ -104,6 +104,134 @@ class Groovy8474Bug extends GroovyTestCase {
         '''
     }
 
+    void testSettingSuperProperty5() {
+        assertScript '''
+            class T {
+              Integer group
+            }
+            
+            class S extends T {
+              S() {
+                super.group = 1
+              }
+            }
+            
+            assert 1 == new S().group
+        '''
+    }
+
+    void testSettingSuperProperty6() {
+        assertScript '''
+            class T {
+              Long group
+            }
+            
+            class S extends T {
+              S() {
+                super.group = 1
+              }
+            }
+            
+            assert 1 == new S().group
+        '''
+    }
+
+    void testSettingSuperProperty7() {
+        assertScript '''
+            class T {
+              Long group
+            }
+            
+            class S extends T {
+              S() {
+                super.group = Long.MAX_VALUE
+              }
+            }
+            
+            assert Long.MAX_VALUE == new S().group
+        '''
+    }
+
+    void testSettingSuperProperty8() {
+        assertScript '''
+            class T {
+              int group
+            }
+            
+            class S extends T {
+              S() {
+                super.group = Integer.MAX_VALUE
+              }
+            }
+            
+            assert Integer.MAX_VALUE == new S().group
+        '''
+    }
+
+    void testSettingSuperProperty9() {
+        assertScript '''
+            class T {
+              long group
+            }
+            
+            class S extends T {
+              S() {
+                super.group = Long.MAX_VALUE
+              }
+            }
+            
+            assert Long.MAX_VALUE == new S().group
+        '''
+    }
+
+    void testSettingSuperProperty10() {
+        assertScript '''
+            class T {
+              int group
+            }
+            
+            class S extends T {
+              S() {
+                super.group = 1
+              }
+            }
+            
+            assert 1 == new S().group
+        '''
+    }
+
+    void testSettingSuperProperty11() {
+        assertScript '''
+            class T {
+              long group
+            }
+            
+            class S extends T {
+              S() {
+                super.group = 123456789123456789
+              }
+            }
+            
+            assert 123456789123456789 == new S().group
+        '''
+    }
+
+    void testSettingSuperProperty12() {
+        assertScript '''
+            class T {
+              boolean group
+            }
+            
+            class S extends T {
+              S() {
+                super.group = true
+              }
+            }
+            
+            assert true == new S().group
+        '''
+    }
+
     void testSettingSuperProtectedField() {
         assertScript '''
             class T {