vazexqi / CodingSpectator

Watches and analyzes code edits in the Eclipse IDE non-invasively
http://codingspectator.cs.illinois.edu
Other
20 stars 14 forks source link

Report refactoring bugs to Eclipse #146

Open reprogrammer opened 13 years ago

reprogrammer commented 13 years ago

We discover some bugs in the refactoring tools of Eclipse while working on CodingSpectator. We need to report to these bugs to the Eclipse bug tracking system. Some of these bugs might have already been reported. So, we need to search for related bug reports before submitting the bug. We will collect all the bugs that we find in the refactoring tools of Eclipse in this issue. We will describe each bug in a separate comment. And, whenever we report the bug to Eclipse or find an existing bug report for it, we will update the comment by putting the link to the bug report in the Eclipse bug tracking system.

reprogrammer commented 13 years ago

Inlining the main method doesn't generate a warning.

If you inline the main method, it deletes the main method without giving any errors or warnings.

reprogrammer commented 13 years ago

The Inline Method refactoring does not handle an erroneous case correctly.

If you inline the method m in the following program, the refactoring tool reports some errors but it lets the user continue without performing any changes. And, the refactoring gets added to the refactoring history.

import java.util.Random;

public class Main {

    public static void main(String[] args) {
        System.out.println("Hi");
        assert m();
    }

    public static boolean m() {
        System.out.println("m");
        return new Random().nextBoolean();
    }

}
reprogrammer commented 13 years ago

The extract method refactoring generates an error if a method with the same signature as the extracted method already exists. But, it still lets the user perform the erroneous refactoring and end up with compilation errors.

reprogrammer commented 13 years ago

The pull up refactoring on methods of inner classes could result in broken code

If you pull up the method m from C1 to D1 in the following piece of code, Eclipse will perform the refactoring but the resulting code will contain the compilation problem "No enclosing instance of the type C is accessible in scope". I believe the precondition checks should prevent or warn the user about performing this refactoring.

abstract class D {
    abstract void m();

    static class D1 {
    }
}

class C {
    void m() {
    }

    class C1 extends D.D1 {
        void m() {
            C.this.m();
        }
    }
}

@reprogrammer reported the bug to Eclipse on 7/6/2011 (See Bug 351383).

reprogrammer commented 13 years ago

Moving a method from an interface to a class generates a compilation error.

If you move a method of an interface to a class, the refactoring will be performed but the result will have a compile error.

reprogrammer commented 13 years ago

The Move refactoring doesn't handle a name conflict properly.

If you move a Java class to a package that has a class with the same name, Eclipse pretends to perform the refactoring but it doesn't remove the original class. See issue #111 for more information.

reprogrammer commented 13 years ago

The configurations of refactorings performed via Quick Assist are not saved correctly.

If you extract a method using quick assist, it will be serialized with the default name "extracted" regardless of what name you choose. We noticed that this bug is not specific to extract method. Invoking other refactorings such as extract local variable through quick assist exhibits the same bug.

reprogrammer commented 13 years ago

UDC does not capture the refactorings performed via Quick Assist correctly.

UDC captures the invocation of quick assist but not the refactorings that are performed through the quick assist, e.g. Extract Method.

reprogrammer commented 13 years ago

The pull up refactoring throws an NPE when pulling up a member that already exists in the superclass

The Pull Up Refactoring has some problems when the same method is defined in both the parent class and the child class. Here is an example:

class Parent {
     public void m() {}
}

class Child extends Parent {
    public void m() {}
}

If the user selects the method m() in the child class and tries to pull it up, Eclipse throws an exception (no warnings).

On 6/30/2011, @reprogrammer reported this bug to Eclipse (See Bug 350885).

rajkuma1 commented 13 years ago

The Move Compilation Unit refactoring fails when the destination does not exist.

Move Refactoring is buggy. The issue is that Eclipse throws an exception when you perform the following steps:

  1. Select a java source file.
  2. On the input page that pops up for the destination for moving the compilation unit, type a letter, say, "x" assuming that there is no folder whose name starts with "x".

Once you hit ok, the following exception is thrown and the IDE shows a dialog that an unexpected error has occurred. Here's the link to the exception stack trace.

http://pastebin.com/5nbejRmA

Wanderer777 commented 13 years ago

Eclipse fails to rename a method in a Java project that transitively depends on the project that declares this method.

