netbeans icon indicating copy to clipboard operation
netbeans copied to clipboard

[Pull Down Method Refactoring Bug] Pull down method refactoring leads to changes in code calling relationships

Open assdfsdafasfa opened this issue 1 year ago • 1 comments

Apache NetBeans version

Apache NetBeans 23

What happened

When test() is selected to perform pull down refactoring into class A, before refactoring, test() method calls method() of class ParentClass, after refactoring, test() method calls method() of class SubClass, resulting in a change in code behavior.

Code before refactoring

class ParentClass {
void method() {
 System.out.println("ParentClass method");
 }
}
class A extends SubClass { 
void method() {
 System.out.println("Class A method");
 }
}
class SubClass extends ParentClass {
void test() {
 method(); 
 }
}

Language / Project Type / NetBeans Component

No response

How to reproduce

When test() is selected to perform pull down refactoring into class A, before refactoring, test() method calls method() of class ParentClass, after refactoring, test() method calls method() of class SubClass, resulting in a change in code behavior.

Code before refactoring

class ParentClass {
void method() {
 System.out.println("ParentClass method");
 }
}
class A extends SubClass { 
void method() {
 System.out.println("Class A method");
 }
}
class SubClass extends ParentClass {
void test() {
 method(); 
 }
}

Did this work correctly in an earlier version?

No / Don't know

Operating System

Windows11

JDK

20

Apache NetBeans packaging

Apache NetBeans provided installer

Anything else

Code after refactoring

class ParentClass {
void method() {
 System.out.println("ParentClass method");
 }
}
class A extends SubClass { 
void method() {
 System.out.println("Class A method");
 }
void test() {
 method(); 
 }
}
class SubClass extends ParentClass {
}

Are you willing to submit a pull request?

No

assdfsdafasfa avatar Sep 26 '24 10:09 assdfsdafasfa

In my first analysis, I misread pullup instead of push down.

What NetBeans refactoring does is pull the element(s) up to the nearest super, which can be a class, an abstract class, or interface. In the later, and optionally in the penultimate (the abstract class) the member (method) can be made abstract.

In your case the Hierarchy is

classDiagram
   SuperClass <|-- SubClass
   SubClass <|-- A
   SubClass: + test()

Pulling up member test from Subclass will and should result in

classDiagram
   SuperClass <|-- SubClass
   SubClass <|-- A
   SuperClass: + test()

This implies that as far as I am concerned, there is nothing wrong. Tested on NetBeans 26, jdk 21, ubuntu LTS 24.04

For Pushdown the conclusion is similar. The test method now lands in class A, like so

class A extends SubClass {

   @Override
   void method() {
       System.out.println("Class A method");
   }

   void test() {
       method();
   }
}

This looks like what needs to be done by this refactoring.

As class diagram

classDiagram
   SuperClass <|-- SubClass
   SubClass <|-- A
   A: + test()

homberghp avatar Jun 16 '25 18:06 homberghp

@homberghp I had a look at your PR and I don't think that the test address the point opened here. In fact I think this is not a bug. The provided code is incomplete so I took the code and made it executable:

public class TestX {

    static class ParentClass {

        void method() {
            System.out.println("ParentClass method");
        }
    }

    static class A extends SubClass {

        void method() {
            System.out.println("Class A method");
        }
    }

    static class SubClass extends ParentClass {

        void test() {
            method();
        }
    }

    public static void main(String[] argv) {
        A a = new A();
        a.test();
    }
}

now @assdfsdafasfa writes:

before refactoring, test() method calls method() of class ParentClass, after refactoring, test() method calls method() of class SubClass

this is a wrong assumption. As method is overriden in A calls to method invoked on an instance of A will always yield Class A method. It does not matter that method is called via SubClass#test.

Running the above code yields:

--- exec:1.5.0:exec (default-cli) @ MavenSpielwiese ---
Class A method

Running "Push down" refactoring yields (as described in the issue):

public class TestX {

    static class ParentClass {

        void method() {
            System.out.println("ParentClass method");
        }
    }

    static class A extends SubClass {

        void method() {
            System.out.println("Class A method");
        }

        void test() {
            method();
        }
    }

    static class SubClass extends ParentClass {
    }

    public static void main(String[] argv) {
        A a = new A();
        a.test();
    }
}

And that yields unsurprisingly:

--- exec:1.5.0:exec (default-cli) @ MavenSpielwiese ---
Class A method

matthiasblaesing avatar Aug 15 '25 19:08 matthiasblaesing