Bugzilla – Full Text Bug Listing
| Summary: | [extract method] Extracting expression of parameterized type that is passed as argument to this constructor yields compilation error | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Milos Gligoric <milos.gligoric> | ||||||
| Component: | UI | Assignee: | Samrat Dhillon <samrat.dhillon> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | daniel_megert, manju656, markus.kell.r, samrat.dhillon | ||||||
| Version: | 4.2.1 | Flags: | manju656:
review+
|
||||||
| Target Milestone: | 4.4 M3 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Linux | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
Issue is reproducible using I20121210-0800. The refactoring results in compiler error. Created attachment 236344 [details] Patch with fix and tests This patch aims to fix two problems 1. In case we are creating a static method, we should add the type parameters of the declaring class. 2. Handle wild card bounds. The wildcard problem can been seen from the snippet below public class A{ public <E> void foo() { List<? extends E> t = new ArrayList<E>(); //extract method on t.size() leads to compilation failure t.size(); } } The extracted method has no type variables. I have added tests for both the cases. This contribution complies with http://www.eclipse.org/legal/CoO.php Manju, please review once you have some spare time during the Java 8 work. Find below my review comments: 1. If you notice carefully, the tests are grouped according to the functionality being tested. In this case 'genericTest()' are written between test1100 and test1120. So the new tests you have added should follow this numbering system. 2. Use Modifier.isStatic(int flags) to determine whether the modifier has static bit set. 3. Adopt to <if.. else if> coding format. ExtractMethodRefactoring #processVariable() the new code added should be in else if, similarly the inner multiple if's also should be in else if's instead of independent if's or you can combine the inner if's to a single if and use || operator as the statement within both if's are the same. 4. In ExtractMethodRefactoring #processVariable() extract arg.getBound() and decl.getParent() to a local variable as it is computed many times. 5. Update contributors list in ExtractMethodTests and ExtractMethodRefactoring. Also pull changes before submitting the next patch. Thanks. Created attachment 236550 [details] updated patch I have updated the name of the tests. Also using Modifier helper methods. Using <if.. else if> now. I am using temporary variable to store the value from arg.getBound() but not for decl.getParent() as decl.getParent will get executed just once from any code path. Also in the new if blocks that I added I was missing null check. I have added null check now. I made two contributions to ExtractMethodRefactoring i.e. fix for Bug 393098 and this one. This contribution was made before Bug 393098, but changes for Bug 393098 were accepted before this one. If changes are not applied/accepted in the chronological order in which they are made, it will lead to problem. When I submitted this patch I was working on the latest code base. If you look at the patch file I initially attached, I did update the contribution list, but since Bug 393098 was applied before this one, when you applied this patch, the contribution list did not get updated. The above contribution complies with http://www.eclipse.org/legal/CoO.php Released the patch with minor changes as: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=ba5280deb3a074bf0d41687d395a2def630f9608 I will try my best to take up reviews in the order in which they are submitted. But a patch must be created against the master at any given time and that means parts of changes of another bug which is not yet in master should never be part of a patch, until unless Bug B is dependent on Bug A in which case the review of Bug A must be completed before Bug B. You can adopt any of the below approach: 1) You already started working on Bug B when you receive the review comments for Bug A. Create a WIP patch for Bug B, revert the file changes, apply Bug A patch, complete it, upload the patch. Now revert file changes of Bug A and apply Bug B patch and continue work on Bug B. 2) Create local branches and use different branches to work on multiple bugs. Refer http://wiki.eclipse.org/EGit/User_Guide#Local_Branches Before creating a patch always compare the files against master and make sure the changes are applicable only for the bug against which the patch will be created. |
Steps to reproduce: 1. Invoke "Extract Method" refactoring on "c.size()" expression in the example below. 2. Resulting file does not compile ("Cannot make a static reference to the non-static type V"). The problem is that the extracted method must be static because it is used as argument to "this" and at the same time it needs type parameter "V" which is non-static. public class ExtractMethodBug<V> { public ExtractMethodBug(Map<?, ? extends V> c) { // Invoke "Extract Method" on "c.size()" this(c.size()); } public ExtractMethodBug(int size) { } } (Thanks to Anirudh Balagopal for helping with this bug report.)