If each of the following three classes is in a separate Java project, let's say A is in projectA, B is in projectB, C is in projectC, and projectB depends on projectA, projectC depends on projectB, then renaming method A.m would correctly update its name in classes A and B, but would not affect class C, thus leading to a compilation error.

public class A {
    public void m() {   }
}

public class B extends A {
    @Override
    public void m() {   }
}

public class C {
    public void someMethod() {
        new B().m();
    }
}

It is not clear whether this is a bug or a feature, but Eclipse looks only for immediate dependencies while performing refactorings. If projectC had a direct dependency on projectA, then rename refactoring would correctly update class C as well.

On 7/07/2011, @Wanderer777 reported this bug to Eclipse (See Bug 351506).

reprogrammer commented 13 years ago

Eclipse doesn't log a move refactoring

If you select the method m2 in the following program and perform a move refactoring ("Refactor -> Move...") on it to move it to class C2, Eclipse will perform the refactoring but it won't log it in .metadata/.plugins/org.eclipse.ltk.core.refactoring and the history dialog at "Refactor -> History..." will be empty.

//The Java file is named C.java
class C1 {
    void m2(C2 c2) {
    }
}

class C2 {
}

@reprogrammer reported the bug to Eclipse on 7/6/2011 (See Bug 351381).

rajkuma1 commented 13 years ago

Eclipse doesn't move multiple fields at once.

926650a78f19f50d59e1fbd16d930176483ede35 brings out a bug in eclipse. When you select multiple fields, only the first field in the textual order is moved.

public class C {
    private int i;
    private int j;
}

class C2 {

}

If you select both i and j in C and move it to C2, only i gets moved.

rajkuma1 commented 13 years ago

The Move Type to New File refactoring generates redundant files.

I noticed this bug in eclipse while working on issue #200,

Consider this code in a file called C.java

public class C {
}

class D {
}

Now, select D and invoke the refactoring Move Type to New File.... This will bring up a dialog. Click OK to dismiss the dialog. Now, you'll notice 2 java files under the package explorer with the same content:

  1. One under the default package.
  2. One under the src folder outside the default package.

This is a bug. There should be only one file under the default package with name D.java.

The reason why this is happening is because the dialog box that appears allows you to create a file under <projectname?\src. There should be a radio button. You can move the source to only one file.

What's even confusing is, when the destination appears, if you uncheck this option to Create 'D.java' - P\src, the whole source code will disappear. This is a bug on the version where CodingSpectator is installed.

On the version where CodingSpectator is not installed, the class is properly created under D.java under the default package. However, the problems view is updated with a non-issue:

Description Resource Path Location Type The type D is already defined D.java /P/src Unknown Java Problem

This is a problem with eclipse.

vazexqi commented 13 years ago

Incorrect selection for the descriptor of inline local variable (quick assist)

Consider this file


public class Simple {
    public static void main(String[] args) {
        String string = "hello";
        System.out.println(string);
    }
}

Steps to reproduce:

  1. Position your cursor in the string local variable (anywhere would do)
  2. Invoke the inline refactoring through quick assist (Ctrl+1)
  3. Check the refactoring descriptor captured by Eclipse.
  4. Notice that the selection is reported as 'string = "Hello"' even though nothing was actually selected in the program.

This is the behavior of Eclipse.

reprogrammer commented 13 years ago

Eclipse logs rename compilation unit refactorings as rename type refactorings

This bug report comes from issue #185.

To reproduce this problem, perform the following steps.

  1. Install a fresh instance of Eclipse Helios SR2 and perform the following steps.
  2. Create a Java project with a Java file called C.java that has the following contents.
public class C {
}
  1. Select C.java from the package explorer view and invoke the rename refactoring. As a result, the "Rename Compilation Unit" dialog will show up.
  2. Apply the refactoring.
  3. If you inspect the refactoring history, you'll notice that Eclipse has captured the refactoring as a rename type refactoring even though the dialog is titled "Rename Compilation Unit".

Note that if you select the class name (C) from the editor or the package explorer view and perform the rename refactoring, a dialog with title "Rename Type" will show up and Eclipse will serialize the refactoring as a rename type refactoring. I didn't find any difference between the results of rename compilation unit and rename type refactorings. So, this bug is perhaps a minor one.

reprogrammer commented 12 years ago

The Change Method Signature refactoring does not get recorded in refactoring histories correctly.

The refactoring descriptor of Change Method Signature captured in refactorings.history doesn't capture the change in the order and name of method parameters (See Bug 365010).