Lombok: @Data needs a callSuper.

[lombokissues]

_Migrated from Google Code (issue 19)_

[lombokissues]

:bust_in_silhouette: reinierz   :clock8: Aug 05, 2009 at 20:17 UTC

This will also involve making @ EqualsAndHashCode and @ ToString aware of @ Data, as they'll need
to read @ Data's callSuper value if the user didn't explicitly specify one for @ EqAHC or @ ToString.

[lombokissues]

:bust_in_silhouette: jacek99   :clock8: Aug 13, 2009 at 14:15 UTC

+1 from me, it makes class inheritance problematic

[lombokissues]

:bust_in_silhouette: pe.fips   :clock8: May 08, 2012 at 19:16 UTC

Here is a proof-of-concept implementation:

https://github.com/peichhorn/lombok/tree/Issue_19_%40Data_needs_a_callSuper

• Philipp

[lombokissues]

:bust_in_silhouette: reinierz   :clock8: May 09, 2012 at 09:32 UTC

[lombokissues]

:bust_in_silhouette: serverperformance   :clock8: Jul 03, 2012 at 23:07 UTC

Is there an expected release date/version for this enhacement? (if any)
Thank you very much!

[lombokissues]

:bust_in_silhouette: askoning   :clock8: May 09, 2013 at 08:15 UTC

Issue #550 has been merged into this issue.

[lombokissues]

:bust_in_silhouette: mederel   :clock8: May 30, 2013 at 11:50 UTC

Hi, same question: is there an expected date version for this enhancement?
I see it marked for label 0.11.1. Does this means it is already included since 0.11.1?

Currently using 0.11.8 I write @ Data on a superclass and @ Data on a subclass and I get the warning that follows on the subclass:
"Generating equals/hashCode implementation but without a call to superclass, even though this class does not extend java.lang.Object. If this is intentional, add '@ EqualsAndHashCode(callSuper=false)' to your type."
Adding the line: "@ EqualsAndHashCode(callSuper = true)" the warning disappear.

Thanks and cheers, Emile

[lombokissues]

:bust_in_silhouette: danielmalcolmabraham   :clock8: Aug 19, 2013 at 12:58 UTC

I'm sure this is obvious, but when this happens, could it happen on @ Value as well please?

Thank you very much.

[lombokissues]

:bust_in_silhouette: r.spilker   :clock8: Nov 08, 2013 at 13:16 UTC

Issue #443 has been merged into this issue.

[lombokissues]

:bust_in_silhouette: [email protected]   :clock8: Jun 11, 2015 at 22:09 UTC

See https://github.com/rzwitserloot/lombok/pull/69

[lombokissues]

_End of migration_

[tburch]

Would love to see this! 👍

[dabrowt1]

This seems simple but would be very helpful.

[jordanms]

Calling the super implementation for Equals and Hashcode should be the default behavior when the target class does not extend java.lang.Object. As changing default value for @EqualsAndHashCode would break compatibility, adding the parameter callSuper to @Data and @Value would add great value as we wouldn't need to add extra @EqualsAndHashCode(callSuper = true) annotation.

[demi365]

This is like a much needed feature, Call super feature in lombok inheritance, I have a project which have a bunch of classes that inherit classes down the line, and it would be much helpful, if we can have this feature in lombok.

[rzwitserloot]

@demi365 that requires Resolution and therefore not going to happen anytime soon; it'd be extremely complicated to get that right. The callSuper that this feature refers to is something else.

[rzwitserloot]

@jordanms there is no guarantee that the parent class has sensible support for either, or for that matter, has fields. Really, we don't _KNOW_ what we ought to do, which is why we emit a warning instead; force you to acknowledge it.

This entire issue looks like it's filled with people who think that this feature request implies that lombok will figure out what parent's fields are or if parent has sensible implementations of equals and hashCode. There is no real way we can know; at best we can detect that lombok generated their equals, hashCode, toString etc and thus automatically do the right thing, but that's NOT what this is about. I'm closing it, as a consequence.

[jordanms]

