You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2020/06/26 18:28:42 UTC
[groovy] 01/01: GROOVY-8999 (pt.1): super.@x should fail for
private fields -- no getX()
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-8999
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit c2bfff3778cadd6d3d3d790f2adf4904654002d6
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Jun 26 13:28:23 2020 -0500
GROOVY-8999 (pt.1): super.@x should fail for private fields -- no getX()
- Error message for super.x and super.@x should refer to super class
---
src/main/java/groovy/lang/MetaClassImpl.java | 4 +-
.../groovy/classgen/AsmClassGenerator.java | 10 +-
src/test/groovy/ThisAndSuperTest.groovy | 116 ++++++++++++++-------
.../groovy/classgen/asm/sc/bugs/Groovy7300.groovy | 77 ++++++--------
4 files changed, 117 insertions(+), 90 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index 5dda2f5..063fe49 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -2936,7 +2936,7 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
}
}
- throw new MissingFieldException(attribute, theClass);
+ throw new MissingFieldException(attribute, sender);
}
/**
@@ -2976,7 +2976,7 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
}
}
- throw new MissingFieldException(attribute, theClass);
+ throw new MissingFieldException(attribute, sender);
}
/**
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index eb181d8..e8fd5ca 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -141,8 +141,8 @@ public class AsmClassGenerator extends ClassGenerator {
// fields
public static final MethodCallerMultiAdapter setField = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setField", false, false);
public static final MethodCallerMultiAdapter getField = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getField", false, false);
- //private static final MethodCallerMultiAdapter setFieldOnSuper = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setFieldOnSuper", false, false);
- //private static final MethodCallerMultiAdapter getFieldOnSuper = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getFieldOnSuper", false, false);
+ private static final MethodCallerMultiAdapter setFieldOnSuper = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setFieldOnSuper", false, false);
+ private static final MethodCallerMultiAdapter getFieldOnSuper = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getFieldOnSuper", false, false);
public static final MethodCallerMultiAdapter setGroovyObjectField = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "setGroovyObjectField", false, false);
public static final MethodCallerMultiAdapter getGroovyObjectField = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "getGroovyObjectField", false, false);
@@ -1126,8 +1126,6 @@ public class AsmClassGenerator extends ClassGenerator {
if (fieldNode != null) {
fieldX(fieldNode).visit(this);
visited = true;
- } else if (isSuperExpression(objectExpression)) {
- visited = tryPropertyOfSuperClass(expression, name);
}
}
}
@@ -1135,9 +1133,9 @@ public class AsmClassGenerator extends ClassGenerator {
if (!visited) {
MethodCallerMultiAdapter adapter;
if (controller.getCompileStack().isLHS()) {
- adapter = isGroovyObject(objectExpression) ? setGroovyObjectField : setField;
+ adapter = isSuperExpression(objectExpression) ? setFieldOnSuper : isGroovyObject(objectExpression) ? setGroovyObjectField : setField;
} else {
- adapter = isGroovyObject(objectExpression) ? getGroovyObjectField : getField;
+ adapter = isSuperExpression(objectExpression) ? getFieldOnSuper : isGroovyObject(objectExpression) ? getGroovyObjectField : getField;
}
visitAttributeOrProperty(expression, adapter);
}
diff --git a/src/test/groovy/ThisAndSuperTest.groovy b/src/test/groovy/ThisAndSuperTest.groovy
index 087756d..b948b76 100644
--- a/src/test/groovy/ThisAndSuperTest.groovy
+++ b/src/test/groovy/ThisAndSuperTest.groovy
@@ -18,22 +18,28 @@
*/
package groovy
-import groovy.test.GroovyTestCase
+import org.junit.Test
-class ThisAndSuperTest extends GroovyTestCase {
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+
+final class ThisAndSuperTest {
+
+ @Test
void testOverwrittenSuperMethod() {
def helper = new TestForSuperHelper2()
assert helper.foo() == 2
assert helper.callFooInSuper() == 1
}
+ @Test
void testClosureUsingSuperAndThis() {
def helper = new TestForSuperHelper2()
assert helper.aClosureUsingThis() == 2
assert helper.aClosureUsingSuper() == 1
// accessing private method should not be changed
// by a public method of the same name and signature!
- assertEquals "bar", helper.closureUsingPrivateMethod()
+ assert helper.closureUsingPrivateMethod() == "bar"
assert helper.bar() == "no bar"
assert helper.aField == "I am a field"
@@ -43,6 +49,7 @@ class ThisAndSuperTest extends GroovyTestCase {
assert helper.aField == 2
}
+ @Test
void testClosureDelegateAndThis() {
def map = [:]
def helper = new TestForSuperHelper2()
@@ -78,6 +85,7 @@ class ThisAndSuperTest extends GroovyTestCase {
assert map.foo == 1
}
+ @Test
void testConstructorChain() {
def helper = new TestForSuperHelper4()
assert helper.x == 1
@@ -85,6 +93,7 @@ class ThisAndSuperTest extends GroovyTestCase {
assert helper.x == "Object"
}
+ @Test
void testChainingForAsType() {
def helper = new TestForSuperHelper1()
def ret = helper as Object[]
@@ -96,43 +105,84 @@ class ThisAndSuperTest extends GroovyTestCase {
}
}
+ @Test
void testSuperEach() {
def x = new TestForSuperEach()
x.each {
x.res << "I am it: ${it.class.name}"
}
- assertEquals 3, x.res.size()
- assertEquals "start each in subclass", x.res[0]
- assertEquals "I am it: groovy.TestForSuperEach", x.res[1]
- assertEquals "end of each in subclass", x.res[2]
- }
-
-// GROOVY-2555
-// void testCallToAbstractSuperMethodShouldResultInMissingMethod () {
-// def x = new TestForSuperHelper6()
-// shouldFail(MissingMethodException) {
-// x.theMethod()
-// }
-// }
-
- void testDgm() {
- assertEquals A.empty(), '123'
+ assert x.res.size() == 3
+ assert x.res[0] == "start each in subclass"
+ assert x.res[1] == "I am it: groovy.TestForSuperEach"
+ assert x.res[2] == "end of each in subclass"
}
+ @Test // GROOVY-2555
void testAbstractSuperMethodShouldBeTreatedLikeMissingMethod() {
- shouldFail(MissingMethodException) {
- new TestForSuperHelper6().theMethod()
- }
- }
-
- static class A {
- static {
- A.metaClass.static.empty << {-> '123' }
- }
+ shouldFail MissingMethodException, '''
+ abstract class A {
+ abstract void m()
+ }
+ class B extends A {
+ void m() {
+ super.m()
+ }
+ }
+ new B().m()
+ '''
+ }
+
+ @Test // GROOVY-8999
+ void testPrivateSuperField1() {
+ def err = shouldFail MissingFieldException, '''
+ abstract class A {
+ private x = 1
+ def getX() { 2 }
+ }
+ class B extends A {
+ private x = 3
+ def m() { super.@x }
+ }
+ new B().m()
+ '''
+
+ assert err =~ /No such field: x for class: A/
+ }
+
+ @Test // GROOVY-8999
+ void testPrivateSuperField2() {
+ def err = shouldFail MissingFieldException, '''
+ abstract class A {
+ private x = 1
+ def getX() { 2 }
+ void setX(x) { this.x = 3 }
+ }
+ class B extends A {
+ private x = 4
+ def m() { super.@x = 5; return x }
+ }
+ new B().m()
+ '''
+
+ assert err =~ /No such field: x for class: A/
+ }
+
+ // https://github.com/apache/groovy/commit/b62e4d3165b4d899a3b6c71dba2858c9362b2e1b
+ @Test // TODO: Does this belong in another test suite?
+ void testStaticMetaClassClosure() {
+ assertScript '''
+ class A {
+ }
+ A.metaClass.static.something << { -> '123' }
+
+ assert A.something() == '123'
+ '''
}
}
+//------------------------------------------------------------------------------
+
class TestForSuperEach {
def res = []
@@ -193,13 +243,3 @@ class TestForSuperHelper4 extends TestForSuperHelper3 {
super(j)
}
}
-
-abstract class TestForSuperHelper5 {
- abstract void theMethod()
-}
-
-class TestForSuperHelper6 extends TestForSuperHelper5 {
- void theMethod() {
- super.theMethod()
- }
-}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7300.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7300.groovy
index 3fba642..2770ba3 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7300.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7300.groovy
@@ -23,72 +23,61 @@ import org.codehaus.groovy.classgen.asm.sc.StaticCompilationTestSupport
final class Groovy7300 extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
- void testShouldNotThrowStackOverflow() {
+ void testUseSuperToBypassOverride1() {
assertScript '''
- class A {
- private String field1 = 'test'
-
- String getField1() {
- return this.field1
- }
+ abstract class A {
+ protected x = 1
+ def getX() { 2 }
}
-
class B extends A {
@Override
- String getField1() {
- super.field1
- }
+ def getX() { super.x }
}
-
- B b = new B()
-
- assert b.field1 == 'test'
+ assert new B().getX() == 1 // TODO: Why use A#x and not A#getX?
'''
}
- void testShouldNotThrowStackOverflowWithSuper1() {
+ void testUseSuperToBypassOverride1a() {
assertScript '''
- class A {
- private String field1 = 'test'
-
- void setField1(String val) { field1 = val }
-
- String getField1() {
- return this.field1
- }
+ abstract class A {
+ protected x = 1
+ def getX() { 2 }
}
-
class B extends A {
@Override
- String getField1() {
- super.field1 = 'test 2'
- super.field1
- }
+ def getX() { super.@x }
}
-
- B b = new B()
-
- assert b.field1 == 'test 2'
+ assert new B().getX() == 1
'''
}
- void testShouldNotThrowStackOverflowWithSuper2() {
+ void testUseSuperToBypassOverride2() {
assertScript '''
- class A {
- private String field = 'value'
- String getField() { return field }
- void setField(String value) { field = value }
+ abstract class A {
+ private x = 1
+ def getX() { 2 }
}
-
class B extends A {
@Override
- String getField() {
- super.@field = 'reset'
- return super.field
- }
+ def getX() { super.x }
}
+ assert new B().getX() == 2
+ '''
+ }
- assert new B().field == 'reset'
+ void testUseSuperToBypassOverride2a() {
+ def err = shouldFail '''
+ abstract class A {
+ private x = 1
+ def getX() { 2 }
+ }
+ class B extends A {
+ @Override
+ def getX() { super.@x }
+ }
+ assert new B().getX() == 1
'''
+
+ assert err =~ /No such field: x for class: A/ // TODO: Replace run-time error with compile-time error.
}
}