This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 9278d262a07624a095ec5ace875e981656f0cb9d Author: Eric Milles <[email protected]> AuthorDate: Tue Dec 10 12:18:00 2024 -0600 GROOVY-10811: `final` or `abstract` is implicit for an `enum` definition --- .../apache/groovy/parser/antlr4/AstBuilder.java | 2 -- .../java/org/codehaus/groovy/antlr/EnumHelper.java | 26 ++++++----------- .../groovy/classgen/ClassCompletionVerifier.java | 2 ++ .../org/codehaus/groovy/classgen/EnumVisitor.java | 33 ++++++++++++++-------- src/test/gls/enums/EnumTest.groovy | 33 ++++++++++++++++++++++ src/test/groovy/bugs/GroovyInnerEnumBug.groovy | 21 +++++++------- .../ASTTransformationCustomizerTest.groovy | 6 ++-- 7 files changed, 80 insertions(+), 43 deletions(-) diff --git a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java index 08c60e9322..6138cd6c64 100644 --- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java +++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java @@ -3360,8 +3360,6 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { InnerClassNode anonymousInnerClass; if (ctx.t == 1) { anonymousInnerClass = new EnumConstantClassNode(outerClass, innerClassName, superClass.getPlainNodeReference()); - // and remove the final modifier from superClass to allow the sub class - superClass.setModifiers(superClass.getModifiers() & ~Opcodes.ACC_FINAL); } else { anonymousInnerClass = new InnerClassNode(outerClass, innerClassName, 0, superClass); } diff --git a/src/main/java/org/codehaus/groovy/antlr/EnumHelper.java b/src/main/java/org/codehaus/groovy/antlr/EnumHelper.java index 2f0ec08e05..9299a98df3 100644 --- a/src/main/java/org/codehaus/groovy/antlr/EnumHelper.java +++ b/src/main/java/org/codehaus/groovy/antlr/EnumHelper.java @@ -29,38 +29,30 @@ import org.codehaus.groovy.ast.expr.ListExpression; import org.objectweb.asm.Opcodes; public class EnumHelper { - private static final int FS = Opcodes.ACC_FINAL | Opcodes.ACC_STATIC; - private static final int PUBLIC_FS = Opcodes.ACC_PUBLIC | FS; - public static ClassNode makeEnumNode(String name, int modifiers, ClassNode[] interfaces, ClassNode outerClass) { - modifiers = modifiers | Opcodes.ACC_FINAL | Opcodes.ACC_ENUM; + public static ClassNode makeEnumNode(final String name, final int modifiers, final ClassNode[] interfaces, final ClassNode outerClass) { ClassNode enumClass; - if (outerClass==null) { - enumClass = new ClassNode(name,modifiers,null,interfaces,MixinNode.EMPTY_ARRAY); + if (outerClass == null) { + enumClass = new ClassNode(name, modifiers | Opcodes.ACC_ENUM, null, interfaces, MixinNode.EMPTY_ARRAY); } else { - name = outerClass.getName() + "$" + name; - modifiers |= Opcodes.ACC_STATIC; - enumClass = new InnerClassNode(outerClass,name,modifiers,null,interfaces,MixinNode.EMPTY_ARRAY); + enumClass = new InnerClassNode(outerClass, outerClass.getName() + "$" + name, modifiers | Opcodes.ACC_ENUM, null, interfaces, MixinNode.EMPTY_ARRAY); } - // set super class and generics info - // "enum X" -> class X extends Enum<X> - GenericsType gt = new GenericsType(enumClass); - ClassNode superClass = ClassHelper.makeWithoutCaching("java.lang.Enum"); - superClass.setGenericsTypes(new GenericsType[]{gt}); + // enum E extends java.lang.Enum<E> + ClassNode superClass = ClassHelper.Enum_Type.getPlainNodeReference(); + superClass.setGenericsTypes(new GenericsType[]{new GenericsType(enumClass)}); enumClass.setSuperClass(superClass); - superClass.setRedirect(ClassHelper.Enum_Type); return enumClass; } - public static FieldNode addEnumConstant(ClassNode enumClass, String name, Expression init) { - int modifiers = PUBLIC_FS | Opcodes.ACC_ENUM; + public static FieldNode addEnumConstant(final ClassNode enumClass, final String name, Expression init) { if (init != null && !(init instanceof ListExpression)) { ListExpression list = new ListExpression(); list.addExpression(init); init = list; } + final int modifiers = Opcodes.ACC_ENUM | Opcodes.ACC_FINAL | Opcodes.ACC_STATIC | Opcodes.ACC_PUBLIC; FieldNode fn = new FieldNode(name, modifiers, enumClass.getPlainNodeReference(), enumClass, init); enumClass.addField(fn); return fn; diff --git a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java index fcdf7bdbeb..30ca775504 100644 --- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java +++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java @@ -59,6 +59,7 @@ import java.util.Map; import static java.lang.reflect.Modifier.isFinal; import static java.lang.reflect.Modifier.isNative; import static java.lang.reflect.Modifier.isPrivate; +import static java.lang.reflect.Modifier.isProtected; import static java.lang.reflect.Modifier.isStatic; import static java.lang.reflect.Modifier.isStrict; import static java.lang.reflect.Modifier.isSynchronized; @@ -257,6 +258,7 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport { List<String> modifiers = new ArrayList<>(); if (!(node instanceof InnerClassNode)) { + if (isProtected(node.getModifiers())) modifiers.add("protected"); if (isPrivate(node.getModifiers())) modifiers.add("private"); if (isStatic(node.getModifiers())) modifiers.add("static"); } diff --git a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java b/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java index a29076de97..851ac8c30d 100644 --- a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java +++ b/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java @@ -73,6 +73,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.propX; import static org.codehaus.groovy.ast.tools.GeneralUtils.returnS; import static org.codehaus.groovy.ast.tools.GeneralUtils.spreadX; import static org.codehaus.groovy.ast.tools.GeneralUtils.varX; +import static org.codehaus.groovy.runtime.StreamGroovyMethods.stream; import static org.objectweb.asm.Opcodes.ACC_ABSTRACT; import static org.objectweb.asm.Opcodes.ACC_FINAL; import static org.objectweb.asm.Opcodes.ACC_PRIVATE; @@ -119,23 +120,25 @@ public class EnumVisitor extends ClassCodeVisitorSupport { } } + // GROOVY-10811: final and abstract are implicit modifiers + boolean isInnerClass = (enumClass.getOuterClass() != null); + if ((enumClass.getModifiers() & (ACC_ABSTRACT | ACC_FINAL)) != 0) { + String name = enumClass.getNameWithoutPackage(); // TODO: substring after the $ + String permitted = (!isInnerClass ? "public is" : "public, private, protected & static are"); + addError(enumClass, "Illegal modifier for the enum " + name + "; only " + permitted + " permitted."); + } + addMethods(enumClass, values, minValue, maxValue); - checkForAbstractMethods(enumClass); + + // for now, inner enum is always static + if (isInnerClass) enumClass.setModifiers(enumClass.getModifiers() | ACC_STATIC); + if (isAnyAbstract(enumClass)) enumClass.setModifiers(enumClass.getModifiers() | ACC_ABSTRACT); + else if (isNotExtended(enumClass)) enumClass.setModifiers(enumClass.getModifiers() | ACC_FINAL); } addInit(enumClass, minValue, maxValue, values, isAIC); } - private static void checkForAbstractMethods(final ClassNode enumClass) { - for (MethodNode method : enumClass.getMethods()) { - if (method.isAbstract()) { - // make the class abstract also; see Effective Java p.152 - enumClass.setModifiers(enumClass.getModifiers() | ACC_ABSTRACT); - break; - } - } - } - private static void addMethods(final ClassNode enumClass, final FieldNode values, FieldNode minValue, FieldNode maxValue) { boolean hasNext = false; @@ -328,4 +331,12 @@ public class EnumVisitor extends ClassCodeVisitorSupport { return enumClass instanceof EnumConstantClassNode && ((EnumConstantClassNode) enumClass).getVariableScope() == null; } + + private static boolean isAnyAbstract(final ClassNode enumClass) { + return enumClass.getMethods().stream().anyMatch(MethodNode::isAbstract); + } + + private static boolean isNotExtended(final ClassNode enumClass) { + return stream(enumClass.getInnerClasses()).noneMatch(it -> it instanceof EnumConstantClassNode); + } } diff --git a/src/test/gls/enums/EnumTest.groovy b/src/test/gls/enums/EnumTest.groovy index c7a849c88a..4d80ac1e0d 100644 --- a/src/test/gls/enums/EnumTest.groovy +++ b/src/test/gls/enums/EnumTest.groovy @@ -826,6 +826,39 @@ class EnumTest extends CompilableTestSupport { assert Outer.props == [bar: 10, baz: 20, foo: 30] ''' } + + // GROOVY-10811 + void testIllegalModifiers() { + for (mod in ['','public','@groovy.transform.PackageScope']) { + shouldCompile """ + $mod enum E { + } + """ + } + for (mod in ['abstract','final','static','private','protected']) { + def err = shouldNotCompile """ + $mod enum E { + } + """ + } + + for (mod in ['','public','private','protected','@groovy.transform.PackageScope','static']) { + shouldCompile """ + class C { + $mod enum E { + } + } + """ + } + for (mod in ['abstract','final']) { + shouldNotCompile """ + class C { + $mod enum E { + } + } + """ + } + } } enum UsCoin { diff --git a/src/test/groovy/bugs/GroovyInnerEnumBug.groovy b/src/test/groovy/bugs/GroovyInnerEnumBug.groovy index 289ed9d404..b14136827b 100644 --- a/src/test/groovy/bugs/GroovyInnerEnumBug.groovy +++ b/src/test/groovy/bugs/GroovyInnerEnumBug.groovy @@ -21,29 +21,30 @@ package groovy.bugs import groovy.test.GroovyTestCase class GroovyInnerEnumBug extends GroovyTestCase { - static public enum MyEnum { - a, b, c - public static MyEnum[] myenums = [a, b, c]; - } // GROOVY-3979 - void testEnumInsideAClass3979() { - assertScript """ + void testInnerEnum1() { + assertScript ''' class EnumTest2 { enum Direction3979 { North, East, South, West } static void main(args) { - for (d in Direction3979) { + for (d in Direction3979) { assert d instanceof Direction3979 } } } - """ + ''' } // GROOVY-3994 - void testEnumInsideAClass3994() { + void testInnerEnum2() { assert MyEnum.a.name() == 'a' assertTrue Enum.isAssignableFrom(MyEnum.class) - assert EnumSet.allOf(MyEnum.class) instanceof EnumSet + assert EnumSet.allOf(MyEnum.class) instanceof EnumSet + } + + public static enum MyEnum { + a, b, c; + public static MyEnum[] myenums = [a, b, c]; } } diff --git a/subprojects/groovy-astbuilder/src/test/groovy/org/codehaus/groovy/control/customizers/ASTTransformationCustomizerTest.groovy b/subprojects/groovy-astbuilder/src/test/groovy/org/codehaus/groovy/control/customizers/ASTTransformationCustomizerTest.groovy index f378ce192f..9c906a3c8f 100644 --- a/subprojects/groovy-astbuilder/src/test/groovy/org/codehaus/groovy/control/customizers/ASTTransformationCustomizerTest.groovy +++ b/subprojects/groovy-astbuilder/src/test/groovy/org/codehaus/groovy/control/customizers/ASTTransformationCustomizerTest.groovy @@ -108,12 +108,12 @@ class ASTTransformationCustomizerTest extends GroovyTestCase { @Retention(RetentionPolicy.SOURCE) @Target([ElementType.TYPE]) @GroovyASTTransformationClass("org.codehaus.groovy.control.customizers.ContractAnnotation") -protected @interface Contract { +@interface Contract { Class value(); } @GroovyASTTransformation(phase=CompilePhase.CONVERSION) -protected class ContractAnnotation implements ASTTransformation, Opcodes { +class ContractAnnotation implements ASTTransformation, Opcodes { void visit(ASTNode[] nodes, SourceUnit source) { def node = nodes[0] def member = node.getMember("value") @@ -123,6 +123,6 @@ protected class ContractAnnotation implements ASTTransformation, Opcodes { @Retention(RetentionPolicy.SOURCE) @Target([ElementType.TYPE]) -protected @interface Contract2 { +@interface Contract2 { Class value(); }