@rzwitserloot I didn't say to force that behavior, I said it should be the default. I also didn't say that lombok has to figure out if the parent class has fields that matter or not. I also didn't see anyone saying what you understood. Seems you didn't understand the proposition and you are being rude to people with no reason.

What we want is very simple: add the configuration so we can avoid few extra lines of code.

It's a pity you couldn't understand that.

[demi365]

@rzwitserloot agreed after going through I could understand that getting
this right is extremely difficult before the code reaches the compiler.
But, inheritance is a key feature, and right now, if not able to call
parents args with lombok is a problem, with a solution that might be next
to impossible. But we are not asking for lombok to do all of it. You could
try a vote to ask people if this is good,
@AllArgsConstructor(super = Bar.class) class Foo or any small value that
could be passed to lombok to figure what it needs to do. And then my
compiled class Foo will be Foo extends Bar with it's fields in args.
Whichever help lombok needs from developers can be given, kept a minimal
rate.
Thanks, again, if this doesn't work, I can fully understand the complexity
of the feature we are requesting, so we will make do with whatever we have
now.

On Fri, Feb 28, 2020 at 3:44 AM Reinier Zwitserloot <
[email protected]> wrote:

@demi365 https://github.com/demi365 that requires Resolution
https://github.com/rzwitserloot/lombok/wiki/LOMBOK-CONCEPT:-Resolution
and therefore not going to happen anytime soon; it'd be extremely
complicated to get that right. The callSuper that this feature refers to
is something else.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/92?email_source=notifications&email_token=AHJDDJWPQV7TWDRDNLSIF4TRFA3N7A5CNFSM4CIJNVQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENGFHHQ#issuecomment-592204702,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AHJDDJVZDB3MUU7MWPRRMMTRFA3N7ANCNFSM4CIJNVQQ
.

[janrieke]

@jordanms There is a reason why callSuper = true is not and should not be the default: It leads to hard-to-find bugs, because people will use it on classes where the superclass does not implement it, and suddenly no instances are equal because it also calls Object.equals().
You could argue that lombok should make callSuper = true the default, but also emit a warning. But where is the difference to the current behaviour (callSuper = false + warning)? You do not want warnings in your code, so you still have to acknowledge this by explicitly setting callSuper = true.

But let's get back to the original topic of this issue: an additional parameter callSuper = false for @Data (and possibly @Value).
Will it allow removing 2 additional annotations? Yes.
Will it make things more complicated? Also yes, because lombok has to check for possible contradictions on the parameter. E.g. what should be the result of @Data(callSuper = true) @ToString(callSuper = false)? An error, a warning, or does the more special annotation (@ToString) take precedence (so only equals() and hashCode() will call super)? It's not that easy.

[janrieke]

@demi365 The problem is not figuring out the name of the superclass; lombok could do that just by looking at the extends clause. The problem is that this superclass is not accessible when lombok processes the subclass, so lombok cannot figure out how the constructor and the fields of this superclass look like.
This is the reason @rzwitserloot refers to when saying "It requires resolution."

[demi365]

Okay, I feel like I can understand but not understand, how
does @SuperBuilder work, and how does lombok know the fields from a class
based on the annotation on the class name, if it can be done for parent,
then knowing the parent should be enough?
I thought the resolution was for cases where you import com.foo.* and the
parent Foo.class is from this package, which is confusing, since lombok can
potentially not be able to search all these .* packages for the parent...
It's confusing, the thing with resolution

On Sat, Feb 29, 2020 at 6:24 PM Jan Rieke [e-mail] wrote:

@demi365 https://github.com/demi365 The problem is not figuring out the
name of the superclass; lombok could do that just by looking at the
extends clause. The problem is that this superclass is not accessible
when lombok processes the subclass, so lombok cannot figure out how the
constructor and the fields of this superclass looks like.
This is the same reason why many other feature requests get close: "It
requires resolution."

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/92?email_source=notifications&email_token=AHJDDJVWMKG3UGTRKLBJGJTRFECP7A5CNFSM4CIJNVQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENLZMIA#issuecomment-592942624,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AHJDDJSXSJRIVSK54FAV2NLRFECP7ANCNFSM4CIJNVQQ
.

