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 2022/10/12 19:20:32 UTC
[groovy] branch GROOVY_3_0_X updated: GROOVY-10717: proxy method if abstract or no abstract overload exists
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_3_0_X by this push:
new c95f0bb9c2 GROOVY-10717: proxy method if abstract or no abstract overload exists
c95f0bb9c2 is described below
commit c95f0bb9c2debc4db4ae5343cbca04c2c0c62926
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Oct 12 11:29:54 2022 -0500
GROOVY-10717: proxy method if abstract or no abstract overload exists
---
src/main/java/groovy/util/ProxyGenerator.java | 70 ++++-----
.../groovy/runtime/ProxyGeneratorAdapter.java | 165 ++++++++++-----------
src/spec/test/CoercionTest.groovy | 33 ++++-
3 files changed, 135 insertions(+), 133 deletions(-)
diff --git a/src/main/java/groovy/util/ProxyGenerator.java b/src/main/java/groovy/util/ProxyGenerator.java
index 06d6a2a16a..7887168230 100644
--- a/src/main/java/groovy/util/ProxyGenerator.java
+++ b/src/main/java/groovy/util/ProxyGenerator.java
@@ -42,28 +42,22 @@ import java.util.Map;
import java.util.Objects;
import java.util.Set;
+import static org.codehaus.groovy.runtime.MetaClassHelper.EMPTY_CLASS_ARRAY;
+
/**
- * Classes to generate 'Proxy' objects which implement interfaces,
- * maps of closures and/or extend classes/delegates.
+ * Generates 'Proxy' objects which implement interfaces, maps of closures and/or
+ * extend classes/delegates.
*/
+@SuppressWarnings("rawtypes")
public class ProxyGenerator {
- private static final Class[] EMPTY_CLASS_ARRAY = new Class[0];
- private static final Class[] EMPTY_INTERFACE_ARRAY = EMPTY_CLASS_ARRAY;
private static final Map<Object,Object> EMPTY_CLOSURE_MAP = Collections.emptyMap();
- private static final Set<String> EMPTY_KEYSET = Collections.emptySet();
static {
// wrap the standard MetaClass with the delegate
setMetaClass(GroovySystem.getMetaClassRegistry().getMetaClass(ProxyGenerator.class));
}
- private ClassLoader override = null;
- private boolean debug = false;
- private boolean emptyMethods = false;
-
- private static final Integer GROOVY_ADAPTER_CACHE_DEFAULT_SIZE = Integer.getInteger("groovy.adapter.cache.default.size", 64);
-
- public static final ProxyGenerator INSTANCE = new ProxyGenerator(); // TODO should we make ProxyGenerator singleton?
+ public static final ProxyGenerator INSTANCE = new ProxyGenerator(); // TODO: Should we make ProxyGenerator singleton?
/**
* The adapter cache is used to cache proxy classes. When, for example, a call like:
@@ -71,7 +65,11 @@ public class ProxyGenerator {
* adapter already exists. If so, then the class is reused, instead of generating a
* new class.
*/
- private final LRUCache adapterCache = new LRUCache(GROOVY_ADAPTER_CACHE_DEFAULT_SIZE);
+ private final LRUCache<CacheKey,ProxyGeneratorAdapter> adapterCache = new LRUCache<>(Integer.getInteger("groovy.adapter.cache.default.size", 64));
+
+ private boolean debug;
+ private boolean emptyMethods;
+ private ClassLoader override;
public boolean getDebug() {
return debug;
@@ -158,7 +156,6 @@ public class ProxyGenerator {
return instantiateAggregate(closureMap, interfaces, clazz, null);
}
- @SuppressWarnings("unchecked")
public GroovyObject instantiateAggregate(Map closureMap, List<Class> interfaces, Class clazz, Object[] constructorArgs) {
if (clazz != null && Modifier.isFinal(clazz.getModifiers())) {
throw new GroovyCastException("Cannot coerce a map to class " + clazz.getName() + " because it is a final class");
@@ -199,7 +196,6 @@ public class ProxyGenerator {
* @param name the name of the proxy, unused, but kept for compatibility with previous versions of Groovy.
* @return a proxy object implementing the specified interfaces, and delegating to the provided object
*/
- @SuppressWarnings("unchecked")
public GroovyObject instantiateDelegateWithBaseClass(Map closureMap, List<Class> interfaces, Object delegate, Class baseClass, String name) {
Map<Object,Object> map = closureMap != null ? closureMap : EMPTY_CLOSURE_MAP;
ProxyGeneratorAdapter adapter = createAdapter(map, interfaces, delegate.getClass(), baseClass);
@@ -207,33 +203,20 @@ public class ProxyGenerator {
return adapter.delegatingProxy(delegate, map, (Object[])null);
}
- private ProxyGeneratorAdapter createAdapter(Map closureMap, List<Class> interfaces, Class delegateClass, Class baseClass) {
- // According to https://shipilev.net/blog/2016/arrays-wisdom-ancients/#_conclusion
- // toArray(new T[0]) seems faster, safer, and contractually cleaner, and therefore should be the default choice now.
- Class[] intfs = interfaces != null ? interfaces.toArray(EMPTY_CLASS_ARRAY) : EMPTY_INTERFACE_ARRAY;
- Class base = baseClass;
- if (base == null) {
- if (intfs.length > 0) {
- base = intfs[0];
- } else {
- base = Object.class;
- }
+ private ProxyGeneratorAdapter createAdapter(final Map<Object,Object> closureMap, final List<Class> interfaceList, final Class delegateClass, final Class baseClass) {
+ Class[] interfaces = interfaceList != null ? interfaceList.toArray(EMPTY_CLASS_ARRAY) : EMPTY_CLASS_ARRAY;
+ final Class base = baseClass != null ? baseClass : (interfaces.length > 0 ? interfaces[0] : Object.class);
+ Set<String> methodNames = closureMap.isEmpty() ? Collections.emptySet() : new HashSet<>();
+ for (Object key : closureMap.keySet()) {
+ methodNames.add(key.toString());
}
- Set<String> keys = closureMap == EMPTY_CLOSURE_MAP ? EMPTY_KEYSET : new HashSet<String>();
- for (Object o : closureMap.keySet()) {
- keys.add(o.toString());
- }
- boolean useDelegate = null != delegateClass;
- CacheKey key = new CacheKey(base, useDelegate ? delegateClass : Object.class, keys, intfs, emptyMethods, useDelegate);
- final Class b = base;
-
- return (ProxyGeneratorAdapter) adapterCache.getAndPut(
- key,
- k -> new ProxyGeneratorAdapter(closureMap, b, intfs, useDelegate
- ? delegateClass.getClassLoader()
- : b.getClassLoader(), emptyMethods, useDelegate ? delegateClass : null
- )
- );
+ boolean useDelegate = (delegateClass != null);
+ CacheKey key = new CacheKey(base, useDelegate ? delegateClass : Object.class, methodNames, interfaces, emptyMethods, useDelegate);
+
+ return adapterCache.getAndPut(key, k -> {
+ ClassLoader classLoader = useDelegate ? delegateClass.getClassLoader() : base.getClassLoader();
+ return new ProxyGeneratorAdapter(closureMap, base, interfaces, classLoader, emptyMethods, useDelegate ? delegateClass : null);
+ });
}
private static void setMetaClass(final MetaClass metaClass) {
@@ -245,7 +228,9 @@ public class ProxyGenerator {
};
GroovySystem.getMetaClassRegistry().setMetaClass(ProxyGenerator.class, newMetaClass);
}
-
+
+ //--------------------------------------------------------------------------
+
private static final class CacheKey {
private static final Comparator<Class> INTERFACE_COMPARATOR = (o1, o2) -> {
// Traits order *must* be preserved
@@ -334,5 +319,4 @@ public class ProxyGenerator {
}
}
}
-
}
diff --git a/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java b/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java
index 4d0cd800e5..e78bd0e315 100644
--- a/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java
+++ b/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java
@@ -45,8 +45,6 @@ import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -84,47 +82,42 @@ import static org.codehaus.groovy.reflection.ReflectionUtils.isSealed;
*
* @since 2.0.0
*/
+@SuppressWarnings("rawtypes")
public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
private static final Map<String, Boolean> EMPTY_DELEGATECLOSURE_MAP = Collections.emptyMap();
- private static final Set<String> EMPTY_STRING_SET = Collections.emptySet();
+ private static final Object[] EMPTY_ARGS = new Object[0];
private static final String CLOSURES_MAP_FIELD = "$closures$delegate$map";
private static final String DELEGATE_OBJECT_FIELD = "$delegate";
+ private static final AtomicLong proxyCounter = new AtomicLong();
- private static final List<Method> OBJECT_METHODS = getInheritedMethods(Object.class, new ArrayList<>());
- private static final List<Method> GROOVYOBJECT_METHODS = getInheritedMethods(GroovyObject.class, new ArrayList<>());
-
- private static final AtomicLong pxyCounter = new AtomicLong();
- private static final Set<String> GROOVYOBJECT_METHOD_NAMESS;
- private static final Object[] EMPTY_ARGS = new Object[0];
- private static final String[] EMPTY_STRING_ARRAY = new String[0];
-
+ private static List<Method> OBJECT_METHODS = getInheritedMethods(Object.class, new ArrayList<>());
+ private static List<Method> GROOVYOBJECT_METHODS = getInheritedMethods(GroovyObject.class, new ArrayList<>());
+ private static final Set<String> GROOVYOBJECT_METHOD_NAMES;
static {
- List<String> names = new ArrayList<>();
+ Set<String> names = new HashSet<>();
for (Method method : GroovyObject.class.getMethods()) {
names.add(method.getName());
}
- GROOVYOBJECT_METHOD_NAMESS = new HashSet<>(names);
+ GROOVYOBJECT_METHOD_NAMES = Collections.unmodifiableSet(names);
}
- private final Class<?> superClass;
- private final Class<?> delegateClass;
- private final InnerLoader loader;
private final String proxyName;
- private final LinkedHashSet<Class> classList;
+ private final Class superClass;
+ private final Class delegateClass;
+ private final InnerLoader innerLoader;
+ private final Set<Class > implClasses;
+ private final Set<Object> visitedMethods;
+ private final Set<String> objectDelegateMethods;
private final Map<String, Boolean> delegatedClosures;
- // if emptyBody == true, then we generate an empty body instead throwing error on unimplemented methods
+ // if emptyBody is true, then we generate an empty body instead throwing error on unimplemented methods
private final boolean emptyBody;
private final boolean hasWildcard;
private final boolean generateDelegateField;
- private final Set<String> objectDelegateMethods;
-
- private final Set<Object> visitedMethods;
- // cached class
private final Class cachedClass;
- private final Constructor<?> cachedNoArgConstructor;
+ private final Constructor cachedNoArgConstructor;
/**
* Construct a proxy generator. This generator is used when we need to create a proxy object for a class or an
@@ -141,12 +134,12 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
public ProxyGeneratorAdapter(
final Map<Object, Object> closureMap,
final Class<?> superClass,
- final Class[] interfaces,
+ final Class[] interfaces,
final ClassLoader proxyLoader,
final boolean emptyBody,
final Class<?> delegateClass) {
super(CompilerConfiguration.ASM_API_VERSION, new ClassWriter(0));
- this.loader = proxyLoader != null ? createInnerLoader(proxyLoader, interfaces) : findClassLoader(superClass, interfaces);
+ this.innerLoader = proxyLoader != null ? createInnerLoader(proxyLoader, interfaces) : findClassLoader(superClass, interfaces);
this.visitedMethods = new LinkedHashSet<>();
this.delegatedClosures = closureMap.isEmpty() ? EMPTY_DELEGATECLOSURE_MAP : new HashMap<>();
boolean wildcard = false;
@@ -159,11 +152,11 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
}
this.hasWildcard = wildcard;
- Class<?> fixedSuperClass = adjustSuperClass(superClass, interfaces);
+ Class fixedSuperClass = adjustSuperClass(superClass, interfaces);
// if we have to delegate to another object, generate the appropriate delegate field
// and collect the name of the methods for which delegation is active
this.generateDelegateField = delegateClass != null;
- this.objectDelegateMethods = generateDelegateField ? createDelegateMethodList(fixedSuperClass, delegateClass, interfaces) : EMPTY_STRING_SET;
+ this.objectDelegateMethods = generateDelegateField ? createDelegateMethodList(fixedSuperClass, delegateClass, interfaces) : Collections.emptySet();
this.delegateClass = delegateClass;
// a proxy is supposed to be a concrete class, so it cannot extend an interface.
@@ -171,17 +164,17 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
// and add this interface to the list of implemented interfaces
this.superClass = fixedSuperClass;
- // create the base list of classes which have possible methods to be overloaded
- this.classList = new LinkedHashSet<>();
- this.classList.add(superClass);
+ // collect classes which have possible methods to be overloaded
+ implClasses = new LinkedHashSet<>();
+ implClasses.add(superClass);
if (generateDelegateField) {
- classList.add(delegateClass);
- for (Class<?> i : delegateClass.getInterfaces()) {
- if (!isSealed(i)) this.classList.add(i);
+ implClasses.add(delegateClass);
+ for (Class i : delegateClass.getInterfaces()) {
+ if (!isSealed(i)) implClasses.add(i);
}
}
if (interfaces != null) {
- Collections.addAll(this.classList, interfaces);
+ Collections.addAll(implClasses, interfaces);
}
this.proxyName = proxyName();
this.emptyBody = emptyBody;
@@ -190,11 +183,10 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
ClassWriter writer = (ClassWriter) cv;
this.visit(Opcodes.V1_5, ACC_PUBLIC, proxyName, null, null, null);
byte[] b = writer.toByteArray();
-// CheckClassAdapter.verify(new ClassReader(b), true, new PrintWriter(System.err));
- cachedClass = loader.defineClass(proxyName.replace('/', '.'), b);
+ cachedClass = innerLoader.defineClass(proxyName.replace('/', '.'), b);
// cache no-arg constructor
Class[] args = generateDelegateField ? new Class[]{Map.class, delegateClass} : new Class[]{Map.class};
- Constructor<?> constructor;
+ Constructor constructor;
try {
constructor = cachedClass.getConstructor(args);
} catch (NoSuchMethodException e) {
@@ -203,7 +195,7 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
cachedNoArgConstructor = constructor;
}
- private Class<?> adjustSuperClass(final Class<?> superClass, Class[] interfaces) {
+ private Class adjustSuperClass(final Class superClass, Class[] interfaces) {
if (!superClass.isInterface()) {
return superClass;
}
@@ -216,9 +208,9 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
if (!traits.isEmpty()) {
String name = superClass.getName() + "$TraitAdapter";
ClassNode cn = new ClassNode(name, ACC_PUBLIC | ACC_ABSTRACT, ClassHelper.OBJECT_TYPE, traits.toArray(ClassNode.EMPTY_ARRAY), null);
- CompilationUnit cu = new CompilationUnit(loader);
+ CompilationUnit cu = new CompilationUnit(innerLoader);
CompilerConfiguration config = new CompilerConfiguration();
- SourceUnit su = new SourceUnit(name + "wrapper", "", config, loader, new ErrorCollector(config));
+ SourceUnit su = new SourceUnit(name + "wrapper", "", config, innerLoader, new ErrorCollector(config));
cu.addSource(su);
cu.compile(Phases.CONVERSION);
su.getAST().addClass(cn);
@@ -226,7 +218,7 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
List<GroovyClass> classes = cu.getClasses();
for (GroovyClass groovyClass : classes) {
if (groovyClass.getName().equals(name)) {
- return loader.defineClass(name, groovyClass.getBytes());
+ return innerLoader.defineClass(name, groovyClass.getBytes());
}
}
}
@@ -236,7 +228,7 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
private static Set<ClassNode> collectTraits(final Class[] interfaces) {
Set<ClassNode> traits = new LinkedHashSet<>();
- for (Class<?> face : interfaces) {
+ for (Class face : interfaces) {
for (ClassNode node : ClassHelper.make(face).getAllInterfaces()) {
if (Traits.isTrait(node)) {
traits.add(node.getPlainNodeReference());
@@ -255,22 +247,22 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
}
private static InnerLoader createInnerLoader(final ClassLoader parent, final Class[] interfaces) {
- return AccessController.doPrivileged((PrivilegedAction<InnerLoader>) () -> new InnerLoader(parent, interfaces));
+ return java.security.AccessController.doPrivileged((java.security.PrivilegedAction<InnerLoader>) () -> new InnerLoader(parent, interfaces));
}
- private InnerLoader findClassLoader(Class<?> clazz, Class[] interfaces) {
+ private InnerLoader findClassLoader(final Class clazz, final Class[] interfaces) {
ClassLoader cl = clazz.getClassLoader();
if (cl == null) cl = this.getClass().getClassLoader();
return createInnerLoader(cl, interfaces);
}
- private static Set<String> createDelegateMethodList(Class<?> superClass, Class<?> delegateClass, Class[] interfaces) {
+ private static Set<String> createDelegateMethodList(final Class superClass, final Class delegateClass, final Class[] interfaces) {
Set<String> selectedMethods = new HashSet<>();
List<Method> interfaceMethods = new ArrayList<>();
List<Method> superClassMethods = new ArrayList<>();
Collections.addAll(superClassMethods, superClass.getDeclaredMethods());
if (interfaces != null) {
- for (Class<?> thisInterface : interfaces) {
+ for (Class thisInterface : interfaces) {
getInheritedMethods(thisInterface, interfaceMethods);
}
for (Method method : interfaceMethods) {
@@ -292,9 +284,9 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
return selectedMethods;
}
- private static List<Method> getInheritedMethods(Class<?> baseClass, List<Method> methods) {
+ private static List<Method> getInheritedMethods(final Class baseClass, final List<Method> methods) {
Collections.addAll(methods, baseClass.getMethods());
- Class<?> currentClass = baseClass;
+ Class currentClass = baseClass;
while (currentClass != null) {
Method[] protectedMethods = currentClass.getDeclaredMethods();
for (Method method : protectedMethods) {
@@ -308,7 +300,7 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
return methods;
}
- private static boolean containsEquivalentMethod(Collection<Method> publicAndProtectedMethods, Method candidate) {
+ private static boolean containsEquivalentMethod(final Collection<Method> publicAndProtectedMethods, final Method candidate) {
for (Method method : publicAndProtectedMethods) {
if (candidate.getName().equals(method.getName()) &&
candidate.getReturnType().equals(method.getReturnType()) &&
@@ -319,7 +311,7 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
return false;
}
- private static boolean hasMatchingParameterTypes(Method method, Method candidate) {
+ private static boolean hasMatchingParameterTypes(final Method method, final Method candidate) {
Class[] candidateParamTypes = candidate.getParameterTypes();
Class[] methodParamTypes = method.getParameterTypes();
if (candidateParamTypes.length != methodParamTypes.length) return false;
@@ -333,22 +325,22 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
public void visit(final int version, final int access, final String name, final String signature, final String superName, final String[] interfaces) {
Set<String> interfacesSet = new LinkedHashSet<>();
if (interfaces != null) Collections.addAll(interfacesSet, interfaces);
- for (Class<?> extraInterface : classList) {
+ for (Class extraInterface : implClasses) {
if (extraInterface.isInterface()) interfacesSet.add(BytecodeHelper.getClassInternalName(extraInterface));
}
final boolean addGroovyObjectSupport = !GroovyObject.class.isAssignableFrom(superClass);
if (addGroovyObjectSupport) interfacesSet.add("groovy/lang/GroovyObject");
if (generateDelegateField) {
- classList.add(GeneratedGroovyProxy.class);
+ implClasses.add(GeneratedGroovyProxy.class);
interfacesSet.add("groovy/lang/GeneratedGroovyProxy");
}
- super.visit(V1_5, ACC_PUBLIC, proxyName, signature, BytecodeHelper.getClassInternalName(superClass), interfacesSet.toArray(EMPTY_STRING_ARRAY));
+ super.visit(V1_5, ACC_PUBLIC, proxyName, signature, BytecodeHelper.getClassInternalName(superClass), interfacesSet.toArray(new String[0]));
visitMethod(ACC_PUBLIC, "<init>", "()V", null, null);
addDelegateFields();
if (addGroovyObjectSupport) {
createGroovyObjectSupport();
}
- for (Class<?> clazz : classList) {
+ for (Class clazz : implClasses) {
visitClass(clazz);
}
}
@@ -359,10 +351,10 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
*
* @param clazz an class for which to generate bytecode
*/
- private void visitClass(final Class<?> clazz) {
+ private void visitClass(final Class clazz) {
Method[] methods = clazz.getDeclaredMethods();
for (Method method : methods) {
- Class<?>[] exceptionTypes = method.getExceptionTypes();
+ Class[] exceptionTypes = method.getExceptionTypes();
String[] exceptions = new String[exceptionTypes.length];
for (int i = 0; i < exceptions.length; i++) {
exceptions[i] = BytecodeHelper.getClassInternalName(exceptionTypes[i]);
@@ -375,8 +367,8 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
exceptions);
}
Constructor[] constructors = clazz.getDeclaredConstructors();
- for (Constructor<?> method : constructors) {
- Class<?>[] exceptionTypes = method.getExceptionTypes();
+ for (Constructor method : constructors) {
+ Class[] exceptionTypes = method.getExceptionTypes();
String[] exceptions = new String[exceptionTypes.length];
for (int i = 0; i < exceptions.length; i++) {
exceptions[i] = BytecodeHelper.getClassInternalName(exceptionTypes[i]);
@@ -389,10 +381,10 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
exceptions);
}
- for (Class<?> intf : clazz.getInterfaces()) {
+ for (Class intf : clazz.getInterfaces()) {
visitClass(intf);
}
- Class<?> superclass = clazz.getSuperclass();
+ Class superclass = clazz.getSuperclass();
if (superclass != null) visitClass(superclass);
// Ultimately, methods can be available in the closure map which are not defined by the superclass
@@ -477,11 +469,11 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
name = name.substring(1, name.length() - 1) + "_array";
}
int index = name.lastIndexOf('.');
- if (index == -1) return name + pxyCounter.incrementAndGet() + "_groovyProxy";
- return name.substring(index + 1) + pxyCounter.incrementAndGet() + "_groovyProxy";
+ if (index == -1) return name + proxyCounter.incrementAndGet() + "_groovyProxy";
+ return name.substring(index + 1) + proxyCounter.incrementAndGet() + "_groovyProxy";
}
- private static boolean isImplemented(Class<?> clazz, String name, String desc) {
+ private static boolean isImplemented(final Class clazz, final String name, final String desc) {
Method[] methods = clazz.getDeclaredMethods();
for (Method method : methods) {
if (method.getName().equals(name)) {
@@ -490,7 +482,7 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
}
}
}
- Class<?> parent = clazz.getSuperclass();
+ Class parent = clazz.getSuperclass();
return parent != null && isImplemented(parent, name, desc);
}
@@ -507,9 +499,10 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
boolean wildcardDelegate = hasWildcard && !"<init>".equals(name);
if ((objectDelegate || closureDelegate || wildcardDelegate) && !Modifier.isStatic(access)) {
- if (!GROOVYOBJECT_METHOD_NAMESS.contains(name)
- // GROOVY-8244: proxy for abstract class/trait/interface only overrides abstract method(s)
- && (!Modifier.isAbstract(superClass.getModifiers()) || !isImplemented(superClass, name, desc))) {
+ if (!GROOVYOBJECT_METHOD_NAMES.contains(name)
+ // GROOVY-8244, GROOVY-10717: replace if abstract or no abstract overload exists
+ && (!Modifier.isAbstract(superClass.getModifiers()) || !isImplemented(superClass, name, desc)
+ || ((objectDelegate || closureDelegate) && Arrays.stream(superClass.getMethods()).filter(m -> m.getName().equals(name)).mapToInt(Method::getModifiers).noneMatch(Modifier::isAbstract)))) {
if (closureDelegate || wildcardDelegate || !(objectDelegate && generateDelegateField)) {
delegatedClosures.put(name, Boolean.TRUE);
@@ -523,7 +516,7 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
} else if ("<init>".equals(name) && (Modifier.isPublic(access) || Modifier.isProtected(access))) {
return createConstructor(access, name, desc, signature, exceptions);
- } else if (Modifier.isAbstract(access) && !GROOVYOBJECT_METHOD_NAMESS.contains(name) && !isImplemented(superClass, name, desc)) {
+ } else if (Modifier.isAbstract(access) && !GROOVYOBJECT_METHOD_NAMES.contains(name) && !isImplemented(superClass, name, desc)) {
MethodVisitor mv = super.visitMethod(access & ~ACC_ABSTRACT, name, desc, signature, exceptions);
mv.visitCode();
Type[] args = Type.getArgumentTypes(desc);
@@ -625,7 +618,7 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
return null;
}
- private void initializeDelegateClosure(final MethodVisitor mv, Type[] args) {
+ private void initializeDelegateClosure(final MethodVisitor mv, final Type[] args) {
int idx = 1 + getTypeArgsRegisterLength(args);
mv.visitIntInsn(ALOAD, 0); // this
@@ -634,7 +627,7 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
mv.visitFieldInsn(PUTFIELD, proxyName, CLOSURES_MAP_FIELD, "Ljava/util/Map;");
}
- private void initializeDelegateObject(final MethodVisitor mv, Type[] args) {
+ private void initializeDelegateObject(final MethodVisitor mv, final Type[] args) {
int idx = 2 + getTypeArgsRegisterLength(args);
mv.visitIntInsn(ALOAD, 0); // this
@@ -684,8 +677,6 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
protected MethodVisitor makeDelegateToClosureCall(final String name, final String desc, final String signature, final String[] exceptions, final int accessFlags) {
MethodVisitor mv = super.visitMethod(accessFlags, name, desc, signature, exceptions);
-// TraceMethodVisitor tmv = new TraceMethodVisitor(mv);
-// mv = tmv;
mv.visitCode();
int stackSize = 0;
// method body should be:
@@ -732,11 +723,10 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
unwrapResult(mv, desc);
mv.visitMaxs(stackSize, arrayStore + 1);
mv.visitEnd();
-// System.out.println("tmv.getText() = " + tmv.getText());
return null;
}
- private void boxPrimitiveType(MethodVisitor mv, int idx, Type arg) {
+ private void boxPrimitiveType(final MethodVisitor mv, final int idx, final Type arg) {
if (isPrimitive(arg)) {
mv.visitIntInsn(getLoadInsn(arg), idx);
String wrappedType = getWrappedClassDescriptor(arg);
@@ -762,7 +752,7 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
}
@SuppressWarnings("unchecked")
- public GroovyObject proxy(Map<Object, Object> map, Object... constructorArgs) {
+ public GroovyObject proxy(final Map<Object, Object> map, Object... constructorArgs) {
if (constructorArgs == null && cachedNoArgConstructor != null) {
// if there isn't any argument, we can make invocation faster using the cached constructor
try {
@@ -779,7 +769,7 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
}
@SuppressWarnings("unchecked")
- public GroovyObject delegatingProxy(Object delegate, Map<Object, Object> map, Object... constructorArgs) {
+ public GroovyObject delegatingProxy(final Object delegate, final Map<Object, Object> map, Object... constructorArgs) {
if (constructorArgs == null && cachedNoArgConstructor != null) {
// if there isn't any argument, we can make invocation faster using the cached constructor
try {
@@ -802,25 +792,25 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
* Do not trust IDEs, this method is used in bytecode.
*/
@SuppressWarnings("unchecked")
- public static Closure ensureClosure(Object o) {
+ public static Closure ensureClosure(final Object o) {
if (o == null) throw new UnsupportedOperationException();
if (o instanceof Closure) return (Closure) o;
return new ReturnValueWrappingClosure(o);
}
- private static int getLoadInsn(Type type) {
+ private static int getLoadInsn(final Type type) {
return TypeUtil.getLoadInsnByType(type);
}
- private static int getReturnInsn(Type type) {
+ private static int getReturnInsn(final Type type) {
return TypeUtil.getReturnInsnByType(type);
}
- private static boolean isPrimitive(Type type) {
+ private static boolean isPrimitive(final Type type) {
return TypeUtil.isPrimitiveType(type);
}
- private static String getWrappedClassDescriptor(Type type) {
+ private static String getWrappedClassDescriptor(final Type type) {
return TypeUtil.getWrappedClassDescriptor(type);
}
@@ -830,7 +820,7 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
protected InnerLoader(final ClassLoader parent, final Class[] interfaces) {
super(parent);
if (interfaces != null) {
- for (Class<?> c : interfaces) {
+ for (Class c : interfaces) {
if (c.getClassLoader() != parent) {
if (internalClassLoaders == null)
internalClassLoaders = new ArrayList<>(interfaces.length);
@@ -842,13 +832,15 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
}
}
- public Class defineClass(String name, byte[] data) {
+ @Override
+ public Class defineClass(final String name, final byte[] data) {
return super.defineClass(name, data, 0, data.length);
}
- public Class loadClass(String name) throws ClassNotFoundException {
+ @Override
+ public Class loadClass(final String name) throws ClassNotFoundException {
// First check whether it's already been loaded, if so use it
- Class<?> loadedClass = findLoadedClass(name);
+ Class loadedClass = findLoadedClass(name);
if (loadedClass != null) return loadedClass;
@@ -893,7 +885,7 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
private static final long serialVersionUID = 1313135457715304501L;
private final V value;
- public ReturnValueWrappingClosure(V returnValue) {
+ public ReturnValueWrappingClosure(final V returnValue) {
super(null);
value = returnValue;
}
@@ -903,5 +895,4 @@ public class ProxyGeneratorAdapter extends ClassVisitor implements Opcodes {
return value;
}
}
-
}
diff --git a/src/spec/test/CoercionTest.groovy b/src/spec/test/CoercionTest.groovy
index ff44750557..c0dd0e9688 100644
--- a/src/spec/test/CoercionTest.groovy
+++ b/src/spec/test/CoercionTest.groovy
@@ -1,5 +1,3 @@
-import groovy.test.GroovyTestCase
-
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
@@ -18,7 +16,8 @@ import groovy.test.GroovyTestCase
* specific language governing permissions and limitations
* under the License.
*/
-class CoercionTest extends GroovyTestCase {
+
+final class CoercionTest extends groovy.test.GroovyTestCase {
void testStringToEnumValue() {
assertScript '''
@@ -299,6 +298,34 @@ println iter.next()
assert map.i==0
// end::use_coerced_iterator[]
'''
+ // "remove()" is an interface default method
+ assertScript '''import static groovy.test.GroovyAssert.shouldFail
+
+ def iter = [next: {->null}, hasNext: {->true}] as Iterator
+ shouldFail(UnsupportedOperationException) {
+ iter.remove()
+ }
+
+ iter = [next: {->null}, hasNext: {->true}, remove: {->}] as Iterator
+ iter.remove()
+ '''
+ }
+
+ // GROOVY-10717
+ void testCoerceMapToAbstract() {
+ assertScript '''import static groovy.test.GroovyAssert.shouldFail
+
+ abstract class A {
+ String b,c
+ }
+
+ def a = [getB: {->'bb'}, getX: {->'xx'}] as A
+ assert a.b == 'bb' // overrides accessor
+ assert a.c == null // generated accessor
+ shouldFail(MissingPropertyException) {
+ a.x
+ }
+ '''
}
void testCoerceThrowsNPE() {