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

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

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

Daniel.Sun
Repository: groovy
Updated Branches:
  refs/heads/master af2073c6b -> 221c13988


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


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

Branch: refs/heads/master
Commit: 221c13988cf904cb21a1c0559272a960918ad45d
Parents: af2073c
Author: sunlan <[hidden email]>
Authored: Fri Feb 9 08:34:47 2018 +0800
Committer: sunlan <[hidden email]>
Committed: Fri Feb 9 08:34:47 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/221c1398/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/221c1398/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 {

Daniel Sun
Apache Groovy committer

Blog: http://blog.sunlan.me
Twitter: @daniel_sun