Description Luke Hutchison 2005-12-02 17:58:59 EST

Initial code:

if (x) {
    d0.add(new Vector<Integer>());
    d1.add(new Vector<Integer>());
}

Select first occurrence of "new Vector<Integer>()" and replace with local variable a0, with the box checked to replace all instances of the expression with the variable:

Vector<Integer> a0 = new Vector<Integer>();
if (x) {
  d0.add(a0);
  d1.add(a0);
}

This is of course never going to be semantically correct.

The refactoring engine can't in general know if the given expression will have side effects like this.  However this is a fairly prominent case to catch as an exception -- if the expression contains a "new", then the checkbox to replace all instances should be greyed-out.  I almost didn't notice that the refactorer did this, because it has always worked for every expression I tried before.  

Perhaps there needs to be a warning displayed in the dialog box if there is a method call in the expression and more than one instance of the expression is being replaced, which says that it is the programmer's responsibility to ensure that the function has no side effects.

In general, I think it would be very valuable for JDT to explicitly know when a method has side effects and when it doesn't.  This may be uncomputable in the general case, but it is certainly possible to prove when a function *doesn't* have side-effects.