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/11/21 23:30:18 UTC
[groovy] branch GROOVY_4_0_X updated: GROOVY-10108: `@Immutable(specialNamedArgHandling=false)` for `Map` prop
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
The following commit(s) were added to refs/heads/GROOVY_4_0_X by this push:
new b39fa7a2cf GROOVY-10108: `@Immutable(specialNamedArgHandling=false)` for `Map` prop
b39fa7a2cf is described below
commit b39fa7a2cf0ab38794f8b20413ead02ca42672ba
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Nov 21 17:03:46 2022 -0600
GROOVY-10108: `@Immutable(specialNamedArgHandling=false)` for `Map` prop
---
.../transform/ImmutableASTTransformation.java | 33 ++++---
.../transform/MapConstructorASTTransformation.java | 104 +++++++++------------
.../groovy/transform/ImmutableTransformTest.groovy | 16 ++++
3 files changed, 75 insertions(+), 78 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java
index b72d16ac18..3b95bd149d 100644
--- a/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java
@@ -110,25 +110,22 @@ public class ImmutableASTTransformation extends AbstractASTTransformation implem
@Override
public void visit(final ASTNode[] nodes, final SourceUnit source) {
init(nodes, source);
- AnnotatedNode parent = (AnnotatedNode) nodes[1];
AnnotationNode anno = (AnnotationNode) nodes[0];
- if (!MY_TYPE.equals(anno.getClassNode())) return;
-
- if (parent instanceof ClassNode) {
- final GroovyClassLoader classLoader = compilationUnit != null ? compilationUnit.getTransformLoader() : source.getClassLoader();
- final PropertyHandler handler = PropertyHandler.createPropertyHandler(this, classLoader, (ClassNode) parent);
- if (handler == null || !handler.validateAttributes(this, anno)) return;
- doMakeImmutable((ClassNode) parent, anno, handler);
+ AnnotatedNode parent = (AnnotatedNode) nodes[1];
+ if (MY_TYPE.equals(anno.getClassNode()) && parent instanceof ClassNode) { ClassNode type = (ClassNode) parent;
+ GroovyClassLoader classLoader = compilationUnit != null ? compilationUnit.getTransformLoader() : source.getClassLoader();
+ PropertyHandler handler = PropertyHandler.createPropertyHandler(this, classLoader, type);
+ if (handler != null
+ && handler.validateAttributes(this, anno)
+ && checkNotInterface(type, MY_TYPE_NAME))
+ doMakeImmutable(type, anno, handler);
}
}
- private void doMakeImmutable(final ClassNode cNode, final AnnotationNode node, final PropertyHandler handler) {
- List<PropertyNode> newProperties = new ArrayList<>();
-
+ private void doMakeImmutable(final ClassNode cNode, final AnnotationNode anno, final PropertyHandler handler) {
String cName = cNode.getName();
- if (!checkNotInterface(cNode, MY_TYPE_NAME)) return;
-
- final List<PropertyNode> pList = getInstanceProperties(cNode);
+ List<PropertyNode> newProperties = new ArrayList<>();
+ List<PropertyNode> pList = getInstanceProperties(cNode);
for (PropertyNode pNode : pList) {
adjustPropertyForImmutability(pNode, newProperties, handler);
}
@@ -136,8 +133,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation implem
cNode.getProperties().remove(pNode);
addProperty(cNode, pNode);
}
- final List<FieldNode> fList = cNode.getFields();
- for (FieldNode fNode : fList) {
+ for (FieldNode fNode : cNode.getFields()) {
ensureNotPublic(this, cName, fNode);
}
if (hasAnnotation(cNode, TupleConstructorASTTransformation.MY_TYPE)) {
@@ -153,7 +149,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation implem
if (unsupportedTupleAttribute(tupleCons, "force")) return;
}
if (hasExplicitConstructor(this, cNode)) return;
- if (!pList.isEmpty() && memberHasValue(node, "copyWith", Boolean.TRUE) && !hasDeclaredMethod(cNode, "copyWith", 1)) {
+ if (!pList.isEmpty() && memberHasValue(anno, "copyWith", Boolean.TRUE) && !hasDeclaredMethod(cNode, "copyWith", 1)) {
createCopyWith(cNode, pList);
}
}
@@ -409,6 +405,9 @@ public class ImmutableASTTransformation extends AbstractASTTransformation implem
throw new RuntimeException(createErrorMessage(clazz.getName(), fieldName, typeName, "constructing"));
}
+ /**
+ * Called during named-arguments constructor execution to check given names.
+ */
public static void checkPropNames(final Object instance, final Map<String, Object> args) {
final MetaClass metaClass = InvokerHelper.getMetaClass(instance);
for (String name : args.keySet()) {
diff --git a/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java
index 40538689bb..0944c8da5d 100644
--- a/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java
@@ -48,18 +48,15 @@ import java.util.ArrayList;
import java.util.HashSet;
import java.util.LinkedHashMap;
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.hasNoArgConstructor;
import static org.apache.groovy.ast.tools.VisibilityUtils.getVisibility;
import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
import static org.codehaus.groovy.ast.tools.GeneralUtils.copyStatementsWithSuperAdjustment;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorThisX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.getAllProperties;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.param;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.params;
import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
@@ -75,7 +72,7 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation i
static final Class<?> MY_CLASS = MapConstructor.class;
static final ClassNode MY_TYPE = ClassHelper.make(MY_CLASS);
static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage();
- private static final ClassNode MAP_TYPE = ClassHelper.makeWithoutCaching(Map.class, false);
+ private static final ClassNode MAP_TYPE = ClassHelper.MAP_TYPE.getPlainNodeReference();
private static final ClassNode LHMAP_TYPE = ClassHelper.makeWithoutCaching(LinkedHashMap.class, false);
@Override
@@ -98,17 +95,17 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation i
if (parent instanceof ClassNode) {
ClassNode cNode = (ClassNode) parent;
if (!checkNotInterface(cNode, MY_TYPE_NAME)) return;
- boolean includeFields = memberHasValue(anno, "includeFields", true);
- boolean includeProperties = !memberHasValue(anno, "includeProperties", false);
- boolean includeSuperProperties = memberHasValue(anno, "includeSuperProperties", true);
- boolean includeSuperFields = memberHasValue(anno, "includeSuperFields", true);
- boolean includeStatic = memberHasValue(anno, "includeStatic", true);
- boolean allProperties = memberHasValue(anno, "allProperties", true);
- boolean noArg = memberHasValue(anno, "noArg", true);
- boolean specialNamedArgHandling = !memberHasValue(anno, "specialNamedArgHandling", false);
+ boolean includeFields = memberHasValue(anno, "includeFields", Boolean.TRUE);
+ boolean includeProperties = !memberHasValue(anno, "includeProperties", Boolean.FALSE);
+ boolean includeSuperProperties = memberHasValue(anno, "includeSuperProperties", Boolean.TRUE);
+ boolean includeSuperFields = memberHasValue(anno, "includeSuperFields", Boolean.TRUE);
+ boolean includeStatic = memberHasValue(anno, "includeStatic", Boolean.TRUE);
+ boolean allProperties = memberHasValue(anno, "allProperties", Boolean.TRUE);
+ boolean noArg = memberHasValue(anno, "noArg", Boolean.TRUE);
+ boolean specialNamedArgHandling = !memberHasValue(anno, "specialNamedArgHandling", Boolean.FALSE);
List<String> excludes = getMemberStringList(anno, "excludes");
List<String> includes = getMemberStringList(anno, "includes");
- boolean allNames = memberHasValue(anno, "allNames", true);
+ boolean allNames = memberHasValue(anno, "allNames", Boolean.TRUE);
if (!checkIncludeExcludeUndefinedAware(anno, excludes, includes, MY_TYPE_NAME)) return;
if (!checkPropertyList(cNode, includes, "includes", anno, MY_TYPE_NAME, includeFields, includeSuperProperties, allProperties))
return;
@@ -141,65 +138,54 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation i
}
}
- private static void createConstructors(AbstractASTTransformation xform, AnnotationNode anno, PropertyHandler handler, ClassNode cNode, boolean includeFields, boolean includeProperties,
- boolean includeSuperProperties, boolean includeSuperFields, boolean noArg,
- boolean allNames, boolean allProperties, boolean specialNamedArgHandling, boolean includeStatic,
- List<String> excludes, List<String> includes, ClosureExpression pre, ClosureExpression post, SourceUnit source) {
+ private static void createConstructors(final AbstractASTTransformation xform, final AnnotationNode anno, final PropertyHandler handler, final ClassNode cNode,
+ final boolean includeFields, final boolean includeProperties, final boolean includeSuperProperties, final boolean includeSuperFields,
+ final boolean noArg, final boolean allNames, final boolean allProperties, final boolean specialNamedArgHandling, final boolean includeStatic,
+ final List<String> excludes, final List<String> includes, final ClosureExpression pre, final ClosureExpression post, final SourceUnit source) {
// HACK: JavaStubGenerator could have snuck in a constructor we don't want
cNode.getDeclaredConstructors().removeIf(next -> next.getFirstStatement() == null);
- Set<String> names = new HashSet<String>();
- List<PropertyNode> superList;
+ Set<String> names = new HashSet<>();
+ List<PropertyNode> properties;
if (includeSuperProperties || includeSuperFields) {
- superList = getAllProperties(names, cNode, cNode.getSuperClass(), includeSuperProperties, includeSuperFields, false, allProperties, true, false, false, allNames, includeStatic);
+ properties = getAllProperties(names, cNode, cNode.getSuperClass(), includeSuperProperties, includeSuperFields, false, allProperties, true, false, false, allNames, includeStatic);
} else {
- superList = new ArrayList<PropertyNode>();
+ properties = new ArrayList<>();
}
- List<PropertyNode> list = getAllProperties(names, cNode, cNode, includeProperties, includeFields, false, allProperties, false, false, false, allNames, includeStatic);
+ properties.addAll(getAllProperties(names, cNode, cNode, includeProperties, includeFields, false, allProperties, false, false, false, allNames, includeStatic));
- Parameter map = param(MAP_TYPE, "args");
- final BlockStatement body = new BlockStatement();
+ BlockStatement body = new BlockStatement();
ClassCodeExpressionTransformer transformer = makeMapTypedArgsTransformer();
if (pre != null) {
ClosureExpression transformed = (ClosureExpression) transformer.transform(pre);
copyStatementsWithSuperAdjustment(transformed, body);
}
- final BlockStatement inner = new BlockStatement();
- superList.addAll(list);
-
- if (!handler.validateProperties(xform, body, cNode, superList)) {
+ if (!handler.validateProperties(xform, body, cNode, properties)) {
return;
}
- boolean specialNamedArgCase = specialNamedArgHandling && ImmutableASTTransformation.isSpecialNamedArgCase(superList, true);
- processProps(xform, anno, cNode, handler, allNames, excludes, includes, superList, map, inner);
+ BlockStatement inner = new BlockStatement();
+ Parameter map = new Parameter(MAP_TYPE, "args");
+ boolean specialNamedArgsCase = specialNamedArgHandling
+ && ImmutableASTTransformation.isSpecialNamedArgCase(properties, true);
+ processProps(xform, anno, cNode, handler, allNames, excludes, includes, properties, map, inner);
+ if (specialNamedArgsCase) map = new Parameter(LHMAP_TYPE, "args");
body.addStatement(inner);
- Parameter[] params = params(specialNamedArgCase ? new Parameter(LHMAP_TYPE, "args") : map);
if (post != null) {
ClosureExpression transformed = (ClosureExpression) transformer.transform(post);
body.addStatement(transformed.getCode());
}
+
int modifiers = getVisibility(anno, cNode, ConstructorNode.class, ACC_PUBLIC);
- doAddConstructor(cNode, new ConstructorNode(modifiers, params, ClassNode.EMPTY_ARRAY, body));
- if (noArg && !superList.isEmpty() && !hasNoArgConstructor(cNode)/* && !specialNamedArgCase*/) {
- createNoArgConstructor(cNode, modifiers);
- }
+ createConstructors(cNode, modifiers, map, body, noArg && !properties.isEmpty()/* && !specialNamedArgsCase*/);
}
- private static void doAddConstructor(final ClassNode cNode, final ConstructorNode constructorNode) {
- markAsGenerated(cNode, constructorNode);
- cNode.addConstructor(constructorNode);
- // GROOVY-5814: Immutable is not compatible with @CompileStatic
- Parameter argsParam = null;
- for (Parameter p : constructorNode.getParameters()) {
- if ("args".equals(p.getName())) {
- argsParam = p;
- break;
- }
- }
- if (argsParam != null) {
- final Parameter arg = argsParam;
+ private static void createConstructors(final ClassNode cNode, final int mods, final Parameter args, final Statement body, final boolean noArg) {
+ Parameter[] parameters = {args};
+ if (cNode.getDeclaredConstructor(parameters) == null) {
+ ConstructorNode ctor = addGeneratedConstructor(cNode, mods, parameters, ClassNode.EMPTY_ARRAY, body);
+ // GROOVY-5814: fix compatibility with @CompileStatic
ClassCodeVisitorSupport variableExpressionFix = new ClassCodeVisitorSupport() {
@Override
protected SourceUnit getSourceUnit() {
@@ -210,11 +196,14 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation i
public void visitVariableExpression(final VariableExpression expression) {
super.visitVariableExpression(expression);
if ("args".equals(expression.getName())) {
- expression.setAccessedVariable(arg);
+ expression.setAccessedVariable(args);
}
}
};
- variableExpressionFix.visitConstructor(constructorNode);
+ variableExpressionFix.visitConstructor(ctor);
+ }
+ if (noArg && !hasNoArgConstructor(cNode)) {
+ addGeneratedConstructor(cNode, mods, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, stmt(ctorThisX(args(new MapExpression()))));
}
}
@@ -229,24 +218,17 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation i
}
}
- private static void createNoArgConstructor(ClassNode cNode, int modifiers) {
- Statement body = stmt(ctorX(ClassNode.THIS, args(new MapExpression())));
- ConstructorNode consNode = new ConstructorNode(modifiers, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, body);
- markAsGenerated(cNode, consNode);
- cNode.addConstructor(consNode);
- }
-
private static ClassCodeExpressionTransformer makeMapTypedArgsTransformer() {
return new ClassCodeExpressionTransformer() {
@Override
- public Expression transform(Expression exp) {
+ public Expression transform(final Expression exp) {
if (exp instanceof ClosureExpression) {
ClosureExpression ce = (ClosureExpression) exp;
ce.getCode().visit(this);
} else if (exp instanceof VariableExpression) {
VariableExpression ve = (VariableExpression) exp;
if ("args".equals(ve.getName()) && ve.getAccessedVariable() instanceof DynamicVariable) {
- VariableExpression newVe = varX(param(MAP_TYPE, "args"));
+ VariableExpression newVe = varX(new Parameter(MAP_TYPE, "args"));
newVe.setSourcePosition(ve);
return newVe;
}
diff --git a/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy b/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy
index cab5f1d309..fa024d0eea 100644
--- a/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy
@@ -1045,4 +1045,20 @@ final class ImmutableTransformTest {
assert new Foo().toString() == 'Foo(null, z)'
'''
}
+
+ // GROOVY-10108
+ @Test
+ void testSpecialNamedArgHandlingForMap() {
+ assertScript shell, '''
+ @Immutable(specialNamedArgHandling=false)
+ class Type {
+ Map<Long, String> data
+ }
+
+ Map<Long, String> data = [(1L):'foo', (2L):'bar']
+ def obj= new Type(data)
+
+ assert obj['data'][2L] == 'bar'
+ '''
+ }
}