Community
Participate
Working Groups
interface I { private static void foo() { // Error [1] } private default void bar() { // Error [2] } } [1] 'Remove invalid modifier' quick fix removes 'static' modifier also along with 'private'. [2] No quick fix is available. The quick fix should be updated to handle static and default interface methods at ModifierCorrectionSubProcessor.addRemoveInvalidModifiersProposal(..).
We also need to handle the case when there are illegal visibility modifier combination in Interface method. e.g. public abstract static void m1(){} public abstract default void m2(){} public default static void m3(){} All the above have illegal modifier combinations. For this a new problem id has to generated in core that can be similar to IllegalVisibilityModifierCombinationForInterfaceMethod. This will be handled in bug 400977 in JDT Core.
Created attachment 232685 [details] Patch with testcases. I have used the patch provided by Manoj from bug 400977 to complete this task. Provided valid quick fixes for invalid access modifier if used for static and default interface methods. No quick fix is provided for invalid modifier combination (IProblem#IllegalModifierCombinationForInterfaceMethod), the error marker informs user of the problem. Added testcases to cover the scenario. @Noopur: Kindly review and give an initial feedback.
Created attachment 235219 [details] Updated Patch with tests Created fresh patch from the newly created remote branch after eliminating duplicate changes.
Created attachment 241759 [details] Correct patch + tests Previous patch did not contain all the changes.
Review comments: - Is there an example for modifying the case for "IllegalModifierForInterfaceMethod" with Java 8 condition in ModifierCorrectionSubProcessor? I could not see a corresponding test case also. - It would be good to place the case for "IllegalModifierForInterfaceMethod18" near other cases for interfaces, like after "IllegalModifierForInterfaceMethod", in both the files. - Can we handle "IllegalStrictfpForAbstractInterfaceMethod" also? For example: strictfp void fun1(); strictfp abstract void fun2();
(In reply to Noopur Gupta from comment #5) > Review comments: (In reply to Noopur Gupta from comment #5) > Review comments: > > - Is there an example for modifying the case for > "IllegalModifierForInterfaceMethod" with Java 8 condition in > ModifierCorrectionSubProcessor? I could not see a corresponding test case > also. > This change was unnecessary, reverted it. > - It would be good to place the case for > "IllegalModifierForInterfaceMethod18" near other cases for interfaces, like > after "IllegalModifierForInterfaceMethod", in both the files. > done. > - Can we handle "IllegalStrictfpForAbstractInterfaceMethod" also? For > example: > strictfp void fun1(); > strictfp abstract void fun2(); Isn't the error message clear enough? "strictfp is not permitted for abstract interface method fun2". For this same reason currently there is no quick fix provided for IProblem#IllegalModifierCombinationForInterfaceMethod.
(In reply to Manju Mathew from comment #6) > > - Can we handle "IllegalStrictfpForAbstractInterfaceMethod" also? For > > example: > > strictfp void fun1(); > > strictfp abstract void fun2(); > > Isn't the error message clear enough? > "strictfp is not permitted for abstract interface method fun2". For this > same reason currently there is no quick fix provided for > IProblem#IllegalModifierCombinationForInterfaceMethod. I was suggesting to add the quick fix that will remove 'strictfp' as suggested by the error message that it is not permitted here. But we can leave it for now and add it later, if required.
Thanks Noopur. Released the fix as : http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=93033f2a57fd8b34d836f67639f3daed6b4e0f75
