You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2022/07/02 18:54:19 UTC

[groovy] branch GROOVY_4_0_X updated (5d5b29d80c -> b594a14075)

This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a change to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git


    from 5d5b29d80c GROOVY-10659: Parrot Parser: named arguments does not support all key expressions
     new 668a188a92 GROOVY-10675: propagate type arguments to supertypes for override checks
     new b594a14075 Reserve `isAnnotationCompatible` for binary compatibility

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/classgen/ExtendedVerifier.java |  48 +++----
 src/test/groovy/OverrideTest.groovy                | 139 +++++++++++++--------
 2 files changed, 105 insertions(+), 82 deletions(-)


[groovy] 02/02: Reserve `isAnnotationCompatible` for binary compatibility

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit b594a140755200bfff3f145185ac54a22db386f1
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sun Jul 3 02:48:18 2022 +0800

    Reserve `isAnnotationCompatible` for binary compatibility
---
 .../java/org/codehaus/groovy/classgen/ExtendedVerifier.java   | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
index ddf44415c2..2b52ac267b 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
@@ -39,6 +39,7 @@ import org.codehaus.groovy.ast.expr.VariableExpression;
 import org.codehaus.groovy.ast.stmt.ReturnStatement;
 import org.codehaus.groovy.ast.stmt.Statement;
 import org.codehaus.groovy.control.AnnotationConstantsVisitor;
+import org.codehaus.groovy.control.CompilerConfiguration;
 import org.codehaus.groovy.control.ErrorCollector;
 import org.codehaus.groovy.control.SourceUnit;
 import org.objectweb.asm.Opcodes;
@@ -458,4 +459,14 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
         }
         return null;
     }
+
+    /**
+     * Check if the current runtime allows Annotation usage.
+     *
+     * @return true if running on a 1.5+ runtime
+     */
+    @Deprecated
+    protected boolean isAnnotationCompatible() {
+        return CompilerConfiguration.isPostJDK5(this.source.getConfiguration().getTargetBytecode());
+    }
 }


[groovy] 01/02: GROOVY-10675: propagate type arguments to supertypes for override checks

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 668a188a92e0d27d4fe10b4aa8572ab7ec41e77a
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Jul 2 10:50:46 2022 -0500

    GROOVY-10675: propagate type arguments to supertypes for override checks
    
    (cherry picked from commit 74f4fd4b25a57b8370345cbb1bd50b1d657cb3e7)
---
 .../codehaus/groovy/classgen/ExtendedVerifier.java |  59 +++------
 src/test/groovy/OverrideTest.groovy                | 139 +++++++++++++--------
 2 files changed, 105 insertions(+), 93 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
index a83e6b8603..ddf44415c2 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
@@ -38,9 +38,7 @@ import org.codehaus.groovy.ast.expr.ListExpression;
 import org.codehaus.groovy.ast.expr.VariableExpression;
 import org.codehaus.groovy.ast.stmt.ReturnStatement;
 import org.codehaus.groovy.ast.stmt.Statement;
-import org.codehaus.groovy.ast.tools.ParameterUtils;
 import org.codehaus.groovy.control.AnnotationConstantsVisitor;
-import org.codehaus.groovy.control.CompilerConfiguration;
 import org.codehaus.groovy.control.ErrorCollector;
 import org.codehaus.groovy.control.SourceUnit;
 import org.objectweb.asm.Opcodes;
@@ -48,8 +46,6 @@ import org.objectweb.asm.Opcodes;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -67,10 +63,11 @@ import static org.codehaus.groovy.ast.AnnotationNode.RECORD_COMPONENT_TARGET;
 import static org.codehaus.groovy.ast.AnnotationNode.TYPE_PARAMETER_TARGET;
 import static org.codehaus.groovy.ast.AnnotationNode.TYPE_TARGET;
 import static org.codehaus.groovy.ast.AnnotationNode.TYPE_USE_TARGET;
-import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.getInterfacesAndSuperInterfaces;
 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.ParameterUtils.parametersEqual;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.evaluateExpression;
 
 /**
@@ -81,11 +78,12 @@ import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.evalua
  * - annotations on local variables are not supported
  */
 public class ExtendedVerifier extends ClassCodeVisitorSupport {
+
     public static final String JVM_ERROR_MESSAGE = "Please make sure you are running on a JVM >= 1.5";
     private static final String EXTENDED_VERIFIER_SEEN = "EXTENDED_VERIFIER_SEEN";
 
-    private final SourceUnit source;
     private ClassNode currentClass;
+    private final SourceUnit source;
     private final Map<String, Boolean> repeatableCache = new HashMap<>();
 
     public ExtendedVerifier(SourceUnit sourceUnit) {
@@ -425,32 +423,25 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
         }
     }
 
