Bug 129150 - Add warning when equals/hashcode is out of date (and quick fix to regenerate)
Summary: Add warning when equals/hashcode is out of date (and quick fix to regenerate)
Status: NEW
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.2   Edit
Hardware: All All
: P3 enhancement with 1 vote (vote)
Target Milestone: ---   Edit
Assignee: JDT-Core-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 129151 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-02-23 08:33 EST by Alex Blewitt CLA
Modified: 2006-06-17 17:56 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Blewitt CLA 2006-02-23 08:33:35 EST
If I add a non-transient field into a class, the hashCode and equals methods become out of date. However, I have no way of knowing this. It would be great if Eclipse could keep track of the fields that are referenced in both hashcode and equals, and give a warning when a new field is added that isn't in the equals/hashcode list.

You could have a quick-fix to re-generate the hashCode and equals methods.

Alternatively, you could have a comment in the hashCode/equals lines, like:

// This method was generated automatically. Remove this line to prevent automatic regeneration.

and then if this comment is present, just automatically regenerate the hashCode/equals methods whenever the set of fields changes in the class. This would also be useful if the type of the fields changed (e.g. int to Integer).

Eclipse 3.2M5
Comment 1 Martin Aeschlimann CLA 2006-02-24 06:33:42 EST
There ane many problems with this suggestion:

- Adding these special comments is something we want to avoid
- It also seems to me that we would need a special marker on the fields to know which ones are needed for equals, 'transient' might match, but really has a different purpose which is related to serialization
- fields can be added in many different ways. Modifying equals and hashcode while I type in a new field will be very nasty and difficult to implement.
- sometimes hashcode and equals can not be completly generated and manual user work is required: for arrays only the user knows if the array can be compared by identity or by element and how deep (for arrays of arrays)
- checking if equals/hashcode is up-to-date on reconcile will be expensive.
Comment 2 Martin Aeschlimann CLA 2006-02-24 06:33:59 EST
*** Bug 129151 has been marked as a duplicate of this bug. ***
Comment 3 Alex Blewitt CLA 2006-02-24 18:11:18 EST
You raise some valid points, but also some invalid ones.

If a variable is added which is not referenced at all in the equals method, this is almost certainly a bug and can be trivially flagged as such. It doesn't matter whether or not an array needs a full comparison or not; however, not having a reference to an array can be considered a bug.

Transient fields by their nature are not supposed to be compared in an equals method, and seems a good marker to me. It may not be perfect, but it's better than not having anything at all (which is your suggestion).

I merely added the possibility to auto-generate. However, the main part of the bug report was to flag a *warning* when the set of fields referred to in the equals method is not the same as the set of non-transient fields in a class, with a quick-fix to regenerate. Even if you do not believe that it would catch all situations in all ways, it would still be useful to have this feature. Like other warnings in Eclipse, there's no reason that this couldn't be enabled/disabled if people so chose individually if they didn't agree with it.

Apologies for the duplicate entry; must have been a refresh issue.
Comment 4 Martin Aeschlimann CLA 2006-02-27 02:48:28 EST
First, please do not reopen bugs. Just add comments and I'll gladly reopen them if I find your points convincing.

If it is only about issuing warnings if equals or hash methods that miss a reference to a non-transient field, I will move the bug to jdt.core for consideration.
I personally think it is a bad idea to misuse 'transient' for this case.
The correct solution would be to introduce a new annotation that mark the variables storing values.

Moving to jdt.core to decide about this bug.

Comment 5 Alex Blewitt CLA 2006-02-28 03:49:50 EST
Just so that it's clear; the current generation dialog prompts for which fields to include in the equals() and hashCode() methods. It only shows non-static methods, and all transient fields are deselected by default, whilst all non-transient fields are selected by default.

If I were to add a new non-transient field, then manually delete the hashCode() and equals() methods, and then re-generate them, my newly added field would be selected by default.

All I'm suggesting is that the warning/quick-fix uses the same definition as the currently existing wizard by default, though it would be nice if there were some Java standard for an annotation (or whatever) for marking fields that you were/weren't interested in.

By the way, I think that the quick fix should also give the option of not regenerating the hashCode() method, but just regenerating the equals() method. It's probably easiest to provide two quick fixes in this case.
Comment 6 Werner Keil CLA 2006-03-02 09:46:17 EST
> Alternatively, you could have a comment in the hashCode/equals lines, like:
> // This method was generated automatically. Remove this line to prevent
> automatic regeneration.
> and then if this comment is present, just automatically regenerate the
> hashCode/equals methods whenever the set of fields changes in the class. This
> would also be useful if the type of the fields changed (e.g. int to Integer).
> Eclipse 3.2M5

using some kind of TODO pattern in this comment could also serve a similar tasks as a warning ;-)

Comment 7 Gunnar Wagenknecht CLA 2006-06-17 17:56:51 EDT
(In reply to comment #3)
> If a variable is added which is not referenced at all in the equals method,
> this is almost certainly a bug and can be trivially flagged as such.

I don't think so. I'm working a lot with beans and manual persistence using JDBC and Spring's JdbcDaoSupport (it's an old application. ;). Almost all of the beans are compared by their primary key. Thus, if I add new fields to the beans non of them are relevant for the equals methods.