[jordanms]

@janrieke I got a bug because I annotated my class with @ToStringAndHashCode and the super methods were not called (I assume it would be the default). In my opinion, implementing toString and hashCode is mandatory and any developer should follow it because not doing so creates lots of hard to find bugs. Even worse than not implementing those methods, is to implement in the child class and not in the parent class (when it is not Object). If you got a case when you only need to look at the child to see if these objects are equal or not, then most likely the class should not have a parent. This is a good OO pattern and libraries should follow and enforce, as maximum as possible, good patterns. Therefore, have callSuper = true as default is the most appropriated. For the developers that don't follow it, this behavior is configurable, so they can set callSuper = false.
Regarding the warning, in my current company, they use @ToStringAndHashCode(callSuper = false) and I asked them why and the answer was that without it they get a warning! I guess the same is happening in other companies and I think this is not the intention of the warning. So I suggest you guys improve the message so people will think if they should set it to true or false.

Orignal topic

Will it make things more complicated?

As you said, yes. However, it will be more complicated for Lombok only. And I don't think it is a big deal for Lombok to solve the problems from it, because you guys already solved for other annotations. For instance, when annotating a class with @Value @Builder, or when in the class level we have @Data and in a field, we have @Getter(AccessLevel.PRIVATE). It is just a matter of setting the rules, which should not be complicated.

I also don't think it would be difficult to implement, the current solution for the Constructor annotations requires the parent class to have a default constructor and this constraint could be kept as @Data is just an umbrella for the other annotations. If we stick the solution to the behaviour we have currently, it is completely doable.

[randakar]

You folks misunderstand what resolution means.

Lombok has no access to the superclass at all when it is invoked. It can
access the class itself, but nothing outside it.

It's not just "more difficult". It's close to impossible. It's not a
compiler nor does it have the same level of access that javac does. If it
were possible it would require cooperation of the java compiler that does
not currently exist.
And the lombok devs have bigger fish to fry.

All of that
means that it can't know if there is a sensible implementation of toString,
hashcode, or equals at all. So it can't warn you if it what it generates
makes zero sense.

For the constructor of the superclass the picture is a bit easier. If the
superclass doesn't have a default constructor you will get the same compile
error that you'd get if you were to call the wrong constructor in your own
code, and the user can fix it in the same way.

But with the equals and hashcode methods especially calling the superclass
version blindly is dangerous and liable to introduce subtle bugs. Lombok is
there to make your life easier, and that means not to giving you "features"
that may blow up in your face.

On Sun, Mar 1, 2020, 13:55 Jordan Silva [e-mail] wrote:

@janrieke https://github.com/janrieke I got a bug because I annotated
my class with @ToStringAndHashCode and the super methods were not called
(I assume it would be the default). In my opinion, implementing toString
and hashCode is mandatory and any developer should follow it because not
doing so creates lots of hard to find bugs. Even worse than not
implementing those methods, is to implement in the child class and not in
the parent class (when it is not Object). If you got a case when you only
need to look at the child to see if these objects are equal or not, then
most likely the class should not have a parent. This is a good OO pattern
and libraries should follow and enforce, as maximum as possible, good
patterns. Therefore, have callSuper = true as default is the most
appropriated. For the developers that don't follow it, this behavior is
configurable, so they can set callSuper = false.
Regarding the warning, in my current company, they use @ToStringAndHashCode(callSuper
= false) and I asked them why and the answer was that without it they get
a warning! I guess the same is happening in other companies and I think
this is not the intention of the warning. So I suggest you guys improve the
message so people will think if they should set it to true or false.

Orignal topic

Will it make things more complicated?

As you said, yes. However, it will be more complicated for Lombok and the
developers that don't follow good practices. And I don't think it is a big
deal for Lombok to solve the problems from it, because you guys already
solved for other annotations. For instance, when annotating a class with @Value
@Builder, or when in the class level we have @Data and in a field, we
have @Getter(AccessLevel.PRIVATE). It is just a matter of setting the
rules, which should not be complicated.

