You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2019/07/17 05:18:32 UTC
[groovy] branch master updated: GROOVY-9106: fix for same-package
check in isDirectAccessAllowed
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 7954154 GROOVY-9106: fix for same-package check in isDirectAccessAllowed
7954154 is described below
commit 7954154005bde8afe2bd693e82e1852ae313ae39
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Jul 14 09:59:32 2019 -0500
GROOVY-9106: fix for same-package check in isDirectAccessAllowed
---
.../classgen/asm/sc/StaticInvocationWriter.java | 2 +-
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 45 ++++++++++------------
.../packageScope/DifferentPackageTest.groovy | 27 +++++++++----
3 files changed, 41 insertions(+), 33 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
index 8c2b6a0..9539b53 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
@@ -668,7 +668,7 @@ public class StaticInvocationWriter extends InvocationWriter {
if (pname!=null && callSiteWriter instanceof StaticTypesCallSiteWriter) {
StaticTypesCallSiteWriter stcsw = (StaticTypesCallSiteWriter) callSiteWriter;
TypeChooser typeChooser = controller.getTypeChooser();
- if (stcsw.makeGetField(receiver, typeChooser.resolveType(receiver, controller.getClassNode()), pname, safe, false, true)) {
+ if (stcsw.makeGetField(receiver, typeChooser.resolveType(receiver, controller.getClassNode()), pname, safe, false)) {
return;
}
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
index 05a28e3..fc035de 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
@@ -94,15 +94,14 @@ import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.isClas
*/
public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes {
- private static final ClassNode INVOKERHELPER_TYPE = ClassHelper.make(InvokerHelper.class);
- private static final MethodNode GROOVYOBJECT_GETPROPERTY_METHOD = GROOVY_OBJECT_TYPE.getMethod("getProperty", new Parameter[]{new Parameter(STRING_TYPE, "propertyName")});
- private static final MethodNode INVOKERHELPER_GETPROPERTY_METHOD = INVOKERHELPER_TYPE.getMethod("getProperty", new Parameter[]{new Parameter(OBJECT_TYPE, "object"), new Parameter(STRING_TYPE, "propertyName")});
- private static final MethodNode INVOKERHELPER_GETPROPERTYSAFE_METHOD = INVOKERHELPER_TYPE.getMethod("getPropertySafe", new Parameter[]{new Parameter(OBJECT_TYPE, "object"), new Parameter(STRING_TYPE, "propertyName")});
- private static final MethodNode CLOSURE_GETTHISOBJECT_METHOD = CLOSURE_TYPE.getMethod("getThisObject", Parameter.EMPTY_ARRAY);
private static final ClassNode COLLECTION_TYPE = make(Collection.class);
+ private static final ClassNode INVOKERHELPER_TYPE = make(InvokerHelper.class);
private static final MethodNode COLLECTION_SIZE_METHOD = COLLECTION_TYPE.getMethod("size", Parameter.EMPTY_ARRAY);
- private static final MethodNode MAP_GET_METHOD = MAP_TYPE.getMethod("get", new Parameter[] { new Parameter(OBJECT_TYPE, "key")});
-
+ private static final MethodNode CLOSURE_GETTHISOBJECT_METHOD = CLOSURE_TYPE.getMethod("getThisObject", Parameter.EMPTY_ARRAY);
+ private static final MethodNode MAP_GET_METHOD = MAP_TYPE.getMethod("get", new Parameter[] {new Parameter(OBJECT_TYPE, "key")});
+ private static final MethodNode GROOVYOBJECT_GETPROPERTY_METHOD = GROOVY_OBJECT_TYPE.getMethod("getProperty", new Parameter[] {new Parameter(STRING_TYPE, "propertyName")});
+ private static final MethodNode INVOKERHELPER_GETPROPERTY_METHOD = INVOKERHELPER_TYPE.getMethod("getProperty", new Parameter[] {new Parameter(OBJECT_TYPE, "object"), new Parameter(STRING_TYPE, "propertyName")});
+ private static final MethodNode INVOKERHELPER_GETPROPERTYSAFE_METHOD = INVOKERHELPER_TYPE.getMethod("getPropertySafe", new Parameter[] {new Parameter(OBJECT_TYPE, "object"), new Parameter(STRING_TYPE, "propertyName")});
private final StaticTypesWriterController controller;
@@ -192,16 +191,16 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
return;
}
if (makeGetPropertyWithGetter(receiver, receiverType, methodName, safe, implicitThis)) return;
- if (makeGetField(receiver, receiverType, methodName, safe, implicitThis, samePackages(receiverType.getPackageName(), classNode.getPackageName()))) return;
+ if (makeGetField(receiver, receiverType, methodName, safe, implicitThis)) return;
if (receiver instanceof ClassExpression) {
- if (makeGetField(receiver, receiver.getType(), methodName, safe, implicitThis, samePackages(receiver.getType().getPackageName(), classNode.getPackageName()))) return;
+ if (makeGetField(receiver, receiver.getType(), methodName, safe, implicitThis)) return;
if (makeGetPropertyWithGetter(receiver, receiver.getType(), methodName, safe, implicitThis)) return;
if (makeGetPrivateFieldWithBridgeMethod(receiver, receiver.getType(), methodName, safe, implicitThis)) return;
}
if (isClassReceiver) {
// we are probably looking for a property of the class
if (makeGetPropertyWithGetter(receiver, CLASS_Type, methodName, safe, implicitThis)) return;
- if (makeGetField(receiver, CLASS_Type, methodName, safe, false, true)) return;
+ if (makeGetField(receiver, CLASS_Type, methodName, safe, false)) return;
}
if (makeGetPrivateFieldWithBridgeMethod(receiver, receiverType, methodName, safe, implicitThis)) return;
@@ -480,7 +479,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
if (makeGetPropertyWithGetter(receiver, receiverType, property, safe, implicitThis)) return;
if (makeGetPrivateFieldWithBridgeMethod(receiver, receiverType, property, safe, implicitThis)) return;
- if (makeGetField(receiver, receiverType, property, safe, implicitThis, samePackages(receiverType.getPackageName(), classNode.getPackageName()))) return;
+ if (makeGetField(receiver, receiverType, property, safe, implicitThis)) return;
MethodCallExpression call = new MethodCallExpression(
receiver,
@@ -565,12 +564,11 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
return false;
}
- boolean makeGetField(final Expression receiver, final ClassNode receiverType, final String fieldName, final boolean safe, final boolean implicitThis, final boolean samePackage) {
+ boolean makeGetField(final Expression receiver, final ClassNode receiverType, final String fieldName, final boolean safe, final boolean implicitThis) {
FieldNode field = receiverType.getField(fieldName);
// direct access is allowed if we are in the same class as the declaring class
// or we are in an inner class
- if (field !=null
- && isDirectAccessAllowed(field, controller.getClassNode(), samePackage)) {
+ if (field != null && isDirectAccessAllowed(field, controller.getClassNode())) {
CompileStack compileStack = controller.getCompileStack();
MethodVisitor mv = controller.getMethodVisitor();
ClassNode replacementType = field.getOriginType();
@@ -613,19 +611,19 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
for (ClassNode intf : receiverType.getInterfaces()) {
// GROOVY-7039
- if (intf!=receiverType && makeGetField(receiver, intf, fieldName, safe, implicitThis, false)) {
+ if (intf != receiverType && makeGetField(receiver, intf, fieldName, safe, implicitThis)) {
return true;
}
}
ClassNode superClass = receiverType.getSuperClass();
- if (superClass !=null) {
- return makeGetField(receiver, superClass, fieldName, safe, implicitThis, false);
+ if (superClass != null) {
+ return makeGetField(receiver, superClass, fieldName, safe, implicitThis);
}
return false;
}
- private static boolean isDirectAccessAllowed(FieldNode field, ClassNode receiver, boolean isSamePackage) {
+ private static boolean isDirectAccessAllowed(FieldNode field, ClassNode receiver) {
ClassNode declaringClass = field.getDeclaringClass().redirect();
ClassNode receiverType = receiver.redirect();
@@ -642,8 +640,8 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
receiverType = receiverType.getOuterClass();
}
- // finally public and inherited
- return field.isPublic() || isSamePackage;
+ // finally public and visible
+ return field.isPublic() || samePackages(receiver.getPackageName(), declaringClass.getPackageName());
}
@Override
@@ -924,13 +922,12 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
}
private boolean getField(PropertyExpression expression, Expression receiver, ClassNode receiverType, String name) {
- ClassNode classNode = controller.getClassNode();
boolean safe = expression.isSafe();
boolean implicitThis = expression.isImplicitThis();
- if (makeGetField(receiver, receiverType, name, safe, implicitThis, samePackages(receiverType.getPackageName(), classNode.getPackageName()))) return true;
+ if (makeGetField(receiver, receiverType, name, safe, implicitThis)) return true;
if (receiver instanceof ClassExpression) {
- if (makeGetField(receiver, receiver.getType(), name, safe, implicitThis, samePackages(receiver.getType().getPackageName(), classNode.getPackageName()))) return true;
+ if (makeGetField(receiver, receiver.getType(), name, safe, implicitThis)) return true;
if (makeGetPrivateFieldWithBridgeMethod(receiver, receiver.getType(), name, safe, implicitThis)) return true;
}
if (makeGetPrivateFieldWithBridgeMethod(receiver, receiverType, name, safe, implicitThis)) return true;
@@ -940,7 +937,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
isClassReceiver = true;
receiverType = receiverType.getGenericsTypes()[0].getType();
}
- if (isClassReceiver && makeGetField(receiver, CLASS_Type, name, safe, false, true)) return true;
+ if (isClassReceiver && makeGetField(receiver, CLASS_Type, name, safe, false)) return true;
if (receiverType.isEnum()) {
controller.getMethodVisitor().visitFieldInsn(GETSTATIC, BytecodeHelper.getClassInternalName(receiverType), name, BytecodeHelper.getTypeDescription(receiverType));
controller.getOperandStack().push(receiverType);
diff --git a/src/test/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy b/src/test/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy
index a041e6a..408955c 100644
--- a/src/test/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy
@@ -18,10 +18,14 @@
*/
package org.codehaus.groovy.transform.packageScope
+import groovy.transform.NotYetImplemented
import org.codehaus.groovy.control.*
import org.codehaus.groovy.tools.GroovyClass
+import org.junit.Test
-final class DifferentPackageTest extends GroovyTestCase {
+import static groovy.test.GroovyAssert.shouldFail
+
+final class DifferentPackageTest {
/** Class in package {@code p} with package-private fields {@code value} and {@code CONST}. */
private static final String P_DOT_ONE = '''
@@ -50,6 +54,7 @@ final class DifferentPackageTest extends GroovyTestCase {
//--------------------------------------------------------------------------
+ @Test
void testSamePackageShouldSeeInstanceProps1() {
def loader = addSources(
One: P_DOT_ONE,
@@ -67,6 +72,7 @@ final class DifferentPackageTest extends GroovyTestCase {
assert loader.loadClass('p.Two').newInstance().valueSize() == 5
}
+ @Test
void testSamePackageShouldSeeInstanceProps2() {
def loader = addSources(
One: P_DOT_ONE,
@@ -84,6 +90,7 @@ final class DifferentPackageTest extends GroovyTestCase {
assert loader.loadClass('p.Peer').newInstance().valueSize() == 5
}
+ @Test
void testSamePackageShouldSeeStaticProps1() {
def loader = addSources(
One: P_DOT_ONE,
@@ -101,6 +108,7 @@ final class DifferentPackageTest extends GroovyTestCase {
assert loader.loadClass('p.Two').half() == 21
}
+ @Test
void testSamePackageShouldSeeStaticProps2() {
def loader = addSources(
One: P_DOT_ONE,
@@ -118,6 +126,7 @@ final class DifferentPackageTest extends GroovyTestCase {
assert loader.loadClass('p.Two').newInstance().half() == 21
}
+ @Test
void testSamePackageShouldSeeStaticProps3() {
def loader = addSources(
One: P_DOT_ONE,
@@ -135,6 +144,7 @@ final class DifferentPackageTest extends GroovyTestCase {
assert loader.loadClass('p.Peer').half() == 21
}
+ @Test
void testSamePackageShouldSeeStaticProps4() {
def loader = addSources(
One: P_DOT_ONE,
@@ -152,8 +162,8 @@ final class DifferentPackageTest extends GroovyTestCase {
assert loader.loadClass('p.Peer').newInstance().half() == 21
}
- // GROOVY-9106
- void _FIXME_testSamePackageShouldSeeStaticProps5() {
+ @Test // GROOVY-9106
+ void testSamePackageShouldSeeStaticProps5() {
def loader = addSources(
One: P_DOT_ONE,
Two: '''
@@ -163,7 +173,7 @@ final class DifferentPackageTest extends GroovyTestCase {
class Two extends p.One {
}
''',
- Peer: '''\
+ Peer: '''
package p
@groovy.transform.CompileStatic
@@ -177,8 +187,8 @@ final class DifferentPackageTest extends GroovyTestCase {
assert loader.loadClass('p.Peer').half() == 21
}
- // GROOVY-9093
- void _FIXME_testDifferentPackageShouldNotSeeInstanceProps() {
+ @Test @NotYetImplemented // GROOVY-9093
+ void testDifferentPackageShouldNotSeeInstanceProps() {
def err = shouldFail CompilationFailedException, {
def loader = addSources(
One: P_DOT_ONE,
@@ -199,8 +209,8 @@ final class DifferentPackageTest extends GroovyTestCase {
assert err =~ / Access to ... value is forbidden /
}
- // GROOVY-9093
- void _FIXME_testDifferentPackageShouldNotSeeStaticProps1() {
+ @Test @NotYetImplemented // GROOVY-9093
+ void testDifferentPackageShouldNotSeeStaticProps1() {
def err = shouldFail CompilationFailedException, {
def loader = addSources(
One: P_DOT_ONE,
@@ -221,6 +231,7 @@ final class DifferentPackageTest extends GroovyTestCase {
assert err =~ / Access to p.One#CONST is forbidden /
}
+ @Test
void testDifferentPackageShouldNotSeeStaticProps2() {
def err = shouldFail CompilationFailedException, {
addSources(