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/01/12 11:44:17 UTC
[groovy] 02/02: GROOVY-10439: `@Delegate`: same interface with different type argument
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 1ff49b92b4b6cd32cb69d03b5698261302edf0c6
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Jan 6 11:38:40 2022 -0600
GROOVY-10439: `@Delegate`: same interface with different type argument
---
.../codehaus/groovy/ast/tools/GeneralUtils.java | 20 +++++++++---
.../org/codehaus/groovy/classgen/Verifier.java | 19 ++---------
.../transform/DelegateASTTransformation.java | 38 +++++++++-------------
.../groovy/transform/DelegateTransformTest.groovy | 23 +++++++++++--
4 files changed, 54 insertions(+), 46 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
index acbfbad..afd3c07 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
@@ -88,6 +88,7 @@ import java.util.function.Consumer;
import static org.codehaus.groovy.antlr.PrimitiveHelper.getDefaultValueForPrimitive;
import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
+import static org.codehaus.groovy.ast.tools.GenericsUtils.parameterizeType;
/**
* Handy methods when working with the Groovy AST
@@ -462,11 +463,22 @@ public class GeneralUtils {
return result;
}
- public static Set<ClassNode> getInterfacesAndSuperInterfaces(final ClassNode type) {
- Set<ClassNode> result = new LinkedHashSet<>();
- for (ClassNode next = type; next != null; next = next.getSuperClass()) {
- result.addAll(next.getAllInterfaces());
+ private static void addAllInterfaces(final Set<ClassNode> result, final ClassNode source) {
+ for (ClassNode in : source.getInterfaces()) {
+ in = parameterizeType(source, in);
+ if (result.add(in))
+ addAllInterfaces(result, in);
+ }
+ ClassNode sc = source.redirect().getUnresolvedSuperClass(false);
+ if (sc != null && !isObjectType(sc)) {
+ addAllInterfaces(result, parameterizeType(source, sc));
}
+ }
+
+ public static Set<ClassNode> getInterfacesAndSuperInterfaces(final ClassNode cNode) {
+ Set<ClassNode> result = new LinkedHashSet<>();
+ if (cNode.isInterface()) result.add(cNode);
+ addAllInterfaces(result, cNode);
return result;
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index edb89de..1a99ca4 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -117,6 +117,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.declS;
import static org.codehaus.groovy.ast.tools.GeneralUtils.fieldX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.getInterfacesAndSuperInterfaces;
import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.param;
import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
@@ -124,7 +125,6 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
import static org.codehaus.groovy.ast.tools.GenericsUtils.addMethodGenerics;
import static org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpec;
import static org.codehaus.groovy.ast.tools.GenericsUtils.createGenericsSpec;
-import static org.codehaus.groovy.ast.tools.GenericsUtils.parameterizeType;
import static org.codehaus.groovy.ast.tools.PropertyNodeUtils.adjustPropertyModifiersForMethod;
/**
@@ -358,23 +358,8 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
};
}
- private static void addAllInterfaces(final Set<ClassNode> result, final ClassNode source) {
- for (ClassNode in : source.getInterfaces()) {
- in = parameterizeType(source, in);
- if (result.add(in))
- addAllInterfaces(result, in);
- }
- ClassNode sc = source.redirect().getUnresolvedSuperClass(false);
- if (sc != null && !isObjectType(sc)) {
- addAllInterfaces(result, parameterizeType(source, sc));
- }
- }
-
private static Set<ClassNode> getAllInterfaces(final ClassNode cn) {
- Set<ClassNode> result = new HashSet<>();
- if (cn.isInterface()) result.add(cn);
- addAllInterfaces(result, cn);
- return result;
+ return getInterfacesAndSuperInterfaces(cn);
}
private static void checkForDuplicateInterfaces(final ClassNode cn) {
diff --git a/src/main/java/org/codehaus/groovy/transform/DelegateASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/DelegateASTTransformation.java
index 4080e31..e67a4e0 100644
--- a/src/main/java/org/codehaus/groovy/transform/DelegateASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/DelegateASTTransformation.java
@@ -35,7 +35,6 @@ import org.codehaus.groovy.ast.expr.ArgumentListExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.MethodCallExpression;
import org.codehaus.groovy.ast.tools.BeanUtils;
-import org.codehaus.groovy.ast.tools.GenericsUtils;
import org.codehaus.groovy.control.CompilePhase;
import org.codehaus.groovy.control.SourceUnit;
@@ -47,8 +46,8 @@ import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Set;
-import java.util.stream.Collectors;
+import static java.util.Arrays.copyOf;
import static java.util.stream.Collectors.toSet;
import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedMethod;
import static org.apache.groovy.util.BeanUtils.capitalize;
@@ -75,6 +74,7 @@ import static org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpec;
import static org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpecRecurse;
import static org.codehaus.groovy.ast.tools.GenericsUtils.createGenericsSpec;
import static org.codehaus.groovy.ast.tools.GenericsUtils.extractSuperClassGenerics;
+import static org.codehaus.groovy.ast.tools.GenericsUtils.nonGeneric;
import static org.codehaus.groovy.ast.tools.ParameterUtils.parametersEqual;
import static org.objectweb.asm.Opcodes.ACC_ABSTRACT;
import static org.objectweb.asm.Opcodes.ACC_FINAL;
@@ -198,24 +198,18 @@ public class DelegateASTTransformation extends AbstractASTTransformation {
if (skipInterfaces) return;
- final Set<ClassNode> allInterfaces =
- getInterfacesAndSuperInterfaces(delegate.type).stream()
- .filter(c -> !c.isSealed())
- .collect(Collectors.toSet());
- final Set<ClassNode> ownerIfaces = delegate.owner.getAllInterfaces();
- Map<String,ClassNode> genericsSpec = createGenericsSpec(delegate.owner);
- genericsSpec = createGenericsSpec(delegate.type, genericsSpec);
- for (ClassNode iface : allInterfaces) {
- if (Modifier.isPublic(iface.getModifiers()) && !ownerIfaces.contains(iface)) {
- final ClassNode[] ifaces = delegate.owner.getInterfaces();
- final int nFaces = ifaces.length;
-
- final ClassNode[] newIfaces = new ClassNode[nFaces + 1];
- for (int i = 0; i < nFaces; i += 1) {
- newIfaces[i] = correctToGenericsSpecRecurse(genericsSpec, ifaces[i]);
+ Set<ClassNode> addedInterfaces = getInterfacesAndSuperInterfaces(delegate.type);
+ addedInterfaces.removeIf(i -> !Modifier.isPublic(i.getModifiers()) || i.isSealed());
+ if (!addedInterfaces.isEmpty()) {
+ Set<ClassNode> ownerInterfaces = getInterfacesAndSuperInterfaces(delegate.owner);
+ for (ClassNode i : addedInterfaces) {
+ if (!ownerInterfaces.contains(i)) {
+ ClassNode[] faces = delegate.owner.getInterfaces();
+ faces = copyOf(faces, faces.length + 1);
+ faces[faces.length - 1] = i;
+
+ delegate.owner.setInterfaces(faces);
}
- newIfaces[nFaces] = correctToGenericsSpecRecurse(genericsSpec, iface);
- delegate.owner.setInterfaces(newIfaces);
}
}
}
@@ -325,7 +319,7 @@ public class DelegateASTTransformation extends AbstractASTTransformation {
setterName,
ACC_PUBLIC,
ClassHelper.VOID_TYPE,
- params(new Parameter(GenericsUtils.nonGeneric(prop.getType()), "value")),
+ params(new Parameter(nonGeneric(prop.getType()), "value")),
null,
assignS(propX(delegate.getOp, name), varX("value"))
);
@@ -356,7 +350,7 @@ public class DelegateASTTransformation extends AbstractASTTransformation {
delegate.owner,
getterName,
ACC_PUBLIC,
- GenericsUtils.nonGeneric(prop.getType()),
+ nonGeneric(prop.getType()),
Parameter.EMPTY_ARRAY,
null,
returnS(propX(delegate.getOp, name))
@@ -369,7 +363,7 @@ public class DelegateASTTransformation extends AbstractASTTransformation {
delegate.owner,
isserName,
ACC_PUBLIC,
- GenericsUtils.nonGeneric(prop.getType()),
+ nonGeneric(prop.getType()),
Parameter.EMPTY_ARRAY,
null,
returnS(propX(delegate.getOp, name))
diff --git a/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy b/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy
index 554ba11..7ad281e 100644
--- a/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy
@@ -37,7 +37,7 @@ import static org.junit.Assert.assertTrue
final class DelegateTransformTest {
@Test // GROOVY-3380
- void testDelegateImplementingANonPublicInterface() {
+ void testDelegateImplementingNonPublicInterface() {
assertScript '''
import org.codehaus.groovy.transform.ClassImplementingANonPublicInterface
@@ -51,11 +51,11 @@ final class DelegateTransformTest {
}
@Test // GROOVY-3380
- void testDelegateImplementingANonPublicInterfaceWithZipFileConcreteCase() {
+ void testDelegateImplementingNonPublicInterfaceWithZipFileConcreteCase() {
assertScript '''
import java.util.zip.*
- class ZipWrapper{
+ class ZipWrapper {
@Delegate ZipFile zipFile
}
@@ -63,6 +63,23 @@ final class DelegateTransformTest {
'''
}
+ @Test // GROOVY-10439
+ void testDelegateImplementingInterfaceWithDifferentTypeArgumentThanOwner() {
+ assertScript '''
+ class C extends ArrayList<String> {
+ @Delegate List<Number> numbers // List<String> takes precedence
+ }
+ new C(numbers:[1,2,3])
+ '''
+
+ def err = shouldFail '''
+ class C extends ArrayList<String> {
+ @Delegate HashSet<Number> numbers // Set<Number> added; Verifier checks
+ }
+ '''
+ assert err =~ /The interface Collection cannot be implemented more than once with different arguments: java.util.Collection<java.lang.Number> and java.util.Collection<java.lang.String>/
+ }
+
@Test // GROOVY-5974
void testDelegateExcludes() {
assertScript '''