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/06 21:19:53 UTC

[groovy] branch master updated: 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 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 eef2a82  GROOVY-10113: check class cycle for outer types and type parameters
eef2a82 is described below

commit eef2a82ec72c7f94e8c60c3e447a85723ac461cf
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
---
 .../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']
     }