groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sun...@apache.org
Subject [1/2] groovy git commit: Refine the fix of GROOVY-8474: check getter and setter
Date Fri, 09 Feb 2018 01:07:06 GMT
Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_5_X e30e86cb9 -> 4f7691eb4


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/b76b91f0
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/b76b91f0
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/b76b91f0

Branch: refs/heads/GROOVY_2_5_X
Commit: b76b91f0e7b304963550e4514ceff9addc6665da
Parents: e30e86c
Author: sunlan <sunlan@apache.org>
Authored: Fri Feb 9 08:34:47 2018 +0800
Committer: sunlan <sunlan@apache.org>
Committed: Fri Feb 9 09:06:51 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/b76b91f0/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/b76b91f0/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 {


Mime
View raw message