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 2021/03/19 00:03:13 UTC
[groovy] branch GROOVY-6747 created (now d9d1827)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-6747
in repository https://gitbox.apache.org/repos/asf/groovy.git.
at d9d1827 GROOVY-6747: ensure enum ctor private and add bridge for const with body
This branch includes the following new commits:
new d9d1827 GROOVY-6747: ensure enum ctor private and add bridge for const with body
The 1 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.
[groovy] 01/01: GROOVY-6747: ensure enum ctor private and add
bridge for const with body
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-6747
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit d9d1827fc848cfc47eea095c3f0f7a5b9d790b0c
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
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).
---
.../groovy/classgen/EnumCompletionVisitor.java | 44 +++++++++++++++++++---
.../org/codehaus/groovy/classgen/EnumVisitor.java | 10 +++++
.../TupleConstructorASTTransformation.java | 14 ++-----
src/test/gls/enums/EnumTest.groovy | 35 +++++++++++++++++
4 files changed, 88 insertions(+), 15 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java b/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
index 72593e1..8b24c4a 100644
--- a/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
@@ -25,6 +25,8 @@ import org.codehaus.groovy.ast.CodeVisitorSupport;
import org.codehaus.groovy.ast.ConstructorNode;
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;
@@ -39,6 +41,11 @@ import org.codehaus.groovy.transform.TupleConstructorASTTransformation;
import java.util.ArrayList;
import java.util.List;
+import static java.util.stream.Collectors.toList;
+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.
* This visitor adds those two arguments into manually created constructors
@@ -63,11 +70,11 @@ public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
}
private void completeEnum(final ClassNode enumClass) {
- if (enumClass.getDeclaredConstructors().isEmpty()) {
+ if (nonSyntheticConstructors(enumClass).isEmpty()) {
addImplicitConstructors(enumClass);
}
- for (ConstructorNode ctor : enumClass.getDeclaredConstructors()) {
+ for (ConstructorNode ctor : nonSyntheticConstructors(enumClass)) {
transformConstructor(ctor);
}
}
@@ -77,10 +84,10 @@ public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
*/
private static void addImplicitConstructors(final ClassNode enumClass) {
if (EnumVisitor.isAnonymousInnerClass(enumClass)) {
- List<ConstructorNode> superCtors = enumClass.getSuperClass().getDeclaredConstructors();
+ List<ConstructorNode> superCtors = nonSyntheticConstructors(enumClass.getSuperClass());
if (!superCtors.isEmpty()) {
for (ConstructorNode ctor : superCtors) {
- enumClass.addConstructor(0, ctor.getParameters(), ClassNode.EMPTY_ARRAY, new BlockStatement());
+ addGeneratedConstructor(enumClass, ACC_PRIVATE, ctor.getParameters(), ClassNode.EMPTY_ARRAY, new BlockStatement());
}
return;
}
@@ -126,8 +133,11 @@ public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
args.add(intVariable);
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();
@@ -151,4 +161,28 @@ public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
if (found[0] != null) return getUniqueVariableName("_" + name, code);
return name;
}
+
+ /**
+ * 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(final ClassNode e, final 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(final ClassNode cn) {
+ return cn.getDeclaredConstructors().stream().filter(c -> !c.isSynthetic()).collect(toList());
+ }
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java b/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
index 2556f6a..2940fa3 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;
@@ -99,6 +100,15 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
values = new FieldNode("$VALUES", ACC_FINAL | ACC_PRIVATE | ACC_STATIC | ACC_SYNTHETIC, enumPlain.makeArray(), enumClass, null);
values.setSynthetic(true);
+ for (ConstructorNode ctor : enumClass.getDeclaredConstructors()) {
+ if (ctor.isSyntheticPublic()) {
+ ctor.setSyntheticPublic(false);
+ ctor.setModifiers((ctor.getModifiers() | ACC_PRIVATE) & ~ACC_PUBLIC);
+ } else if (!ctor.isPrivate()) {
+ addError(ctor, "Illegal modifier for the enum constructor; only private is permitted.");
+ }
+ }
+
addMethods(enumClass, values);
checkForAbstractMethods(enumClass);
}
diff --git a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
index 26b3ceb..fe87d0f 100644
--- a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
@@ -55,7 +55,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.ConstructorNodeUtils.checkPropNamesS;
import static org.apache.groovy.ast.tools.VisibilityUtils.getVisibility;
@@ -254,9 +254,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);
@@ -315,16 +313,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 542b4bf..d2982cb 100644
--- a/src/test/gls/enums/EnumTest.groovy
+++ b/src/test/gls/enums/EnumTest.groovy
@@ -425,6 +425,30 @@ class EnumTest extends CompilableTestSupport {
'''
}
+ // GROOVY-6747
+ void testOverridingMethodsWithExplicitConstructor2() {
+ 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'
+ '''
+ }
+
// GROOVY-4641
void testAbstractMethodOverriding() {
assertScript """
@@ -603,6 +627,17 @@ class EnumTest extends CompilableTestSupport {
'''
}
+ // GROOVY-6747
+ void testEnumConstructorHasPrivateModifier() {
+ assertScript '''
+ enum Foo {
+ BAR, BAZ
+ Foo() {}
+ }
+ assert java.lang.reflect.Modifier.isPrivate(Foo.declaredConstructors[0].modifiers)
+ '''
+ }
+
// GROOVY-8360
void testNestedEnumHasStaticModifier() {
assertScript '''