Refactoring Verification


I posted a blog post from JavaOne where i stated:  “I did try out
several of [Netbean’s] refactorings, and was pleased with them for the
most part. 
However, i tried a rename refactoring that changed the meaning of my
code, and they did no verification of the rename to detect and warn me
about this.  This makes refactorings highly suspect and dangerous
and is definitely something that needs to be addressed asap.”



I then received a message from PDeva who says: “Could you give us an
example of the rename refactoring that changed the meaning of the code?
… just curious to know
whether i screwed up my code accidentally while using the netbeans
refactoring.”



PDeva: Absolutely.  Here we go.



Let’s start with the following simple code sample:


class C {
int a; //rename a --> b

void F() {
int b = 0;
int c = a;
}
}

Now, as you can tell, if you go ahead and just blindly rename all the
symbols that reference the ‘a’ field, you’re going to end up with a
problem.  Specifically:

class C {
int b;

void F() {
int b = 0;
int c = b; //uh-oh, 'b' now refers to the local, not the field
}
}

The is an example of a conflict that you need to detect and make the
user aware of.  Of course, this is addressable.  Refactoring
tools could be smart and instead change the code into:

class C {
int b;

void F() {
int b = 0;
int c = this.b; //Now everything is fine.
}
}

However,
we don’t provide this sort of workaround. Because of certain
architectural choices we made, it can make certain things difficult if
we change the number of tokens in a file in the middle of a
refactoring.  In the future i’d like for us to provide these sort
of “fixes” to allow the refactoring to continue while still preserving
the meaning of your code.  However, it’s not always the case that
there is a way to work around
such a conflict, and so it is important to tell the user about these
problems.



Why is it important? Well, if we look at Martin Fowler’s Refactoring Site, we’ll find the following information:

Q: What is Refactoring?

A: Refactoring is a disciplined technique for restructuring an existing
body of code, altering its internal structure without changing its
external behavior.

However, in the above examples if we blindly change ‘a’ to ‘b’ we’re
violate the requirement that external behavior is not changed. 



If you use Netbeans3.1 then you’ll the IDE willingly transform the code from:


into    Ack!

However, with Visual Studio and Eclipse you will get:

  or 



This will allow the user to know that the refactoring will most likely
not be doing what they were expecting and will ensure that they don’t
accidently change the meaning of their code.


Comments (6)

  1. Jeff says:

    I’m curious about "certain architectural choices " that "can make certain things difficult if we change the number of tokens in a file in the middle of a refactoring".

    This sounds like a fairly fundamental problem (obviously there is more to it) – can you shed any light on this?

  2. CyrusN says:

    Jeff, it’s no big deal, but it has to do with how we locate and update the tokens we need to rename. If we store their token indices before making the change, then it’s important not to have those indices change otherwise we’ll rename incorrectly. This is, of course, a solvable problem, but it’s currently what we’re faced with right now.

    When we started writing the refactoring this was an example of DoTheSimplestThingThatCouldPossibly work. At that time we never considered that a rename might actually do more than change one identifier to another. We’ve since rethought the rename refactoring so that it’s definition for us is: Renames a symbol and then updates all references to that symbol, doing what is necessary to keep the symantics the same, or notifying the user if that’s not possible. It’s the "doing what is necessary to keep the symantics the same" part that we realized late into the feature and at that point we’d made certain architectural choices (like i mentioned) that made that difficult.

    Hope that this helps!

  3. PDeva says:

    I am switching over to eclipse starting this moment 😉

  4. Radu Grigore says:

    It would be interesting if the rename operation would point out (possible) in-comment references to the old variable name.

    (I don’t know if it doesn’t already do it: I don’t have Express Edition close now and I’ll probably forget to post this by the time I get home to try)

  5. I wanted to write a post today about a very interesting bug we just came across related to Refactoring. …