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/03/25 20:15:15 UTC
[groovy] branch GROOVY_4_0_X updated: GROOVY-8244: proxy for trait/interface only overrides abstract method(s)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_4_0_X by this push:
new 06b6448 GROOVY-8244: proxy for trait/interface only overrides abstract method(s)
06b6448 is described below
commit 06b64482cbcfb440eb5a55fd48686042f52cc575
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Mar 25 14:45:05 2022 -0500
GROOVY-8244: proxy for trait/interface only overrides abstract method(s)
---
.../org/codehaus/groovy/classgen/Verifier.java | 43 ++++++++++---------
.../groovy/runtime/ProxyGeneratorAdapter.java | 49 ++++++++++------------
.../transform/trait/TraitASTTransformation.java | 16 +++----
.../traitx/TraitASTTransformationTest.groovy | 13 ++++++
4 files changed, 63 insertions(+), 58 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index 78db90a..6eed40f 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -228,19 +228,19 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
if (node.isRecord()) {
detectInvalidRecordComponentNames(node);
if (node.isAbstract()) {
- throw new RuntimeParserException("Record '" + classNode.getNameWithoutPackage() + "' must not be abstract", classNode);
+ throw new RuntimeParserException("Record '" + node.getNameWithoutPackage() + "' must not be abstract", node);
}
}
checkForDuplicateInterfaces(node);
- if (classNode.isInterface() || Traits.isTrait(node)) {
+ if (node.isInterface() || Traits.isTrait(node)) {
// interfaces have no constructors but this expects one,
// so create a dummy but do not add it to the class node
addInitialization(node, new ConstructorNode(0, null));
node.visitContents(this);
- if (classNode.getNodeMetaData(ClassNodeSkip.class) == null) {
- classNode.setNodeMetaData(ClassNodeSkip.class, Boolean.TRUE);
+ if (node.getNodeMetaData(ClassNodeSkip.class) == null) {
+ node.setNodeMetaData(ClassNodeSkip.class, Boolean.TRUE);
}
return;
}
@@ -855,18 +855,20 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
Parameter[] parameters = method.getParameters();
ClassNode methodReturnType = method.getReturnType();
for (MethodNode node : classNode.getAbstractMethods()) {
- if (!node.getDeclaringClass().equals(classNode)) continue;
- if (node.getName().equals(methodName) && node.getParameters().length == parameters.length) {
- if (parameters.length == 1) {
- // setter
+ if (node.getName().equals(methodName)
+ && node.getDeclaringClass().equals(classNode)
+ && node.getParameters().length == parameters.length) {
+ if (parameters.length == 1) { // setter
ClassNode abstractMethodParameterType = node.getParameters()[0].getType();
ClassNode methodParameterType = parameters[0].getType();
- if (!methodParameterType.isDerivedFrom(abstractMethodParameterType) && !methodParameterType.implementsInterface(abstractMethodParameterType)) {
+ if (!methodParameterType.isDerivedFrom(abstractMethodParameterType)
+ && !methodParameterType.implementsInterface(abstractMethodParameterType)) {
continue;
}
}
ClassNode nodeReturnType = node.getReturnType();
- if (!methodReturnType.isDerivedFrom(nodeReturnType) && !methodReturnType.implementsInterface(nodeReturnType)) {
+ if (!methodReturnType.isDerivedFrom(nodeReturnType)
+ && !methodReturnType.implementsInterface(nodeReturnType)) {
continue;
}
// matching method, remove abstract status and use the same body
@@ -887,9 +889,9 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
protected void addDefaultParameterMethods(final ClassNode type) {
List<MethodNode> methods = new ArrayList<>(type.getMethods());
addDefaultParameters(methods, (arguments, params, method) -> {
- BlockStatement code = new BlockStatement();
+ BlockStatement block = new BlockStatement();
- MethodNode newMethod = new MethodNode(method.getName(), method.getModifiers(), method.getReturnType(), params, method.getExceptions(), code);
+ MethodNode newMethod = new MethodNode(method.getName(), method.getModifiers() & ~ACC_ABSTRACT, method.getReturnType(), params, method.getExceptions(), block);
MethodNode oldMethod = type.getDeclaredMethod(method.getName(), params);
if (oldMethod != null) {
@@ -900,10 +902,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
sourceOf(method));
}
- List<AnnotationNode> annotations = method.getAnnotations();
- if (annotations != null && !annotations.isEmpty()) {
- newMethod.addAnnotations(annotations);
- }
+ newMethod.addAnnotations(method.getAnnotations());
newMethod.setGenericsTypes(method.getGenericsTypes());
// GROOVY-5632, GROOVY-9151: check for references to parameters that have been removed
@@ -922,7 +921,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
if (e.getAccessedVariable() instanceof Parameter) {
Parameter p = (Parameter) e.getAccessedVariable();
if (p.hasInitialExpression() && !Arrays.asList(params).contains(p)) {
- VariableScope blockScope = code.getVariableScope();
+ VariableScope blockScope = block.getVariableScope();
VariableExpression localVariable = (VariableExpression) blockScope.getDeclaredVariable(p.getName());
if (localVariable == null) {
// create a variable declaration so that the name can be found in the new method
@@ -930,7 +929,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
localVariable.setModifiers(p.getModifiers());
blockScope.putDeclaredVariable(localVariable);
localVariable.setInStaticContext(blockScope.isInStaticContext());
- code.addStatement(declS(localVariable, p.getInitialExpression()));
+ block.addStatement(declS(localVariable, p.getInitialExpression()));
}
if (!localVariable.isClosureSharedVariable()) {
localVariable.setClosureSharedVariable(inClosure);
@@ -950,7 +949,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
for (Parameter p : method.getParameters()) {
if (p.hasInitialExpression() && p.getInitialExpression() == argument) {
- if (code.getVariableScope().getDeclaredVariable(p.getName()) != null) {
+ if (block.getVariableScope().getDeclaredVariable(p.getName()) != null) {
it.set(varX(p.getName()));
}
break;
@@ -964,9 +963,9 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
call.setImplicitThis(true);
if (method.isVoidMethod()) {
- code.addStatement(new ExpressionStatement(call));
+ block.addStatement(new ExpressionStatement(call));
} else {
- code.addStatement(new ReturnStatement(call));
+ block.addStatement(new ReturnStatement(call));
}
// GROOVY-5681: set anon. inner enclosing method reference
@@ -979,7 +978,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
super.visitConstructorCallExpression(call);
}
};
- visitor.visitBlockStatement(code);
+ visitor.visitBlockStatement(block);
addPropertyMethod(newMethod);
newMethod.putNodeMetaData(DEFAULT_PARAMETER_GENERATED, Boolean.TRUE);
diff --git a/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java b/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java
index 13e571f..5a3d380 100644
--- a/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java
+++ b/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java
@@ -63,6 +63,7 @@ import static org.codehaus.groovy.reflection.ReflectionUtils.isSealed;
import static org.objectweb.asm.Opcodes.AASTORE;
import static org.objectweb.asm.Opcodes.ACC_ABSTRACT;
import static org.objectweb.asm.Opcodes.ACC_FINAL;
+import static org.objectweb.asm.Opcodes.ACC_NATIVE;
import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
@@ -527,40 +528,35 @@ public class ProxyGeneratorAdapter extends ClassVisitor {
@Override
public MethodVisitor visitMethod(final int access, final String name, final String desc, final String signature, final String[] exceptions) {
+ // do not generate bytecode for final, native, private or synthetic methods
+ if ((access & (ACC_FINAL | ACC_NATIVE | ACC_PRIVATE | ACC_SYNTHETIC)) != 0) return null;
+
Object key = Arrays.asList(name, desc);
- if (visitedMethods.contains(key)) return null;
- if (Modifier.isPrivate(access) || Modifier.isNative(access) || ((access & ACC_SYNTHETIC) != 0)) {
- // do not generate bytecode for private methods
- return null;
- }
- int accessFlags = access;
- visitedMethods.add(key);
- if ((objectDelegateMethods.contains(name + desc) || delegatedClosures.containsKey(name) || (!"<init>".equals(name) && hasWildcard)) && !Modifier.isStatic(access) && !Modifier.isFinal(access)) {
- if (!GROOVYOBJECT_METHOD_NAMESS.contains(name)) {
- if (Modifier.isAbstract(access)) {
- // prevents the proxy from being abstract
- accessFlags -= ACC_ABSTRACT;
- }
- if (delegatedClosures.containsKey(name) || (!"<init>".equals(name) && hasWildcard)) {
+ if (!visitedMethods.add(key)) return null;
+
+ boolean objectDelegate = objectDelegateMethods.contains(name + desc);
+ boolean closureDelegate = delegatedClosures.containsKey(name);
+ 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 (closureDelegate || wildcardDelegate || !(objectDelegate && generateDelegateField)) {
delegatedClosures.put(name, Boolean.TRUE);
- return makeDelegateToClosureCall(name, desc, signature, exceptions, accessFlags);
+ return makeDelegateToClosureCall(name, desc, signature, exceptions, access & ~ACC_ABSTRACT);
}
- if (generateDelegateField && objectDelegateMethods.contains(name + desc)) {
- return makeDelegateCall(name, desc, signature, exceptions, accessFlags);
- }
- delegatedClosures.put(name, Boolean.TRUE);
- return makeDelegateToClosureCall(name, desc, signature, exceptions, accessFlags);
+ return makeDelegateCall(name, desc, signature, exceptions, access & ~ACC_ABSTRACT);
}
} else if ("getProxyTarget".equals(name) && "()Ljava/lang/Object;".equals(desc)) {
return createGetProxyTargetMethod(access, name, desc, signature, exceptions);
+
} 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)) {
- if (isImplemented(superClass, name, desc)) {
- return null;
- }
- accessFlags -= ACC_ABSTRACT;
- MethodVisitor mv = super.visitMethod(accessFlags, name, desc, signature, exceptions);
+
+ } else if (Modifier.isAbstract(access) && !GROOVYOBJECT_METHOD_NAMESS.contains(name) && !isImplemented(superClass, name, desc)) {
+ MethodVisitor mv = super.visitMethod(access & ~ACC_ABSTRACT, name, desc, signature, exceptions);
mv.visitCode();
Type[] args = Type.getArgumentTypes(desc);
if (emptyBody) {
@@ -599,6 +595,7 @@ public class ProxyGeneratorAdapter extends ClassVisitor {
}
mv.visitEnd();
}
+
return null;
}
diff --git a/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
index 902870e..4f89097 100644
--- a/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
@@ -156,7 +156,12 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
}
private static void generateMethodsWithDefaultArgs(final ClassNode cNode) {
- new DefaultArgsMethodsAdder().addDefaultParameterMethods(cNode);
+ new Verifier() {
+ @Override
+ public void addDefaultParameterMethods(final ClassNode cn) {
+ setClassNode(cn); super.addDefaultParameterMethods(cn);
+ }
+ }.addDefaultParameterMethods(cNode);
}
private ClassNode createHelperClass(final ClassNode cNode) {
@@ -631,15 +636,6 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
//--------------------------------------------------------------------------
- private static class DefaultArgsMethodsAdder extends Verifier {
-
- @Override
- public void addDefaultParameterMethods(final ClassNode node) {
- setClassNode(node);
- super.addDefaultParameterMethods(node);
- }
- }
-
private static class PostTypeCheckingExpressionReplacer extends ClassCodeExpressionTransformer {
private final SourceUnit sourceUnit;
diff --git a/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy b/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
index b266c38..3f429db 100644
--- a/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
@@ -1591,6 +1591,19 @@ final class TraitASTTransformationTest {
'''
}
+ @Test // GROOVY-8244
+ void testSAMCoercion6() {
+ assertScript '''
+ trait T {
+ abstract def foo(int a, int b = 2)
+ }
+ T t = { int a, int b ->
+ return a + b
+ }
+ assert t.foo(40) == 42
+ '''
+ }
+
@Test
void testMethodMissingInTrait() {
assertScript '''