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:20 UTC
[groovy] 01/02: GROOVY-10675: propagate type arguments to supertypes for override checks
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()
+ '''
+ }
}