netbeans icon indicating copy to clipboard operation
netbeans copied to clipboard

Bug: Renaming a method results in a change in the calling relationship

Open assdfsdafasfa opened this issue 1 year ago • 4 comments

Apache NetBeans version

Apache NetBeans 23

What happened

The "methodToBeRenamed()" method is selected to be renamed to ''method()''; Code before refactoring, the body of “testMethod()” method calls ‘’method()‘’ in the ParentClass class ; Code after refactoring, the "method()" in the SubClass class is called in the body of "testMethod()" method , and the rename method refactoring has resulted in a change in the method call within the body of "testMethod()" method .

Code before refactoring:

class ParentClass{
    void method(){}
}
class SubClass extends ParentClass{
   void methodToBeRenamed(){}
   void test(){
    method();
  }
}

Language / Project Type / NetBeans Component

No response

How to reproduce

see above

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

No response

Are you willing to submit a pull request?

No

assdfsdafasfa avatar Oct 08 '24 12:10 assdfsdafasfa

please ask before submitting dozens generated bug reports. We have to review those in not automated manner. Generating issues/warnings is in most cases not the bottleneck.

Refactorings do require review since side effects can't always be excluded, NB does also open previews for non-trivial refactorings before the code is transformed.

As written elsewhere, in some cases a dev might even want to temporarily break the program during refactoring (trivial example: rename variable to overlap with other variable before removing one of the two variable declarations).

IDEs could show warnings in some cases but they should not prevent devs from refactoring code.

I question a bit that something as described here is an issue. Again: IDE driven code transformations are just a tool in a tool box and not something which can be blindly applied.

mbien avatar Oct 08 '24 21:10 mbien

please ask before submitting dozens generated bug reports. We have to review those in not automated manner. Generating issues/warnings is in most cases not the bottleneck.

Refactorings do require review since side effects can't always be excluded, NB does also open previews for non-trivial refactorings before the code is transformed.

As written elsewhere, in some cases a dev might even want to temporarily break the program during refactoring (trivial example: rename variable to overlap with other variable before removing one of the two variable declarations).

IDEs could show warnings in some cases but they should not prevent devs from refactoring code.

I question a bit that something as described here is an issue. Again: IDE driven code transformations are just a tool in a tool box and not something which can be blindly applied.

Thanks for the heads up, I was going through the test methodology and found the cause of the error for the different refactoring types, they are different in detail, I hope you can take a look at it and fix it

assdfsdafasfa avatar Oct 09 '24 00:10 assdfsdafasfa

@assdfsdafasfa can you edit the issues you posted and use code blocks for the code sections? There is also no point to paste the same thing into different sections - someone has to read that. I updated this issue here.

mbien avatar Oct 09 '24 01:10 mbien

nb is going to detect the namespace conflict and refactor the sample into:

class SubClass extends ParentClass{
   void method(){}
   void test(){
        ParentClass.this.method();
  }
}

this will cause an compiler error however: not an enclosing class: ParentClass

what it could do is to use super:

class SubClass extends ParentClass{
   void method(){}
   void test(){
        super.method();
  }
}

the fact that methodToBeRenamed got renamed to method and is now overriding ParentClass#method could be intended by the user. NB will also show the add-override hint once renamed. Using super.method() here might be already a bit of a stretch.

mbien avatar Oct 09 '24 01:10 mbien