Community
Participate
Working Groups
Using I20140318-0830, the quick fixes from bug 241696 for for loops are not shown in the following situations: List<String> list = ...; list<-- Cursor position here It also doesn't work for arguments or fields. Also not for other collection types or arrays. What works is this: Map<String, String> map = ...; map.entrySet()<-- Cursor position here The first examples shows the following errors in the ruler: - Syntax error, insert ";" to complete LocalVariableDeclarationStatement - list cannot be resolved to a type - Syntax error, insert "VariableDeclarators" to complete LocalVariableDeclaration The second example shows only the following error (which may be the cause for the difference): - Syntax error, insert ";" to complete Statement
Yes, and we have 12 test failures in master for this. The problem is the bad AST recovery, see bug 405780. We should first try to fix that problem in JDT Core. If that doesn't work out for 4.4, then we can work around it in the quick fix implementation.
Oops, cited wrong bug. Bug 430336 is the blocker for this one.
Disabled 12 failing tests for now: org.eclipse.jdt.ui.tests.quickfix.LocalCorrectionsQuickFixTest.testGenerateForeachNotAddedForLowVersion org.eclipse.jdt.ui.tests.quickfix.LocalCorrectionsQuickFixTest.testLoopOverAddedToFixesForVariablehNotAddedForLowVersion and 10 in org.eclipse.jdt.ui.tests.quickfix.AssistQuickFixTest#testGenerateFor* http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=67e6de8681d025aa60eef7e3f6dbe2211cc0fb18
Lukas, your nice new "Create 'for' loop" quick fixes don't work any more, and bug 430336 seems to be too hard to fix for 4.4. Would you have time to adapt them to the current structure of the recovered AST?
Ok, I'll take a closer look at it.
Created attachment 242505 [details] This patch fixes the bug I adapted the implementation to the available information of Luna's AST recovery. Please note, I had to change the tests as well, because some assertions depend on the results of other quick assists/fixes (e.g. assign to local variables?) that are broken in Luna :/.
Manju, could you please review this patch? Don't release it yet, since I need to review it as well, and since there's still a faint hope that we might find a solution for bug 430336.
All the tests pass with this patch. 1. Update Copyright year for GenerateForLoopAssistProposal. 2. Consider the below code snippet: package snippet; import java.util.Collection; public class E { void foo(Collection<String> collection) { collection } } Hovering over 'collection' shows all quick fixes available except create for loop, but when user press Ctrl+1 then the additional 2 quick assists are added to the list. This looks inconsistent. Also this doesn't solve the issue reported in Bug 434188.
Fixed with http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=ff852ca64a584ee5ec86f0dd71d0cccc3d80b4d5 I've added a relevance boost in QuickAssistProcessor#getGenerateForLoopProposals(..) to get the for-loop quick assists to show up in front of the "Create class ..." quick fixes (which only show up due to the bad recovery). We will disable the quick fixes in bug 434188 because we don't know how to compute their availability accurately enough. The quick assists will still be available via Ctrl+1 (but not in the Quick Fix hover). That's not perfect, but the best we can do for Luna.
Created attachment 243135 [details] fix for hovering Hi, I'm not sure if it's still needed, but the above patch adds the generate for loop quick assist on hovering again. The proposals to generate for loops will be added correctly now, but there are still inconsistencies for the 'assign to local variable'-quickfix. If the hovering feature shall stay disabled for consistency purposes, just ignore this patch. It can be applied to the current master branch.
(In reply to Lukas Hanke from comment #10) > Created attachment 243135 [details] [diff] > fix for hovering Nice try :-) For the case in comment 8, you rely on the bad AST recovery that already makes "Create class"-like quick fixes appear and then put the good ones on top. Let me think a bit more about this -- maybe I'll even accept that hack for now. But the "case IProblem.ParsingErrorInsertToComplete:" in QuickFixProcessor#hasCorrections(ICompilationUnit, int) is still a no-go I've added some more explanation for this in bug 434188 comment 7.
Verified as working (quick assists) in build id: I20140515-1230.
