Community
Participate
Working Groups
Build Identifier: 20110916-0149 The input dialog of the Extract Method refactorings lets users change the order of the parameters of the extracted method. However, the descriptor of the Extract Method refactoring doesn't capture the reordering of method parameters. As a result, the refactoring cannot be later replayed correctly. Reproducible: Always Steps to Reproduce: 1. Add class "C" with the following contents to a Java project: ---- public class C { public static void main(String[] args) { String a = "a"; String b = "b"; System.out.println(a + b); } } ---- 2. Select "System.out.println(a + b);" and invoke the Extract Method refactoring from the Refactor menu. 3. Set the name of the new method to "m" and reorder the parameters of the method. Then, hit the OK button. 4. The resulting code should look like the following: ---- public class C { public static void main(String[] args) { String a = "a"; String b = "b"; m(b, a); } private static void m(String b, String a) { System.out.println(a + b); } } ---- The contents of ".metadata/.plugins/org.eclipse.ltk.core.refactoring/.refactorings/P/*/*/*/refactorings.history" should look like the following: ---- <refactoring comment="Extract method 'private static void m(String b, String a)' from 'C.main()' to 'C' - Original project: 'P' - Method name: 'm' - Destination type: 'C' - Declared visibility: 'private'" comments="false" description="Extract method 'm'" destination="0" exceptions="false" flags="786434" id="org.eclipse.jdt.ui.extract.method" input="/src<{C.java" name="m" replace="false" selection="98 26" stamp="1322678132297" version="1.0" visibility="2"/> ---- 5. Create a script from the Extract Method refactoring by going to "Refactor -> Create Script...". 6. Undo the Extract Method refactoring. 7. Apply the refactoring script you created in step 5. The resulting code will look like the following: ---- public class C { public static void main(String[] args) { String a = "a"; String b = "b"; m(a, b); } private static void m(String a, String b) { System.out.println(a + b); } } ---- Note that the order of the parameters of method "m" in steps 4 and 7 are different. The problem is that the refactoring descriptor shown in step 4 did not capture the reordering of parameters on the Extract Method input dialog.
CodingSpectator <http://codingspectator.cs.illinois.edu/> adds a few pieces of information to the refactoring descriptors captured by Eclipse to help us better understand how programmers use the refactoring tool. While working on CodingSpectator, we decided to improve the descriptor of the Extract Method refactoring to capture the reordering of method parameters. Nicholas Chen and Balaji Ambresh Rajkumar included the ordering of parameters in the descriptor of the Extract Method refactoring. Please see their patch at <codingspectator.cs.illinois.edu/github/commit/cd63360f1005d5c935e9ac15a51661a1fdfc90e2>.
(In reply to comment #1) > While working on CodingSpectator, we decided to improve the descriptor of the > Extract Method refactoring to capture the reordering of method parameters. > Nicholas Chen and Balaji Ambresh Rajkumar included the ordering of parameters > in the descriptor of the Extract Method refactoring. Please see their patch at > <codingspectator.cs.illinois.edu/github/commit/cd63360f1005d5c935e9ac15a51661a1fdfc90e2>. I took a quick look at the commit mentioned above, and it does not look to be on the latest source from JDT/UI. Can you please attach a patch here against the latest JDT/UI source? You may also find the info on the following wiki page useful -http://wiki.eclipse.org/Platform_UI/How_to_Contribute#Coding_Conventions
(In reply to comment #2) > I took a quick look at the commit mentioned above, and it does not look to be > on the latest source from JDT/UI. Can you please attach a patch here against > the latest JDT/UI source? Could you please review my commit at <https://github.com/reprogrammer/eclipse.jdt.ui/commit/201193dad6feaf7c2ca55e09ef91d1b2c2aec239>?
(In reply to comment #3) > (In reply to comment #2) > > I took a quick look at the commit mentioned above, and it does not look to be > > on the latest source from JDT/UI. Can you please attach a patch here against > > the latest JDT/UI source? > > Could you please review my commit at > <https://github.com/reprogrammer/eclipse.jdt.ui/commit/201193dad6feaf7c2ca55e09ef91d1b2c2aec239>? For legal reasons please attach a patch here. We don't go to github.
Created attachment 208095 [details] Patch of the commit at https://github.com/reprogrammer/eclipse.jdt.ui/commit/201193dad6feaf7c2ca55e09ef91d1b2c2aec239
(In reply to comment #4) > For legal reasons please attach a patch here. We don't go to github. I was following instructions at <http://wiki.eclipse.org/Development_Resources/Handling_Git_Contributions>. Doesn't JDT UI accept Git contributions? Anyways, I created attachment 208095 [details] for the patch.
(In reply to comment #6) > (In reply to comment #4) > > For legal reasons please attach a patch here. We don't go to github. > > I was following instructions at > <http://wiki.eclipse.org/Development_Resources/Handling_Git_Contributions>. > Doesn't JDT UI accept Git contributions? Nope. We prefer the patch until the foundation has setup a Gerrit server.
Another alternative is remove the widget that lets the user reorder the parameters of the extracted method. Removing this widget will simplify the configuration dialog. I'll analyze CodingSpectator data to see how often our participants have reordered the parameters of extracted methods using this widget.
(In reply to comment #8) > Another alternative is remove the widget that lets the user reorder the > parameters of the extracted method. Removing this widget will simplify the > configuration dialog. I'll analyze CodingSpectator data to see how often our > participants have reordered the parameters of extracted methods using this > widget. Unfortunately, CodingSpectator only records the final ordering of parameters of the extracted methods. In other words, it isn't easy to tell if a programmers has reordered the parameters on the Extract Method dialog. However, our participants generally preferred less configuration and simpler refactoring dialogs. Therefore, it might be better to just remove the widget that lets the user reorder the parameters of the extracted method. Programmers can still use the Change Method Signature refactoring to reorder the parameters of the extracted method.
(In reply to comment #9) > Therefore, it might be better to just remove the widget > that lets the user reorder the parameters of the extracted method. Programmers > can still use the Change Method Signature refactoring to reorder the parameters > of the extracted method. I don't think that is a good idea, as you need to then invoke an additional refactoring. Even I sometimes reorder parameters in Extract Method refactoring dialog. I looked at the patch, and functionality wise it is mostly Ok. However, things fail when the refactoring script is applied after one or more variables (passed as arguments to the new method) have been renamed. Example: Create a extract method script with this snippet... public static void main(String[] args) { String a = "a"; String b = "b"; System.out.println(a + b); } ... and then apply to this snippet public static void main(String[] args) { String x = "a"; String y = "b"; System.out.println(x + y); } You get the following NPE java.lang.NullPointerException at org.eclipse.jdt.internal.corext.dom.ASTNodes.findVariableDeclaration(ASTNodes.java:250) at org.eclipse.jdt.internal.corext.refactoring.code.ExtractMethodRefactoring.getVariableDeclaration(ExtractMethodRefactoring.java:1155) at org.eclipse.jdt.internal.corext.refactoring.code.ExtractMethodRefactoring.createNewMethodDeclaration(ExtractMethodRefactoring.java:1025) at org.eclipse.jdt.internal.corext.refactoring.code.ExtractMethodRefactoring.getSignature(ExtractMethodRefactoring.java:719) at org.eclipse.jdt.internal.corext.refactoring.code.ExtractMethodRefactoring.getSignature(ExtractMethodRefactoring.java:709) at org.eclipse.jdt.internal.corext.refactoring.code.ExtractMethodRefactoring.getRefactoringDescriptor(ExtractMethodRefactoring.java:665) at org.eclipse.jdt.internal.corext.refactoring.code.ExtractMethodRefactoring.createChange(ExtractMethodRefactoring.java:498) at org.eclipse.ltk.core.refactoring.CreateChangeOperation.run(CreateChangeOperation.java:132) at org.eclipse.ltk.ui.refactoring.history.RefactoringHistoryWizard.createChange(RefactoringHistoryWizard.java:532) at org.eclipse.ltk.ui.refactoring.history.RefactoringHistoryWizard.access$4(RefactoringHistoryWizard.java:528) at org.eclipse.ltk.ui.refactoring.history.RefactoringHistoryWizard$7.run(RefactoringHistoryWizard.java:885) at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:121) I think the right solution would be to also record whether the user has made any changes to the order or parameter just like Change Method Signature refactoring. There we store what was the original index of the parameter and the final index, i.e. the same thing you allude to in your comment. (In reply to comment #9) > Unfortunately, CodingSpectator only records the final ordering of parameters of > the extracted methods. In other words, it isn't easy to tell if a programmers > has reordered the parameters on the Extract Method dialog. Also in future patches fill out this template and add it to the header comment of each changed file: "Your Name <email@example.com> - Bug Title - https://bugs.eclipse.org/BUG_NUMBER"
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
