groovy
  1. groovy
  2. GROOVY-4827

@EqualsAndHashCode.excludes doesn't work correctly with class inheritance

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.8.0
    • Fix Version/s: 1.8.1
    • Component/s: None
    • Labels:
      None
    • Testcase included:
      yes
    • Number of attachments :
      1

      Description

      I cannot inherit exclusions of EqualsAndHashCode, I have to duplicate exclusions in every child class.
      Please see the tescase, only testA() and testE() pass.

      class EqualsTest {
          @EqualsAndHashCode(excludes = 'a1')
          static class A {
              def a1, a2;
          }
      
          @Test
          void testA() {
              assert new A(a1: 'same', a2: 'same') == new A(a1: 'same', a2: 'same') // ok
              assert new A(a1: 'diff', a2: 'same') == new A(a1: 'erent', a2: 'same') // ok
          }
      
          static class B extends A {
              def b
          }
      
          @Test
          void testB() {
              assert new B(a1: 'diff', a2: 'same', b: 'same') == new B(a1: 'erent', a2: 'same', b: 'same')
              // failed! Shouldn't equals() be inherited from A?
          }
      
          //ok. let's try to specify explicitly that we call super.equals()
          @EqualsAndHashCode(callSuper = true)
          static class C extends A {
              def c
          }
      
          @Test
          void testC() {
              assert new C(a1: 'diff', a2: 'same', c: 'same') == new C(a1: 'erent', a2: 'same', c: 'same')
              // failed!
          }
      
          //hm. what about excluding it again in the child class?
          @EqualsAndHashCode(callSuper = true, excludes = 'a1')
          static class D extends A {
              def d
          }
      
          @Test
          void testD() {
              assert new D(a1: 'diff', a2: 'same', d: 'same') == new D(a1: 'erent', a2: 'same', d: 'same')
              // failed!
          }
      
          //hm. should I forget about inheritance??
          @EqualsAndHashCode(excludes = 'a1')
          static class E extends A {
              def e
          }
      
          @Test
          void testE() {
              assert new E(a1: 'diff', a2: 'same', e: 'same') == new E(a1: 'erent', a2: 'same', e: 'same')
              // YES :(
          }
      
      }
      

        Activity

        Hide
        Paul King added a comment - - edited

        There is no intention to allow exclusions from properties inherited from super classes. By using 'callSuper' you get to decide whether the identity as defined in the base class (already specified by its equals and hashCode methods) are important to you or not. Having said that, your examples reveal a bug in how callSuper is working. This came about when we made the implementation of equals stricter to ensure that the equals contract was always fulfilled evening when using inheritance. We will probably alter the implementation to use a canEqual method ala project Lombok 0.10+ or Scala case classes - so that it will work properly for inheritance but also for JPA proxy classes which also use inheritance but only as an implementation detail. We might make this an option or configurable to some extent.

        Reference to an explanation of what we would probably implement:
        http://www.artima.com/lejava/articles/equality.html

        We would probably have a "useCanEqual" flag which defaults to true and results in a public "canEqual" method being defined as per the above article (and project lombok 0.10+ and recent scala case classes). If you set the "useCanEqual" flag to false you will get the current implementation which compares getClass() and you won't pollute the class with a "canEqual" method but the following won't return true (and ditto for JPA proxy instances) which most people would expect to work:

        @Canonical class Point { def x, y }
        assert new Point(1, 2) == new Point(1, 1) { def getY() { 2 } }
        
        Show
        Paul King added a comment - - edited There is no intention to allow exclusions from properties inherited from super classes. By using 'callSuper' you get to decide whether the identity as defined in the base class (already specified by its equals and hashCode methods) are important to you or not. Having said that, your examples reveal a bug in how callSuper is working. This came about when we made the implementation of equals stricter to ensure that the equals contract was always fulfilled evening when using inheritance. We will probably alter the implementation to use a canEqual method ala project Lombok 0.10+ or Scala case classes - so that it will work properly for inheritance but also for JPA proxy classes which also use inheritance but only as an implementation detail. We might make this an option or configurable to some extent. Reference to an explanation of what we would probably implement: http://www.artima.com/lejava/articles/equality.html We would probably have a "useCanEqual" flag which defaults to true and results in a public "canEqual" method being defined as per the above article (and project lombok 0.10+ and recent scala case classes). If you set the "useCanEqual" flag to false you will get the current implementation which compares getClass() and you won't pollute the class with a "canEqual" method but the following won't return true (and ditto for JPA proxy instances) which most people would expect to work: @Canonical class Point { def x, y } assert new Point(1, 2) == new Point(1, 1) { def getY() { 2 } }
        Hide
        Paul King added a comment -

        This should be fixed. Thanks for spotting this. Please let me know if you observe any strange behavior with this functionality.

        Show
        Paul King added a comment - This should be fixed. Thanks for spotting this. Please let me know if you observe any strange behavior with this functionality.

          People

          • Assignee:
            Paul King
            Reporter:
            Pavlo Morgunyuk
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: