You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by jk...@apache.org on 2015/10/14 15:47:44 UTC
tapestry-5 git commit: TAP5-2508: methods with default visibility
cannot be overridden from a different package
Repository: tapestry-5
Updated Branches:
refs/heads/master 1e73d6b52 -> 38f1d479c
TAP5-2508: methods with default visibility cannot be overridden from a different package
Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/38f1d479
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/38f1d479
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/38f1d479
Branch: refs/heads/master
Commit: 38f1d479cf115c9de93fc501bd47c130f0cff7f1
Parents: 1e73d6b
Author: Jochen Kemnade <jo...@eddyson.de>
Authored: Mon Oct 5 14:13:28 2015 +0200
Committer: Jochen Kemnade <jo...@eddyson.de>
Committed: Wed Oct 14 15:47:08 2015 +0200
----------------------------------------------------------------------
.../internal/plastic/InheritanceData.java | 65 ++++--
.../internal/plastic/PlasticClassImpl.java | 214 ++++++++++---------
.../internal/plastic/PlasticClassPool.java | 2 +-
.../internal/plastic/PlasticMethodImpl.java | 2 +-
.../tapestry5/plastic/InheritanceSpec.groovy | 58 +++++
.../test/java/testsubjects/foo/BaseClass.java | 9 +
6 files changed, 228 insertions(+), 122 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/38f1d479/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java
----------------------------------------------------------------------
diff --git a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java
index a9dc87d..cb98a37 100644
--- a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java
+++ b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java
@@ -12,6 +12,7 @@
package org.apache.tapestry5.internal.plastic;
+import java.util.Map;
import java.util.Set;
/**
@@ -22,18 +23,21 @@ public class InheritanceData
{
private final InheritanceData parent;
+ private final String packageName;
+
private final Set<String> methodNames = PlasticInternalUtils.newSet();
- private final Set<String> methods = PlasticInternalUtils.newSet();
+ private final Map<String, Boolean> methods = PlasticInternalUtils.newMap();
private final Set<String> interfaceNames = PlasticInternalUtils.newSet();
- public InheritanceData()
+ public InheritanceData(String packageName)
{
- this(null);
+ this(null, packageName);
}
- private InheritanceData(InheritanceData parent)
+ private InheritanceData(InheritanceData parent, String packageName)
{
this.parent = parent;
+ this.packageName = packageName;
}
/**
@@ -50,11 +54,13 @@ public class InheritanceData
* Returns a new MethodBundle that represents the methods of a child class
* of this bundle. The returned bundle will always be {@linkplain #isTransformed() transformed}.
*
+ * @param packageName
+ * the package that the child class will be created in
* @return new method bundle
*/
- public InheritanceData createChild()
+ public InheritanceData createChild(String packageName)
{
- return new InheritanceData(this);
+ return new InheritanceData(this, packageName);
}
/**
@@ -65,37 +71,66 @@ public class InheritanceData
* name of method
* @param desc
* describes the parameters and return value of the method
+ * @param samePackageOnly
+ * whether the method can only be overridden in classes that are in the same package
*/
- public void addMethod(String name, String desc)
+ public void addMethod(String name, String desc, boolean samePackageOnly)
{
- methods.add(toValue(name, desc));
+ methods.put(toValue(name, desc), samePackageOnly);
methodNames.add(name);
}
/**
- * Returns true if a transformed parent class contains an implementation of, or abstract placeholder for, the method.
+ * Returns true if this class or a transformed parent class contains an implementation of,
+ * or abstract placeholder for, the method.
*
* @param name
* method name
* @param desc
* method descriptor
- * @return true if a base class implements the method (including abstract methods)
+ * @return true if this class or a base class implements the method (including abstract methods)
*/
public boolean isImplemented(String name, String desc)
{
- return checkForMethod(toValue(name, desc));
+ return checkForMethod(toValue(name, desc), this);
+ }
+
+ /**
+ * Returns true if the method is an override of a base class method
+ *
+ * @param name
+ * method name
+ * @param desc
+ * method descriptor
+ * @return true if a base class implements the method (including abstract methods)
+ */
+ public boolean isOverride(String name, String desc)
+ {
+ return checkForMethod(toValue(name, desc), parent);
}
- private boolean checkForMethod(String value)
+ private boolean checkForMethod(String value, InheritanceData cursor)
{
- InheritanceData cursor = this;
+
+ String thisPackageName = packageName;
while (cursor != null)
{
- if (cursor.methods.contains(value))
+ if (cursor.methods.containsKey(value))
{
- return true;
+ boolean mustBeInSamePackage = cursor.methods.get(value);
+
+ if (!mustBeInSamePackage)
+ {
+ return true;
+ }
+ boolean isInSamePackage = thisPackageName.equals(cursor.packageName);
+
+ if (isInSamePackage)
+ {
+ return true;
+ }
}
cursor = cursor.parent;
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/38f1d479/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
----------------------------------------------------------------------
diff --git a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
index 46f3b41..264825a 100644
--- a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
+++ b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
@@ -194,6 +194,9 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
className = PlasticInternalUtils.toClassName(classNode.name);
superClassName = PlasticInternalUtils.toClassName(classNode.superName);
+ int lastIndexOfDot = className.lastIndexOf('.');
+
+ String packageName = lastIndexOfDot > -1 ? className.substring(0, lastIndexOfDot) : "";
fieldInstrumentations = new FieldInstrumentations(classNode.superName);
@@ -202,7 +205,7 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
this.parentInheritanceData = parentInheritanceData;
- inheritanceData = parentInheritanceData.createChild();
+ inheritanceData = parentInheritanceData.createChild(packageName);
for (String interfaceName : classNode.interfaces)
{
@@ -239,7 +242,7 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
{
if (isInheritableMethod(node))
{
- inheritanceData.addMethod(node.name, node.desc);
+ inheritanceData.addMethod(node.name, node.desc, node.access == 0);
}
methodNames.add(node.name);
@@ -261,7 +264,7 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
if (isInheritableMethod(node))
{
- inheritanceData.addMethod(node.name, node.desc);
+ inheritanceData.addMethod(node.name, node.desc, node.access == 0);
}
methodNames.add(node.name);
@@ -338,7 +341,7 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
return annotationAccess.getAnnotation(annotationType);
}
-
+
private static void addMethodAndParameterAnnotationsFromExistingClass(MethodNode methodNode, MethodNode implementationMethodNode)
{
// visits the method attributes
@@ -400,78 +403,78 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
methodNode.visitEnd();
}
-
+
private static void removeDuplicatedAnnotations(MethodNode node)
{
-
- removeDuplicatedAnnotations(node.visibleAnnotations);
- removeDuplicatedAnnotations(node.invisibleAnnotations);
-
- if (node.visibleParameterAnnotations != null)
- {
- for (List<AnnotationNode> list : node.visibleParameterAnnotations)
- {
- removeDuplicatedAnnotations(list);
- }
- }
-
- if (node.invisibleParameterAnnotations != null)
- {
- for (List<AnnotationNode> list : node.invisibleParameterAnnotations)
- {
- removeDuplicatedAnnotations(list);
- }
- }
-
+
+ removeDuplicatedAnnotations(node.visibleAnnotations);
+ removeDuplicatedAnnotations(node.invisibleAnnotations);
+
+ if (node.visibleParameterAnnotations != null)
+ {
+ for (List<AnnotationNode> list : node.visibleParameterAnnotations)
+ {
+ removeDuplicatedAnnotations(list);
+ }
+ }
+
+ if (node.invisibleParameterAnnotations != null)
+ {
+ for (List<AnnotationNode> list : node.invisibleParameterAnnotations)
+ {
+ removeDuplicatedAnnotations(list);
+ }
+ }
+
}
private static void removeDuplicatedAnnotations(ClassNode node)
{
- removeDuplicatedAnnotations(node.visibleAnnotations, true);
- removeDuplicatedAnnotations(node.invisibleAnnotations, true);
+ removeDuplicatedAnnotations(node.visibleAnnotations, true);
+ removeDuplicatedAnnotations(node.invisibleAnnotations, true);
}
-
+
private static void removeDuplicatedAnnotations(List<AnnotationNode> list) {
- removeDuplicatedAnnotations(list, false);
+ removeDuplicatedAnnotations(list, false);
}
-
+
private static void removeDuplicatedAnnotations(List<AnnotationNode> list, boolean reverse) {
-
- if (list != null)
- {
-
- final Set<String> annotations = new HashSet<String>();
- final List<AnnotationNode> toBeRemoved = new ArrayList<AnnotationNode>();
- final List<AnnotationNode> toBeIterated;
-
- if (reverse)
- {
- toBeIterated = new ArrayList<AnnotationNode>(list);
- Collections.reverse(toBeIterated);
- }
- else {
- toBeIterated = list;
- }
-
- for (AnnotationNode annotationNode : toBeIterated)
- {
- if (annotations.contains(annotationNode.desc))
- {
- toBeRemoved.add(annotationNode);
- }
- else
- {
- annotations.add(annotationNode.desc);
- }
- }
-
- for (AnnotationNode annotationNode : toBeRemoved)
- {
- list.remove(annotationNode);
- }
-
- }
-
+
+ if (list != null)
+ {
+
+ final Set<String> annotations = new HashSet<String>();
+ final List<AnnotationNode> toBeRemoved = new ArrayList<AnnotationNode>();
+ final List<AnnotationNode> toBeIterated;
+
+ if (reverse)
+ {
+ toBeIterated = new ArrayList<AnnotationNode>(list);
+ Collections.reverse(toBeIterated);
+ }
+ else {
+ toBeIterated = list;
+ }
+
+ for (AnnotationNode annotationNode : toBeIterated)
+ {
+ if (annotations.contains(annotationNode.desc))
+ {
+ toBeRemoved.add(annotationNode);
+ }
+ else
+ {
+ annotations.add(annotationNode.desc);
+ }
+ }
+
+ for (AnnotationNode annotationNode : toBeRemoved)
+ {
+ list.remove(annotationNode);
+ }
+
+ }
+
}
private static String getParametersDesc(MethodNode methodNode) {
@@ -480,33 +483,33 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
private static MethodNode findExactMatchMethod(MethodNode methodNode, ClassNode source) {
- MethodNode found = null;
-
- final String methodDescription = getParametersDesc(methodNode);
-
+ MethodNode found = null;
+
+ final String methodDescription = getParametersDesc(methodNode);
+
for (MethodNode implementationMethodNode : source.methods)
{
-
+
final String implementationMethodDescription = getParametersDesc(implementationMethodNode);
- if (methodNode.name.equals(implementationMethodNode.name) &&
- // We don't want synthetic methods.
- ((implementationMethodNode.access & Opcodes.ACC_SYNTHETIC) == 0)
- && (methodDescription.equals(implementationMethodDescription)))
+ if (methodNode.name.equals(implementationMethodNode.name) &&
+ // We don't want synthetic methods.
+ ((implementationMethodNode.access & Opcodes.ACC_SYNTHETIC) == 0)
+ && (methodDescription.equals(implementationMethodDescription)))
{
- found = implementationMethodNode;
- break;
+ found = implementationMethodNode;
+ break;
}
}
-
+
return found;
}
-
+
private static List<Class> getJavaParameterTypes(MethodNode methodNode) {
final ClassLoader classLoader = PlasticInternalUtils.class.getClassLoader();
Type[] parameterTypes = Type.getArgumentTypes(methodNode.desc);
- List<Class> list = new ArrayList<Class>();
- for (Type type : parameterTypes)
+ List<Class> list = new ArrayList<Class>();
+ for (Type type : parameterTypes)
{
try
{
@@ -517,9 +520,9 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
throw new RuntimeException(e); // shouldn't happen anyway
}
}
- return list;
+ return list;
}
-
+
/**
* Returns the first method which matches the given methodNode.
* FIXME: this may not find the correct method if the correct one is declared after
@@ -551,12 +554,12 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
for (int i = 0; i < parameterTypes.size(); i++)
{
final Class implementationParameterType = implementationParameterTypes.get(i);
- final Class parameterType = parameterTypes.get(i);
- if (!parameterType.isAssignableFrom(implementationParameterType)) {
+ final Class parameterType = parameterTypes.get(i);
+ if (!parameterType.isAssignableFrom(implementationParameterType)) {
matches = false;
break;
}
-
+
}
if (matches && !isBridge(implementationMethodNode))
@@ -564,7 +567,7 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
found = implementationMethodNode;
break;
}
-
+
}
}
@@ -574,39 +577,40 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
return found;
}
-
+
private static void addMethodAndParameterAnnotationsFromExistingClass(MethodNode methodNode, ClassNode source)
{
if (source != null)
{
- MethodNode candidate = findExactMatchMethod(methodNode, source);
+ MethodNode candidate = findExactMatchMethod(methodNode, source);
+
+ final String parametersDesc = getParametersDesc(methodNode);
+
+ // candidate will be null when the method has generic parameters
+ if (candidate == null && parametersDesc.trim().length() > 0)
+ {
+ candidate = findGenericMethod(methodNode, source);
+ }
- final String parametersDesc = getParametersDesc(methodNode);
-
- // candidate will be null when the method has generic parameters
- if (candidate == null && parametersDesc.trim().length() > 0)
- {
- candidate = findGenericMethod(methodNode, source);
- }
-
if (candidate != null)
{
- addMethodAndParameterAnnotationsFromExistingClass(methodNode, candidate);
+ addMethodAndParameterAnnotationsFromExistingClass(methodNode, candidate);
}
-
+
}
}
- /**
+ /**
* Tells whether a given method is a bridge one or not.
* Notice the flag for bridge method is the same as volatile field. Java 6 doesn't have
* Modifiers.isBridge(), so we use a workaround.
*/
- private static boolean isBridge(MethodNode methodNode) {
- return Modifier.isVolatile(methodNode.access);
- }
+ private static boolean isBridge(MethodNode methodNode)
+ {
+ return Modifier.isVolatile(methodNode.access);
+ }
@Override
public PlasticClass proxyInterface(Class interfaceType, PlasticField field)
@@ -976,7 +980,7 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
if (isInheritableMethod(methodNode))
{
- inheritanceData.addMethod(methodNode.name, methodNode.desc);
+ inheritanceData.addMethod(methodNode.name, methodNode.desc, methodNode.access == 0);
}
}
@@ -1001,7 +1005,7 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
if (!isOverride)
{
- addMethodAndParameterAnnotationsFromExistingClass(methodNode, implementationClassNode);
+ addMethodAndParameterAnnotationsFromExistingClass(methodNode, implementationClassNode);
addMethodAndParameterAnnotationsFromExistingClass(methodNode, interfaceClassNode);
removeDuplicatedAnnotations(methodNode);
}
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/38f1d479/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
----------------------------------------------------------------------
diff --git a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
index 290821b..3cf1700 100644
--- a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
+++ b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
@@ -58,7 +58,7 @@ public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticCl
*/
private final Map<String, ClassInstantiator> instantiators = PlasticInternalUtils.newMap();
- private final InheritanceData emptyInheritanceData = new InheritanceData();
+ private final InheritanceData emptyInheritanceData = new InheritanceData(null);
private final StaticContext emptyStaticContext = new StaticContext();
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/38f1d479/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticMethodImpl.java
----------------------------------------------------------------------
diff --git a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticMethodImpl.java b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticMethodImpl.java
index bc3c14c..4bb27cc 100644
--- a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticMethodImpl.java
+++ b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticMethodImpl.java
@@ -79,7 +79,7 @@ class PlasticMethodImpl extends PlasticMember implements PlasticMethod, Comparab
{
plasticClass.check();
- return plasticClass.parentInheritanceData.isImplemented(node.name, node.desc);
+ return plasticClass.inheritanceData.isOverride(node.name, node.desc);
}
@Override
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/38f1d479/plastic/src/test/groovy/org/apache/tapestry5/plastic/InheritanceSpec.groovy
----------------------------------------------------------------------
diff --git a/plastic/src/test/groovy/org/apache/tapestry5/plastic/InheritanceSpec.groovy b/plastic/src/test/groovy/org/apache/tapestry5/plastic/InheritanceSpec.groovy
new file mode 100644
index 0000000..38a53e4
--- /dev/null
+++ b/plastic/src/test/groovy/org/apache/tapestry5/plastic/InheritanceSpec.groovy
@@ -0,0 +1,58 @@
+package org.apache.tapestry5.plastic
+
+import org.apache.tapestry5.internal.plastic.NoopDelegate
+import org.spockframework.util.NotThreadSafe
+
+import spock.lang.Ignore
+import spock.lang.Issue;
+import testannotations.SimpleAnnotation;
+
+import java.lang.reflect.InvocationTargetException
+
+import testsubjects.foo.BaseClass
+
+
+class InheritanceSpec extends AbstractPlasticSpecification {
+
+ def "default visible method from same package is inherited"() {
+ setup:
+ def mgr = PlasticManager.withContextClassLoader().packages(["testsubjects.foo",]).create()
+ PlasticClass pcBase = mgr.getPlasticClass(BaseClass.name)
+ when:
+ PlasticClass pcExtending = mgr.pool.createTransformation(pcBase.className, BaseClass.name+"2").plasticClass
+ then:
+ pcExtending != null
+ when:
+ MethodDescription methodDescription = findMethod(pcBase, 'method').description
+
+ def method2Extending = pcExtending.introduceMethod(methodDescription)
+
+ then:
+ method2Extending != null
+
+ method2Extending.isOverride()
+ }
+
+ @Issue("TAP5-2508")
+ def "default visible method from different package is not inherited"() {
+ setup:
+ def mgr = PlasticManager.withContextClassLoader().packages([
+ "testsubjects.foo",
+ "testsubjects.bar"
+ ]).create()
+ PlasticClass pcBase = mgr.getPlasticClass(BaseClass.name)
+ when:
+ PlasticClass pcExtending = mgr.pool.createTransformation(pcBase.className, "testsubjects.bar.ExtendingClass").plasticClass
+ then:
+ pcExtending != null
+ when:
+ MethodDescription methodDescription = findMethod(pcBase, 'method').description
+
+ def method2Extending = pcExtending.introduceMethod(methodDescription)
+
+ then:
+ method2Extending != null
+
+ !method2Extending.isOverride()
+ }
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/38f1d479/plastic/src/test/java/testsubjects/foo/BaseClass.java
----------------------------------------------------------------------
diff --git a/plastic/src/test/java/testsubjects/foo/BaseClass.java b/plastic/src/test/java/testsubjects/foo/BaseClass.java
new file mode 100644
index 0000000..101d32a
--- /dev/null
+++ b/plastic/src/test/java/testsubjects/foo/BaseClass.java
@@ -0,0 +1,9 @@
+package testsubjects.foo;
+
+public class BaseClass {
+
+ void method() {
+
+ }
+
+}