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 c8f17c4a82a82d911ad8125e662c5b167c6ea682 Author: Eric Milles <[email protected]> AuthorDate: Thu Apr 24 15:33:21 2025 -0500 GROOVY-11634: check method modifiers: `abstract`, `static`, `final` 4_0_X backport --- .../groovy/classgen/ClassCompletionVerifier.java | 38 ++++++++-------------- .../classgen/ClassCompletionVerifierTest.java | 14 +++++--- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java index 6f7ae6f359..f749f96711 100644 --- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java +++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java @@ -66,7 +66,6 @@ import static java.lang.reflect.Modifier.isTransient; import static java.lang.reflect.Modifier.isVolatile; import static org.objectweb.asm.Opcodes.ACC_ABSTRACT; import static org.objectweb.asm.Opcodes.ACC_FINAL; -import static org.objectweb.asm.Opcodes.ACC_INTERFACE; import static org.objectweb.asm.Opcodes.ACC_NATIVE; import static org.objectweb.asm.Opcodes.ACC_PRIVATE; import static org.objectweb.asm.Opcodes.ACC_PROTECTED; @@ -120,7 +119,6 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport { checkInterfaceMethodVisibility(node); checkAbstractMethodVisibility(node); checkClassForExtendingFinalOrSealed(node); - checkMethodsForIncorrectModifiers(node); checkMethodsForIncorrectName(node); checkMethodsForWeakerAccess(node); checkMethodsForOverridingFinal(node); @@ -411,20 +409,6 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport { } } - private void checkMethodsForIncorrectModifiers(final ClassNode cn) { - if (!cn.isInterface()) return; - for (MethodNode method : cn.getMethods()) { - if (method.isFinal()) { - addError("The " + getDescription(method) + " from " + getDescription(cn) + - " must not be final. It is by definition abstract.", method); - } - if (method.isStatic() && !method.isStaticConstructor()) { - addError("The " + getDescription(method) + " from " + getDescription(cn) + - " must not be static. Only fields may be static in an interface.", method); - } - } - } - private void checkMethodsForWeakerAccess(final ClassNode cn) { for (MethodNode method : cn.getMethods()) { checkMethodForWeakerAccessPrivileges(method, cn); @@ -496,7 +480,7 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport { checkAbstractDeclaration(node); checkRepetitiveMethod(node); checkOverloadingPrivateAndPublic(node); - checkMethodModifiers(node); + checkMethodForIncorrectModifiers(node); checkGenericsUsage(node, node.getParameters()); checkGenericsUsage(node, node.getReturnType()); for (Parameter param : node.getParameters()) { @@ -507,16 +491,22 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport { super.visitMethod(node); } - private void checkMethodModifiers(final MethodNode node) { - // don't check volatile here as it overlaps with ACC_BRIDGE - // additional modifiers not allowed for interfaces - if ((this.currentClass.getModifiers() & ACC_INTERFACE) != 0) { + private void checkMethodForIncorrectModifiers(final MethodNode node) { + if (node.isFinal() && currentClass.isInterface()) { + addError("The " + getDescription(node) + " from " + getDescription(currentClass) + " must not be final. It is by definition abstract.", node); + } else if (node.isStatic() && currentClass.isInterface()) { + if (!node.isStaticConstructor()) addError("The " + getDescription(node) + " from " + getDescription(currentClass) + " must not be static. Only fields may be static in an interface.", node); + } else if (node.isAbstract() && (node.isStatic() || node.isFinal())) { + addError("The " + getDescription(node) + " can only be one of abstract, static, final.", node); // GROOVY-11634 + } + + if (currentClass.isInterface()) { + checkMethodForModifier(node, isNative(node.getModifiers()), "native"); checkMethodForModifier(node, isStrict(node.getModifiers()), "strictfp"); checkMethodForModifier(node, isSynchronized(node.getModifiers()), "synchronized"); - checkMethodForModifier(node, isNative(node.getModifiers()), "native"); } - // transient overlaps with varargs but we don't add varargs until AsmClassGenerator - // but we might have varargs set from e.g. @Delegate of a varargs method so skip generated + // transient overlaps with varargs but we do not add varargs until AsmClassGenerator + // but we might have varargs set from @Delegate of varargs method, so skip generated if (!AnnotatedNodeUtils.isGenerated(node)) { checkMethodForModifier(node, isTransient(node.getModifiers()), "transient"); } diff --git a/src/test/groovy/org/codehaus/groovy/classgen/ClassCompletionVerifierTest.java b/src/test/groovy/org/codehaus/groovy/classgen/ClassCompletionVerifierTest.java index acd1652586..1a8dcdcb5b 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/ClassCompletionVerifierTest.java +++ b/src/test/groovy/org/codehaus/groovy/classgen/ClassCompletionVerifierTest.java @@ -170,15 +170,21 @@ public final class ClassCompletionVerifierTest { @Test public void shouldDetectIncorrectMethodModifiersInClass() { - ClassNode node = new ClassNode("zzz", ACC_PUBLIC, ClassHelper.OBJECT_TYPE); - node.addMethod(new MethodNode("tr", ACC_TRANSIENT, ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null)); + ClassNode node = new ClassNode("zzz", ACC_ABSTRACT | ACC_PUBLIC, ClassHelper.OBJECT_TYPE); + node.addMethod(new MethodNode("tr", ACC_TRANSIENT , ClassHelper.VOID_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null)); + node.addMethod(new MethodNode("xx", ACC_ABSTRACT | ACC_FINAL , ClassHelper.VOID_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null)); // GROOVY-11634 + node.addMethod(new MethodNode("yy", ACC_ABSTRACT | ACC_STATIC, ClassHelper.VOID_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null)); + node.addMethod(new MethodNode("zz", ACC_FINAL | ACC_STATIC, ClassHelper.VOID_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null)); // can't check volatile here as it doubles up with bridge addStaticConstructor(node); verifier.visitClass(node); - checkErrorCount(1); - checkErrorMessage("The method 'java.lang.Object tr()' has an incorrect modifier transient."); + checkErrorCount(3); + checkErrorMessage("The method 'void tr()' has an incorrect modifier transient."); + checkErrorMessage("The method 'void xx()' can only be one of abstract, static, final."); + checkErrorMessage("The method 'void yy()' can only be one of abstract, static, final."); + //checkErrorMessage("The method 'void zz()' can only be one of abstract, static, final."); } @Test
