Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 334992 - [quick fix] "Return the allocated Object" hides "Assign statement to local variable"
Summary: [quick fix] "Return the allocated Object" hides "Assign statement to local va...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.7   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 3.7 M6   Edit
Assignee: Markus Keller CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-21 05:48 EST by Markus Keller CLA
Modified: 2011-03-09 08:36 EST (History)
3 users (show)

See Also:


Attachments
Fix (4.42 KB, patch)
2011-01-31 12:46 EST, Markus Keller CLA
no flags Details | Diff
Fix (12.58 KB, patch)
2011-01-31 12:50 EST, Markus Keller CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2011-01-21 05:48:10 EST
HEAD

The "Return the allocated Object" quick fix hides the more interesting "Assign statement to local variable" quick assist.

Have a line like this:

        new StringBuffer();

With the warning for unused objects enabled and the caret at the end of the line, Ctrl+1 jumps to the warning and only offers "Return the allocated object" and "Remove", but does not show the "Assign statement to local variable/field" quick assists. The quick assists should also be shown in this situation, and they should have precedence over the quick fixes.
Comment 1 Deepak Azad CLA 2011-01-31 00:29:32 EST
Isn't this a general problem/feature ? 

Whenever Ctrl+1 jumps to a warning or error only the quick fixes are shown
(see JavaCorrectionProcessor.computeQuickAssistProposals(..) line 242)

You can always press Ctrl+1 again to jump back to the original location and use the quick assists. In this case "Assign statement to local variable/field" is available when you press Ctrl+1 again.
Comment 2 Dani Megert CLA 2011-01-31 03:20:56 EST
(In reply to comment #1)
> Isn't this a general problem/feature ? 
Yes.
Comment 3 Markus Keller CLA 2011-01-31 12:46:42 EST
Created attachment 187984 [details]
Fix

In general, it's good that we don't show quick assists after jumping to the error location. But in this case, "Assign to local variable" can just as well be considered as a quick fix (since it indeed fixes the problem).

I've implemented it that way, which has the additional advantage, that it also shows up in the quick fix hover. Finally, I've tweaked the relevance to prefer assigning to a local variable if the type of the allocated object is not assignable to the return type.
Comment 4 Markus Keller CLA 2011-01-31 12:50:08 EST
Created attachment 187985 [details]
Fix

Second try, just for bug 294650 :-(
Comment 5 Markus Keller CLA 2011-01-31 12:51:02 EST
Fixed in HEAD.
Comment 6 Deepak Azad CLA 2011-01-31 23:10:34 EST
(In reply to comment #3)
> But in this case, "Assign to local variable" can just as well
> be considered as a quick fix (since it indeed fixes the problem).
Fair enough, the fix looks good.
Comment 7 Raksha Vasisht CLA 2011-03-07 03:40:49 EST
(In reply to comment #3)
> Created attachment 187984 [details] [diff]
> Fix
> 
> In general, it's good that we don't show quick assists after jumping to the
> error location. But in this case, "Assign to local variable" can just as well
> be considered as a quick fix (since it indeed fixes the problem).
> 
> I've implemented it that way, which has the additional advantage, that it also
> shows up in the quick fix hover. Finally, I've tweaked the relevance to prefer
> assigning to a local variable if the type of the allocated object is not
> assignable to the return type.

I see the order gets swapped depending on the return type, but the "Return the allocated Object" comes in between "Assign
statement to local variable" and "Assign statement to new field". Don't we always show the similar quick fixes/assists together (like Rename, Extract...)? Plus both the assign assists solve the warning/error. 

public class A {	
	int foo(){
		new StringBuffer();
		return 1;		
	}
}

=> I would expect that the 'Return the allocated Object' quick fix comes before or after both the Assign quick assists.
Comment 8 Raksha Vasisht CLA 2011-03-07 03:49:08 EST
(In reply to comment #7)

Pls see bug 339056
Comment 9 Raksha Vasisht CLA 2011-03-09 08:36:14 EST
Verified for 3.6 M6 with I20110307-2110.