Skip to content
Permalink
Browse files
GROOVY-8244: proxy for trait/interface only overrides abstract method(s)
  • Loading branch information
eric-milles committed Mar 25, 2022
1 parent 72481ef commit aa173c4d5740a3c3e3dcb901a2c30e64c4891fce
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 58 deletions.
@@ -228,19 +228,19 @@ public void visitClass(final ClassNode node) {
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 @@ protected void addPropertyMethod(final MethodNode method) {
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 interface DefaultArgsAction {
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 @@ protected void addDefaultParameterMethods(final ClassNode type) {
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,15 +921,15 @@ public void visitVariableExpression(final VariableExpression e) {
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
localVariable = localVarX(p.getName(), p.getType());
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 void visitVariableExpression(final VariableExpression e) {

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 void visitVariableExpression(final VariableExpression e) {
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 void visitConstructorCallExpression(final ConstructorCallExpression call)
super.visitConstructorCallExpression(call);
}
};
visitor.visitBlockStatement(code);
visitor.visitBlockStatement(block);

addPropertyMethod(newMethod);
newMethod.putNodeMetaData(DEFAULT_PARAMETER_GENERATED, Boolean.TRUE);
@@ -63,6 +63,7 @@
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 @@ private static boolean isImplemented(final Class<?> clazz, final String name, fi

@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 MethodVisitor visitMethod(final int access, final String name, final Stri
}
mv.visitEnd();
}

return null;
}

@@ -156,7 +156,12 @@ private static void replaceExtendsByImplements(final ClassNode cNode) {
}

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 @@ private Statement processBody(final VariableExpression thisObject, final Stateme

//--------------------------------------------------------------------------

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;

@@ -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 '''

0 comments on commit aa173c4

Please sign in to comment.