-    private static boolean isOverrideMethod(MethodNode method) {
-        ClassNode cNode = method.getDeclaringClass();
-        ClassNode next = cNode;
+    private static boolean isOverrideMethod(final MethodNode method) {
+        ClassNode declaringClass = method.getDeclaringClass();
+        ClassNode next = declaringClass;
         outer:
         while (next != null) {
-            Map<String, ClassNode> genericsSpec = createGenericsSpec(next);
-            MethodNode mn = correctToGenericsSpec(genericsSpec, method);
-            if (next != cNode) {
-                ClassNode correctedNext = correctToGenericsSpecRecurse(genericsSpec, next);
-                MethodNode found = getDeclaredMethodCorrected(genericsSpec, mn, correctedNext);
-                if (found != null) break;
+            Map<String, ClassNode> nextSpec = createGenericsSpec(next);
+            MethodNode mn = correctToGenericsSpec(nextSpec, method);
+            if (next != declaringClass) {
+                if (getDeclaredMethodCorrected(nextSpec, mn, next) != null) break;
             }
-            List<ClassNode> ifaces = new ArrayList<>(Arrays.asList(next.getInterfaces()));
-            while (!ifaces.isEmpty()) {
-                ClassNode origInterface = ifaces.remove(0);
-                if (!isObjectType(origInterface)) {
-                    genericsSpec = createGenericsSpec(origInterface, genericsSpec);
-                    ClassNode iNode = correctToGenericsSpecRecurse(genericsSpec, origInterface);
-                    MethodNode found2 = getDeclaredMethodCorrected(genericsSpec, mn, iNode);
-                    if (found2 != null) break outer;
-                    Collections.addAll(ifaces, iNode.getInterfaces());
-                }
+
+            for (ClassNode face : getInterfacesAndSuperInterfaces(next)) {
+                Map<String, ClassNode> faceSpec = createGenericsSpec(face, nextSpec);
+                if (getDeclaredMethodCorrected(faceSpec, mn, face) != null) break outer;
             }
+
             ClassNode superClass = next.getUnresolvedSuperClass();
             if (superClass != null) {
-                next = correctToGenericsSpecRecurse(genericsSpec, superClass);
+                next = correctToGenericsSpecRecurse(nextSpec, superClass);
             } else {
                 next = null;
             }
@@ -458,23 +449,13 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
         return next != null;
     }
 
-    private static MethodNode getDeclaredMethodCorrected(Map<String, ClassNode> genericsSpec, MethodNode mn, ClassNode correctedNext) {
-        for (MethodNode declared : correctedNext.getDeclaredMethods(mn.getName())) {
+    private static MethodNode getDeclaredMethodCorrected(final Map<String, ClassNode> genericsSpec, final MethodNode mn, final ClassNode cn) {
+        for (MethodNode declared : cn.getDeclaredMethods(mn.getName())) {
             MethodNode corrected = correctToGenericsSpec(genericsSpec, declared);
-            if (ParameterUtils.parametersEqual(corrected.getParameters(), mn.getParameters())) {
+            if (parametersEqual(corrected.getParameters(), mn.getParameters())) {
                 return corrected;
             }
         }
         return null;
     }
-
-    /**
-     * Check if the current runtime allows Annotation usage.
-     *
-     * @return true if running on a 1.5+ runtime
-     */
-    @Deprecated
-    protected boolean isAnnotationCompatible() {
-        return CompilerConfiguration.isPostJDK5(this.source.getConfiguration().getTargetBytecode());
-    }
 }
diff --git a/src/test/groovy/OverrideTest.groovy b/src/test/groovy/OverrideTest.groovy
index b5108381e0..c29618a8d9 100644
--- a/src/test/groovy/OverrideTest.groovy
+++ b/src/test/groovy/OverrideTest.groovy
@@ -18,11 +18,16 @@
  */
 package groovy
 
-import groovy.test.GroovyTestCase
+import org.junit.Test
 
-class OverrideTest extends GroovyTestCase {
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+
+final class OverrideTest {
+
+    @Test
     void testHappyPath() {
-        assertScript """
+        assertScript '''
             abstract class Parent<T> {
                 abstract method()
                 void methodTakeT(T t) { }
@@ -47,11 +52,12 @@ class OverrideTest extends GroovyTestCase {
             }
 
             new OverrideAnnotationTest()
-        """
+        '''
     }
 
+    @Test
     void testUnhappyPath() {
-        def message = shouldFail """
+        def err = shouldFail '''
             abstract class Parent<T> {
                 abstract method()
                 void methodTakeT(T t) { }
@@ -76,27 +82,14 @@ class OverrideTest extends GroovyTestCase {
             }
 
             new OverrideAnnotationTest()
-        """
-        assert message.contains(/The return type of java.lang.Double methodMakeT() in OverrideAnnotationTest is incompatible with java.lang.Integer in Parent/)
-        assert message.contains(/Method 'methodTakeT' from class 'OverrideAnnotationTest' does not override method from its superclass or interfaces but is annotated with @Override./)
-    }
-
-    void testGroovy6654() {
-        assertScript '''
-class Base<T> {
-    void foo(T t) {}
-}
-
-class Derived extends Base<String> {
-    @Override
-    void foo(String s) {}
-}
-def d = new Derived()
-'''
+        '''
+        assert err.message.contains(/The return type of java.lang.Double methodMakeT() in OverrideAnnotationTest is incompatible with java.lang.Integer in Parent/)
+        assert err.message.contains(/Method 'methodTakeT' from class 'OverrideAnnotationTest' does not override method from its superclass or interfaces but is annotated with @Override./)
     }
 
+    @Test
     void testSpuriousMethod() {
-        def message = shouldFail """
+        def err = shouldFail '''
             interface Intf<U> {
                 def method()
             }
@@ -107,12 +100,13 @@ def d = new Derived()
                 @Override method() {}
                 @Override someOtherMethod() {}
             }
-        """
-        assert message.contains("Method 'someOtherMethod' from class 'HasSpuriousMethod' does not override method from its superclass or interfaces but is annotated with @Override.")
+        '''
+        assert err.message.contains("Method 'someOtherMethod' from class 'HasSpuriousMethod' does not override method from its superclass or interfaces but is annotated with @Override.")
     }
 
+    @Test
     void testBadReturnType() {
-        def message = shouldFail """
+        def err = shouldFail '''
             interface Intf<U> {
                 def method()
                 U method6()
@@ -124,12 +118,13 @@ def d = new Derived()
                 @Override method() {}
                 @Override methodReturnsObject() {}
             }
-        """
-        assert message.contains("Method 'methodReturnsObject' from class 'HasMethodWithBadReturnType' does not override method from its superclass or interfaces but is annotated with @Override.")
+        '''
+        assert err.message.contains("Method 'methodReturnsObject' from class 'HasMethodWithBadReturnType' does not override method from its superclass or interfaces but is annotated with @Override.")
     }
 
-    void testBadArgType() {
-        def message = shouldFail """
+    @Test
+    void testBadParameterType() {
+        def err = shouldFail '''
             interface Intf<U> {
                 def method()
                 void method6(U u)
@@ -141,44 +136,46 @@ def d = new Derived()
                 @Override method() {}
                 @Override void methodTakesObject(arg) {}
             }
-        """
-        assert message.contains("Method 'methodTakesObject' from class 'HasMethodWithBadArgType' does not override method from its superclass or interfaces but is annotated with @Override.")
+        '''
+        assert err.message.contains("Method 'methodTakesObject' from class 'HasMethodWithBadArgType' does not override method from its superclass or interfaces but is annotated with @Override.")
     }
 
-    void testOverrideOnMethodWithDefaultParameters() {
+    @Test // GROOVY-6654
+    void testCovariantParameterType1() {
         assertScript '''
-            interface TemplatedInterface {
-                String execute(Map argument)
+            class C<T> {
+                void proc(T t) {}
             }
 
-            class TemplatedInterfaceImplementation implements TemplatedInterface {
+            class D extends C<String> {
                 @Override
-                String execute(Map argument = [:]) {
-                    return null
-                }
+                void proc(String s) {}
             }
-            new TemplatedInterfaceImplementation()
+
+            def d = new D()
         '''
     }
 
-    void testOverrideOnMethodWithDefaultParametersVariant() {
+    @Test // GROOVY-10675
+    void testCovariantParameterType2() {
         assertScript '''
-            interface TemplatedInterface {
-                String execute(Map argument)
+            @FunctionalInterface
+            interface A<I, O> {
+                O apply(I in)
             }
-
-            class TemplatedInterfaceImplementation implements TemplatedInterface {
-                @Override
-                String execute(Map argument, String foo = null) {
-                    return foo
-                }
+            interface B<X, Y> extends A<X, Y> {
             }
-            new TemplatedInterfaceImplementation()
+            class C implements B<Number, String> {
+                @Override String apply(Number n) { 'x' }
+            }
+
+            def result = new C().apply(42)
+            assert result == 'x'
         '''
     }
 
-    //GROOVY-7849
-    void testArrayReturnTypeCovariance() {
+    @Test // GROOVY-7849
+    void testCovariantArrayReturnType1() {
         assertScript '''
             interface Base {}
 
@@ -195,8 +192,8 @@ def d = new Derived()
         '''
     }
 
-    //GROOVY-7185
-    void testArrayReturnTypeCovarianceGenericsVariant() {
+    @Test // GROOVY-7185
+    void testCovariantArrayReturnType2() {
         assertScript '''
             interface A<T> {
                 T[] process();
@@ -218,4 +215,38 @@ def d = new Derived()
             assert new C().process()[0] == 'foo'
         '''
     }
+
+    @Test
+    void testOverrideOnMethodWithDefaultParameters() {
+        assertScript '''
+            interface TemplatedInterface {
+                String execute(Map argument)
+            }
+
+            class TemplatedInterfaceImplementation implements TemplatedInterface {
+                @Override
+                String execute(Map argument = [:]) {
+                    return null
+                }
+            }
+            new TemplatedInterfaceImplementation()
+        '''
+    }
+
+    @Test
+    void testOverrideOnMethodWithDefaultParametersVariant() {
+        assertScript '''
+            interface TemplatedInterface {
+                String execute(Map argument)
+            }
+
+            class TemplatedInterfaceImplementation implements TemplatedInterface {
+                @Override
+                String execute(Map argument, String foo = null) {
+                    return foo
+                }
+            }
+            new TemplatedInterfaceImplementation()
+        '''
+    }
 }