Potentially unnecessary warning: Generating equals/hashCode implementation but without a call to superclass, even though this class does not extend java.lang.Object.

3,135 views
Skip to first unread message

v6ak

unread,
Aug 4, 2009, 8:31:21 AM8/4/09
to Project Lombok
I have a class with abstract hashCode and equals. The default
implementation is not usable for the class, but I haven't better one.
So, every subclass has to implement these methods.
When I do it manually, it is OK. But if I try to use
@EqualsAndHashCode, i get this warning that I cannot suppress.

Reinier Zwitserloot

unread,
Aug 4, 2009, 11:17:11 AM8/4/09
to Project Lombok
add (callSuper=false), just like the warning says you should.

Daniel Aborg

unread,
Aug 5, 2009, 3:51:43 PM8/5/09
to Project Lombok
This actually surprised me and my team - we expected that Lombok would
just generate super calls depending on whether we're extending a class
or not.

Now that I've seen this warning I'm even more confounded - clearly
there's a lot of effort put in to highlight what makes sense to the
user when extending or not extending, so wouldn't it be better to just
do the expected thing and just default to calling super when extending
a class?

I mean, if the user absolutely doesn't want this they can just
explicitly turn it off, but at least the default if nothing is
specified is going to make sense most of the time.

Should also make @Data a lot more useful as you won't have to keep
adding an extra @EqualsAndHashCode(callSuper=true) - seems so
unnecessary.

What do you think?

Cheers,

Daniel

Reinier Zwitserloot

unread,
Aug 5, 2009, 4:18:02 PM8/5/09
to Project Lombok
Lombok doesn't automatically generate calls to super, because there
are many situations where that would be entirely the wrong thing to
do.

public class CommonMarker {}

public @EqualsAndHashCode class ActualClass extends CommonMarker {
}



In the above scenario, calling super.hashCode and super.equals() would
in fact turn the generated hashCode and equals into having the same
behaviour as Object's own version (i.e. equal if they are the exact
same instance, otherwise, not equal), except they'd be slower.

Even if the superclass DOES have an implementation for equals and
hashCode, many such implementations aren't written correctly, and a
supercall will fail. Therefore, you are required to manually confirm
the right thing to do and add a callSuper= value.

The concept of going for sane defaults is fine and very much something
lombok is designed around, but it stops being a useful rule of thumb
if, in the few cases where the default isn't what you wanted, things
blow up in your face and/or shoot your foot off. This is such a case.
It would be entirely non-obvious and could result in many hours of bug
hunting when lombok's generated equals and hashCode methods
mysteriously appear but seem to not be doing their job right. We could
and probably should add a link to an html page with more information
about the vagaries of equals and hashCode to that warning someday.

We were going to add callSuper to data itself, but that would add a
lot more interdependencies; right now Data knows about
EqualsAndHashCode and will simply not generate equals and hashCode
implementations if it sees that annotation (knowing that it will end
up generating them instead). EqualsAndHashCode is oblivious to the
existence of Data. By adding callSuper, we'd have to make
EqualsAndHashCode and ToString aware of Data and copy its callSuper
value if the user hasn't specified one for @EqualsAndHashCode or
@ToString. Though, in retrospect, that may be the right idea anyway. I
filed an issue to do just that:

http://code.google.com/p/projectlombok/issues/detail?id=19

v6ak

unread,
Aug 6, 2009, 7:48:01 AM8/6/09
to Project Lombok
@EqualsAndHashCode throws the warning.
@EqualsAndHashCode(callSuper=false) throws the warning, too.
@EqualsAndHashCode(callSuper=true) throws compilation error, because
the parent't equals and hashCode are abstract.
I'm not sure it is a good behavior.
Note that the warning does not say to add callSuper=false. It says
that I maybe should add callSuper=true.
Reply all
Reply to author
Forward
0 new messages