You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2022/02/14 07:17:15 UTC
[groovy] branch master updated: GROOVY-10472: @AutoImplement is failing when covariant returns are involved
This is an automated email from the ASF dual-hosted git repository.
paulk 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 5601ea1 GROOVY-10472: @AutoImplement is failing when covariant returns are involved
5601ea1 is described below
commit 5601ea14304b67e71586b0196e38f90fa1a46f5b
Author: Paul King <pa...@asert.com.au>
AuthorDate: Fri Feb 11 14:41:19 2022 +1000
GROOVY-10472: @AutoImplement is failing when covariant returns are involved
---
.../apache/groovy/ast/tools/ClassNodeUtils.java | 7 ++
.../transform/AutoImplementASTTransformation.java | 16 ++--
.../transform/AutoImplementTransformTest.groovy | 92 ++++++++++++++++++++++
3 files changed, 108 insertions(+), 7 deletions(-)
diff --git a/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java b/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java
index 993c959..510888e 100644
--- a/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java
+++ b/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java
@@ -50,6 +50,7 @@ import static org.apache.groovy.ast.tools.AnnotatedNodeUtils.markAsGenerated;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveBoolean;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.isOrImplements;
import static org.codehaus.groovy.runtime.ArrayTypeUtils.dimension;
import static org.codehaus.groovy.runtime.ArrayTypeUtils.elementType;
import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
@@ -448,4 +449,10 @@ public class ClassNodeUtils {
return null;
}
+
+ public static boolean isSubtype(ClassNode maybeSuperclassOrInterface, ClassNode candidateChild) {
+ return maybeSuperclassOrInterface.isInterface() || candidateChild.isInterface()
+ ? isOrImplements(candidateChild, maybeSuperclassOrInterface)
+ : candidateChild.isDerivedFrom(maybeSuperclassOrInterface);
+ }
}
diff --git a/src/main/java/org/codehaus/groovy/transform/AutoImplementASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/AutoImplementASTTransformation.java
index a89a3ca..00db645 100644
--- a/src/main/java/org/codehaus/groovy/transform/AutoImplementASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/AutoImplementASTTransformation.java
@@ -42,6 +42,7 @@ import java.util.List;
import java.util.Map;
import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedMethod;
+import static org.apache.groovy.ast.tools.ClassNodeUtils.isSubtype;
import static org.apache.groovy.ast.tools.MethodNodeUtils.getPropertyName;
import static org.apache.groovy.ast.tools.MethodNodeUtils.methodDescriptorWithoutReturnType;
import static org.apache.groovy.util.BeanUtils.capitalize;
@@ -161,9 +162,7 @@ public class AutoImplementASTTransformation extends AbstractASTTransformation {
MethodNode found = getDeclaredMethodCorrected(genericsSpec, correctedMethod, correctedClass);
if (found != null) {
String td = methodDescriptorWithoutReturnType(found);
- if (result.containsKey(td) && !result.get(td).isAbstract()) {
- continue;
- }
+ if (result.containsKey(td) && isWeakerCandidate(result.get(td), found)) continue;
result.put(td, found);
}
}
@@ -182,9 +181,7 @@ public class AutoImplementASTTransformation extends AbstractASTTransformation {
MethodNode found = getDeclaredMethodCorrected(updatedGenericsSpec, correctedMethod, correctedInterface);
if (found != null) {
String td = methodDescriptorWithoutReturnType(found);
- if (result.containsKey(td) && !result.get(td).isAbstract()) {
- continue;
- }
+ if (result.containsKey(td) && isWeakerCandidate(result.get(td), found)) continue;
result.put(td, found);
}
}
@@ -221,10 +218,15 @@ public class AutoImplementASTTransformation extends AbstractASTTransformation {
}
}
}
-
return result;
}
+ private static boolean isWeakerCandidate(MethodNode existing, MethodNode found) {
+ return !(existing.isAbstract() && !found.isAbstract()) &&
+ // GROOVY-10472: prefer covariant method with more concrete type
+ isSubtype(found.getReturnType(), existing.getReturnType());
+ }
+
private static MethodNode getDeclaredMethodCorrected(final Map<String, ClassNode> genericsSpec, final MethodNode origMethod, final ClassNode correctedClass) {
for (MethodNode nameMatch : correctedClass.getDeclaredMethods(origMethod.getName())) {
MethodNode correctedMethod = correctToGenericsSpec(genericsSpec, nameMatch);
diff --git a/src/test/org/codehaus/groovy/transform/AutoImplementTransformTest.groovy b/src/test/org/codehaus/groovy/transform/AutoImplementTransformTest.groovy
index ba86e77..05b108f 100644
--- a/src/test/org/codehaus/groovy/transform/AutoImplementTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/AutoImplementTransformTest.groovy
@@ -216,4 +216,96 @@ final class AutoImplementTransformTest {
assert new Foo().compare('bar', 'baz') == 0
'''
}
+
+ @Test // GROOVY-10472
+ void testCovariantReturnTypes() {
+ assertScript '''
+ import groovy.transform.AutoImplement
+
+ interface Super { List findAll() }
+ interface Sub extends Super { Iterable findAll() }
+
+ @AutoImplement
+ class ThisClassFails implements Sub{}
+
+ assert !(new ThisClassFails().findAll())
+ '''
+ assertScript '''
+ import groovy.transform.AutoImplement
+
+ interface Super { ArrayList findAll() }
+ interface Sub extends Super { Iterable findAll() }
+
+ @AutoImplement
+ class ThisClassFails implements Sub{}
+
+ assert !(new ThisClassFails().findAll())
+ '''
+ assertScript '''
+ import groovy.transform.AutoImplement
+
+ interface Super { Iterable findAll() }
+ interface Sub extends Super { List findAll() }
+
+ @AutoImplement
+ class ThisClassFails implements Sub{}
+
+ assert !(new ThisClassFails().findAll())
+ '''
+ assertScript '''
+ import groovy.transform.AutoImplement
+
+ interface Super { Iterable findAll() }
+ interface Sub extends Super { ArrayList findAll() }
+
+ @AutoImplement
+ class ThisClassFails implements Sub{}
+
+ assert !(new ThisClassFails().findAll())
+ '''
+ assertScript '''
+ import groovy.transform.AutoImplement
+
+ interface Super { AbstractList findAll() }
+ interface Sub extends Super { List findAll() }
+
+ @AutoImplement
+ class ThisClassFails implements Sub{}
+
+ assert !(new ThisClassFails().findAll())
+ '''
+ assertScript '''
+ import groovy.transform.AutoImplement
+
+ interface Super { List findAll() }
+ interface Sub extends Super { AbstractList findAll() }
+
+ @AutoImplement
+ class ThisClassFails implements Sub{}
+
+ assert !(new ThisClassFails().findAll())
+ '''
+ assertScript '''
+ import groovy.transform.AutoImplement
+
+ interface Super { AbstractList findAll() }
+ interface Sub extends Super { ArrayList findAll() }
+
+ @AutoImplement
+ class ThisClassFails implements Sub{}
+
+ assert !(new ThisClassFails().findAll())
+ '''
+ assertScript '''
+ import groovy.transform.AutoImplement
+
+ interface Super { ArrayList findAll() }
+ interface Sub extends Super { AbstractList findAll() }
+
+ @AutoImplement
+ class ThisClassFails implements Sub{}
+
+ assert !(new ThisClassFails().findAll())
+ '''
+ }
}