Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 339222 - [quick assist] "Change modifiers to final where possible" too prominent
Summary: [quick assist] "Change modifiers to final where possible" too prominent
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.7   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.7 M7   Edit
Assignee: Deepak Azad CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-08 08:48 EST by Deepak Azad CLA
Modified: 2011-04-11 01:15 EDT (History)
1 user (show)

See Also:


Attachments
fix+tests (25.26 KB, patch)
2011-04-11 01:11 EDT, Deepak Azad CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Deepak Azad CLA 2011-03-08 08:48:23 EST
-----------------------------------------------------------
package p;

class A {   // warning here
	int a;
	
	@Override
	public boolean equals(Object obj) {
		if (this == obj)
			return true;
		if (obj == null)
			return false;
		if (getClass() != obj.getClass())
			return false;
		ClassA other = (ClassA) obj;
		if (a != other.a)
			return false;
		return true;
	}
}
----------------------------------------------------------
- Enable 'Class overrides equals() but not hashcode()' warning
- Click on the warning in the vertical ruler
- 'Override hascode()' is number 2 and 'Generate hashcode and equals' is number 3.
Expected: These should be 1 and 2.
Comment 1 Markus Keller CLA 2011-03-09 12:45:14 EST
Hmm, I'm not sure. If you already have an equals() method, then "Generate hashCode() and equals()" will override that method with a generated boilerplate implementation that is often not the best solution for the specific problem.

I'd rather encourage people to take a breath and think about how to be implement hashCode(), rather than proposing to use a generated method. People who often need to implement these methods should better use one of the many available frameworks.
Comment 2 Markus Keller CLA 2011-03-09 13:00:06 EST
Wheew, I missed the "Change modifiers to final where possible" quick assist that shows up when "A" is selected (with and without a problem on A). My comment 1 assumed you wanted to change the ordering of the 2 applicable quick fixes.

The quick assist is completely wrong and should never be shown in this situation. I'm not even sure if it should be shown in any situation :-/

Unless someone sees a god reason, we should remove the quick assist completely. Users can use the Clean Up if they want to suffer from 'final' measles.
Comment 3 Markus Keller CLA 2011-03-11 09:11:55 EST
(In reply to comment #2)
> Unless someone sees a god reason, we should remove the quick assist completely.

I meant a "good" reason, of course.

Deepak, can you please remove the quick assist?
Comment 4 Deepak Azad CLA 2011-04-11 01:11:47 EDT
Created attachment 192913 [details]
fix+tests

Markus, as discussed on sametime I have reduced the visibility of the quick assist. Now it only works inside the selection, i.e. it does not show up when only "A" is selected, but it show up when the whole class or the whole equals method or a set of statements are selected.
Comment 5 Deepak Azad CLA 2011-04-11 01:15:41 EDT
Fixed in HEAD.