Community
Participate
Working Groups
@FunctionalInterface interface I { public int foo(I i); } public class X { public static int bar(I i) { return 0;} public int foo(I i) { I i1 = X::;// shows probable quick fix but says no suggestions. return 0; } } [not java 8 specific]
(In reply to Manoj Palat from comment #0) > @FunctionalInterface > interface I { > public int foo(I i); > } > public class X { > public static int bar(I i) { return 0;} > public int foo(I i) { > I i1 = X::;// shows probable quick fix but says no suggestions. > return 0; > } > } > > [not java 8 specific] In 4.3 no quick fix is suggested. The error in 4.4 is: Syntax error, insert "Identifier" to complete Expression Should be fixed during RC1 if possible.
(In reply to Dani Megert from comment #1) > In 4.3 no quick fix is suggested. Sorry, I meant 4.2 / 3.8.
This is due to the "generate for loop" quick fix, which is currently broken (bug 430818). QuickFixProcessor#hasCorrections(ICompilationUnit, int) needs to be very fast, (see Javadoc), and it doesn't get the necessary context to do any further analysis on the actual problem location. Unfortunately, the compile error ParsingErrorInsertToComplete is not very specific and occurs in many cases where LocalCorrectionsSubProcessor#getGenerateForLoopProposals(..) won't find a quick fix. We will have to see how this works after bug 430818 or bug 430336 is fixed. Maybe we should just remove handling of ParsingErrorInsertToComplete in the QuickFixProcessor. The QuickAssistProcessor should still show the fix in that case on Ctrl+1. It won't show up in the Quick Fix hover, but that's an acceptable price for not showing false quick fix lightbulbs elsewhere.
Created attachment 243099 [details] Fix (In reply to Markus Keller from comment #3) > Maybe we should just remove handling of ParsingErrorInsertToComplete in the > QuickFixProcessor. The QuickAssistProcessor should still show the fix in > that case on Ctrl+1. It won't show up in the Quick Fix hover, but that's an > acceptable price for not showing false quick fix lightbulbs elsewhere. The fix implements this.
Dani could you please review and push this?
Note that your test cases pass without the fix. Would be good to have some tests that are red before the fix and green afterwards. Committed with http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=51a68d396c4f907bb13e4f3854465718e4ee9aa2
There's a myriad of ways you can create IProblem.ParsingErrorInsertToComplete, so all a negative test could do is ensure there are no quick fixes for one of them. We also don't do that for all the other IProblems for which we don't have quick fixes. Just to give a small example that shows 4 different ways for getting the problemID that the for-loop quick fixed tries to handle: class E2 { public class MyLayout { int indent; } public void foo() { new MyLayout().indent // no real quick fix } private int[] fField; public void bar() { fField[0] // no quick fix } void // no quick fix void foo(Map<String, String> map) { map..keySet(); // no quick fix } } In all these cases, we wrongly showed a quick fix lightbulb -- and there are many more.
Verified using Build id: I20140515-1230 that the code snippet from comment 0 and comment 7 do not show bogus quick fix sign.
(In reply to Dani Megert from comment #6) > Note that your test cases pass without the fix. Would be good to have some > tests that are red before the fix and green afterwards. Added http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=df7fe5bb670aee8facb0cd636646bf2f102d7ae4 to prevent cases like bug 338785 comment 11. Needs to be revisited when we try to fix bug 437927.
From Manju: > The new test added "testNoFixFor_ParsingErrorInsertToComplete" pass with or without the fix from Sandra. I understood that the cases being tested are those for which there can't be any real quick fix possible. Hence is there a way we can test if the light bulbs are added for those invalid cases when a quick fix for a valid case is provided? e.g when Sandra's patch from bug 338785 is applied, light bulbs appear for the invalid cases, how to make the test fail in such a case? Thanks for raising that. I really intended the test to do that, but was tricked by our bad testing infrastructure. Added Javadoc warnings and fixed the test with http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=157d905b49ee6fa2cff213ebecddf82e47b3ee80
And finally the right fix that asserts the absence of the lightbulb also without the cheat that comment 10 inserted into the test CU source: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=b0ce2b48b7caf8f557aff6ae8372c9d50d076cbf
