Repository: incubator-groovy Updated Branches: refs/heads/master d37a7dcc8 -> cde936058
GROOVY-7394: @ToString could support non-field properties (Closes #2) Project: http://git-wip-us.apache.org/repos/asf/incubator-groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-groovy/commit/cde93605 Tree: http://git-wip-us.apache.org/repos/asf/incubator-groovy/tree/cde93605 Diff: http://git-wip-us.apache.org/repos/asf/incubator-groovy/diff/cde93605 Branch: refs/heads/master Commit: cde9360582bf03e98f9eb84b257ba51db39e33c6 Parents: d37a7dc Author: Paul King <[email protected]> Authored: Mon Apr 20 20:52:34 2015 +1000 Committer: Paul King <[email protected]> Committed: Wed Apr 29 15:56:47 2015 +1000 ---------------------------------------------------------------------- src/main/groovy/transform/ToString.java | 12 ++ .../codehaus/groovy/ast/tools/BeanUtils.java | 120 +++++++++++++++++++ .../transform/ToStringASTTransformation.java | 53 ++++---- src/spec/doc/core-metaprogramming.adoc | 5 + .../test/CodeGenerationASTTransformsTest.groovy | 16 +++ .../CanonicalComponentsTransformTest.groovy | 7 +- .../transform/ToStringTransformTest.groovy | 55 +++++++++ 7 files changed, 235 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/cde93605/src/main/groovy/transform/ToString.java ---------------------------------------------------------------------- diff --git a/src/main/groovy/transform/ToString.java b/src/main/groovy/transform/ToString.java index 9389433..bbc40a1 100644 --- a/src/main/groovy/transform/ToString.java +++ b/src/main/groovy/transform/ToString.java @@ -172,6 +172,18 @@ public @interface ToString { boolean includePackage() default true; /** + * Whether to include all properties (as per the JavaBean spec) in the generated toString. + * Groovy recognizes any field-like definitions with no explicit visibility as property definitions + * and always includes them in the {@code @ToString} generated toString (as well as auto-generating the + * appropriate getters and setters). Groovy also treats any explicitly created getXxx() or isYyy() + * methods as property getters as per the JavaBean specification. Old versions of Groovy did not. + * So set this flag to false for the old behavior or if you want to explicitly exclude such properties. + * + * @since 2.5 + */ + boolean allProperties() default true; + + /** * Whether to cache toString() calculations. You should only set this to true if * you know the object is immutable (or technically mutable but never changed). * @since 2.1.0 http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/cde93605/src/main/org/codehaus/groovy/ast/tools/BeanUtils.java ---------------------------------------------------------------------- diff --git a/src/main/org/codehaus/groovy/ast/tools/BeanUtils.java b/src/main/org/codehaus/groovy/ast/tools/BeanUtils.java new file mode 100644 index 0000000..32d4a81 --- /dev/null +++ b/src/main/org/codehaus/groovy/ast/tools/BeanUtils.java @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.codehaus.groovy.ast.tools; + +import org.codehaus.groovy.ast.ClassHelper; +import org.codehaus.groovy.ast.ClassNode; +import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.PropertyNode; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import static java.beans.Introspector.decapitalize; + +public class BeanUtils { + static final String GET_PREFIX = "get"; + static final String IS_PREFIX = "is"; + + /** + * Get all properties including JavaBean pseudo properties matching getter conventions. + * + * @param type the ClassNode + * @param includeSuperProperties whether to include super properties + * @param includeStatic whether to include static properties + * @param includePseudoGetters whether to include JavaBean pseudo (getXXX/isYYY) properties with no corresponding field + * @return the list of found property nodes + */ + public static List<PropertyNode> getAllProperties(ClassNode type, boolean includeSuperProperties, boolean includeStatic, boolean includePseudoGetters) { + // TODO add generics support so this can be used for @EAHC + // TODO add an includePseudoSetters so this can be used for @TupleConstructor + ClassNode node = type; + List<PropertyNode> result = new ArrayList<PropertyNode>(); + Set<String> names = new HashSet<String>(); + while (node != null) { + addExplicitProperties(node, result, names, includeStatic); + if (!includeSuperProperties) break; + node = node.getSuperClass(); + } + addPseudoProperties(type, result, names, includeStatic, includePseudoGetters, includeSuperProperties); + return result; + } + + private static void addExplicitProperties(ClassNode cNode, List<PropertyNode> result, Set<String> names, boolean includeStatic) { + for (PropertyNode pNode : cNode.getProperties()) { + if (includeStatic || !pNode.isStatic()) { + if (!names.contains(pNode.getName())) { + result.add(pNode); + names.add(pNode.getName()); + } + } + } + } + + private static void addPseudoProperties(ClassNode cNode, List<PropertyNode> result, Set<String> names, boolean includeStatic, boolean includePseudoGetters, boolean includeSuperProperties) { + if (!includePseudoGetters) return; + List<MethodNode> methods = cNode.getAllDeclaredMethods(); + ClassNode node = cNode.getSuperClass(); + if (includeSuperProperties) { + while (node != null) { + for (MethodNode next : node.getAllDeclaredMethods()) { + if (!next.isPrivate()) { + methods.add(next); + } + } + node = node.getSuperClass(); + } + } + for (MethodNode mNode : methods) { + if (!includeStatic && mNode.isStatic()) continue; + String name = mNode.getName(); + if ((name.length() <= 3 && !name.startsWith(IS_PREFIX)) || name.equals("getClass") || name.equals("getMetaClass") || name.equals("getDeclaringClass")) { + // Optimization: skip invalid propertyNames + continue; + } + if (mNode.getDeclaringClass() != cNode && mNode.isPrivate()) { + // skip private super methods + continue; + } + int paramCount = mNode.getParameters().length; + ClassNode returnType = mNode.getReturnType(); + if (paramCount == 0) { + if (name.startsWith(GET_PREFIX)) { + // Simple getter + String propName = decapitalize(name.substring(3)); + if (!names.contains(propName)) { + result.add(new PropertyNode(propName, mNode.getModifiers(), returnType, cNode, null, mNode.getCode(), null)); + names.add(propName); + } + } else { + if (name.startsWith(IS_PREFIX) && returnType.equals(ClassHelper.boolean_TYPE)) { + // boolean getter + String propName = decapitalize(name.substring(2)); + if (!names.contains(propName)) { + names.add(propName); + result.add(new PropertyNode(propName, mNode.getModifiers(), returnType, cNode, null, mNode.getCode(), null)); + } + } + } + } + } + } +} http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/cde93605/src/main/org/codehaus/groovy/transform/ToStringASTTransformation.java ---------------------------------------------------------------------- diff --git a/src/main/org/codehaus/groovy/transform/ToStringASTTransformation.java b/src/main/org/codehaus/groovy/transform/ToStringASTTransformation.java index b93ccbb..5847c4b 100644 --- a/src/main/org/codehaus/groovy/transform/ToStringASTTransformation.java +++ b/src/main/org/codehaus/groovy/transform/ToStringASTTransformation.java @@ -34,12 +34,13 @@ import org.codehaus.groovy.ast.expr.MethodCallExpression; import org.codehaus.groovy.ast.expr.VariableExpression; import org.codehaus.groovy.ast.stmt.BlockStatement; import org.codehaus.groovy.ast.stmt.Statement; +import org.codehaus.groovy.ast.tools.BeanUtils; import org.codehaus.groovy.control.CompilePhase; import org.codehaus.groovy.control.SourceUnit; import org.codehaus.groovy.runtime.InvokerHelper; +import org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport; import java.util.ArrayList; -import java.util.Iterator; import java.util.List; import static org.codehaus.groovy.ast.ClassHelper.make; @@ -47,9 +48,6 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.*; /** * Handles generation of code for the @ToString annotation. - * - * @author Paul King - * @author Andre Steingress */ @GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION) public class ToStringASTTransformation extends AbstractASTTransformation { @@ -81,6 +79,7 @@ public class ToStringASTTransformation extends AbstractASTTransformation { List<String> includes = getMemberList(anno, "includes"); boolean ignoreNulls = memberHasValue(anno, "ignoreNulls", true); boolean includePackage = !memberHasValue(anno, "includePackage", false); + boolean allProperties = !memberHasValue(anno, "allProperties", false); if (hasAnnotation(cNode, CanonicalASTTransformation.MY_TYPE)) { AnnotationNode canonical = cNode.getAnnotations(CanonicalASTTransformation.MY_TYPE).get(0); @@ -88,7 +87,7 @@ public class ToStringASTTransformation extends AbstractASTTransformation { if (includes == null || includes.isEmpty()) includes = getMemberList(canonical, "includes"); } if (!checkIncludeExclude(anno, excludes, includes, MY_TYPE_NAME)) return; - createToString(cNode, includeSuper, includeFields, excludes, includes, includeNames, ignoreNulls, includePackage, cacheToString, includeSuperProperties); + createToString(cNode, includeSuper, includeFields, excludes, includes, includeNames, ignoreNulls, includePackage, cacheToString, includeSuperProperties, allProperties); } } @@ -109,6 +108,10 @@ public class ToStringASTTransformation extends AbstractASTTransformation { } public static void createToString(ClassNode cNode, boolean includeSuper, boolean includeFields, List<String> excludes, List<String> includes, boolean includeNames, boolean ignoreNulls, boolean includePackage, boolean cache, boolean includeSuperProperties) { + createToString(cNode, includeSuper, includeFields, excludes, includes, includeNames, ignoreNulls, includePackage, cache, includeSuperProperties, true); + } + + public static void createToString(ClassNode cNode, boolean includeSuper, boolean includeFields, List<String> excludes, List<String> includes, boolean includeNames, boolean ignoreNulls, boolean includePackage, boolean cache, boolean includeSuperProperties, boolean allProperties) { // make a public method if none exists otherwise try a private method with leading underscore boolean hasExistingToString = hasDeclaredMethod(cNode, "toString", 0); if (hasExistingToString && hasDeclaredMethod(cNode, "_toString", 0)) return; @@ -120,11 +123,11 @@ public class ToStringASTTransformation extends AbstractASTTransformation { final Expression savedToString = varX(cacheField); body.addStatement(ifS( equalsNullX(savedToString), - assignS(savedToString, calculateToStringStatements(cNode, includeSuper, includeFields, excludes, includes, includeNames, ignoreNulls, includePackage, includeSuperProperties, body)) + assignS(savedToString, calculateToStringStatements(cNode, includeSuper, includeFields, excludes, includes, includeNames, ignoreNulls, includePackage, includeSuperProperties, allProperties, body)) )); tempToString = savedToString; } else { - tempToString = calculateToStringStatements(cNode, includeSuper, includeFields, excludes, includes, includeNames, ignoreNulls, includePackage, includeSuperProperties, body); + tempToString = calculateToStringStatements(cNode, includeSuper, includeFields, excludes, includes, includeNames, ignoreNulls, includePackage, includeSuperProperties, allProperties, body); } body.addStatement(returnS(tempToString)); @@ -132,7 +135,7 @@ public class ToStringASTTransformation extends AbstractASTTransformation { ClassHelper.STRING_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, body)); } - private static Expression calculateToStringStatements(ClassNode cNode, boolean includeSuper, boolean includeFields, List<String> excludes, List<String> includes, boolean includeNames, boolean ignoreNulls, boolean includePackage, boolean includeSuperProperties, BlockStatement body) { + private static Expression calculateToStringStatements(ClassNode cNode, boolean includeSuper, boolean includeFields, List<String> excludes, List<String> includes, boolean includeNames, boolean ignoreNulls, boolean includePackage, boolean includeSuperProperties, boolean allProperties, BlockStatement body) { // def _result = new StringBuilder() final Expression result = varX("_result"); body.addStatement(declS(result, ctorX(STRINGBUILDER_TYPE))); @@ -146,23 +149,11 @@ public class ToStringASTTransformation extends AbstractASTTransformation { body.addStatement(appendS(result, constX(className + "("))); // append properties - List<PropertyNode> pList; - if (includeSuperProperties) { - pList = getAllProperties(cNode); - Iterator<PropertyNode> pIterator = pList.iterator(); - while (pIterator.hasNext()) { - if (pIterator.next().isStatic()) { - pIterator.remove(); - } - } - } else { - pList = getInstanceProperties(cNode); - } + List<PropertyNode> pList = BeanUtils.getAllProperties(cNode, includeSuperProperties, false, allProperties); for (PropertyNode pNode : pList) { if (shouldSkip(pNode.getName(), excludes, includes)) continue; Expression getter = getterX(cNode, pNode); - - appendValue(body, result, first, getter, pNode.getName(), includeNames, ignoreNulls); + appendValue(cNode, body, result, first, getter, pNode.getOriginType(), pNode.getName(), includeNames, ignoreNulls); } // append fields if needed @@ -171,7 +162,7 @@ public class ToStringASTTransformation extends AbstractASTTransformation { fList.addAll(getInstanceNonPropertyFields(cNode)); for (FieldNode fNode : fList) { if (shouldSkip(fNode.getName(), excludes, includes)) continue; - appendValue(body, result, first, varX(fNode), fNode.getName(), includeNames, ignoreNulls); + appendValue(cNode, body, result, first, varX(fNode), fNode.getType(), fNode.getName(), includeNames, ignoreNulls); } } @@ -190,15 +181,21 @@ public class ToStringASTTransformation extends AbstractASTTransformation { return toString; } - private static void appendValue(BlockStatement body, Expression result, VariableExpression first, Expression value, String name, boolean includeNames, boolean ignoreNulls) { + private static void appendValue(ClassNode cNode, BlockStatement body, Expression result, VariableExpression first, Expression value, ClassNode valueType, String name, boolean includeNames, boolean ignoreNulls) { final BlockStatement thenBlock = new BlockStatement(); final Statement appendValue = ignoreNulls ? ifS(notNullX(value), thenBlock) : thenBlock; appendCommaIfNotFirst(thenBlock, result, first); appendPrefix(thenBlock, result, name, includeNames); - thenBlock.addStatement(ifElseS( - sameX(value, new VariableExpression("this")), - appendS(result, constX("(this)")), - appendS(result, callX(INVOKER_TYPE, "toString", value)))); + boolean canBeSelf = StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(valueType, cNode); + if (canBeSelf) { + thenBlock.addStatement(ifElseS( + sameX(value, new VariableExpression("this")), + appendS(result, constX("(this)")), + appendS(result, callX(INVOKER_TYPE, "toString", value)))); + } else { + thenBlock.addStatement(appendS(result, callX(INVOKER_TYPE, "toString", value))); + + } body.addStatement(appendValue); } http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/cde93605/src/spec/doc/core-metaprogramming.adoc ---------------------------------------------------------------------- diff --git a/src/spec/doc/core-metaprogramming.adoc b/src/spec/doc/core-metaprogramming.adoc index 27154fe..b3f3faa 100644 --- a/src/spec/doc/core-metaprogramming.adoc +++ b/src/spec/doc/core-metaprogramming.adoc @@ -690,6 +690,11 @@ include::{projectdir}/src/spec/test/CodeGenerationASTTransformsTest.groovy[tags= ---- include::{projectdir}/src/spec/test/CodeGenerationASTTransformsTest.groovy[tags=tostring_example_includePackage,indent=0] ---- +|allPropeties|True|Include all JavaBean properties in toString| +[source,groovy] +---- +include::{projectdir}/src/spec/test/CodeGenerationASTTransformsTest.groovy[tags=tostring_example_allProperties,indent=0] +---- |cache|False|Cache the toString string. Should only be set to true if the class is immutable.| [source,groovy] ---- http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/cde93605/src/spec/test/CodeGenerationASTTransformsTest.groovy ---------------------------------------------------------------------- diff --git a/src/spec/test/CodeGenerationASTTransformsTest.groovy b/src/spec/test/CodeGenerationASTTransformsTest.groovy index 476d649..9a75078 100644 --- a/src/spec/test/CodeGenerationASTTransformsTest.groovy +++ b/src/spec/test/CodeGenerationASTTransformsTest.groovy @@ -198,6 +198,22 @@ assert p.toString() == 'acme.Person(Jack, Nicholson)' ''' + assertScript '''package acme +import groovy.transform.ToString + +// tag::tostring_example_allProperties[] +@ToString(includeNames=true) +class Person { + String firstName + String getLastName() { 'Nicholson' } +} + +def p = new Person(firstName: 'Jack') +assert p.toString() == 'acme.Person(firstName:Jack, lastName:Nicholson)' +// end::tostring_example_allProperties[] + +''' + } http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/cde93605/src/test/org/codehaus/groovy/transform/CanonicalComponentsTransformTest.groovy ---------------------------------------------------------------------- diff --git a/src/test/org/codehaus/groovy/transform/CanonicalComponentsTransformTest.groovy b/src/test/org/codehaus/groovy/transform/CanonicalComponentsTransformTest.groovy index 8525028..2db0b70 100644 --- a/src/test/org/codehaus/groovy/transform/CanonicalComponentsTransformTest.groovy +++ b/src/test/org/codehaus/groovy/transform/CanonicalComponentsTransformTest.groovy @@ -29,9 +29,6 @@ import static groovy.transform.AutoCloneStyle.* import groovy.transform.ToString import groovy.transform.Canonical -/** - * @author Paul King - */ class CanonicalComponentsTransformTest extends GroovyShellTestCase { void testTupleConstructorWithEnum() { @@ -374,9 +371,9 @@ class CanonicalComponentsTransformTest extends GroovyShellTestCase { def p = new Person(21) // $first setter is an implementation detail p.$first = 'Mary' - "$p.first $p.last ${p.toString()}" + p.toString() ''') - assert result == 'Mary Smith Person(21)' + assert result == 'Person(21, Mary, Smith)' } // GROOVY-5901 http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/cde93605/src/test/org/codehaus/groovy/transform/ToStringTransformTest.groovy ---------------------------------------------------------------------- diff --git a/src/test/org/codehaus/groovy/transform/ToStringTransformTest.groovy b/src/test/org/codehaus/groovy/transform/ToStringTransformTest.groovy index 6f73955..675c900 100644 --- a/src/test/org/codehaus/groovy/transform/ToStringTransformTest.groovy +++ b/src/test/org/codehaus/groovy/transform/ToStringTransformTest.groovy @@ -245,6 +245,61 @@ class ToStringTransformTest extends GroovyShellTestCase { assertEquals("Person()", toString) } + void testPseudoProperties() { + def toString = evaluate(''' + import groovy.transform.* + + class Person { + boolean isAdult() { false } + boolean golfer = true + boolean isSenior() { false } + private getAge() { 40 } + protected getBorn() { 1975 } + } + + @ToString(excludes='last', includeNames=true, includeSuperProperties=true) + class SportsPerson extends Person { + private String _first + String last, title + boolean golfer = false + boolean adult = true + Boolean cyclist = true + Boolean isMale() { true } + void setFirst(String first) { this._first = first } + String getFull() { "$_first $last" } + } + new SportsPerson(first: 'John', last: 'Smith', title: 'Mr').toString() + ''') + assert toString == "SportsPerson(title:Mr, golfer:false, adult:true, cyclist:true, full:John Smith, senior:false, born:1975)" + // same again but with allProperties=false and with @CompileStatic for test coverage purposes + toString = evaluate(''' + import groovy.transform.* + + class Person { + boolean isAdult() { false } + boolean golfer = true + boolean isSenior() { false } + private getAge() { 40 } + protected getBorn() { 1975 } + } + + @CompileStatic + @ToString(excludes='last', includeNames=true, includeSuperProperties=true, allProperties=false) + class SportsPerson extends Person { + private String _first + String last, title + boolean golfer = false + boolean adult = true + Boolean cyclist = true + Boolean isMale() { true } + void setFirst(String first) { this._first = first } + String getFull() { "$_first $last" } + } + new SportsPerson(first: 'John', last: 'Smith', title: 'Mr').toString() + ''') + assert toString == "SportsPerson(title:Mr, golfer:false, adult:true, cyclist:true)" + } + void testSelfReference() { def toString = evaluate("""
