Bug 365820

Description Mohsen Vakilian 2011-12-06 18:40:05 EST

Build Identifier: 20110916-0149

CodingSpectator <http://codingspectator.cs.illinois.edu/> records the problems that the Eclipse refactoring tool reports to programmers. This report is based on our study of 26 programmers using CodingSpectator.

Seven of our participants got the following the error message for a total of 17 times while using the Extract Method refactoring tool of Eclipse.

>Selected statements contain a return statement but not all possible execution flows end in a return. Semantics may not be preserved if you proceed.

This error message was the second most frequent problem that the Extract Method refactoring reported to our participants. This error message accounts for 29% of the problems that the Extract Method refactoring reported to our participants. When Eclipse reports the above error message, it still lets the user perform the refactoring, but, the resulting code may not compile. Some of our participants dismissed the error message and performed it. Others canceled the refactoring once or more but finally performed it. In other words, there were several attempts to perform some of the refactorings. Overall, there were 11 batches of independent refactoring events that reported the above error message. In six batches, our participants canceled the refactoring the first time it reported the above error message. However, in only two cases the participants were able to change their code and selection to avoid the error message. In all other nine batches, our participants ended up performing the refactoring in spite of the error message. So, it was a good choice to let the programmer continue the refactoring in spite of the error message.

Five of the eleven cases that our participants ran into and got the above error message were similar to the following example:

----
public class C {
  public boolean m(boolean condition) {
    // START SELECTION
    if (condition)
      return true;
    else
      System.out.println("else");
    // END SELECTION - Invoke Extract Method
    return false;
  }
}
----

In the above example, the Extract Method refactoring reports an error because the "else" branch does not have a "return" statement and generates the following code with compilation problems:

----
public class C {
  public boolean m(boolean condition) {
    n(condition);
    return false;
  }
 
  private void n(boolean condition) {
    if (condition)
      return true;
    else
      System.out.println("else");
  }
}
----

However, because all of the "return" statements in the selected piece of code return the same "boolean" literal, i.e. "true," the refactoring tool could automatically perform the following changes to avoid any compilation problems:

1. Change the return type of the extracted method to "boolean".
2. Insert a "return false;" statement as the last statement of the extracted method.
3. Check the return value of the extracted method at its call site and return to preserve the control flow.

----
public class C {
  public boolean m(boolean condition) {
    if (n(condition) == true) {
      return true;
    }
    return false;
  }
 
  private boolean n(boolean condition) {
    if (condition)
      return true;
    else
      System.out.println("else");
    return false;
  }
}
----

Two of the eleven cases that our participants ran into were similar to the following example:

----
public class C {
  public void m(boolean condition) {
    // START SELECTION
    if (condition)
      return;
    else
      System.out.println("else");
    // END SELECTION - Invoke Extract Method
  }
}
----

Again, the Extract Method refactoring reports an error in the above example because the "else" branch does not have a "return" statement and generates the following code that doesn't preserve the control flow of the original program:

----
public class C {
  public void m(boolean condition) {
    n(condition);
  }
 
  private void n(boolean condition)) {
    if (condition)
      return;
    else
      System.out.println("else");
  }
}
----

However, the refactoring tool could change the "return;" statement inside the extracted method to return a specific value in order to preserve the control flow of the original method. The following shows how the refactoring tool could extract the selected statements without changing the control flow of the program.

----
public class C {
  public void m(boolean condition) {
    if (n(condition) == true) {
      return;
    }
  }
 
  private boolean n(boolean condition) {
    if (condition)
      return true;
    else
      System.out.println("else");
    return false;
  }
}
----

In summary, there are two techniques to avoid the above error message in most cases (seven out of eleven):

1. When all "return" statements in the selected block return some literal values, add a "return" statement at the end of the extracted method that returns a different literal. This different literal will help the tool determine the condition for returning out of the caller.
2. If the selected statements contain "return;" statements, change the return type of the extracted method to "boolean", replace "return;" by "return true", add "return false" to the end of the extracted method, and return from the caller when the return value of the callee is "true".

I think that the above two techniques will reduce the frequency of the error message and make the Extract Method refactoring tool easier to use. Do you think it's a good idea to implement these techniques in Eclipse?