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 2024/12/10 18:54:42 UTC
(groovy) branch master updated (f30c96b7a7 -> 634ce6b09d)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
from f30c96b7a7 fix typo
new 9278d262a0 GROOVY-10811: `final` or `abstract` is implicit for an `enum` definition
new 634ce6b09d GROOVY-10811: error for explicit `super(...)` call in `enum` constructor
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../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 | 43 +++++++++++++++------
src/test/gls/enums/EnumTest.groovy | 45 ++++++++++++++++++++++
src/test/groovy/bugs/GroovyInnerEnumBug.groovy | 21 +++++-----
.../ASTTransformationCustomizerTest.groovy | 6 +--
7 files changed, 102 insertions(+), 43 deletions(-)
(groovy) 02/02: GROOVY-10811: error for explicit `super(...)` call in `enum` constructor
Posted by em...@apache.org.
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 634ce6b09da431b0c2df6ff6fa6aad486a6cae43
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Dec 10 12:37:25 2024 -0600
GROOVY-10811: error for explicit `super(...)` call in `enum` constructor
---
src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java | 10 ++++++++++
src/test/gls/enums/EnumTest.groovy | 12 ++++++++++++
2 files changed, 22 insertions(+)
diff --git a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java b/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
index 851ac8c30d..4d5b3dd16d 100644
--- a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
@@ -36,6 +36,7 @@ import org.codehaus.groovy.ast.expr.ListExpression;
import org.codehaus.groovy.ast.expr.MapEntryExpression;
import org.codehaus.groovy.ast.expr.MethodCallExpression;
import org.codehaus.groovy.ast.expr.VariableExpression;
+import org.codehaus.groovy.ast.stmt.ExpressionStatement;
import org.codehaus.groovy.ast.stmt.Statement;
import org.codehaus.groovy.control.CompilationUnit;
import org.codehaus.groovy.control.SourceUnit;
@@ -44,6 +45,7 @@ import org.codehaus.groovy.syntax.SyntaxException;
import java.util.ArrayList;
import java.util.List;
+import java.util.StringJoiner;
import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedMethod;
import static org.codehaus.groovy.ast.ClassHelper.int_TYPE;
@@ -118,6 +120,14 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
} else if (!ctor.isPrivate()) {
addError(ctor, "Illegal modifier for the enum constructor; only private is permitted.");
}
+ if (ctor.firstStatementIsSpecialConstructorCall()) {
+ var ctorCall = (ConstructorCallExpression) ((ExpressionStatement) ctor.getFirstStatement()).getExpression();
+ if (ctorCall.isSuperCall()) {
+ var spec = new StringJoiner(",", enumClass.getNameWithoutPackage() + "(", ")");
+ for (Parameter p : ctor.getParameters()) spec.add(p.getType().getUnresolvedName());
+ addError(ctorCall, "Cannot invoke super constructor from enum constructor " + spec);
+ }
+ }
}
// GROOVY-10811: final and abstract are implicit modifiers
diff --git a/src/test/gls/enums/EnumTest.groovy b/src/test/gls/enums/EnumTest.groovy
index 4d80ac1e0d..ab736d037f 100644
--- a/src/test/gls/enums/EnumTest.groovy
+++ b/src/test/gls/enums/EnumTest.groovy
@@ -859,6 +859,18 @@ class EnumTest extends CompilableTestSupport {
"""
}
}
+
+ // GROOVY-10811
+ void testSuperCtorCall() {
+ shouldNotCompile '''
+ enum E {
+ FOO;
+ E() {
+ super()
+ }
+ }
+ '''
+ }
}
enum UsCoin {
(groovy) 01/02: GROOVY-10811: `final` or `abstract` is implicit for an `enum` definition
Posted by em...@apache.org.
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 <er...@thomsonreuters.com>
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();
}