MapConstructor should have an includeSuperFields annotation attribute plus some refactoring
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/4be8c777 Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/4be8c777 Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/4be8c777 Branch: refs/heads/GROOVY_2_6_X Commit: 4be8c777537b8775eb82f840c9ae1e8d31f43f34 Parents: 3ad9a0e Author: paulk <pa...@asert.com.au> Authored: Sun Jan 28 17:00:13 2018 +1000 Committer: paulk <pa...@asert.com.au> Committed: Wed Jan 31 22:40:31 2018 +1000 ---------------------------------------------------------------------- .../groovy/groovy/transform/MapConstructor.java | 5 ++ .../java/org/codehaus/groovy/ast/FieldNode.java | 7 +++ .../codehaus/groovy/ast/tools/GeneralUtils.java | 21 +++++--- .../MapConstructorASTTransformation.java | 54 +++++++++----------- .../TupleConstructorASTTransformation.java | 4 +- 5 files changed, 52 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/groovy/blob/4be8c777/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 ccd127a..55dee78 100644 --- a/src/main/groovy/groovy/transform/MapConstructor.java +++ b/src/main/groovy/groovy/transform/MapConstructor.java @@ -111,6 +111,11 @@ public @interface MapConstructor { boolean includeSuperProperties() default false; /** + * Include fields from super classes in the constructor. + */ + boolean includeSuperFields() default false; + + /** * Whether to include all properties (as per the JavaBean spec) in the generated constructor. * When true, Groovy treats any explicitly created setXxx() methods as property setters as per the JavaBean * specification. http://git-wip-us.apache.org/repos/asf/groovy/blob/4be8c777/src/main/java/org/codehaus/groovy/ast/FieldNode.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/codehaus/groovy/ast/FieldNode.java b/src/main/java/org/codehaus/groovy/ast/FieldNode.java index d38f4f4..33e15c3 100644 --- a/src/main/java/org/codehaus/groovy/ast/FieldNode.java +++ b/src/main/java/org/codehaus/groovy/ast/FieldNode.java @@ -143,6 +143,13 @@ public class FieldNode extends AnnotatedNode implements Opcodes, Variable, Groov return (modifiers & ACC_PROTECTED) != 0; } + /** + * @return true if the field is private + */ + public boolean isPrivate() { + return (modifiers & ACC_PRIVATE) != 0; + } + /** * @param owner The owner to set. */ http://git-wip-us.apache.org/repos/asf/groovy/blob/4be8c777/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 807ea7d..4c55515 100644 --- a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java +++ b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java @@ -466,16 +466,16 @@ public class GeneralUtils { return result; } - public static List<PropertyNode> getAllProperties(Set<String> names, ClassNode cNode, boolean includeProperties, boolean includeFields, boolean allProperties, boolean traverseSuperClasses) { - return getAllProperties(names, cNode, cNode, includeProperties, includeFields, allProperties, traverseSuperClasses); + public static List<PropertyNode> getAllProperties(Set<String> names, ClassNode cNode, boolean includeProperties, boolean includeFields, boolean allProperties, boolean traverseSuperClasses, boolean skipReadonly) { + return getAllProperties(names, cNode, cNode, includeProperties, includeFields, allProperties, traverseSuperClasses, skipReadonly); } - private static List<PropertyNode> getAllProperties(Set<String> names, ClassNode origType, ClassNode cNode, boolean includeProperties, boolean includeFields, boolean allProperties, boolean traverseSuperClasses) { + public static List<PropertyNode> getAllProperties(Set<String> names, ClassNode origType, ClassNode cNode, boolean includeProperties, boolean includeFields, boolean allProperties, boolean traverseSuperClasses, boolean skipReadonly) { final List<PropertyNode> result; if (cNode == ClassHelper.OBJECT_TYPE || !traverseSuperClasses) { result = new ArrayList<PropertyNode>(); } else { - result = getAllProperties(names, origType, cNode.getSuperClass(), includeProperties, includeFields, allProperties, true); + result = getAllProperties(names, origType, cNode.getSuperClass(), includeProperties, includeFields, allProperties, true, skipReadonly); } if (includeProperties) { for (PropertyNode pNode : cNode.getProperties()) { @@ -490,10 +490,17 @@ public class GeneralUtils { } if (includeFields) { for (FieldNode fNode : cNode.getFields()) { - if (!fNode.isStatic() && cNode.getProperty(fNode.getName()) == null && !names.contains(fNode.getName())) { - result.add(new PropertyNode(fNode, fNode.getModifiers(), null, null)); - names.add(fNode.getName()); + if (fNode.isStatic() || cNode.getProperty(fNode.getName()) != null || names.contains(fNode.getName())) { + continue; } + if (fNode.isPrivate() && !cNode.equals(origType)) { + continue; + } + if (fNode.isFinal() && fNode.getInitialExpression() != null && skipReadonly) { + continue; + } + result.add(new PropertyNode(fNode, fNode.getModifiers(), null, null)); + names.add(fNode.getName()); } } return result; http://git-wip-us.apache.org/repos/asf/groovy/blob/4be8c777/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 b6407d3..fb2f94b 100644 --- a/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java @@ -41,8 +41,10 @@ import org.codehaus.groovy.control.CompilePhase; import org.codehaus.groovy.control.SourceUnit; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import static org.apache.groovy.ast.tools.ClassNodeUtils.hasNoArgConstructor; import static org.codehaus.groovy.ast.ClassHelper.make; @@ -54,10 +56,8 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.callX; import static org.codehaus.groovy.ast.tools.GeneralUtils.constX; 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.getInstanceNonPropertyFields; -import static org.codehaus.groovy.ast.tools.GeneralUtils.getInstancePropertyFields; +import static org.codehaus.groovy.ast.tools.GeneralUtils.getAllProperties; import static org.codehaus.groovy.ast.tools.GeneralUtils.getSetterName; -import static org.codehaus.groovy.ast.tools.GeneralUtils.getSuperPropertyFields; import static org.codehaus.groovy.ast.tools.GeneralUtils.ifS; import static org.codehaus.groovy.ast.tools.GeneralUtils.notNullX; import static org.codehaus.groovy.ast.tools.GeneralUtils.param; @@ -76,9 +76,6 @@ 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 ClassNode IMMUTABLE_CLASS_TYPE = makeWithoutCaching(KnownImmutable.class, false); -// private static final ClassNode IMMUTABLE_BASE_TYPE = makeWithoutCaching(ImmutableBase.class, false); -// private static final ClassNode CHECK_METHOD_TYPE = make(ImmutableASTTransformation.class); public void visit(ASTNode[] nodes, SourceUnit source) { init(nodes, source); @@ -92,6 +89,7 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation { 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 useSetters = memberHasValue(anno, "useSetters", true); boolean allProperties = memberHasValue(anno, "allProperties", true); boolean noArg = memberHasValue(anno, "noArg", true); @@ -116,7 +114,7 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation { return; } - createConstructors(this, cNode, includeFields, includeProperties, includeSuperProperties, useSetters, noArg, allNames, allProperties, makeImmutable, excludes, includes, (ClosureExpression) pre, (ClosureExpression) post, source); + createConstructors(this, cNode, includeFields, includeProperties, includeSuperProperties, includeSuperFields, useSetters, noArg, allNames, allProperties, makeImmutable, excludes, includes, (ClosureExpression) pre, (ClosureExpression) post, source); if (pre != null) { anno.setMember("pre", new ClosureExpression(new Parameter[0], EmptyStatement.INSTANCE)); @@ -127,37 +125,33 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation { } } - private static void createConstructors(AbstractASTTransformation xform, ClassNode cNode, boolean includeFields, boolean includeProperties, boolean includeSuperProperties, 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, boolean makeImmutable, 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 if (foundEmpty) constructors.remove(0); - // TODO remove duplicated code from alternative paths below + Set<String> names = new HashSet<String>(); + List<PropertyNode> superList; + if (includeSuperProperties || includeSuperFields) { + superList = getAllProperties(names, cNode, cNode.getSuperClass(), includeSuperProperties, includeSuperFields, allProperties, true, true); + } else { + superList = new ArrayList<PropertyNode>(); + } + List<PropertyNode> list = getAllProperties(names, cNode, true, includeFields, allProperties, false, true); + if (makeImmutable) { - List<PropertyNode> list = ImmutableASTTransformation.getProperties(cNode, includeSuperProperties, allProperties); - boolean specialHashMapCase = ImmutableASTTransformation.isSpecialHashMapCase(list); + boolean specialHashMapCase = (ImmutableASTTransformation.isSpecialHashMapCase(list) && superList.isEmpty()) || + (ImmutableASTTransformation.isSpecialHashMapCase(superList) && list.isEmpty()); + superList.addAll(list); if (specialHashMapCase) { - ImmutableASTTransformation.createConstructorMapSpecial(cNode, list); + ImmutableASTTransformation.createConstructorMapSpecial(cNode, superList); } else { - ImmutableASTTransformation.createConstructorMap(xform, cNode, list); + ImmutableASTTransformation.createConstructorMap(xform, cNode, superList); } return; } - List<FieldNode> superList = new ArrayList<FieldNode>(); - if (includeSuperProperties) { - superList.addAll(getSuperPropertyFields(cNode.getSuperClass())); - } - - List<FieldNode> list = new ArrayList<FieldNode>(); - if (includeProperties) { - list.addAll(getInstancePropertyFields(cNode)); - } - if (includeFields) { - list.addAll(getInstanceNonPropertyFields(cNode)); - } - Parameter map = param(MAP_TYPE, "args"); final BlockStatement body = new BlockStatement(); ClassCodeExpressionTransformer transformer = makeMapTypedArgsTransformer(); @@ -166,13 +160,13 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation { copyStatementsWithSuperAdjustment(transformed, body); } final BlockStatement inner = new BlockStatement(); - for (FieldNode fNode : superList) { - String name = fNode.getName(); + for (PropertyNode pNode : superList) { + String name = pNode.getName(); if (shouldSkip(name, excludes, includes, allNames)) continue; assignField(useSetters, map, inner, name); } - for (FieldNode fNode : list) { - String name = fNode.getName(); + for (PropertyNode pNode : list) { + String name = pNode.getName(); if (shouldSkip(name, excludes, includes, allNames)) continue; assignField(useSetters, map, inner, name); } http://git-wip-us.apache.org/repos/asf/groovy/blob/4be8c777/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 7cefac6..ec6a4c0 100644 --- a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java @@ -179,12 +179,12 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation Set<String> names = new HashSet<String>(); List<PropertyNode> superList; if (includeSuperProperties || includeSuperFields) { - superList = getAllProperties(names, cNode.getSuperClass(), includeSuperProperties, includeSuperFields, allProperties, true); + superList = getAllProperties(names, cNode.getSuperClass(), includeSuperProperties, includeSuperFields, allProperties, true, true); } else { superList = new ArrayList<PropertyNode>(); } - List<PropertyNode> list = getAllProperties(names, cNode, true, includeFields, allProperties, false); + List<PropertyNode> list = getAllProperties(names, cNode, true, includeFields, allProperties, false, true); if (makeImmutable) { boolean specialHashMapCase = (ImmutableASTTransformation.isSpecialHashMapCase(list) && superList.isEmpty()) ||
