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/08/19 15:43:41 UTC
[groovy] 04/05: GROOVY-10113: check class cycle for outer types and
type parameters
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit fc02ed9d7911917fd3ac88401e3e429f426e4d9d
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Jun 6 16:19:45 2021 -0500
GROOVY-10113: check class cycle for outer types and type parameters
Conflicts:
src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
---
.../codehaus/groovy/control/ResolveVisitor.java | 86 +++++++++++---------
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, 205 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 eb54daf..61c4235 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -73,6 +73,7 @@ import java.lang.annotation.Annotation;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.reflect.Modifier;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
@@ -1461,13 +1462,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);
@@ -1476,6 +1490,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();
@@ -1507,41 +1548,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 (parentToCompare == ClassHelper.OBJECT_TYPE) 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);
if (cs.getExceptionType() == ClassHelper.DYNAMIC_TYPE) {
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']
}