I also don't think it would be difficult to implement, the current
solution for the Constructor annotations requires the parent class to have
a default constructor and this constraint could be kept as @Data is just
an umbrella for the other annotations. If we stick the solution to the
behaviour we have currently, it is completely doable.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/92?email_source=notifications&email_token=AABIERJU63PSUHJ65BVDGI3RFJLLFA5CNFSM4CIJNVQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENM6KUA#issuecomment-593093968,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIERIRWY52N4CQKMO67NDRFJLLFANCNFSM4CIJNVQQ
.

[jordanms]

I really don't understand how you guys read that we want to change the current behavior. Pay attention: WE DO NOT WANT TO CHANGE CURRENT BEHAVIOR!!

Current solutions:

@Data
@ToString(callSuper = true)
@ToStringAndHashcode(callSuper = true)
public class A extends B {
}

Proposed solution:

@Data(callSuper = true)
public class A extends B {
}

How is it impossible? How does it change any current behavour?

[Maaartinus]

@janrieke

Will it make things more complicated? Also yes, because lombok has to check for possible contradictions on the parameter. E.g. what should be the result of @Data(callSuper = true) @ToString(callSuper = false)? An error, a warning, or does the more special annotation (@ToString) take precedence (so only equals() and hashCode() will call super)? It's not that easy.

The more specific annotation should take precedence as this is what Lombok always does and also the reason for using them. OTOH, using both @Data(callSuper = true) and @ToString(callSuper = false) on the same scope does make little sense and should produce a warning. I'd vote for both: precedence and warning. Maybe someone can come up with a good use case for this combination and then the warning could go.

@jordanms

How is it impossible? How does it change any current behavour?

One problem is the possible setting conflict (s. above). Another problem is increasing the number of annotation arguments, which Lombok always tries to avoid. While you proposal saves just two lines per class, it seems to make sense as it makes Lombok simpler to use.

@jordanms

Regarding the warning, in my current company, they use @ToStringAndHashCode(callSuper = false) and I asked them why and the answer was that without it they get a warning! I guess the same is happening in other companies and I think this is not the intention of the warning. So I suggest you guys improve the message so people will think if they should set it to true or false.

I guess, you can help to make this happen by providing a PR or a better message.

Disclaimer: I'm not the project owner.

[rzwitserloot]

@jordanms Different cultural standards, perhaps? It wasn't intended as rude, and rereading what I wrote, I don't think I'm being rude. I apologize if it comes across as such. Note, however, that I'm having a very hard time understanding how what I wrote was rude, but this:

Pay attention: WE DO NOT WANT TO CHANGE CURRENT BEHAVIOR!!

is not. I ask you to keep an open mind before immediately jumping to the conclusion that rudeness is intended (It was not), whilst also trying to avoid coming across as rude if you can, which that line certainly did.

@randakar did a good job explaining the issues with resolution and the like.

[rzwitserloot]

Now for some specific points:

• I now get bugs because I did want to call super impls, and lombok by default does not.

That's what the warning is for. Unfortunately, the semantic meaning of 'warning' is not universally understood. Team lombok espouses the idea that warnings are virtually as bad as errors are, they only real difference is that, unlike with an error, you can continue your test run or debug session with warnings. Just like with errors, you should definitely not leave them in, check them into source control (unless in a debug branch), and they should absolutely never make it to production.

With that mindset, this problem goes away. Unfortunately it is not possible to cater to the mindset that warnings may safely be ignored; there is no default behaviour that works in all cases. You've run into a situation where 'call the supers' was a better choice, but there are cases where it is the wrong choice; there is no one-answer-fits-all, which is why we generate that warning. Somebody needs to make a choice and lombok is not technically capable of automatically figuring out which one is correct. We cannot fix this.

• ... but my team did read the warning, didn't understand it, stuck 'callSuper = false' on it because it makes the warning go away, but now we still have bugs.

