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 16:54:07 UTC
[groovy] branch GROOVY_2_5_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_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_2_5_X by this push:
new 46441aa6cc GROOVY-10675: propagate type arguments to supertypes for override checks
46441aa6cc is described below
commit 46441aa6ccf4851deab49eea2c701929832f5b49
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
2_5_X backport
---
.../codehaus/groovy/ast/tools/GeneralUtils.java | 28 +++--
.../codehaus/groovy/classgen/ExtendedVerifier.java | 49 +++-----
src/test/groovy/OverrideTest.groovy | 136 +++++++++++++--------
src/test/groovy/bugs/groovy10675/A.java | 23 ++++
src/test/groovy/bugs/groovy10675/B.java | 22 ++++
5 files changed, 163 insertions(+), 95 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
index b42d95f800..9de5360732 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
@@ -390,18 +390,22 @@ public class GeneralUtils {
return result;
}
- public static Set<ClassNode> getInterfacesAndSuperInterfaces(final ClassNode type) {
- Set<ClassNode> res = new LinkedHashSet<ClassNode>();
- if (type.isInterface()) {
- res.add(type);
- return res;
- }
- ClassNode next = type;
- while (next != null) {
- res.addAll(next.getAllInterfaces());
- next = next.getSuperClass();
- }
- return res;
+ public static Set<ClassNode> getInterfacesAndSuperInterfaces(final ClassNode cNode) {
+ Set<ClassNode> result = new LinkedHashSet<ClassNode>();
+ if (cNode.isInterface()) result.add(cNode);
+ addAllInterfaces(result, cNode);
+ return result;
+ }
+
+ private static void addAllInterfaces(final Set<ClassNode> result, final ClassNode source) {
+ for (ClassNode in : source.getInterfaces()) {
+ in = GenericsUtils.parameterizeType(source, in);
+ if (result.add(in)) addAllInterfaces(result, in);
+ }
+ ClassNode sc = source.redirect().getUnresolvedSuperClass(false);
+ if (sc != null && ClassHelper.OBJECT_TYPE != sc) {
+ addAllInterfaces(result, GenericsUtils.parameterizeType(source, sc));
+ }
}
public static List<FieldNode> getSuperNonPropertyFields(final ClassNode cNode) {
diff --git a/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
index b0c7c00469..c55f2bb51b 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
@@ -22,7 +22,6 @@ import org.codehaus.groovy.ast.ASTNode;
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;
@@ -38,7 +37,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;
@@ -49,15 +47,15 @@ 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 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;
/**
* A specialized Groovy AST visitor meant to perform additional verifications upon the
@@ -182,8 +180,8 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
checkForDuplicateAnnotations(node, nonSourceAnnotations);
}
- private void checkForDuplicateAnnotations(AnnotatedNode node, Map<String, List<AnnotationNode>> runtimeAnnotations) {
- for (Map.Entry<String, List<AnnotationNode>> next : runtimeAnnotations.entrySet()) {
+ private void checkForDuplicateAnnotations(AnnotatedNode node, Map<String, List<AnnotationNode>> nonSourceAnnotations) {
+ for (Map.Entry<String, List<AnnotationNode>> next : nonSourceAnnotations.entrySet()) {
if (next.getValue().size() > 1) {
ClassNode repeatable = null;
AnnotationNode repeatee = next.getValue().get(0);
@@ -272,32 +270,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<ClassNode>(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;
}
@@ -305,10 +296,10 @@ 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;
}
}
diff --git a/src/test/groovy/OverrideTest.groovy b/src/test/groovy/OverrideTest.groovy
index 9740d168ec..d1f24c8b85 100644
--- a/src/test/groovy/OverrideTest.groovy
+++ b/src/test/groovy/OverrideTest.groovy
@@ -18,9 +18,16 @@
*/
package groovy
-class OverrideTest extends GroovyTestCase {
+import org.junit.Test
+
+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) { }
@@ -45,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) { }
@@ -74,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()
}
@@ -105,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()
@@ -122,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)
@@ -139,44 +136,41 @@ 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() {
- assertScript '''
- interface TemplatedInterface {
- String execute(Map argument)
- }
+ @Test // GROOVY-10675
+ void testCovariantParameterType2() {
+ assertScript '''import groovy.bugs.groovy10675.B
- class TemplatedInterfaceImplementation implements TemplatedInterface {
- @Override
- String execute(Map argument, String foo = null) {
- return foo
- }
+ class C implements B<Number, String> {
+ @Override String apply(Number n) { 'x' }
}
- new TemplatedInterfaceImplementation()
+
+ def result = new C().apply(42)
+ assert result == 'x'
'''
}
- //GROOVY-7849
- void testArrayReturnTypeCovariance() {
+ @Test // GROOVY-7849
+ void testCovariantArrayReturnType1() {
assertScript '''
interface Base {}
@@ -193,8 +187,8 @@ def d = new Derived()
'''
}
- //GROOVY-7185
- void testArrayReturnTypeCovarianceGenericsVariant() {
+ @Test // GROOVY-7185
+ void testCovariantArrayReturnType2() {
assertScript '''
interface A<T> {
T[] process();
@@ -216,4 +210,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()
+ '''
+ }
}
diff --git a/src/test/groovy/bugs/groovy10675/A.java b/src/test/groovy/bugs/groovy10675/A.java
new file mode 100644
index 0000000000..d2c25836f8
--- /dev/null
+++ b/src/test/groovy/bugs/groovy10675/A.java
@@ -0,0 +1,23 @@
+/*
+ * 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.groovy10675;
+
+public interface A<I, O> {
+ O apply(I in);
+}
diff --git a/src/test/groovy/bugs/groovy10675/B.java b/src/test/groovy/bugs/groovy10675/B.java
new file mode 100644
index 0000000000..63063e4ab8
--- /dev/null
+++ b/src/test/groovy/bugs/groovy10675/B.java
@@ -0,0 +1,22 @@
+/*
+ * 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.groovy10675;
+
+public interface B<X, Y> extends A<X, Y> {
+}