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:15 UTC
[groovy] branch GROOVY_4_0_X updated (fe7d7ba -> 1ff49b9)
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a change to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from fe7d7ba GROOVY-4266: stubgen: only write static imports
new 76dc939 GROOVY-5106: verify repeat interface has same type argumnet(s)
new 1ff49b9 GROOVY-10439: `@Delegate`: same interface with different type argument
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../codehaus/groovy/ast/tools/GeneralUtils.java | 20 ++++--
.../org/codehaus/groovy/classgen/Verifier.java | 73 +++++++++++++++++-----
.../transform/DelegateASTTransformation.java | 38 +++++------
src/test/groovy/InterfaceTest.groovy | 42 +++++++++----
.../groovy/transform/DelegateTransformTest.groovy | 23 ++++++-
5 files changed, 139 insertions(+), 57 deletions(-)
[groovy] 02/02: GROOVY-10439: `@Delegate`: same interface with different type argument
Posted by pa...@apache.org.
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 '''
[groovy] 01/02: GROOVY-5106: verify repeat interface has same type argumnet(s)
Posted by pa...@apache.org.
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 76dc939d115ebe1af48dcd6ec7dec4c4c57c0572
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Jan 5 14:30:47 2022 -0600
GROOVY-5106: verify repeat interface has same type argumnet(s)
---
.../org/codehaus/groovy/classgen/Verifier.java | 88 +++++++++++++++++-----
src/test/groovy/InterfaceTest.groovy | 42 ++++++++---
2 files changed, 102 insertions(+), 28 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index 12ce5d3..edb89de 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -124,6 +124,7 @@ 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;
/**
@@ -221,8 +222,6 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
@Override
public void visitClass(final ClassNode node) {
- boolean skipGroovify = !node.getAnnotations(ClassHelper.make(CompileStatic.class)).isEmpty() &&
- !node.getAnnotations(ClassHelper.make(POJO.class)).isEmpty();
this.classNode = node;
if (node.isRecord()) {
@@ -231,8 +230,11 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
throw new RuntimeParserException("Record '" + classNode.getNameWithoutPackage() + "' must not be abstract", classNode);
}
}
- if (Traits.isTrait(node) // maybe possible to have this true in joint compilation mode
- || classNode.isInterface()) {
+
+ checkForDuplicateInterfaces(node);
+
+ if (classNode.isInterface()
+ || Traits.isTrait(node)) { // maybe possible to have this true in joint compilation mode
//interfaces have no constructors, but this code expects one,
//so create a dummy and don't add it to the class node
ConstructorNode dummy = new ConstructorNode(0, null);
@@ -245,21 +247,12 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
return;
}
- ClassNode[] classNodes = classNode.getInterfaces();
- List<String> interfaces = new ArrayList<>();
- for (ClassNode classNode : classNodes) {
- interfaces.add(classNode.getName());
- }
- Set<String> interfaceSet = new HashSet<>(interfaces);
- if (interfaceSet.size() != interfaces.size()) {
- throw new RuntimeParserException("Duplicate interfaces in implements list: " + interfaces, classNode);
- }
-
addDefaultParameterMethods(node);
addDefaultParameterConstructors(node);
final String classInternalName = BytecodeHelper.getClassInternalName(node);
-
+ boolean skipGroovify = !node.getAnnotations(ClassHelper.make(POJO.class)).isEmpty()
+ && !node.getAnnotations(ClassHelper.make(CompileStatic.class)).isEmpty();
if (!skipGroovify) {
addStaticMetaClassField(node, classInternalName);
@@ -287,11 +280,11 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
checkFinalVariables(node);
}
- private static final List<String> invalidNames = Arrays.asList("clone", "finalize", "getClass", "hashCode", "notify", "notifyAll", "toString", "wait");
+ private static final String[] INVALID_COMPONENTS = {"clone", "finalize", "getClass", "hashCode", "notify", "notifyAll", "toString", "wait"};
private void detectInvalidRecordComponentNames(ClassNode node) {
for (FieldNode fn : node.getFields()) {
- if (invalidNames.contains(fn.getName())) {
+ if (Arrays.binarySearch(INVALID_COMPONENTS, fn.getName()) >= 0) {
throw new RuntimeParserException("Illegal record component name '" + fn.getName() + "'", fn);
}
}
@@ -365,6 +358,67 @@ 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;
+ }
+
+ private static void checkForDuplicateInterfaces(final ClassNode cn) {
+ ClassNode[] interfaces = cn.getInterfaces();
+ int nInterfaces = interfaces.length;
+ if (nInterfaces == 0) return;
+
+ if (nInterfaces > 1) {
+ List<String> interfaceNames = new ArrayList<>(nInterfaces);
+ for (ClassNode in : interfaces) interfaceNames.add(in.getName());
+ if (interfaceNames.size() != new HashSet<>(interfaceNames).size()) {
+ throw new RuntimeParserException("Duplicate interfaces in implements list: " + interfaceNames, cn);
+ }
+ }
+
+ // GROOVY-5106: check for same interface with different type argument(s)
+ List< Set<ClassNode> > allInterfaces = new ArrayList<>(nInterfaces + 1);
+ for (ClassNode in : interfaces) allInterfaces.add(getAllInterfaces(in));
+ allInterfaces.add(getAllInterfaces(cn.getUnresolvedSuperClass()));
+ if (nInterfaces == 1 && allInterfaces.get(1).isEmpty())
+ return; // no peer interface(s) to verify
+
+ for (int i = 0; i < nInterfaces; i += 1) {
+ for (ClassNode in : allInterfaces.get(i)) {
+ if (in.redirect().getGenericsTypes() != null) {
+ for (int j = i + 1; j < nInterfaces + 1; j += 1) {
+ Set<ClassNode> set = allInterfaces.get(j);
+ if (set.contains(in)) {
+ for (ClassNode t : set) { // find match and check generics
+ if (t.equals(in)) {
+ String one = in.toString(false), two = t.toString(false);
+ if (!one.equals(two))
+ throw new RuntimeParserException("The interface " + in.getNameWithoutPackage() +
+ " cannot be implemented more than once with different arguments: " + one + " and " + two, cn);
+ break;
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
private static void checkForDuplicateMethods(final ClassNode cn) {
Set<String> descriptors = new HashSet<>();
for (MethodNode mn : cn.getMethods()) {
diff --git a/src/test/groovy/InterfaceTest.groovy b/src/test/groovy/InterfaceTest.groovy
index 0a1d27e..50afb49 100644
--- a/src/test/groovy/InterfaceTest.groovy
+++ b/src/test/groovy/InterfaceTest.groovy
@@ -20,20 +20,40 @@ package groovy
import gls.CompilableTestSupport
-class InterfaceTest extends CompilableTestSupport {
+final class InterfaceTest extends CompilableTestSupport {
void testGenericsInInterfaceMembers() {
// control
- shouldCompile """
- interface I1 {
- public <T> T copy1(T arg)
- public <U extends CharSequence> U copy2(U arg)
- public <V, W> V copy3(W arg)
- public <N extends Number> void foo()
- }
- """
+ shouldCompile '''
+ interface I {
+ def <T> T m1(T x)
+ def <U extends CharSequence> U m2(U x)
+ def <V, W> V m3(W x)
+ def <N extends Number> void m4( )
+ }
+ '''
// erroneous
- shouldNotCompile "interface I2 { public <?> copy1(arg) }"
- shouldNotCompile "interface I3 { public <? extends CharSequence> copy1(arg) }"
+ shouldNotCompile 'interface I { def <?> m(x) }'
+ shouldNotCompile 'interface I { def <? extends CharSequence> m(x) }'
+ }
+
+ // GROOVY-5106
+ void testReImplementsInterface1() {
+ def err = shouldFail '''
+ interface I<T> {}
+ interface J<T> extends I<T> {}
+ class X implements I<String>, J<Number> {}
+ '''
+ assert err.contains('The interface I cannot be implemented more than once with different arguments: I<java.lang.String> and I<java.lang.Number>')
+ }
+
+ // GROOVY-5106
+ void testReImplementsInterface2() {
+ def err = shouldFail '''
+ interface I<T> {}
+ class X implements I<Number> {}
+ class Y extends X implements I<String> {}
+ '''
+ assert err.contains('The interface I cannot be implemented more than once with different arguments: I<java.lang.String> and I<java.lang.Number>')
}
}