This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY_4_0_X in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 2a2a9668675d6335776f9409aa6f125d1dcc7afd Author: Eric Milles <[email protected]> AuthorDate: Sun Mar 9 12:34:39 2025 -0500 GROOVY-11579: skip bridge method in final method override checking 4_0_X backport --- .../groovy/classgen/ClassCompletionVerifier.java | 36 ++++++++++------- .../org/codehaus/groovy/classgen/Verifier.java | 7 ++-- src/test/groovy/OverrideTest.groovy | 46 ++++++++++++++++------ 3 files changed, 59 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java index 8ff9c1ef00..6f7ae6f359 100644 --- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java +++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java @@ -74,6 +74,7 @@ import static org.objectweb.asm.Opcodes.ACC_PUBLIC; import static org.objectweb.asm.Opcodes.ACC_STATIC; import static org.objectweb.asm.Opcodes.ACC_STRICT; import static org.objectweb.asm.Opcodes.ACC_SYNCHRONIZED; +import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC; import static org.objectweb.asm.Opcodes.ACC_TRANSIENT; import static org.objectweb.asm.Opcodes.ACC_VOLATILE; @@ -431,27 +432,33 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport { } private void checkMethodsForOverridingFinal(final ClassNode cn) { + final int skips = ACC_SYNTHETIC | ACC_STATIC | ACC_PRIVATE; for (MethodNode method : cn.getMethods()) { + if ((method.getModifiers() & skips) != 0) continue; // GROOVY-11579 + Parameter[] params = method.getParameters(); for (MethodNode superMethod : cn.getSuperClass().getMethods(method.getName())) { - Parameter[] superParams = superMethod.getParameters(); - if (!ParameterUtils.parametersEqual(params, superParams)) continue; - if (!superMethod.isFinal()) break; - addInvalidUseOfFinalError(method, params, superMethod.getDeclaringClass()); - return; + if ((superMethod.getModifiers() & skips + ACC_FINAL) == ACC_FINAL + && ParameterUtils.parametersEqual(params, superMethod.getParameters())) { + StringBuilder sb = new StringBuilder(); + sb.append("You are not allowed to override the final method "); + if (method.getName().contains(" ")) { + sb.append('"').append(method.getName()).append('"'); + } else { + sb.append(method.getName()); + } + appendParamsDescription(params, sb); + sb.append(" from "); + sb.append(getDescription(superMethod.getDeclaringClass())); + sb.append("."); + + addError(sb.toString(), method.getLineNumber() > 0 ? method : cn); + break; + } } } } - private void addInvalidUseOfFinalError(final MethodNode method, final Parameter[] parameters, final ClassNode superCN) { - StringBuilder msg = new StringBuilder(); - msg.append("You are not allowed to override the final method ").append(method.getName()); - appendParamsDescription(parameters, msg); - msg.append(" from ").append(getDescription(superCN)); - msg.append("."); - addError(msg.toString(), method); - } - private void appendParamsDescription(final Parameter[] parameters, final StringBuilder msg) { msg.append('('); boolean needsComma = false; @@ -478,6 +485,7 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport { msg.append(superMethod.getDeclaringClass().getName()); msg.append("; attempting to assign weaker access privileges; was "); msg.append(superMethod.isPublic() ? "public" : (superMethod.isProtected() ? "protected" : "package-private")); + addError(msg.toString(), method); } diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java index df5de6bb6b..60678f7470 100644 --- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java +++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java @@ -1602,9 +1602,10 @@ public class Verifier implements GroovyClassVisitor, Opcodes { } private static ClassNode cleanType(final ClassNode type) { - // TODO: Should this be directly handled by getPlainNodeReference? - if (type.isArray()) return cleanType(type.getComponentType()).makeArray(); - return type.getPlainNodeReference(); + if (type.isArray()) { + return cleanType(type.getComponentType()).makeArray(); + } + return type.redirect().getPlainNodeReference(); } private void storeMissingCovariantMethods(final Iterable<MethodNode> methods, final MethodNode method, final Map<String, MethodNode> methodsToAdd, final Map<String, ClassNode> genericsSpec, final boolean ignoreError) { diff --git a/src/test/groovy/OverrideTest.groovy b/src/test/groovy/OverrideTest.groovy index 2280822ba3..a7699d7002 100644 --- a/src/test/groovy/OverrideTest.groovy +++ b/src/test/groovy/OverrideTest.groovy @@ -140,7 +140,8 @@ final class OverrideTest { assert err.message.contains("Method 'methodTakesObject' from class 'HasMethodWithBadArgType' does not override method from its superclass or interfaces but is annotated with @Override.") } - @Test // GROOVY-6654 + // GROOVY-6654 + @Test void testCovariantParameterType1() { assertScript ''' class C<T> { @@ -156,7 +157,8 @@ final class OverrideTest { ''' } - @Test // GROOVY-10675 + // GROOVY-10675 + @Test void testCovariantParameterType2() { assertScript ''' @FunctionalInterface @@ -174,48 +176,66 @@ final class OverrideTest { ''' } - @Test // GROOVY-7849 + // GROOVY-7849 + @Test void testCovariantArrayReturnType1() { assertScript ''' - interface Base {} - - interface Derived extends Base {} - + interface A { + } + interface B extends A { + } interface I { - Base[] foo() + A[] foo() } - class C implements I { - Derived[] foo() { null } + B[] foo() { null } } + new C().foo() ''' } - @Test // GROOVY-7185 + // GROOVY-7185 + @Test void testCovariantArrayReturnType2() { assertScript ''' interface A<T> { T[] process(); } - class B implements A<String> { @Override public String[] process() { ['foo'] } } - class C extends B { @Override String[] process() { super.process() } } + assert new C().process()[0] == 'foo' ''' } + // GROOVY-11579 + @Test + void testCovariantBridgeReturnType() { + assertScript ''' + interface I<T> { + T m() + } + abstract class A { + final String m() { 'A' } + } + class C extends A implements I<String> { + } + + assert new C().m() == 'A' + ''' + } + @Test void testOverrideOnMethodWithDefaultParameters() { assertScript '''
