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 14:00:45 UTC
[groovy] branch master updated: GROOVY-8243: support closure for functional interface that extends trait
This is an automated email from the ASF dual-hosted git repository.
emilles 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 4d9c61cb05 GROOVY-8243: support closure for functional interface that extends trait
4d9c61cb05 is described below
commit 4d9c61cb054a780e04d13fe52df185ac5c22d28c
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Oct 12 08:42:24 2022 -0500
GROOVY-8243: support closure for functional interface that extends trait
---
.../reflection/stdclasses/CachedSAMClass.java | 151 ++++++++++-----------
.../groovy/runtime/DefaultGroovyMethods.java | 10 +-
.../traitx/TraitASTTransformationTest.groovy | 8 +-
3 files changed, 81 insertions(+), 88 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/reflection/stdclasses/CachedSAMClass.java b/src/main/java/org/codehaus/groovy/reflection/stdclasses/CachedSAMClass.java
index 7483619106..e75b9eb5e8 100644
--- a/src/main/java/org/codehaus/groovy/reflection/stdclasses/CachedSAMClass.java
+++ b/src/main/java/org/codehaus/groovy/reflection/stdclasses/CachedSAMClass.java
@@ -31,72 +31,83 @@ import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.Proxy;
import java.security.PrivilegedAction;
+import java.util.ArrayDeque;
import java.util.Arrays;
import java.util.Collections;
+import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
-import java.util.Map;
+import java.util.Queue;
import java.util.Set;
import java.util.stream.Collectors;
public class CachedSAMClass extends CachedClass {
- private static final int ABSTRACT_STATIC_PRIVATE = Modifier.ABSTRACT | Modifier.PRIVATE | Modifier.STATIC;
- private static final int VISIBILITY = 5; // public|protected
- private static final Method[] EMPTY_METHOD_ARRAY = new Method[0];
private final Method method;
- public CachedSAMClass(Class klazz, ClassInfo classInfo) {
- super(klazz, classInfo);
- method = getSAMMethod(klazz);
+ public CachedSAMClass(Class clazz, ClassInfo classInfo) {
+ super(clazz, classInfo);
+ method = getSAMMethod(clazz);
if (method == null) throw new GroovyBugError("assigned method should not have been null!");
}
@Override
public boolean isAssignableFrom(Class argument) {
- return argument == null ||
- Closure.class.isAssignableFrom(argument) ||
- ReflectionCache.isAssignableFrom(getTheClass(), argument);
+ return argument == null
+ || Closure.class.isAssignableFrom(argument)
+ || ReflectionCache.isAssignableFrom(getTheClass(), argument);
+ }
+
+ @Override
+ public Object coerceArgument(Object argument) {
+ if (argument instanceof Closure) {
+ Class<?> clazz = getTheClass();
+ return coerceToSAM((Closure<?>) argument, method, clazz);
+ } else {
+ return argument;
+ }
}
+ //--------------------------------------------------------------------------
+
+ private static final Method[] EMPTY_METHOD_ARRAY = new Method[0];
+ private static final int PUBLIC_OR_PROTECTED = Modifier.PUBLIC | Modifier.PROTECTED;
+ private static final int ABSTRACT_STATIC_PRIVATE = Modifier.ABSTRACT | Modifier.STATIC | Modifier.PRIVATE;
+ private static final Set<String> OBJECT_METHOD_NAMES = Arrays.stream(Object.class.getMethods()).map(Method::getName).collect(Collectors.toUnmodifiableSet());
+
public static Object coerceToSAM(Closure argument, Method method, Class clazz) {
return coerceToSAM(argument, method, clazz, clazz.isInterface());
}
- /* Should we make the following method private? */
- @SuppressWarnings("unchecked")
public static Object coerceToSAM(Closure argument, Method method, Class clazz, boolean isInterface) {
if (argument != null && clazz.isAssignableFrom(argument.getClass())) {
return argument;
}
- final String name = method.getName();
- if (isInterface) {
- if (Traits.isTrait(clazz)) {
- Map<String, Closure> impl = Collections.singletonMap(name, argument);
- return ProxyGenerator.INSTANCE.instantiateAggregate(impl, Collections.singletonList(clazz));
- }
- return Proxy.newProxyInstance(
- clazz.getClassLoader(),
- new Class[] { clazz },
- new ConvertedClosure(argument));
+ if (!isInterface) {
+ return ProxyGenerator.INSTANCE.instantiateAggregateFromBaseClass(Collections.singletonMap(method.getName(), argument), clazz);
+ } else if (method != null && isOrImplementsTrait(clazz)) { // GROOVY-8243
+ return ProxyGenerator.INSTANCE.instantiateAggregate(Collections.singletonMap(method.getName(), argument), Collections.singletonList(clazz));
} else {
- Map<String, Object> m = Collections.singletonMap(name, argument);
- return ProxyGenerator.INSTANCE.instantiateAggregateFromBaseClass(m, clazz);
+ return Proxy.newProxyInstance(clazz.getClassLoader(), new Class[]{clazz}, new ConvertedClosure(argument));
}
}
- @Override
- public Object coerceArgument(Object argument) {
- if (argument instanceof Closure) {
- Class clazz = getTheClass();
- return coerceToSAM((Closure) argument, method, clazz);
- } else {
- return argument;
+ private static boolean isOrImplementsTrait(Class<?> c) {
+ if (Traits.isTrait(c)) return true; // quick check
+
+ Queue<Class<?>> todo = new ArrayDeque<>(Arrays.asList(c.getInterfaces()));
+ Set<Class<?>> done = new HashSet<>();
+ while ((c = todo.poll()) != null) {
+ if (done.add(c)) {
+ if (Traits.isTrait(c)) return true;
+ Collections.addAll(todo, c.getInterfaces());
+ }
}
+ return false;
}
- @SuppressWarnings("removal") // TODO a future Groovy version should remove the security check
+ @SuppressWarnings("removal") // TODO: a future Groovy version should remove the security check
private static Method[] getDeclaredMethods(final Class<?> c) {
try {
Method[] methods = java.security.AccessController.doPrivileged((PrivilegedAction<Method[]>) c::getDeclaredMethods);
@@ -127,7 +138,7 @@ public class CachedSAMClass extends CachedClass {
found = c.getMethod(m.getName(), m.getParameterTypes());
final int modifiers = found.getModifiers();
int asp = modifiers & ABSTRACT_STATIC_PRIVATE;
- int visible = modifiers & VISIBILITY;
+ int visible = modifiers & PUBLIC_OR_PROTECTED;
if (visible != 0 && asp == 0) return true;
} catch (NoSuchMethodException ignore) {
}
@@ -150,56 +161,44 @@ public class CachedSAMClass extends CachedClass {
}
/**
- * returns the abstract method from a SAM type, if it is a SAM type.
- * @param c the SAM class
- * @return null if nothing was found, the method otherwise
+ * Finds the abstract method of given class, if it is a SAM type.
*/
public static Method getSAMMethod(Class<?> c) {
- try {
- return getSAMMethodImpl(c);
- } catch (NoClassDefFoundError ignore) {
- return null;
- }
- }
-
- private static Method getSAMMethodImpl(Class<?> c) {
- // SAM = single public abstract method
// if the class is not abstract there is no abstract method
if (!Modifier.isAbstract(c.getModifiers())) return null;
- if (c.isInterface()) {
- // res stores the first found abstract method
- Method res = null;
- for (Method mi : c.getMethods()) {
- // ignore methods, that are not abstract and from Object
- if (!Modifier.isAbstract(mi.getModifiers())) continue;
- // ignore trait methods which have a default implementation
- if (mi.getAnnotation(Traits.Implemented.class) != null) continue;
-
- final String name = mi.getName();
- // avoid throwing `NoSuchMethodException` as possible as we could
- if (OBJECT_METHOD_NAME_SET.contains(name)) {
- try {
- Object.class.getMethod(name, mi.getParameterTypes());
- continue;
- } catch (NoSuchMethodException ignore) {
+ try {
+ if (c.isInterface()) {
+ // res stores the first found abstract method
+ Method res = null;
+ for (Method mi : c.getMethods()) {
+ // ignore methods, that are not abstract and from Object
+ if (!Modifier.isAbstract(mi.getModifiers())) continue;
+ // ignore trait methods which have a default implementation
+ if (mi.getAnnotation(Traits.Implemented.class) != null) continue;
+
+ String name = mi.getName();
+ // avoid throwing `NoSuchMethodException` as possible as we could
+ if (OBJECT_METHOD_NAMES.contains(name)) {
+ try {
+ Object.class.getMethod(name, mi.getParameterTypes());
+ continue;
+ } catch (NoSuchMethodException ignore) {
+ }
}
+ // we have two methods, so no SAM
+ if (res != null) return null;
+ res = mi;
}
-
- // we have two methods, so no SAM
- if (res != null) return null;
- res = mi;
+ return res;
+ } else {
+ List<Method> methods = new LinkedList<>();
+ getAbstractMethods(c, methods);
+ if (methods.isEmpty()) return null;
+ methods.removeIf(m -> hasUsableImplementation(c, m));
+ return getSingleNonDuplicateMethod(methods);
}
- return res;
-
- } else {
- List<Method> methods = new LinkedList<>();
- getAbstractMethods(c, methods);
- if (methods.isEmpty()) return null;
- methods.removeIf(m -> hasUsableImplementation(c, m));
- return getSingleNonDuplicateMethod(methods);
+ } catch (NoClassDefFoundError ignore) {
+ return null;
}
}
-
- private static final Set<String> OBJECT_METHOD_NAME_SET =
- Collections.unmodifiableSet(Arrays.stream(Object.class.getMethods()).map(m -> m.getName()).collect(Collectors.toSet()));
}
diff --git a/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java b/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
index 737461feb1..19d59f935e 100644
--- a/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
+++ b/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
@@ -57,7 +57,6 @@ import groovy.util.PermutationGenerator;
import groovy.util.ProxyGenerator;
import org.apache.groovy.io.StringBuilderWriter;
import org.apache.groovy.util.ReversedList;
-import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.classgen.Verifier;
import org.codehaus.groovy.reflection.ClassInfo;
import org.codehaus.groovy.reflection.MixinInMetaClass;
@@ -121,7 +120,6 @@ import java.io.Writer;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Array;
import java.lang.reflect.Field;
-import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.Proxy;
import java.math.BigDecimal;
@@ -11972,13 +11970,7 @@ public class DefaultGroovyMethods extends DefaultGroovyMethodsSupport {
@SuppressWarnings("unchecked")
public static <T> T asType(final Closure impl, final Class<T> type) {
if (type.isInterface() && !type.isInstance(impl)) {
- if (Traits.isTrait(type) || ClassHelper.make(type).getAllInterfaces().stream().anyMatch(Traits::isTrait)) { // GROOVY-8243
- Method sam = CachedSAMClass.getSAMMethod(type);
- if (sam != null) {
- return (T) ProxyGenerator.INSTANCE.instantiateAggregate(Collections.singletonMap(sam.getName(), impl), Collections.singletonList(type));
- }
- }
- return (T) Proxy.newProxyInstance(type.getClassLoader(), new Class[]{type}, new ConvertedClosure(impl));
+ return (T) CachedSAMClass.coerceToSAM(impl, CachedSAMClass.getSAMMethod(type), type, true);
}
try {
diff --git a/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy b/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
index 4763b3701c..1af26fb1c3 100644
--- a/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
@@ -1576,13 +1576,15 @@ final class TraitASTTransformationTest {
void testSAMCoercion5() {
assertScript shell, '''
trait T {
- abstract def foo(int i)
- def bar(double j) { "trait $j".toString() }
+ abstract foo(int n)
+ def bar(double n) {
+ "trait $n".toString()
+ }
}
interface I extends T {
}
- def obj = { "proxy $it".toString() } as I
+ I obj = { "proxy $it".toString() }
assert obj.foo(123) == 'proxy 123'
assert obj.bar(4.5) == 'trait 4.5'
'''