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 2021/06/05 00:45:09 UTC
[groovy] branch GROOVY-10113 created (now 1c801e1)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-10113
in repository https://gitbox.apache.org/repos/asf/groovy.git.
at 1c801e1 GROOVY-10113: check class cycle for outer types and type parameters
This branch includes the following new commits:
new 1c801e1 GROOVY-10113: check class cycle for outer types and type parameters
The 1 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.
[groovy] 01/01: GROOVY-10113: check class cycle for outer types and
type parameters
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-10113
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 1c801e1c7721c649f162c38f9e2b7292076165e0
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Jun 4 19:14:17 2021 -0500
GROOVY-10113: check class cycle for outer types and type parameters
---
.../codehaus/groovy/control/ResolveVisitor.java | 87 +++++++++++---------
src/test/groovy/bugs/Groovy10113.groovy | 94 ++++++++++++++++++++++
.../{Groovy1465Bug.groovy => Groovy1465.groovy} | 39 +++++----
.../{Groovy3904Bug.groovy => Groovy3904.groovy} | 55 ++++++-------
.../{Groovy8048Bug.groovy => Groovy8048.groovy} | 32 ++++----
5 files changed, 206 insertions(+), 101 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index 687a556..16440cf 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -70,6 +70,7 @@ import org.codehaus.groovy.vmplugin.VMPluginFactory;
import org.objectweb.asm.Opcodes;
import java.lang.reflect.Modifier;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
@@ -82,8 +83,8 @@ import java.util.function.BiConsumer;
import java.util.function.Predicate;
import static groovy.lang.Tuple.tuple;
-import static org.codehaus.groovy.ast.tools.ClosureUtils.getParametersSafe;
import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
+import static org.codehaus.groovy.ast.tools.ClosureUtils.getParametersSafe;
/**
* Visitor to resolve Types and convert VariableExpression to
@@ -1474,13 +1475,26 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
}
ClassNode sn = node.getUnresolvedSuperClass();
- if (sn != null) resolveOrFail(sn, "", node, true);
-
- for (ClassNode anInterface : node.getInterfaces()) {
- resolveOrFail(anInterface, "", node, true);
+ if (sn != null) {
+ resolveOrFail(sn, "", node, true);
+ }
+ for (ClassNode in : node.getInterfaces()) {
+ resolveOrFail(in, "", node, true);
}
- checkCyclicInheritance(node, node.getUnresolvedSuperClass(), node.getInterfaces());
+ if (sn != null) checkCyclicInheritance(node, sn);
+ for (ClassNode in : node.getInterfaces()) {
+ checkCyclicInheritance(node, in);
+ }
+ if (node.getGenericsTypes() != null) {
+ for (GenericsType gt : node.getGenericsTypes()) {
+ if (gt != null && gt.getUpperBounds() != null) {
+ for (ClassNode variant : gt.getUpperBounds()) {
+ if (variant.isGenericsPlaceHolder()) checkCyclicInheritance(gt.getType().redirect(), variant);
+ }
+ }
+ }
+ }
super.visitClass(node);
@@ -1489,6 +1503,33 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
currentClass = oldNode;
}
+ private void checkCyclicInheritance(final ClassNode node, final ClassNode type) {
+ if (type.redirect() == node || type.getOuterClasses().contains(node)) {
+ addError("Cycle detected: the type " + node.getName() + " cannot extend/implement itself or one of its own member types", type);
+ } else if (type != ClassHelper.OBJECT_TYPE) {
+ Set<ClassNode> done = new HashSet<>();
+ done.add(ClassHelper.OBJECT_TYPE);
+ done.add(null);
+
+ LinkedList<ClassNode> todo = new LinkedList<>();
+ Collections.addAll(todo, type.getInterfaces());
+ todo.add(type.getUnresolvedSuperClass());
+ todo.add(type.getOuterClass());
+ do {
+ ClassNode next = todo.poll();
+ if (!done.add(next)) continue;
+ if (next.redirect() == node) {
+ ClassNode cn = type; while (cn.getOuterClass() != null) cn = cn.getOuterClass();
+ addError("Cycle detected: a cycle exists in the type hierarchy between " + node.getName() + " and " + cn.getName(), type);
+ return;
+ }
+ Collections.addAll(todo, next.getInterfaces());
+ todo.add(next.getUnresolvedSuperClass());
+ todo.add(next.getOuterClass());
+ } while (!todo.isEmpty());
+ }
+ }
+
// GROOVY-7812(#2): Static inner classes cannot be accessed from other files when running by 'groovy' command
private void resolveOuterNestedClassFurther(final ClassNode node) {
CompileUnit compileUnit = currentClass.getCompileUnit();
@@ -1520,40 +1561,6 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
}
}
- private void checkCyclicInheritance(final ClassNode originalNode, final ClassNode parentToCompare, final ClassNode[] interfacesToCompare) {
- if (!originalNode.isInterface()) {
- if (parentToCompare == null) return;
- if (originalNode == parentToCompare.redirect()) {
- addError("Cyclic inheritance involving " + parentToCompare.getName() + " in class " + originalNode.getName(), originalNode);
- return;
- }
- if (interfacesToCompare != null && interfacesToCompare.length > 0) {
- for (ClassNode intfToCompare : interfacesToCompare) {
- if (originalNode == intfToCompare.redirect()) {
- addError("Cycle detected: the type " + originalNode.getName() + " cannot implement itself" , originalNode);
- return;
- }
- }
- }
- if (isObjectType(parentToCompare)) return;
- checkCyclicInheritance(originalNode, parentToCompare.getUnresolvedSuperClass(), null);
- } else {
- if (interfacesToCompare != null && interfacesToCompare.length > 0) {
- // check interfaces at this level first
- for (ClassNode intfToCompare : interfacesToCompare) {
- if(originalNode == intfToCompare.redirect()) {
- addError("Cyclic inheritance involving " + intfToCompare.getName() + " in interface " + originalNode.getName(), originalNode);
- return;
- }
- }
- // check next level of interfaces
- for (ClassNode intf : interfacesToCompare) {
- checkCyclicInheritance(originalNode, null, intf.getInterfaces());
- }
- }
- }
- }
-
@Override
public void visitCatchStatement(final CatchStatement cs) {
resolveOrFail(cs.getExceptionType(), cs);
diff --git a/src/test/groovy/bugs/Groovy10113.groovy b/src/test/groovy/bugs/Groovy10113.groovy
new file mode 100644
index 0000000..142f2f3
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy10113.groovy
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package groovy.bugs
+
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.shouldFail
+
+final class Groovy10113 {
+
+ @Test
+ void testTypeParamCycle() {
+ def err = shouldFail '''
+ class C<T extends T> {
+ }
+ '''
+ assert err =~ /Cycle detected: the type T cannot extend.implement itself or one of its own member types/
+ }
+
+ @Test
+ void testInnerClassCycle1() {
+ def err = shouldFail '''
+ class C extends C.Inner {
+ class Inner {
+ }
+ }
+ '''
+ assert err =~ /Cycle detected: the type C cannot extend.implement itself or one of its own member types/
+ }
+
+ @Test
+ void testInnerClassCycle2() {
+ def err = shouldFail '''
+ class C extends D {
+ class Inner {
+ }
+ }
+ class D extends C.Inner {
+ }
+ '''
+ assert err =~ /Cycle detected: a cycle exists in the type hierarchy between D and C/
+ }
+
+ @Test
+ void testInnerInterfaceCycle1() {
+ def err = shouldFail '''
+ class C implements C.I {
+ interface I {
+ }
+ }
+ '''
+ assert err =~ /Cycle detected: the type C cannot extend.implement itself or one of its own member types/
+ }
+
+ @Test
+ void testInnerInterfaceCycle2() {
+ def err = shouldFail '''
+ class C extends D {
+ interface I {
+ }
+ }
+ class D implements C.I {
+ }
+ '''
+ assert err =~ /Cycle detected: a cycle exists in the type hierarchy between D and C/
+ }
+
+ @Test
+ void testClassExtendsInterfaceCycle() {
+ def err = shouldFail '''
+ interface A extends B {
+ }
+ class B extends A {
+ }
+ '''
+ assert err =~ /Cycle detected: a cycle exists in the type hierarchy between B and A/
+ }
+}
diff --git a/src/test/groovy/bugs/Groovy1465Bug.groovy b/src/test/groovy/bugs/Groovy1465.groovy
similarity index 63%
rename from src/test/groovy/bugs/Groovy1465Bug.groovy
rename to src/test/groovy/bugs/Groovy1465.groovy
index bef408a..fc40e09 100644
--- a/src/test/groovy/bugs/Groovy1465Bug.groovy
+++ b/src/test/groovy/bugs/Groovy1465.groovy
@@ -18,41 +18,40 @@
*/
package groovy.bugs
-import groovy.test.GroovyTestCase
-import org.codehaus.groovy.control.MultipleCompilationErrorsException
+import org.junit.Test
-class Groovy1465Bug extends GroovyTestCase {
-
- void compileAndVerifyCyclicInheritenceCompilationError(script) {
- try {
- new GroovyShell().parse(script)
- fail('The compilation should have failed as it is a cyclic reference')
- } catch (MultipleCompilationErrorsException e) {
- def syntaxError = e.errorCollector.getSyntaxError(0)
- assert syntaxError.message.contains('Cyclic inheritance')
- }
+import static groovy.test.GroovyAssert.shouldFail
+
+final class Groovy1465 {
+
+ private static void compileAndVerifyCyclicInheritenceCompilationError(String sourceCode) {
+ def err = shouldFail(sourceCode)
+ assert err =~ /Cycle detected/
}
-
+
+ @Test
void testInterfaceCyclicInheritenceTC1() {
- compileAndVerifyCyclicInheritenceCompilationError """
+ compileAndVerifyCyclicInheritenceCompilationError '''
interface G1465Tt extends G1465Tt { }
def tt = {} as G1465Tt
- """
+ '''
}
+ @Test
void testInterfaceCyclicInheritenceTC2() {
- compileAndVerifyCyclicInheritenceCompilationError """
+ compileAndVerifyCyclicInheritenceCompilationError '''
interface G1465Rr extends G1465Ss { }
interface G1465Ss extends G1465Rr { }
def ss = {} as G1465Ss
- """
+ '''
}
+ @Test
void testInterfaceCyclicInheritenceTC3() {
- compileAndVerifyCyclicInheritenceCompilationError """
+ compileAndVerifyCyclicInheritenceCompilationError '''
interface G1465A extends G1465B { }
interface G1465B extends G1465C { }
interface G1465C extends G1465B { }
- """
+ '''
}
-}
\ No newline at end of file
+}
diff --git a/src/test/groovy/bugs/Groovy3904Bug.groovy b/src/test/groovy/bugs/Groovy3904.groovy
similarity index 62%
rename from src/test/groovy/bugs/Groovy3904Bug.groovy
rename to src/test/groovy/bugs/Groovy3904.groovy
index a8b7464..419d2f0 100644
--- a/src/test/groovy/bugs/Groovy3904Bug.groovy
+++ b/src/test/groovy/bugs/Groovy3904.groovy
@@ -18,68 +18,69 @@
*/
package groovy.bugs
-import groovy.test.GroovyTestCase
-import org.codehaus.groovy.control.MultipleCompilationErrorsException
+import org.junit.Test
-class Groovy3904Bug extends GroovyTestCase {
-
- void compileAndVerifyCyclicInheritenceCompilationError(script) {
- try {
- new GroovyShell().parse(script)
- fail('The compilation should have failed as it is a cyclic reference')
- } catch (MultipleCompilationErrorsException e) {
- def syntaxError = e.errorCollector.getSyntaxError(0)
- assert syntaxError.message.contains('Cyclic inheritance')
- }
+import static groovy.test.GroovyAssert.shouldFail
+
+final class Groovy3904 {
+
+ private static void compileAndVerifyCyclicInheritenceCompilationError(String sourceCode) {
+ def err = shouldFail(sourceCode)
+ assert err =~ /Cycle detected/
}
-
+
+ @Test
void testCyclicInheritenceTC1() {
- compileAndVerifyCyclicInheritenceCompilationError """
+ compileAndVerifyCyclicInheritenceCompilationError '''
class G3904R1A extends G3904R1A {}
- """
+ '''
}
+ @Test
void testCyclicInheritenceTC2() {
- compileAndVerifyCyclicInheritenceCompilationError """
+ compileAndVerifyCyclicInheritenceCompilationError '''
class G3904R2A extends G3904R2A {
public static void main(String []argv) {
print 'hey'
}
}
- """
+ '''
}
/* next 2 tests are similar but in reverse order */
+
+ @Test
void testCyclicInheritenceTC3() {
- compileAndVerifyCyclicInheritenceCompilationError """
+ compileAndVerifyCyclicInheritenceCompilationError '''
class G3904R3A extends G3904R3B {}
class G3904R3B extends G3904R3A {}
- """
+ '''
}
+ @Test
void testCyclicInheritenceTC4() {
- compileAndVerifyCyclicInheritenceCompilationError """
+ compileAndVerifyCyclicInheritenceCompilationError '''
class G3904R4B extends G3904R4A {}
class G3904R4A extends G3904R4B {}
- """
+ '''
}
- // cyclic inheritence is between 2 parent classes
+ @Test // cyclic inheritence is between 2 parent classes
void testCyclicInheritenceTC5() {
- compileAndVerifyCyclicInheritenceCompilationError """
+ compileAndVerifyCyclicInheritenceCompilationError '''
class G3904R5A extends G3904R5B {}
class G3904R5B extends G3904R5C {}
class G3904R5C extends G3904R5B {}
- """
+ '''
}
- // cyclic inheritence is between 2 parent classes with a normal level in-between
+ @Test // cyclic inheritence is between 2 parent classes with a normal level in-between
void testCyclicInheritenceTC6() {
- compileAndVerifyCyclicInheritenceCompilationError """
+ compileAndVerifyCyclicInheritenceCompilationError '''
class G3904R6A extends G3904R6B {}
class G3904R6B extends G3904R6C {}
class G3904R6C extends G3904R6D {}
class G3904R6D extends G3904R6B {}
- """
+ '''
}
}
diff --git a/src/test/groovy/bugs/Groovy8048Bug.groovy b/src/test/groovy/bugs/Groovy8048.groovy
similarity index 65%
rename from src/test/groovy/bugs/Groovy8048Bug.groovy
rename to src/test/groovy/bugs/Groovy8048.groovy
index 23b4cf4..6a85e39 100644
--- a/src/test/groovy/bugs/Groovy8048Bug.groovy
+++ b/src/test/groovy/bugs/Groovy8048.groovy
@@ -18,30 +18,34 @@
*/
package groovy.bugs
-import groovy.test.GroovyTestCase
+import org.junit.Test
-class Groovy8048Bug extends GroovyTestCase {
+import static groovy.test.GroovyAssert.assertScript
+
+final class Groovy8048 {
+
+ @Test
void testFinalFieldInPreCompiledTrait() {
- def shell = new GroovyShell(getClass().classLoader)
- shell.evaluate('''
- class Bar implements groovy.bugs.Groovy8048Bug.Groovy8048Trait, BarTrait {
+ assertScript """
+ class C implements ${getClass().getName()}.Foo, Bar {
static void main(args) {
- assert Bar.otherItems1 && Bar.otherItems1[0] == 'bar1'
- assert Bar.items1 && Bar.items1[0] == 'foo1'
- new Bar().with {
+ assert this.otherItems1 && this.otherItems1[0] == 'bar1'
+ assert this.items1 && this.items1[0] == 'foo1'
+ new C().with {
assert otherItems2 && otherItems2[0] == 'bar2'
assert items2 && items2[0] == 'foo2'
}
}
- trait BarTrait {
- final static List otherItems1 = ['bar1']
- final List otherItems2 = ['bar2']
- }
}
- ''', 'testScript')
+
+ trait Bar {
+ final static List otherItems1 = ['bar1']
+ final List otherItems2 = ['bar2']
+ }
+ """
}
- trait Groovy8048Trait {
+ trait Foo {
final static List items1 = ['foo1']
final List items2 = ['foo2']
}