At risk of belittling your team's intelligence, I'm disregarding this one as a problem of lombok. For the same reason: If we posit that that's how development works, it is impossible for lombok to do the right thing. I'm taking this mindset (hey, if I do this, the warning goes away! Clearly that means I wrote correct code) – as a kind of code style I sometimes observe: You more or less type random stuff into your editor until things seem to work out (no errors, in this case no warnings, and a first run seems to indicate things appear to be working). If you are neither willing to fully test your code nor willing to take the time to understand what code (or in this case, a warning) means, then you're going to have bugs. There is nothing lombok can do to stop it from happening, and thus whilst I am totally on board that the best would be that lombok somehow prevents bugs here, we... can't. There's no point keeping an impossible feature request on the books.

• how does SuperBuilder work?

... it works, and without resolution. If you delombok an @SuperBuilder class you can see the magic. It's.. a lot of weird generics. The way superbuilder was done simply cannot be applied sensibly to normal classes, and note that superbuilder inherently only works if the _entire chain_ was _all_ built with superbuilder; lombok's callSuper parameter is intended to work even if your parent's class has no lombok-generated implementations of toString and the like (though, given that it is _impossible_ to just get it right silently if they aren't, lombok generates that warning: It's asking you to tell lombok that the parent's impls are compatible, and that you are willing to sign up to the notion that the API of the parent class is now signed up to maintaining this compatibility. Lombok obviously cannot silently inject future promises, which is why it has to be explicit).

• Okay, so back to the simple case, the original intent of this feature request: @Data(callSuper = false)

Other than the precedence issue (which can be solved with Maaartinus' idea: The more specific one takes precedence), there's the constructor issue: @Data covers many annotations, and one of them doesn't _have_ a callSuper: The @RequiredArgsConstructor. I can imagine someone reading the code would assume that Data(callSuper=true) also implies the non-existent @RequiredArgsConstructor(callSuper = true), and that this non-existent feature implies to call that constructor which covers each required arg. Which can be done – but only with resolution, and I assume everyone is aware that many lombok users are unaware that lombok cannot easily provide resolution-based features, and if not, well, I submit this very thread as Exhibit A :)

Given that I feel unsure that the text that appears in your source code will be properly understood, and that the feature only saves a relatively small and stable amount of boilerplate (does not expand if you add more fields, nor does it require that you rewrite it if you change your fields), the feature is not accepted.

PRs will not be merged.

To change this conclusion, I think it'd be a good idea to start up a new feature request (this one has been all over the place), referencing this one for historic context, but it would need a solid, implementable plan to solve the above issues. And frankly, I don't see any feasible route to that other than 'lombok does resolution now, with no caveats or maintenance burden'. Which we do want, one day. That day isn't today. I'm not worried about this feature once that day arrives: Changing what @Data and friends do will be the very first thing we'd use our shiny new 'oooh we have resolution now!' hammer on, I don't need an issue to remind me of how nice that'd be :)

[jordanms]

@jordanms Different cultural standards, perhaps? It wasn't intended as rude, and rereading what I wrote, I don't think I'm being rude. I apologize if it comes across as such. Note, however, that I'm having a very hard time understanding how what I wrote was rude, but this:

Pay attention: WE DO NOT WANT TO CHANGE CURRENT BEHAVIOR!!

is not. I ask you to keep an open mind before immediately jumping to the conclusion that rudeness is intended (It was not), whilst also trying to avoid coming across as rude if you can, which that line certainly did.

Not my fault if people don't read things before commenting. This is just a way of catching people's attention for something that was mentioned at least 4 times before.

@rzwitserloot, I'm not saying Lombok has to catch bugs for me, I'm saying that the reasons you presented are not sufficient to justify your decisions as they apply to both sides. And I'm glad you recognized that. Also, this is not a matter if it is technical possible or not to implement the requested feature (as you tried to put it in the beginning), but how you think it should be (or how you think people will understand the change). This is not a technical discussion, just people giving their opinion. It's your project, your decision. As a user of your library, I'm already avoiding it in my new project. The classes are getting overloaded with annotations and it makes the code not took nice.