Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 347792

Summary: [1.7][quick fix] Update "Convert 'switch' to 'if-else'" quick assist
Product: [Eclipse Project] JDT Reporter: Deepak Azad <deepakazad>
Component: UIAssignee: Deepak Azad <deepakazad>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: daniel_megert, markus.kell.r, Michael_Rennie
Version: 3.7   
Target Milestone: 3.7.1   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
fix + tests none

Description Deepak Azad CLA 2011-05-31 10:24:33 EDT 
The quick assist needs to be updated to use String#equals() instead of ==

I will attach a patch.
Comment 1 Markus Keller CLA 2011-05-31 12:33:43 EDT 
Good catch. Please make sure you generate the checks as
"if (key.equals(value))". That will also preserve the NPE if key == null.
Comment 2 Deepak Azad CLA 2011-05-31 13:30:33 EDT 
(In reply to comment #1)
> Good catch. Please make sure you generate the checks as
> "if (key.equals(value))". That will also preserve the NPE if key == null.

hmm... I had implemented this as "if (value.equals(key))".  I know that in general quick assists should preserve behavior, however I wonder if in this case we should generate the 'right' code i.e. one that does not result in an NPE. I am assuming most users will find it desirable to avoid the NPE.
Comment 3 Markus Keller CLA 2011-05-31 14:27:05 EDT 
We should not silently change behavior. An alternative would be to add an explicit

    if (key == null)
        throw new NullPointerException();

in front that can easily be deleted. But we would look bad if the compiler immediately flags the null check as unnecessary...

So maybe it's better to just add " (removing null check on condition)" to the quick assist label in this case.
Comment 4 Deepak Azad CLA 2011-06-09 04:35:28 EDT 
Created attachment 197671 [details]
fix + tests

(In reply to comment #3)
> So maybe it's better to just add " (removing null check on condition)" to the
> quick assist label in this case.

I have changed the label to "Convert 'switch' to 'if-else' (removing null check on switch expression)".
Comment 5 Deepak Azad CLA 2011-06-09 04:35:47 EDT 
Fixed in BETA_JAVA7
Comment 6 Markus Keller CLA 2011-06-16 11:58:46 EDT 
Fixed NPE in BETA_JAVA7 of AdvancedQuickAssistProcessor when switch expression type could not be resolved (e.g. an unresolved variable).
Comment 7 Michael Rennie CLA 2011-07-19 16:44:02 EDT 
verified
Comment 8 Dani Megert CLA 2011-08-03 09:09:18 EDT 
The label of that quick fix is too long and not clear. I've removed it (HEAD and R3_7_maintenance) and filed bug 353759 to have a generic way for marking quick fixes/assists that change the code semantics.