Bugzilla – Full Text Bug Listing
| Summary: | [1.8][extract method] Extract Method refactoring in interfaces not handled | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Noopur Gupta <noopur_gupta> | ||||||||
| Component: | UI | Assignee: | Noopur Gupta <noopur_gupta> | ||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P3 | CC: | daniel_megert, manju656, markus.kell.r, noopur_gupta, timo.kinnunen | ||||||||
| Version: | 4.3 | Flags: | markus.kell.r:
review+
|
||||||||
| Target Milestone: | BETA J8 | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Bug Depends on: | 413592 | ||||||||||
| Bug Blocks: | 405305, 407985 | ||||||||||
| Attachments: |
|
||||||||||
*** Bug 411608 has been marked as a duplicate of this bug. *** If the method is being extracted from a default method, then the extracted method will be default; otherwise it will be static. Some open points regarding the expected behavior: 1. Access Modifier radio buttons: If Destination type while opening the wizard is an interface, should 'public' be selected by default with all radio buttons disabled, or should we hide the access modifier composite itself? And, when user changes the Destination type from an interface to non-interface and vice-versa, the composite would need to be updated accordingly. 2. When the expr being extracted from a default method, is also present in a static method and "Replace additional occurrences..." is checked, we get compilation error after refactoring since the new default method cannot be accessed from the static method. The same issue already happens in a class also. 3. Currently if we try to extract a method from a field initialization expr in an interface for source level < 1.8, we get NPE. We can add a check and show error message "Cannot extract to an interface." in this case. Markus, please share your thoughts. > 1. Access Modifier radio buttons: We should keep the access modifier composite, but disable the modifiers that are not allowed for the selected destination. If the destination is an interface, then the extracted method should have the same keyword modifiers as the source method (i.e. if the original doesn't have a 'public' keyword, then the extracted method also shouldn't have it, although 'public' was selected in the UI). > 2. When the expr being extracted from a default method, ... Yeah, that's bug 393098. The "default method" variant can also be handled there. > 3. Currently if we try to extract a method from a field initialization expr > in an interface for source level < 1.8, we get NPE. > We can add a check and show error message "Cannot extract to an interface." > in this case. Yes, please do that. Created attachment 234655 [details] Patch + Tests Patch contains the following major changes: - ExtractMethodInputPage.java: * If destination is an interface, all access modifiers in the wizard should be disabled and 'public' should be selected. The access modifier setting for next invocation of the wizard should not be changed by the earlier forced 'public' for interface. --------------------------------- - ExtractMethodRefactoring.java * createChange(..) : Updated since the first type in destinations list need not be the direct parent type of code being extracted. Example: class C { @interface A { int i= /*[*/0;/*]*/ } } * initializeDestinations() : Updated so that combo displays only valid destination types. * createNewMethodDeclaration() : - If the user extracts an expression from a method into the directly enclosing interface, the extracted method has 'public' iff the enclosing method also was public. - New method will be 'static' if any of its ancestor nodes up to the destination type (excluding the destination) is static. In case of interface as destination, if method is not 'static' then it will be 'default'. --------------------------------- - ExtractMethodAnalyzer.java: * isValidDestination(ASTNode) : To check for interfaces < 1.8 and annotation types. * checkInitialConditions(ImportRewrite) : check if any valid destination type exists for extraction, otherwise return with error. * checkInput(RefactoringStatus, String, ASTNode) : Existing impl ignored the methods from direct super-interfaces hierarchies of destination type, added check for that. And added null check for super-class in interface. --------------------------------- - Checks.java: * checkMethodInType(ITypeBinding, String, ITypeBinding[]) : Updated as constructor warning is already added and it is not an 'error' to have a method with same name as constructor. * checkMethodInHierarchy(ITypeBinding, String, ITypeBinding, ITypeBinding[]) : Added a different explicit warning message for constructors. --------------------------------- - ASTResolving.java: * Updated javadocs of #findParentType(..) methods. --------------------------------- - TODO: * Update test cases with formatted default methods (instead of the unformatted ones currently), once bug 413592 is fixed. --------------------------------- - This patch also contains changes from bug 403924 in ExtractMethodRefactoring.java. Markus, please have a look. Created attachment 236008 [details] Patch + Tests (In reply to Noopur Gupta from comment #4) > - TODO: > * Update test cases with formatted default methods (instead of the > unformatted ones currently), once bug 413592 is fixed. Updated the tests with formatted default methods. *** Bug 425759 has been marked as a duplicate of this bug. *** Created attachment 239048 [details] Patch + Tests (In reply to Markus Keller from comment #3) > > 2. When the expr being extracted from a default method, ... > > Yeah, that's bug 393098. The "default method" variant can also be handled > there. This patch handles the case when code is extracted from default method and the duplicate code is present in static method also. The fix was given in bug 393098, i have merged the changes from master and added a testcase. The patch is checked in to mmathew/BETA_JAVA8 branch. Released all necessary commits to BETA_JAVA8, see http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=c656ba52079370b803406e5b65ffe3b2c3f6e8f6 and 4 parent commits. Verified using Kepler SR2 + Java 8 RC1 + Eclipse Java Development Tools Patch for Java 8 Support (BETA) 1.0.0.v20140220-2054 |
Consider the following interfaces: I2 (having lambda expression) and I3 (having default and static methods). Perform "Extract Method" refactoring on the lines with comments. We get exception and no refactoring is performed. It should be possible to extract methods in interfaces which would be created as default or static methods in those interfaces. @FunctionalInterface public interface I1 { int foo(int a); } interface I2 { I1 i1= (a) -> { int b= 10; // Exception on extracting to method return a + b; }; } interface I3 { default int foo () { int a= 10; // Exception on extracting to method return a; } static int bar() { int a= 10; // Exception on extracting to method return a; } }