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
commit ca1805d4422ea59657eaaf2e0f2973db5868b272 Author: Eric Milles <[email protected]> AuthorDate: Sun Mar 9 12:34:39 2025 -0500 GROOVY-11579: skip bridge method in final method override checking 3_0_X backport --- .../groovy/classgen/ClassCompletionVerifier.java | 44 ++++++++++++--------- .../org/codehaus/groovy/classgen/Verifier.java | 7 ++-- src/test/groovy/OverrideTest.groovy | 46 ++++++++++++++++------ 3 files changed, 63 insertions(+), 34 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java index dff9da3a94..eae9ce9089 100644 --- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java +++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java @@ -73,6 +73,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; /** @@ -366,40 +367,46 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport { return method.getName().equals("<clinit>"); } - private void checkMethodsForOverridingFinal(ClassNode cn) { + 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 (!hasEqualParameterTypes(params, superParams)) continue; - if (!superMethod.isFinal()) break; - addInvalidUseOfFinalError(method, params, superMethod.getDeclaringClass()); - return; + if ((superMethod.getModifiers() & skips + ACC_FINAL) == ACC_FINAL + && hasEqualParameterTypes(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(MethodNode method, Parameter[] parameters, 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(Parameter[] parameters, StringBuilder msg) { - msg.append("("); + msg.append('('); boolean needsComma = false; for (Parameter parameter : parameters) { if (needsComma) { - msg.append(","); + msg.append(','); } else { needsComma = true; } msg.append(parameter.getType()); } - msg.append(")"); + msg.append(')'); } private void addWeakerAccessError(ClassNode cn, MethodNode method, Parameter[] parameters, MethodNode superMethod) { @@ -414,6 +421,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 34c2c962f7..b5fcf86146 100644 --- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java +++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java @@ -1510,9 +1510,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 c29618a8d9..ed7ccb0a08 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 '''
