GROOVY-8440: Groovy's @Immutable annotation should be re-vamped to be a meta-annotation (additional refactoring to remove dup and plumb in pre/post conditions)
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/8cd0820f Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/8cd0820f Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/8cd0820f Branch: refs/heads/GROOVY_2_6_X Commit: 8cd0820f988db59a668daf71af0cf07ade15e8c8 Parents: 4be8c77 Author: paulk <pa...@asert.com.au> Authored: Mon Jan 29 23:57:26 2018 +1000 Committer: paulk <pa...@asert.com.au> Committed: Wed Jan 31 22:40:31 2018 +1000 ---------------------------------------------------------------------- .../groovy/groovy/transform/Immutable.groovy | 4 +- .../groovy/groovy/transform/MapConstructor.java | 5 - .../groovy/transform/TupleConstructor.java | 7 -- .../codehaus/groovy/ast/tools/GeneralUtils.java | 17 +++ .../transform/ImmutableASTTransformation.java | 107 +++++++++---------- .../MapConstructorASTTransformation.java | 69 +++++++----- .../TupleConstructorASTTransformation.java | 25 +++-- 7 files changed, 127 insertions(+), 107 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/groovy/blob/8cd0820f/src/main/groovy/groovy/transform/Immutable.groovy ---------------------------------------------------------------------- diff --git a/src/main/groovy/groovy/transform/Immutable.groovy b/src/main/groovy/groovy/transform/Immutable.groovy index e59342b..3fa518a 100644 --- a/src/main/groovy/groovy/transform/Immutable.groovy +++ b/src/main/groovy/groovy/transform/Immutable.groovy @@ -174,8 +174,8 @@ package groovy.transform @ToString(cache = true, includeSuperProperties = true) @EqualsAndHashCode(cache = true) @ImmutableBase -@TupleConstructor(makeImmutable = true, defaults = false) -@MapConstructor(makeImmutable = true, noArg = true, includeSuperProperties = true) +@TupleConstructor(defaults = false) +@MapConstructor(noArg = true, includeSuperProperties = true) @KnownImmutable @AnnotationCollector(mode=AnnotationCollectorMode.PREFER_EXPLICIT_MERGED) @interface Immutable { } http://git-wip-us.apache.org/repos/asf/groovy/blob/8cd0820f/src/main/groovy/groovy/transform/MapConstructor.java ---------------------------------------------------------------------- diff --git a/src/main/groovy/groovy/transform/MapConstructor.java b/src/main/groovy/groovy/transform/MapConstructor.java index 55dee78..d4d5657 100644 --- a/src/main/groovy/groovy/transform/MapConstructor.java +++ b/src/main/groovy/groovy/transform/MapConstructor.java @@ -96,11 +96,6 @@ public @interface MapConstructor { boolean includeFields() default false; /** - * Whether immutable pre-cautions (defensive copying, cloning, etc.) should be applied to incoming/outgoing properties. - */ - boolean makeImmutable() default false; - - /** * Include properties in the constructor. */ boolean includeProperties() default true; http://git-wip-us.apache.org/repos/asf/groovy/blob/8cd0820f/src/main/groovy/groovy/transform/TupleConstructor.java ---------------------------------------------------------------------- diff --git a/src/main/groovy/groovy/transform/TupleConstructor.java b/src/main/groovy/groovy/transform/TupleConstructor.java index bad5518..eb7cb6e 100644 --- a/src/main/groovy/groovy/transform/TupleConstructor.java +++ b/src/main/groovy/groovy/transform/TupleConstructor.java @@ -232,13 +232,6 @@ public @interface TupleConstructor { boolean force() default false; /** - * Whether immutable pre-cautions (defensive copying, cloning, etc.) should be applied to incoming/outgoing properties. - * - * @since 2.5.0 - */ - boolean makeImmutable() default false; - - /** * Used to set whether default value processing is enabled (the default) or disabled. * * By default, every constructor parameter is given a default value. This value will http://git-wip-us.apache.org/repos/asf/groovy/blob/8cd0820f/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java index 4c55515..8c949e7 100644 --- a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java +++ b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java @@ -42,6 +42,7 @@ import org.codehaus.groovy.ast.expr.ConstructorCallExpression; import org.codehaus.groovy.ast.expr.DeclarationExpression; import org.codehaus.groovy.ast.expr.Expression; import org.codehaus.groovy.ast.expr.FieldExpression; +import org.codehaus.groovy.ast.expr.ListExpression; import org.codehaus.groovy.ast.expr.MethodCallExpression; import org.codehaus.groovy.ast.expr.NotExpression; import org.codehaus.groovy.ast.expr.PropertyExpression; @@ -604,6 +605,22 @@ public class GeneralUtils { return new BooleanExpression(new BinaryExpression(expr, EQ, new ConstantExpression(0))); } + public static ListExpression list2args(List args) { + ListExpression result = new ListExpression(); + for (Object o : args) { + result.addExpression(new ConstantExpression(o)); + } + return result; + } + + public static ListExpression classList2args(List<String> args) { + ListExpression result = new ListExpression(); + for (Object o : args) { + result.addExpression(new ClassExpression(ClassHelper.make(o.toString()))); + } + return result; + } + public static BinaryExpression ltX(Expression lhv, Expression rhv) { return new BinaryExpression(lhv, LT, rhv); } http://git-wip-us.apache.org/repos/asf/groovy/blob/8cd0820f/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java index dcbe6d0..5536783 100644 --- a/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java @@ -73,6 +73,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.block; import static org.codehaus.groovy.ast.tools.GeneralUtils.callThisX; import static org.codehaus.groovy.ast.tools.GeneralUtils.callX; import static org.codehaus.groovy.ast.tools.GeneralUtils.castX; +import static org.codehaus.groovy.ast.tools.GeneralUtils.classList2args; import static org.codehaus.groovy.ast.tools.GeneralUtils.constX; import static org.codehaus.groovy.ast.tools.GeneralUtils.createConstructorStatementDefault; import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorX; @@ -89,6 +90,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.isInstanceOfX; import static org.codehaus.groovy.ast.tools.GeneralUtils.isOneX; import static org.codehaus.groovy.ast.tools.GeneralUtils.isOrImplements; import static org.codehaus.groovy.ast.tools.GeneralUtils.isTrueX; +import static org.codehaus.groovy.ast.tools.GeneralUtils.list2args; import static org.codehaus.groovy.ast.tools.GeneralUtils.neX; import static org.codehaus.groovy.ast.tools.GeneralUtils.notX; import static org.codehaus.groovy.ast.tools.GeneralUtils.orX; @@ -136,7 +138,6 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { private static final Class MY_CLASS = ImmutableBase.class; private static final Class<? extends Annotation> KNOWN_IMMUTABLE_CLASS = KnownImmutable.class; private static final Class<? extends Annotation> IMMUTABLE_BASE_CLASS = ImmutableBase.class; - private static final ClassNode IMMUTABLE_BASE_TYPE = makeWithoutCaching(IMMUTABLE_BASE_CLASS, false); public static final ClassNode MY_TYPE = make(MY_CLASS); static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage(); private static final String MEMBER_KNOWN_IMMUTABLE_CLASSES = "knownImmutableClasses"; @@ -192,10 +193,8 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { for (FieldNode fNode : fList) { ensureNotPublic(this, cName, fNode); } - boolean includeSuperProperties = false; if (hasAnnotation(cNode, TupleConstructorASTTransformation.MY_TYPE)) { AnnotationNode tupleCons = cNode.getAnnotations(TupleConstructorASTTransformation.MY_TYPE).get(0); -// includeSuperProperties = memberHasValue(tupleCons, "includeSuperProperties", true); if (unsupportedTupleAttribute(tupleCons, "excludes")) return; if (unsupportedTupleAttribute(tupleCons, "includes")) return; if (unsupportedTupleAttribute(tupleCons, "includeFields")) return; @@ -205,7 +204,6 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { if (unsupportedTupleAttribute(tupleCons, "force")) return; } if (!validateConstructors(cNode)) return; -// createConstructors(this, cNode, knownImmutableClasses, knownImmutables, includeSuperProperties); if (memberHasValue(node, MEMBER_ADD_COPY_WITH, true) && !pList.isEmpty() && !hasDeclaredMethod(cNode, COPY_WITH_METHOD, 1)) { createCopyWith(cNode, pList); @@ -255,6 +253,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { static List<String> getKnownImmutableClasses(AbstractASTTransformation xform, AnnotationNode node) { final List<String> immutableClasses = new ArrayList<String>(); + if (node == null) return immutableClasses; final Expression expression = node.getMember(MEMBER_KNOWN_IMMUTABLE_CLASSES); if (expression == null) return immutableClasses; @@ -276,6 +275,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { static List<String> getKnownImmutables(AbstractASTTransformation xform, AnnotationNode node) { final List<String> immutables = new ArrayList<String>(); + if (node == null) return immutables; final Expression expression = node.getMember(MEMBER_KNOWN_IMMUTABLES); if (expression == null) return immutables; @@ -305,17 +305,6 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { } } - private static void createConstructors(AbstractASTTransformation xform, ClassNode cNode, List<String> knownImmutableClasses, List<String> knownImmutables, boolean includeSuperProperties, boolean allProperties) { - List<PropertyNode> list = getProperties(cNode, includeSuperProperties, allProperties); - boolean specialHashMapCase = isSpecialHashMapCase(list); - if (specialHashMapCase) { - createConstructorMapSpecial(cNode, list); - } else { - createConstructorMap(xform, cNode, list, knownImmutableClasses, knownImmutables); - createConstructorOrdered(cNode, list); - } - } - static boolean isSpecialHashMapCase(List<PropertyNode> list) { return list.size() == 1 && list.get(0).getField().getType().equals(HASHMAP_TYPE); } @@ -388,39 +377,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { return castX(type, smce); } - static void createConstructorMapSpecial(ClassNode cNode, List<PropertyNode> list) { - final BlockStatement body = new BlockStatement(); - body.addStatement(createConstructorStatementMapSpecial(list.get(0).getField())); - createConstructorMapCommon(cNode, body); - } - - static void createConstructorMap(AbstractASTTransformation xform, ClassNode cNode, List<PropertyNode> list) { - AnnotationNode annoImmutable = cNode.getAnnotations(IMMUTABLE_BASE_TYPE).get(0); - final List<String> knownImmutableClasses = getKnownImmutableClasses(xform, annoImmutable); - final List<String> knownImmutables = getKnownImmutables(xform, annoImmutable); - createConstructorMap(xform, cNode, list, knownImmutableClasses, knownImmutables); - } - - private static void createConstructorMap(AbstractASTTransformation xform, ClassNode cNode, List<PropertyNode> list, List<String> knownImmutableClasses, List<String> knownImmutables) { - final BlockStatement body = new BlockStatement(); - body.addStatement(ifS(equalsNullX(varX("args")), assignS(varX("args"), new MapExpression()))); - for (PropertyNode pNode : list) { - body.addStatement(createConstructorStatement(xform, cNode, pNode, knownImmutableClasses, knownImmutables)); - } - // check for missing properties - body.addStatement(stmt(callX(SELF_TYPE, "checkPropNames", args("this", "args")))); - createConstructorMapCommon(cNode, body); - if (!list.isEmpty()) { - createNoArgConstructor(cNode); - } - } - - private static void createNoArgConstructor(ClassNode cNode) { - Statement body = stmt(ctorX(ClassNode.THIS, args(new MapExpression()))); - doAddConstructor(cNode, new ConstructorNode(ACC_PUBLIC, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, body)); - } - - private static void createConstructorMapCommon(ClassNode cNode, BlockStatement body) { + static void createConstructorMapCommon(ClassNode cNode, BlockStatement body) { final List<FieldNode> fList = cNode.getFields(); for (FieldNode fNode : fList) { if (fNode.isPublic()) continue; // public fields will be rejected elsewhere @@ -444,7 +401,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { ))); } - private static Statement createConstructorStatementMapSpecial(FieldNode fNode) { + static Statement createConstructorStatementMapSpecial(FieldNode fNode) { final Expression fieldExpr = varX(fNode); final ClassNode fieldType = fieldExpr.getType(); final Expression initExpr = fNode.getInitialValueExpression(); @@ -505,7 +462,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { return true; } - private static Statement createConstructorStatement(AbstractASTTransformation xform, ClassNode cNode, PropertyNode pNode, List<String> knownImmutableClasses, List<String> knownImmutables) { + static Statement createConstructorStatement(AbstractASTTransformation xform, ClassNode cNode, PropertyNode pNode, List<String> knownImmutables, List<String> knownImmutableClasses) { FieldNode fNode = pNode.getField(); final ClassNode fieldType = fNode.getType(); Statement statement; @@ -521,26 +478,26 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { xform.addError(createErrorMessage(cNode.getName(), fNode.getName(), fieldType.getName(), "compiling"), fNode); statement = EmptyStatement.INSTANCE; } else { - statement = createConstructorStatementGuarded(cNode, fNode); + statement = createConstructorStatementGuarded(cNode, fNode, knownImmutables, knownImmutableClasses); } return statement; } - private static Statement createConstructorStatementGuarded(ClassNode cNode, FieldNode fNode) { + private static Statement createConstructorStatementGuarded(ClassNode cNode, FieldNode fNode, List<String> knownImmutables, List<String> knownImmutableClasses) { final Expression fieldExpr = varX(fNode); Expression initExpr = fNode.getInitialValueExpression(); final Statement assignInit; if (initExpr == null || (initExpr instanceof ConstantExpression && ((ConstantExpression) initExpr).isNullExpression())) { assignInit = assignS(fieldExpr, ConstantExpression.EMPTY_EXPRESSION); } else { - assignInit = assignS(fieldExpr, checkUnresolved(fNode, initExpr)); + assignInit = assignS(fieldExpr, checkUnresolved(fNode, initExpr, knownImmutables, knownImmutableClasses)); } Expression unknown = findArg(fNode.getName()); - return ifElseS(equalsNullX(unknown), assignInit, assignS(fieldExpr, checkUnresolved(fNode, unknown))); + return ifElseS(equalsNullX(unknown), assignInit, assignS(fieldExpr, checkUnresolved(fNode, unknown, knownImmutables, knownImmutableClasses))); } - private static Expression checkUnresolved(FieldNode fNode, Expression value) { - Expression args = args(callThisX("getClass"), constX(fNode.getName()), value); + private static Expression checkUnresolved(FieldNode fNode, Expression value, List<String> knownImmutables, List<String> knownImmutableClasses) { + Expression args = args(callThisX("getClass"), constX(fNode.getName()), value, list2args(knownImmutables), classList2args(knownImmutableClasses)); return callX(SELF_TYPE, "checkImmutable", args); } @@ -794,6 +751,44 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { } @SuppressWarnings("Unchecked") + public static Object checkImmutable(Class<?> clazz, String fieldName, Object field, List<String> knownImmutableFieldNames, List<Class> knownImmutableClasses) { + if (field == null || field instanceof Enum || knownImmutable(field.getClass()) || knownImmutableFieldNames.contains(fieldName) || knownImmutableClasses.contains(field.getClass())) { + return field; + } + + boolean isImmutable = false; + for (Annotation an : field.getClass().getAnnotations()) { + if (an.getClass().getName().startsWith("groovy.transform.Immutable")) { + isImmutable = true; + break; + } + } + if (isImmutable) return field; + + if (field instanceof Collection) { + Field declaredField; + try { + declaredField = clazz.getDeclaredField(fieldName); + Class<?> fieldType = declaredField.getType(); + if (Collection.class.isAssignableFrom(fieldType)) { + return DefaultGroovyMethods.asImmutable((Collection) field); + } + // potentially allow Collection coercion for a constructor + if (knownImmutable(fieldType)) { + return field; + } + } catch (NoSuchFieldException ignore) { + // ignore + } + } + final String typeName = field.getClass().getName(); + throw new RuntimeException(createErrorMessage(clazz.getName(), fieldName, typeName, "constructing")); + } + + /* + * For compatibility with pre 2.5 compiled classes + */ + @SuppressWarnings("Unchecked") public static Object checkImmutable(Class<?> clazz, String fieldName, Object field) { if (field == null || field instanceof Enum || knownImmutable(field.getClass())) { return field; http://git-wip-us.apache.org/repos/asf/groovy/blob/8cd0820f/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java index fb2f94b..8f24342 100644 --- a/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java @@ -18,6 +18,7 @@ */ package org.codehaus.groovy.transform; +import groovy.transform.ImmutableBase; import groovy.transform.MapConstructor; import org.codehaus.groovy.ast.ASTNode; import org.codehaus.groovy.ast.AnnotatedNode; @@ -26,7 +27,6 @@ import org.codehaus.groovy.ast.ClassCodeExpressionTransformer; import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.ConstructorNode; import org.codehaus.groovy.ast.DynamicVariable; -import org.codehaus.groovy.ast.FieldNode; import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.PropertyNode; import org.codehaus.groovy.ast.expr.ArgumentListExpression; @@ -40,6 +40,7 @@ import org.codehaus.groovy.ast.stmt.Statement; import org.codehaus.groovy.control.CompilePhase; import org.codehaus.groovy.control.SourceUnit; +import java.lang.annotation.Annotation; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -65,6 +66,11 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.params; import static org.codehaus.groovy.ast.tools.GeneralUtils.propX; import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt; import static org.codehaus.groovy.ast.tools.GeneralUtils.varX; +import static org.codehaus.groovy.transform.ImmutableASTTransformation.createConstructorMapCommon; +import static org.codehaus.groovy.transform.ImmutableASTTransformation.createConstructorStatement; +import static org.codehaus.groovy.transform.ImmutableASTTransformation.createConstructorStatementMapSpecial; +import static org.codehaus.groovy.transform.ImmutableASTTransformation.getKnownImmutableClasses; +import static org.codehaus.groovy.transform.ImmutableASTTransformation.getKnownImmutables; /** * Handles generation of code for the @MapConstructor annotation. @@ -76,6 +82,9 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation { static final ClassNode MY_TYPE = make(MY_CLASS); static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage(); private static final ClassNode MAP_TYPE = makeWithoutCaching(Map.class, false); + private static final Class<? extends Annotation> IMMUTABLE_BASE_CLASS = ImmutableBase.class; + private static final ClassNode IMMUTABLE_BASE_TYPE = makeWithoutCaching(IMMUTABLE_BASE_CLASS, false); + private static final ClassNode IMMUTABLE_XFORM_TYPE = make(ImmutableASTTransformation.class); public void visit(ASTNode[] nodes, SourceUnit source) { init(nodes, source); @@ -96,7 +105,6 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation { List<String> excludes = getMemberStringList(anno, "excludes"); List<String> includes = getMemberStringList(anno, "includes"); boolean allNames = memberHasValue(anno, "allNames", true); - boolean makeImmutable = memberHasValue(anno, "makeImmutable", true); if (!checkIncludeExcludeUndefinedAware(anno, excludes, includes, MY_TYPE_NAME)) return; if (!checkPropertyList(cNode, includes, "includes", anno, MY_TYPE_NAME, includeFields, includeSuperProperties, false)) return; @@ -114,7 +122,7 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation { return; } - createConstructors(this, cNode, includeFields, includeProperties, includeSuperProperties, includeSuperFields, useSetters, noArg, allNames, allProperties, makeImmutable, excludes, includes, (ClosureExpression) pre, (ClosureExpression) post, source); + createConstructors(this, cNode, includeFields, includeProperties, includeSuperProperties, includeSuperFields, useSetters, noArg, allNames, allProperties, excludes, includes, (ClosureExpression) pre, (ClosureExpression) post, source); if (pre != null) { anno.setMember("pre", new ClosureExpression(new Parameter[0], EmptyStatement.INSTANCE)); @@ -125,7 +133,7 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation { } } - private static void createConstructors(AbstractASTTransformation xform, ClassNode cNode, boolean includeFields, boolean includeProperties, boolean includeSuperProperties, boolean includeSuperFields, boolean useSetters, boolean noArg, boolean allNames, boolean allProperties, boolean makeImmutable, List<String> excludes, List<String> includes, ClosureExpression pre, ClosureExpression post, SourceUnit source) { + private static void createConstructors(AbstractASTTransformation xform, ClassNode cNode, boolean includeFields, boolean includeProperties, boolean includeSuperProperties, boolean includeSuperFields, boolean useSetters, boolean noArg, boolean allNames, boolean allProperties, List<String> excludes, List<String> includes, ClosureExpression pre, ClosureExpression post, SourceUnit source) { List<ConstructorNode> constructors = cNode.getDeclaredConstructors(); boolean foundEmpty = constructors.size() == 1 && constructors.get(0).getFirstStatement() == null; // HACK: JavaStubGenerator could have snuck in a constructor we don't want @@ -140,17 +148,11 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation { } List<PropertyNode> list = getAllProperties(names, cNode, true, includeFields, allProperties, false, true); - if (makeImmutable) { - boolean specialHashMapCase = (ImmutableASTTransformation.isSpecialHashMapCase(list) && superList.isEmpty()) || - (ImmutableASTTransformation.isSpecialHashMapCase(superList) && list.isEmpty()); - superList.addAll(list); - if (specialHashMapCase) { - ImmutableASTTransformation.createConstructorMapSpecial(cNode, superList); - } else { - ImmutableASTTransformation.createConstructorMap(xform, cNode, superList); - } - return; - } + List<AnnotationNode> annotations = cNode.getAnnotations(IMMUTABLE_BASE_TYPE); + AnnotationNode annoImmutable = annotations.isEmpty() ? null : annotations.get(0); + boolean makeImmutable = annoImmutable != null; + final List<String> knownImmutableClasses = getKnownImmutableClasses(xform, annoImmutable); + final List<String> knownImmutables = getKnownImmutables(xform, annoImmutable); Parameter map = param(MAP_TYPE, "args"); final BlockStatement body = new BlockStatement(); @@ -160,27 +162,42 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation { copyStatementsWithSuperAdjustment(transformed, body); } final BlockStatement inner = new BlockStatement(); - for (PropertyNode pNode : superList) { - String name = pNode.getName(); - if (shouldSkip(name, excludes, includes, allNames)) continue; - assignField(useSetters, map, inner, name); - } - for (PropertyNode pNode : list) { - String name = pNode.getName(); - if (shouldSkip(name, excludes, includes, allNames)) continue; - assignField(useSetters, map, inner, name); + superList.addAll(list); + boolean specialHashMapCase = ImmutableASTTransformation.isSpecialHashMapCase(superList); + if (!specialHashMapCase) { + processProps(xform, cNode, makeImmutable && !specialHashMapCase, useSetters, allNames, excludes, includes, superList, map, inner, knownImmutables, knownImmutableClasses); } body.addStatement(ifS(notNullX(varX("args")), inner)); if (post != null) { ClosureExpression transformed = (ClosureExpression) transformer.transform(post); body.addStatement(transformed.getCode()); } - cNode.addConstructor(new ConstructorNode(ACC_PUBLIC, params(map), ClassNode.EMPTY_ARRAY, body)); - if (noArg && !(list.isEmpty() && superList.isEmpty()) && !hasNoArgConstructor(cNode)) { + if (makeImmutable && !specialHashMapCase) { + body.addStatement(stmt(callX(IMMUTABLE_XFORM_TYPE, "checkPropNames", args("this", "args")))); + createConstructorMapCommon(cNode, body); + } else if (specialHashMapCase) { + inner.addStatement(createConstructorStatementMapSpecial(superList.get(0).getField())); + createConstructorMapCommon(cNode, body); + } else { + cNode.addConstructor(new ConstructorNode(ACC_PUBLIC, params(map), ClassNode.EMPTY_ARRAY, body)); + } + if (noArg && !superList.isEmpty() && !hasNoArgConstructor(cNode) && !specialHashMapCase) { createNoArgConstructor(cNode); } } + private static void processProps(AbstractASTTransformation xform, ClassNode cNode, boolean makeImmutable, boolean useSetters, boolean allNames, List<String> excludes, List<String> includes, List<PropertyNode> superList, Parameter map, BlockStatement inner, List<String> knownImmutables, List<String> knownImmutableClasses) { + for (PropertyNode pNode : superList) { + String name = pNode.getName(); + if (shouldSkip(name, excludes, includes, allNames)) continue; + if (makeImmutable) { + inner.addStatement(createConstructorStatement(xform, cNode, pNode, knownImmutables, knownImmutableClasses)); + } else { + assignField(useSetters, map, inner, name); + } + } + } + private static void createNoArgConstructor(ClassNode cNode) { Statement body = stmt(ctorX(ClassNode.THIS, args(new MapExpression()))); cNode.addConstructor(new ConstructorNode(ACC_PUBLIC, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, body)); http://git-wip-us.apache.org/repos/asf/groovy/blob/8cd0820f/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java index ec6a4c0..5f0e381 100644 --- a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java @@ -18,7 +18,7 @@ */ package org.codehaus.groovy.transform; -import groovy.transform.KnownImmutable; +import groovy.transform.ImmutableBase; import groovy.transform.TupleConstructor; import org.codehaus.groovy.ast.ASTNode; import org.codehaus.groovy.ast.AnnotatedNode; @@ -40,6 +40,7 @@ import org.codehaus.groovy.classgen.VariableScopeVisitor; import org.codehaus.groovy.control.CompilePhase; import org.codehaus.groovy.control.SourceUnit; +import java.lang.annotation.Annotation; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -83,7 +84,8 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation private static final ClassNode LHMAP_TYPE = makeWithoutCaching(LinkedHashMap.class, false); private static final ClassNode HMAP_TYPE = makeWithoutCaching(HashMap.class, false); private static final ClassNode CHECK_METHOD_TYPE = make(ImmutableASTTransformation.class); - private static final ClassNode IMMUTABLE_CLASS_TYPE = makeWithoutCaching(KnownImmutable.class, false); + private static final Class<? extends Annotation> IMMUTABLE_BASE_CLASS = ImmutableBase.class; + private static final ClassNode IMMUTABLE_BASE_TYPE = makeWithoutCaching(IMMUTABLE_BASE_CLASS, false); private static final Map<Class<?>, Expression> primitivesInitialValues; static { @@ -118,7 +120,6 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation boolean defaults = !memberHasValue(anno, "defaults", false); boolean useSetters = memberHasValue(anno, "useSetters", true); boolean allProperties = memberHasValue(anno, "allProperties", true); - boolean makeImmutable = memberHasValue(anno, "makeImmutable", true); List<String> excludes = getMemberStringList(anno, "excludes"); List<String> includes = getMemberStringList(anno, "includes"); boolean allNames = memberHasValue(anno, "allNames", true); @@ -137,7 +138,7 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation } createConstructor(this, cNode, includeFields, includeProperties, includeSuperFields, includeSuperProperties, - callSuper, force, excludes, includes, useSetters, defaults, allNames, allProperties, makeImmutable, + callSuper, force, excludes, includes, useSetters, defaults, allNames, allProperties, sourceUnit, (ClosureExpression) pre, (ClosureExpression) post); if (pre != null) { @@ -163,18 +164,17 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation } public static void createConstructor(AbstractASTTransformation xform, ClassNode cNode, boolean includeFields, - boolean includeProperties, boolean includeSuperFields, boolean - includeSuperProperties, boolean callSuper, boolean force, - List<String> excludes, final List<String> includes, boolean useSetters, boolean - defaults, boolean allNames, SourceUnit sourceUnit, ClosureExpression - pre, ClosureExpression post) { - createConstructor(xform, cNode, includeFields, includeProperties, includeSuperFields, includeSuperProperties, callSuper, force, excludes, includes, useSetters, defaults, allNames, false, false, sourceUnit, pre, post); + boolean includeProperties, boolean includeSuperFields, boolean includeSuperProperties, + boolean callSuper, boolean force, List<String> excludes, final List<String> includes, + boolean useSetters, boolean defaults, boolean allNames, + SourceUnit sourceUnit, ClosureExpression pre, ClosureExpression post) { + createConstructor(xform, cNode, includeFields, includeProperties, includeSuperFields, includeSuperProperties, callSuper, force, excludes, includes, useSetters, defaults, allNames,false, sourceUnit, pre, post); } public static void createConstructor(AbstractASTTransformation xform, ClassNode cNode, boolean includeFields, boolean includeProperties, boolean includeSuperFields, boolean includeSuperProperties, boolean callSuper, boolean force, List<String> excludes, final List<String> includes, - boolean useSetters, boolean defaults, boolean allNames, boolean allProperties, boolean makeImmutable, + boolean useSetters, boolean defaults, boolean allNames, boolean allProperties, SourceUnit sourceUnit, ClosureExpression pre, ClosureExpression post) { Set<String> names = new HashSet<String>(); List<PropertyNode> superList; @@ -186,6 +186,9 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation List<PropertyNode> list = getAllProperties(names, cNode, true, includeFields, allProperties, false, true); + List<AnnotationNode> annotations = cNode.getAnnotations(IMMUTABLE_BASE_TYPE); + AnnotationNode annoImmutable = annotations.isEmpty() ? null : annotations.get(0); + boolean makeImmutable = annoImmutable != null; if (makeImmutable) { boolean specialHashMapCase = (ImmutableASTTransformation.isSpecialHashMapCase(list) && superList.isEmpty()) || (ImmutableASTTransformation.isSpecialHashMapCase(superList) && list.isEmpty());
