You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2018/02/01 12:06:21 UTC
groovy git commit: GROOVY-7390: @EqualsAndHashCode incorrect when
using non-field properties
Repository: groovy
Updated Branches:
refs/heads/master d9e6f8aaa -> 8403a9399
GROOVY-7390: @EqualsAndHashCode incorrect when using non-field properties
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/8403a939
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/8403a939
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/8403a939
Branch: refs/heads/master
Commit: 8403a93997445abcb8bddb4c5eb8600bc36b0bbe
Parents: d9e6f8a
Author: paulk <pa...@asert.com.au>
Authored: Thu Feb 1 22:06:08 2018 +1000
Committer: paulk <pa...@asert.com.au>
Committed: Thu Feb 1 22:06:08 2018 +1000
----------------------------------------------------------------------
.../groovy/transform/EqualsAndHashCode.java | 11 ++++++
.../EqualsAndHashCodeASTTransformation.java | 37 +++++++++++++-------
.../CanonicalComponentsTransformTest.groovy | 12 +++++++
3 files changed, 48 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/8403a939/src/main/groovy/groovy/transform/EqualsAndHashCode.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/groovy/transform/EqualsAndHashCode.java b/src/main/groovy/groovy/transform/EqualsAndHashCode.java
index 592c6ba..552a65f 100644
--- a/src/main/groovy/groovy/transform/EqualsAndHashCode.java
+++ b/src/main/groovy/groovy/transform/EqualsAndHashCode.java
@@ -267,6 +267,17 @@ public @interface EqualsAndHashCode {
boolean useCanEqual() default true;
/**
+ * 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.
+ * JavaBean properties come after any Groovy properties but before any fields for a given class
+ * (unless 'includes' is used to determine the order).
+ *
+ * @since 2.5.0
+ */
+ boolean allProperties() default false;
+
+ /**
* Whether to include all fields and/or properties in equals and hashCode calculations, including those
* with names that are considered internal.
*
http://git-wip-us.apache.org/repos/asf/groovy/blob/8403a939/src/main/java/org/codehaus/groovy/transform/EqualsAndHashCodeASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/transform/EqualsAndHashCodeASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/EqualsAndHashCodeASTTransformation.java
index 86dcd30..a5068f2 100644
--- a/src/main/java/org/codehaus/groovy/transform/EqualsAndHashCodeASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/EqualsAndHashCodeASTTransformation.java
@@ -41,7 +41,9 @@ import org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport;
import org.codehaus.groovy.util.HashCodeHelper;
import java.util.ArrayList;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import static org.codehaus.groovy.ast.ClassHelper.make;
import static org.codehaus.groovy.ast.tools.GeneralUtils.andX;
@@ -53,9 +55,9 @@ 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.declS;
import static org.codehaus.groovy.ast.tools.GeneralUtils.equalsNullX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.getAllProperties;
import static org.codehaus.groovy.ast.tools.GeneralUtils.getGetterName;
import static org.codehaus.groovy.ast.tools.GeneralUtils.getInstanceNonPropertyFields;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.getInstanceProperties;
import static org.codehaus.groovy.ast.tools.GeneralUtils.getterThisX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.hasClassX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.hasDeclaredMethod;
@@ -106,11 +108,12 @@ public class EqualsAndHashCodeASTTransformation extends AbstractASTTransformatio
List<String> excludes = getMemberStringList(anno, "excludes");
List<String> includes = getMemberStringList(anno, "includes");
final boolean allNames = memberHasValue(anno, "allNames", true);
+ final boolean allProperties = memberHasValue(anno, "allProperties", true);
if (!checkIncludeExcludeUndefinedAware(anno, excludes, includes, MY_TYPE_NAME)) return;
if (!checkPropertyList(cNode, includes, "includes", anno, MY_TYPE_NAME, includeFields)) return;
if (!checkPropertyList(cNode, excludes, "excludes", anno, MY_TYPE_NAME, includeFields)) return;
- createHashCode(cNode, cacheHashCode, includeFields, callSuper, excludes, includes, allNames);
- createEquals(cNode, includeFields, callSuper, useCanEqual, excludes, includes, allNames);
+ createHashCode(cNode, cacheHashCode, includeFields, callSuper, excludes, includes, allNames, allProperties);
+ createEquals(cNode, includeFields, callSuper, useCanEqual, excludes, includes, allNames, allProperties);
}
}
@@ -119,6 +122,10 @@ public class EqualsAndHashCodeASTTransformation extends AbstractASTTransformatio
}
public static void createHashCode(ClassNode cNode, boolean cacheResult, boolean includeFields, boolean callSuper, List<String> excludes, List<String> includes, boolean allNames) {
+ createHashCode(cNode, cacheResult, includeFields, callSuper, excludes, includes, allNames,false);
+ }
+
+ public static void createHashCode(ClassNode cNode, boolean cacheResult, boolean includeFields, boolean callSuper, List<String> excludes, List<String> includes, boolean allNames, boolean allProperties) {
// make a public method if none exists otherwise try a private method with leading underscore
boolean hasExistingHashCode = hasDeclaredMethod(cNode, "hashCode", 0);
if (hasExistingHashCode && hasDeclaredMethod(cNode, "_hashCode", 0)) return;
@@ -130,11 +137,11 @@ public class EqualsAndHashCodeASTTransformation extends AbstractASTTransformatio
final Expression hash = varX(hashField);
body.addStatement(ifS(
isZeroX(hash),
- calculateHashStatements(cNode, hash, includeFields, callSuper, excludes, includes, allNames)
+ calculateHashStatements(cNode, hash, includeFields, callSuper, excludes, includes, allNames, allProperties)
));
body.addStatement(returnS(hash));
} else {
- body.addStatement(calculateHashStatements(cNode, null, includeFields, callSuper, excludes, includes, allNames));
+ body.addStatement(calculateHashStatements(cNode, null, includeFields, callSuper, excludes, includes, allNames, allProperties));
}
cNode.addMethod(new MethodNode(
@@ -146,8 +153,9 @@ public class EqualsAndHashCodeASTTransformation extends AbstractASTTransformatio
body));
}
- private static Statement calculateHashStatements(ClassNode cNode, Expression hash, boolean includeFields, boolean callSuper, List<String> excludes, List<String> includes, boolean allNames) {
- final List<PropertyNode> pList = getInstanceProperties(cNode);
+ private static Statement calculateHashStatements(ClassNode cNode, Expression hash, boolean includeFields, boolean callSuper, List<String> excludes, List<String> includes, boolean allNames, boolean allProperties) {
+ final Set<String> names = new HashSet<String>();
+ final List<PropertyNode> pList = getAllProperties(names, cNode, true, false, allProperties, false, false, false);
final List<FieldNode> fList = new ArrayList<FieldNode>();
if (includeFields) {
fList.addAll(getInstanceNonPropertyFields(cNode));
@@ -158,7 +166,7 @@ public class EqualsAndHashCodeASTTransformation extends AbstractASTTransformatio
body.addStatement(declS(result, callX(HASHUTIL_TYPE, "initHash")));
for (PropertyNode pNode : pList) {
- if (shouldSkip(pNode.getName(), excludes, includes, allNames)) continue;
+ if (shouldSkipUndefinedAware(pNode.getName(), excludes, includes, allNames)) continue;
// _result = HashCodeHelper.updateHash(_result, getProperty()) // plus self-reference checking
Expression getter = getterThisX(cNode, pNode);
final Expression current = callX(HASHUTIL_TYPE, "updateHash", args(result, getter));
@@ -168,7 +176,7 @@ public class EqualsAndHashCodeASTTransformation extends AbstractASTTransformatio
}
for (FieldNode fNode : fList) {
- if (shouldSkip(fNode.getName(), excludes, includes, allNames)) continue;
+ if (shouldSkipUndefinedAware(fNode.getName(), excludes, includes, allNames)) continue;
// _result = HashCodeHelper.updateHash(_result, field) // plus self-reference checking
final Expression fieldExpr = varX(fNode);
final Expression current = callX(HASHUTIL_TYPE, "updateHash", args(result, fieldExpr));
@@ -212,6 +220,10 @@ public class EqualsAndHashCodeASTTransformation extends AbstractASTTransformatio
}
public static void createEquals(ClassNode cNode, boolean includeFields, boolean callSuper, boolean useCanEqual, List<String> excludes, List<String> includes, boolean allNames) {
+ createEquals(cNode, includeFields, callSuper, useCanEqual, excludes, includes, allNames,false);
+ }
+
+ public static void createEquals(ClassNode cNode, boolean includeFields, boolean callSuper, boolean useCanEqual, List<String> excludes, List<String> includes, boolean allNames, boolean allProperties) {
if (useCanEqual) createCanEqual(cNode);
// make a public method if none exists otherwise try a private method with leading underscore
boolean hasExistingEquals = hasDeclaredMethod(cNode, "equals", 1);
@@ -239,9 +251,10 @@ public class EqualsAndHashCodeASTTransformation extends AbstractASTTransformatio
body.addStatement(ifS(notX(callX(otherTyped, "canEqual", varX("this"))), returnS(constX(Boolean.FALSE,true))));
}
- List<PropertyNode> pList = getInstanceProperties(cNode);
+ final Set<String> names = new HashSet<String>();
+ final List<PropertyNode> pList = getAllProperties(names, cNode, true, includeFields, allProperties, false, false, false);
for (PropertyNode pNode : pList) {
- if (shouldSkip(pNode.getName(), excludes, includes, allNames)) continue;
+ if (shouldSkipUndefinedAware(pNode.getName(), excludes, includes, allNames)) continue;
boolean canBeSelf = StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(
pNode.getOriginType(), cNode
);
@@ -264,7 +277,7 @@ public class EqualsAndHashCodeASTTransformation extends AbstractASTTransformatio
fList.addAll(getInstanceNonPropertyFields(cNode));
}
for (FieldNode fNode : fList) {
- if (shouldSkip(fNode.getName(), excludes, includes, allNames)) continue;
+ if (shouldSkipUndefinedAware(fNode.getName(), excludes, includes, allNames)) continue;
body.addStatement(
ifS(notX(hasSameFieldX(fNode, otherTyped)),
ifElseS(differentSelfRecursiveFieldX(fNode, otherTyped),
http://git-wip-us.apache.org/repos/asf/groovy/blob/8403a939/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 f2ffe43..c951435 100644
--- a/src/test/org/codehaus/groovy/transform/CanonicalComponentsTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/CanonicalComponentsTransformTest.groovy
@@ -773,6 +773,18 @@ class CanonicalComponentsTransformTest extends GroovyShellTestCase {
assert new FieldAndPropertyIncludedInHashCode().hashCode() == 442087
"""
}
+
+ void testHashCodeForInstanceWithNullPropertyAndJavaBeanProperty() {
+ new GroovyShell().evaluate '''
+ import groovy.transform.*
+ @EqualsAndHashCode(allProperties = true)
+ class FieldAndPropertyIncludedInHashCode {
+ String property
+ String getField() { null }
+ }
+ assert new FieldAndPropertyIncludedInHashCode().hashCode() == 442087
+ '''
+ }
}
@TupleConstructor