You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by pa...@apache.org on 2015/05/08 06:06:50 UTC
incubator-groovy git commit: GROOVY-7417: @EqualsAndHashCode
inconsistent when using boolean properties for classes with explicit getters
Repository: incubator-groovy
Updated Branches:
refs/heads/master 94806c3c2 -> a38bc8fd9
GROOVY-7417: @EqualsAndHashCode inconsistent when using boolean properties for classes with explicit getters
Project: http://git-wip-us.apache.org/repos/asf/incubator-groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-groovy/commit/a38bc8fd
Tree: http://git-wip-us.apache.org/repos/asf/incubator-groovy/tree/a38bc8fd
Diff: http://git-wip-us.apache.org/repos/asf/incubator-groovy/diff/a38bc8fd
Branch: refs/heads/master
Commit: a38bc8fd901508b28822df25d1c79605acffa313
Parents: 94806c3
Author: Paul King <pa...@asert.com.au>
Authored: Fri May 8 14:06:42 2015 +1000
Committer: Paul King <pa...@asert.com.au>
Committed: Fri May 8 14:06:42 2015 +1000
----------------------------------------------------------------------
.../codehaus/groovy/ast/tools/GeneralUtils.java | 34 ++++++++++-
.../EqualsAndHashCodeASTTransformation.java | 6 +-
.../transform/ToStringASTTransformation.java | 2 +-
.../CanonicalComponentsTransformTest.groovy | 62 ++++++++++++++++++++
4 files changed, 97 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/a38bc8fd/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java b/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
index 39eacc4..7fa98a1 100644
--- a/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
+++ b/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
@@ -462,6 +462,11 @@ public class GeneralUtils {
return eqX(varX(fNode), propX(other, fNode.getName()));
}
+ public static BinaryExpression hasEqualPropertyX(ClassNode annotatedNode, PropertyNode pNode, VariableExpression other) {
+ return eqX(getterThisX(annotatedNode, pNode), getterX(other.getOriginType(), other, pNode));
+ }
+
+ @Deprecated
public static BinaryExpression hasEqualPropertyX(PropertyNode pNode, Expression other) {
String getterName = getGetterName(pNode);
return eqX(callThisX(getterName), callX(other, getterName));
@@ -623,15 +628,38 @@ public class GeneralUtils {
* @param pNode the property being accessed
* @return a method call expression or a property expression
*/
- public static Expression getterX(ClassNode annotatedNode, PropertyNode pNode) {
+ public static Expression getterThisX(ClassNode annotatedNode, PropertyNode pNode) {
ClassNode owner = pNode.getDeclaringClass();
if (annotatedNode.equals(owner)) {
String getterName = "get" + MetaClassHelper.capitalize(pNode.getName());
- if (ClassHelper.boolean_TYPE.equals(pNode.getOriginType())) {
+ boolean existingExplicitGetter = annotatedNode.getMethod(getterName, Parameter.EMPTY_ARRAY) != null;
+ if (ClassHelper.boolean_TYPE.equals(pNode.getOriginType()) && !existingExplicitGetter) {
getterName = "is" + MetaClassHelper.capitalize(pNode.getName());
}
- return callX(new VariableExpression("this"), getterName, ArgumentListExpression.EMPTY_ARGUMENTS);
+ return callThisX(getterName);
}
return propX(new VariableExpression("this"), pNode.getName());
}
+
+ /**
+ * This method is similar to {@link #propX(Expression, Expression)} but will make sure that if the property
+ * being accessed is defined inside the classnode provided as a parameter, then a getter call is generated
+ * instead of a field access.
+ * @param annotatedNode the class node where the property node is accessed from
+ * @param receiver the object having the property
+ * @param pNode the property being accessed
+ * @return a method call expression or a property expression
+ */
+ public static Expression getterX(ClassNode annotatedNode, Expression receiver, PropertyNode pNode) {
+ ClassNode owner = pNode.getDeclaringClass();
+ if (annotatedNode.equals(owner)) {
+ String getterName = "get" + MetaClassHelper.capitalize(pNode.getName());
+ boolean existingExplicitGetter = annotatedNode.getMethod(getterName, Parameter.EMPTY_ARRAY) != null;
+ if (ClassHelper.boolean_TYPE.equals(pNode.getOriginType()) && !existingExplicitGetter) {
+ getterName = "is" + MetaClassHelper.capitalize(pNode.getName());
+ }
+ return callX(receiver, getterName);
+ }
+ return propX(receiver, pNode.getName());
+ }
}
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/a38bc8fd/src/main/org/codehaus/groovy/transform/EqualsAndHashCodeASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/EqualsAndHashCodeASTTransformation.java b/src/main/org/codehaus/groovy/transform/EqualsAndHashCodeASTTransformation.java
index a440b52..f1d0efb 100644
--- a/src/main/org/codehaus/groovy/transform/EqualsAndHashCodeASTTransformation.java
+++ b/src/main/org/codehaus/groovy/transform/EqualsAndHashCodeASTTransformation.java
@@ -126,7 +126,7 @@ public class EqualsAndHashCodeASTTransformation extends AbstractASTTransformatio
for (PropertyNode pNode : pList) {
if (shouldSkip(pNode.getName(), excludes, includes)) continue;
// _result = HashCodeHelper.updateHash(_result, getProperty()) // plus self-reference checking
- Expression getter = getterX(cNode, pNode);
+ Expression getter = getterThisX(cNode, pNode);
final Expression current = callX(HASHUTIL_TYPE, "updateHash", args(result, getter));
body.addStatement(ifS(
notX(sameX(getter, varX("this"))),
@@ -208,14 +208,14 @@ public class EqualsAndHashCodeASTTransformation extends AbstractASTTransformatio
pNode.getOriginType(), cNode
);
if (!canBeSelf) {
- body.addStatement(ifS(notX(hasEqualPropertyX(pNode, otherTyped)), returnS(constX(Boolean.FALSE, true))));
+ body.addStatement(ifS(notX(hasEqualPropertyX(otherTyped.getOriginType(), pNode, otherTyped)), returnS(constX(Boolean.FALSE, true))));
} else {
body.addStatement(
ifS(notX(hasSamePropertyX(pNode, otherTyped)),
ifElseS(differentSelfRecursivePropertyX(pNode, otherTyped),
returnS(constX(Boolean.FALSE, true)),
ifS(notX(bothSelfRecursivePropertyX(pNode, otherTyped)),
- ifS(notX(hasEqualPropertyX(pNode, otherTyped)), returnS(constX(Boolean.FALSE, true))))
+ ifS(notX(hasEqualPropertyX(otherTyped.getOriginType(), pNode, otherTyped)), returnS(constX(Boolean.FALSE, true))))
)
)
);
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/a38bc8fd/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 e08702f..f915887 100644
--- a/src/main/org/codehaus/groovy/transform/ToStringASTTransformation.java
+++ b/src/main/org/codehaus/groovy/transform/ToStringASTTransformation.java
@@ -152,7 +152,7 @@ public class ToStringASTTransformation extends AbstractASTTransformation {
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);
+ Expression getter = getterThisX(cNode, pNode);
appendValue(cNode, body, result, first, getter, pNode.getOriginType(), pNode.getName(), includeNames, ignoreNulls);
}
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/a38bc8fd/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 2db0b70..6e6e1b7 100644
--- a/src/test/org/codehaus/groovy/transform/CanonicalComponentsTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/CanonicalComponentsTransformTest.groovy
@@ -610,6 +610,68 @@ class CanonicalComponentsTransformTest extends GroovyShellTestCase {
assert new Foo2('cat', 'dog').toString() == 'Foo2(cat, dog)'
"""
}
+
+ void testEqualsHashCodeToStringConsistencyWithExplicitBooleanGetters_GROOVY7417() {
+ new GroovyShell().evaluate """
+ import groovy.transform.*
+
+ @ToString
+ @EqualsAndHashCode
+ class A {
+ boolean x
+ }
+
+ def a1 = new A(x: true)
+ def a2 = new A(x: true)
+ def a3 = new A(x: false)
+ assert a1.toString() == a2.toString()
+ assert a1.hashCode() == a2.hashCode()
+ assert a1 == a2
+ assert a1.toString() != a3.toString()
+ assert a1.hashCode() != a3.hashCode()
+ assert a1 != a3
+
+ @ToString
+ @EqualsAndHashCode
+ class B {
+ boolean x
+ boolean isX() { false }
+ }
+
+ def b1 = new B(x: true)
+ def b2 = new B(x: false)
+ assert b1.toString() == b2.toString()
+ assert b1.hashCode() == b2.hashCode()
+ assert b1 == b2
+
+ @ToString
+ @EqualsAndHashCode
+ class C {
+ boolean x
+ boolean getX() { false }
+ }
+
+ def c1 = new C(x: true)
+ def c2 = new C(x: false)
+ assert c1.toString() == c2.toString()
+ assert c1.hashCode() == c2.hashCode()
+ assert c1 == c2
+
+ @ToString
+ @EqualsAndHashCode
+ class D {
+ boolean x
+ boolean isX() { false }
+ boolean getX() { false }
+ }
+
+ def d1 = new D(x: true)
+ def d2 = new D(x: false)
+ assert d1.toString() == d2.toString()
+ assert d1.hashCode() == d2.hashCode()
+ assert d1 == d2
+ """
+ }
}
@TupleConstructor