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 2022/07/19 15:02:13 UTC
[groovy] branch GROOVY_3_0_X updated: GROOVY-10675: propagate type arguments to supertypes for override checks
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
The following commit(s) were added to refs/heads/GROOVY_3_0_X by this push:
new d10c2d3edf GROOVY-10675: propagate type arguments to supertypes for override checks
d10c2d3edf is described below
commit d10c2d3edf775637deb653ef940fe6e146e0213d
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
3_0_X backport
---
.../codehaus/groovy/classgen/ExtendedVerifier.java | 45 +++----
src/test/groovy/OverrideTest.groovy | 139 +++++++++++++--------
2 files changed, 103 insertions(+), 81 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
index 4f29cbfcbf..ad5cb2712d 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
@@ -21,7 +21,6 @@ package org.codehaus.groovy.classgen;
import org.codehaus.groovy.ast.AnnotatedNode;
import org.codehaus.groovy.ast.AnnotationNode;
import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
-import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.ConstructorNode;
import org.codehaus.groovy.ast.FieldNode;
@@ -36,7 +35,6 @@ import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.ListExpression;
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;
@@ -46,16 +44,16 @@ 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.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
+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;
/**
@@ -280,32 +278,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 (!origInterface.equals(ClassHelper.OBJECT_TYPE)) {
- 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;
}
@@ -313,10 +304,10 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
return next != null;
}
- private static MethodNode getDeclaredMethodCorrected(Map 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;
}
}
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()
+ '''
+ }
}