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 2022/01/31 18:00:21 UTC
[groovy] 02/05: GROOVY-6747: ensure enum ctor private and add bridge for const with body
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit e0fc7d0ad7951cdcf43d096de39b54134ee1a495
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Mar 18 18:58:18 2021 -0500
GROOVY-6747: ensure enum ctor private and add bridge for const with body
2_5_X backport
enum E {
X {
},
Y
}
E provides a private, implicit constructor (String,int) for Y and a
package-private, synthetic constructor (String,int,E) for X (aka E$1).
Conflicts:
src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
---
.../groovy/classgen/EnumCompletionVisitor.java | 97 +++++++++++++---------
.../org/codehaus/groovy/classgen/EnumVisitor.java | 15 +++-
.../TupleConstructorASTTransformation.java | 14 +---
src/test/gls/enums/EnumTest.groovy | 34 ++++++++
4 files changed, 109 insertions(+), 51 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java b/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
index 8ef3a04..c209a3f 100644
--- a/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
@@ -23,10 +23,10 @@ import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.CodeVisitorSupport;
import org.codehaus.groovy.ast.ConstructorNode;
-import org.codehaus.groovy.ast.EnumConstantClassNode;
-import org.codehaus.groovy.ast.InnerClassNode;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.expr.ArgumentListExpression;
+import org.codehaus.groovy.ast.expr.CastExpression;
+import org.codehaus.groovy.ast.expr.ConstantExpression;
import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.TupleExpression;
@@ -41,7 +41,9 @@ import org.codehaus.groovy.transform.TupleConstructorASTTransformation;
import java.util.ArrayList;
import java.util.List;
-import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
+import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedConstructor;
+import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
+import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
/**
* Enums have a parent constructor with two arguments from java.lang.Enum.
@@ -49,57 +51,55 @@ import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
* and performs the necessary super call.
*/
public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
+
private final SourceUnit sourceUnit;
public EnumCompletionVisitor(CompilationUnit cu, SourceUnit su) {
sourceUnit = su;
}
- public void visitClass(ClassNode node) {
- if (!node.isEnum()) return;
- completeEnum(node);
- }
-
+ @Override
protected SourceUnit getSourceUnit() {
return sourceUnit;
}
+ @Override
+ public void visitClass(ClassNode node) {
+ if (node.isEnum()) completeEnum(node);
+ }
+
private void completeEnum(ClassNode enumClass) {
- boolean isAic = isAnonymousInnerClass(enumClass);
- if (enumClass.getDeclaredConstructors().isEmpty()) {
- addImplicitConstructors(enumClass, isAic);
+ if (nonSyntheticConstructors(enumClass).isEmpty()) {
+ addImplicitConstructors(enumClass);
}
- for (ConstructorNode ctor : enumClass.getDeclaredConstructors()) {
- transformConstructor(ctor, isAic);
+ for (ConstructorNode ctor : nonSyntheticConstructors(enumClass)) {
+ transformConstructor(ctor);
}
}
/**
* Add map and no-arg constructor or mirror those of the superclass (i.e. base enum).
*/
- private static void addImplicitConstructors(ClassNode enumClass, boolean aic) {
- if (aic) {
- ClassNode sn = enumClass.getSuperClass();
- List<ConstructorNode> sctors = new ArrayList<ConstructorNode>(sn.getDeclaredConstructors());
- if (sctors.isEmpty()) {
- addMapConstructors(enumClass);
- } else {
- for (ConstructorNode constructorNode : sctors) {
- ConstructorNode init = new ConstructorNode(ACC_PUBLIC, constructorNode.getParameters(), ClassNode.EMPTY_ARRAY, new BlockStatement());
- enumClass.addConstructor(init);
+ private static void addImplicitConstructors(ClassNode enumClass) {
+ if (EnumVisitor.isAnonymousInnerClass(enumClass)) {
+ List<ConstructorNode> superCtors = nonSyntheticConstructors(enumClass.getSuperClass());
+ if (!superCtors.isEmpty()) {
+ for (ConstructorNode ctor : superCtors) {
+ addGeneratedConstructor(enumClass, ACC_PRIVATE, ctor.getParameters(), ClassNode.EMPTY_ARRAY, new BlockStatement());
}
+ return;
}
- } else {
- addMapConstructors(enumClass);
}
+ TupleConstructorASTTransformation.addSpecialMapConstructors(ACC_PRIVATE, enumClass, "One of the enum constants for enum " +
+ enumClass.getName() + " was initialized with null. Please use a non-null value or define your own constructor.", true);
}
/**
* If constructor does not define a call to super, then transform constructor
* to get String,int parameters at beginning and add call super(String,int).
*/
- private void transformConstructor(ConstructorNode ctor, boolean isAic) {
+ private void transformConstructor(ConstructorNode ctor) {
boolean chainedThisConstructorCall = false;
ConstructorCallExpression cce = null;
if (ctor.firstStatementIsSpecialConstructorCall()) {
@@ -127,13 +127,16 @@ public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
argsExprs.add(1, intVariable);
} else {
// add a super call
- List<Expression> args = new ArrayList<Expression>();
+ List<Expression> args = new ArrayList<>();
args.add(stringVariable);
args.add(intVariable);
- if (isAic) {
+ if (EnumVisitor.isAnonymousInnerClass(ctor.getDeclaringClass())) {
for (Parameter parameter : oldP) {
- args.add(new VariableExpression(parameter.getName()));
+ args.add(new VariableExpression(parameter));
}
+ ClassNode enumClass = ctor.getDeclaringClass().getSuperClass();
+ makeBridgeConstructor(enumClass, newP); // GROOVY-6747: bridge enum's private constructor
+ args.add(new CastExpression(enumClass.getPlainNodeReference(), ConstantExpression.NULL));
}
cce = new ConstructorCallExpression(ClassNode.SUPER, new ArgumentListExpression(args));
BlockStatement code = new BlockStatement();
@@ -144,15 +147,11 @@ public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
}
}
- private static void addMapConstructors(ClassNode enumClass) {
- TupleConstructorASTTransformation.addSpecialMapConstructors(ACC_PUBLIC, enumClass, "One of the enum constants for enum " + enumClass.getName() +
- " was initialized with null. Please use a non-null value or define your own constructor.", true);
- }
-
private String getUniqueVariableName(final String name, Statement code) {
if (code == null) return name;
final Object[] found = new Object[1];
CodeVisitorSupport cv = new CodeVisitorSupport() {
+ @Override
public void visitVariableExpression(VariableExpression expression) {
if (expression.getName().equals(name)) found[0] = Boolean.TRUE;
}
@@ -162,9 +161,31 @@ public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
return name;
}
- private static boolean isAnonymousInnerClass(ClassNode enumClass) {
- if (!(enumClass instanceof EnumConstantClassNode)) return false;
- InnerClassNode ic = (InnerClassNode) enumClass;
- return ic.getVariableScope() == null;
+ /**
+ * Ensures the enum type {@code e} has an accessible constructor for its AIC
+ * constant class to call. This constructor delegates to the enum's private
+ * constructor.
+ */
+ private static void makeBridgeConstructor(ClassNode e, Parameter[] p) {
+ Parameter[] newP = new Parameter[p.length + 1];
+ for (int i = 0; i < p.length; i += 1) {
+ newP[i] = new Parameter(p[i].getType(), "p" + i);
+ }
+ newP[p.length] = new Parameter(e.getPlainNodeReference(), "$anonymous");
+
+ if (e.getDeclaredConstructor(newP) == null) {
+ ArgumentListExpression args = new ArgumentListExpression();
+ for (int i = 0; i < p.length; i += 1) args.addExpression(new VariableExpression(newP[i]));
+ Statement thisCtorCall = new ExpressionStatement(new ConstructorCallExpression(ClassNode.THIS, args));
+ addGeneratedConstructor(e, ACC_SYNTHETIC, newP, ClassNode.EMPTY_ARRAY, thisCtorCall).setSynthetic(true);
+ }
+ }
+
+ private static List<ConstructorNode> nonSyntheticConstructors(ClassNode cn) {
+ List<ConstructorNode> ctors = new ArrayList<>(4);
+ for (ConstructorNode ctor : cn.getDeclaredConstructors()) {
+ if (!ctor.isSynthetic()) ctors.add(ctor);
+ }
+ return ctors;
}
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java b/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
index 208a34e..cdf900b 100644
--- a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
@@ -22,6 +22,7 @@ import org.codehaus.groovy.ast.AnnotatedNode;
import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.ConstructorNode;
import org.codehaus.groovy.ast.EnumConstantClassNode;
import org.codehaus.groovy.ast.FieldNode;
import org.codehaus.groovy.ast.InnerClassNode;
@@ -98,6 +99,15 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
values = new FieldNode("$VALUES", PRIVATE_FS | Opcodes.ACC_SYNTHETIC, enumRef.makeArray(), enumClass, null);
values.setSynthetic(true);
+ for (ConstructorNode ctor : enumClass.getDeclaredConstructors()) {
+ if (ctor.isSyntheticPublic()) {
+ ctor.setSyntheticPublic(false);
+ ctor.setModifiers((ctor.getModifiers() | Opcodes.ACC_PRIVATE) & ~Opcodes.ACC_PUBLIC);
+ } else if (!ctor.isPrivate()) {
+ addError(ctor, "Illegal modifier for the enum constructor; only private is permitted.");
+ }
+ }
+
addMethods(enumClass, values);
checkForAbstractMethods(enumClass);
@@ -296,7 +306,7 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
// calling the constructor may require a table with MetaClass
// selecting the constructor for each enum value. So instead we
// use this method to have a central point for constructor selection
- // and only one table. The whole construction is needed because
+ // and only one table. The whole construction is needed because
// Reflection forbids access to the enum constructor.
// code:
// def $INIT(Object[] para) {
@@ -437,10 +447,9 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
);
}
- private static boolean isAnonymousInnerClass(ClassNode enumClass) {
+ static boolean isAnonymousInnerClass(ClassNode enumClass) {
if (!(enumClass instanceof EnumConstantClassNode)) return false;
InnerClassNode ic = (InnerClassNode) enumClass;
return ic.getVariableScope() == null;
}
-
}
diff --git a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
index 1422fe7..a9271cf 100644
--- a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
@@ -58,7 +58,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
-import static org.apache.groovy.ast.tools.AnnotatedNodeUtils.markAsGenerated;
+import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedConstructor;
import static org.apache.groovy.ast.tools.ClassNodeUtils.hasExplicitConstructor;
import static org.apache.groovy.ast.tools.VisibilityUtils.getVisibility;
import static org.codehaus.groovy.ast.ClassHelper.make;
@@ -260,9 +260,7 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
boolean hasMapCons = AnnotatedNodeUtils.hasAnnotation(cNode, MapConstructorASTTransformation.MY_TYPE);
int modifiers = getVisibility(anno, cNode, ConstructorNode.class, ACC_PUBLIC);
- ConstructorNode consNode = new ConstructorNode(modifiers, params.toArray(Parameter.EMPTY_ARRAY), ClassNode.EMPTY_ARRAY, body);
- markAsGenerated(cNode, consNode);
- cNode.addConstructor(consNode);
+ addGeneratedConstructor(cNode, modifiers, params.toArray(Parameter.EMPTY_ARRAY), ClassNode.EMPTY_ARRAY, body);
if (sourceUnit != null && !body.isEmpty()) {
VariableScopeVisitor scopeVisitor = new VariableScopeVisitor(sourceUnit);
@@ -321,16 +319,12 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
code.addStatement(ifElseS(equalsNullX(namedArgs),
illegalArgumentBlock(message),
processArgsBlock(cNode, namedArgs)));
- ConstructorNode init = new ConstructorNode(modifiers, parameters, ClassNode.EMPTY_ARRAY, code);
- markAsGenerated(cNode, init);
- cNode.addConstructor(init);
+ addGeneratedConstructor(cNode, modifiers, parameters, ClassNode.EMPTY_ARRAY, code);
// potentially add a no-arg constructor too
if (addNoArg) {
code = new BlockStatement();
code.addStatement(stmt(ctorX(ClassNode.THIS, ctorX(LHMAP_TYPE))));
- init = new ConstructorNode(modifiers, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, code);
- markAsGenerated(cNode, init);
- cNode.addConstructor(init);
+ addGeneratedConstructor(cNode, modifiers, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, code);
}
}
diff --git a/src/test/gls/enums/EnumTest.groovy b/src/test/gls/enums/EnumTest.groovy
index 0e72f0f..79da52f 100644
--- a/src/test/gls/enums/EnumTest.groovy
+++ b/src/test/gls/enums/EnumTest.groovy
@@ -424,6 +424,30 @@ class EnumTest extends CompilableTestSupport {
"""
}
+ void testOverridingMethodsWithExplicitConstructor2() {
+ // GROOVY-6747
+ assertScript """
+ enum Codes {
+ YES('Y') {
+ @Override String getCode() { /*string*/ }
+ },
+ NO('N') {
+ @Override String getCode() { /*string*/ }
+ }
+
+ abstract String getCode()
+
+ private final String string
+
+ private Codes(String string) {
+ this.string = string
+ }
+ }
+
+ assert Codes.YES.code == null // TODO: 'Y'
+ """
+ }
+
void testAbstractMethodOverriding() {
// GROOVY-4641
assertScript """
@@ -600,6 +624,16 @@ class EnumTest extends CompilableTestSupport {
'''
}
+ void testEnumConstructorHasPrivateModifier() {
+ assertScript '''
+ enum Foo {
+ BAR, BAZ
+ Foo() {}
+ }
+ assert java.lang.reflect.Modifier.isPrivate(Foo.declaredConstructors[0].modifiers)
+ '''
+ }
+
void testNestedEnumHasStaticModifier_GROOVY_8360() {
assertScript '''
class Foo {