Community
Participate
Working Groups
Basically, ExtractLocalVariable ignores relevant type guards and side-effects when computing where to move the extracted expression. Each of the three "main()" methods below provides a distinct test-case. All of them fail under version 2.1M4. I have a partial patch for the type-guard case (partial because the already-existing createAndInsertTempDeclaration() isn't prepared to create a block to surround the extracted temp variable declaration statement, so I still have to fix that). Also, I'm in the process of creating a proper testcase for inclusion in the regression suite in org.eclipse.jdt.ui.tests.refactoring. How should I go about submitting these patches? The side-effect case, on the other hand, requires some flow analysis (computing and examining use/def chains) which I'm guessing someone has already done somewhere inside the JDT. public class ExtractVariableTest { static public void main(String[] args) { Object x = System.getProperty("foo"); if (x instanceof String) System.out.println(((String) x).substring(0, 5)); // Select: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ // and invoke Refactor->Extract Local Variable // Expression will be moved outside the type guard! } static public void main2(String[] args) { Object x; if ((x = System.getProperty("foo")) != null) System.out.println(x.toString()); // Select: ^^^^^^^^^^^^ // and invoke Refactor->Extract Local Variable // Expression will be moved above the assignment to x! } static public void main3(String[] args) { Object x; System.out.println(((x = foo()) != null) ? x.toString() : ""); // Select: ^^^^^^^^^^^^ // and invoke Refactor->Extract Local Variable // Expression will be moved above the assignment to x! } static public Object foo() { return null; } }
After looking at this more closely, it's a more widespread issue than I first thought. (a) it effects all control constructs, not just "if" statements, and (b) the nature of the control condition doesn't matter (i.e. it need not be a type guard or something with side-effects). Here are 2 more testcases: static public void main(String[] args) { String x = System.getProperty("foo"); if (x != null) System.out.println(x.substring(0, 5)); // Select: ^^^^^^^^^^^^^^^^^ // and invoke Refactor->Extract Local Variable // Expression will be moved outside the null guard! } static public void main0(String[] args) { String x = System.getProperty("foo"); while (x.length() > 0) x = x.substring(0, x.length()-1); // Select: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ // and invoke Refactor->Extract Local Variable // Expression will be hoisted outside the while loop! }
extract temp is implemented in a simple-minded way that cuts the expression and pastes it in an initializer that is inserted before the first occurrence of the expression no real analysis is performed we can work on it for 2.2
*** Bug 28243 has been marked as a duplicate of this bug. ***
*** Bug 24073 has been marked as a duplicate of this bug. ***
*** Bug 8149 has been marked as a duplicate of this bug. ***
Bob, i heard you did some work on this - is that true?
Yes, I've got a partial fix for the problem. I wrote it against 2.1M4 source, along with other things, so I'm in the process of "porting" it forward to the 2.1 release source. I'll be doing this in the next few days.
The presence of a block makes the difference. This fails: if ( false ) someExpression This works as expected: if ( false ) { someExpression }
Bob, do you still have that code you mentioned in comment 7? :-)
It is clear now that this problem is a matter of if there is a block surrounding the conditionnal flow of not Could it just be simple to handle this special case (where there is no block) and to create a block surrounding it have to handle <code>if</code> <code>else</code> <code>while</code> <code>for</code> <code>do</code> ...<code>while</code> and may be some that i forget Could it be targeted to 3.0
The fix for bug 48231 has solved the problems with conditionals (comment 8 & 10). *** This bug has been marked as a duplicate of 27740 ***
