Bug 70135 - Add "Deprecate" to Rename refactoring [refactoring]
Summary: Add "Deprecate" to Rename refactoring [refactoring]
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.0   Edit
Hardware: All All
: P3 enhancement with 1 vote (vote)
Target Milestone: 3.2 M5   Edit
Assignee: Philip Mayer CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-07-15 13:54 EDT by Gary Gregory CLA
Modified: 2006-01-03 10:07 EST (History)
3 users (show)

See Also:


Attachments
Extra code added to ChangeSignatureRefactoring and ChangeSignatureWizard to implement requested functionality (26.91 KB, patch)
2005-03-12 05:28 EST, Ben Aveling CLA
no flags Details | Diff
updated junit test file ChangeSignatureTests and new and changed in and out files for same (60.54 KB, patch)
2005-03-12 05:32 EST, Ben Aveling CLA
no flags Details | Diff
jdt.ui / jdt.ui.tests.refactoring (52.03 KB, patch)
2006-01-03 04:33 EST, Philip Mayer CLA
no flags Details | Diff
jdt.ui / jdt.ui.tests.refactoring (59.53 KB, patch)
2006-01-03 06:16 EST, Philip Mayer CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gary Gregory CLA 2004-07-15 13:54:50 EDT
I would like to be able to deprecate methods and types when renaming them. The
Rename... dialog would have a checkbox with a "Deprecate" label followed by a
"Comment" entry field. Turning on the checkbox would perform the same operation
as "Rename" except that the renamed item would NOT be deleted. Instead it would
be marked with the Javadoc @deprecated tag followed by the comment the user
entered if any. Eclipse rocks.
Comment 1 Frederic Fusier CLA 2004-07-16 15:53:18 EDT
Move to JDT/UI.
Comment 2 Dirk Baeumer CLA 2004-07-19 08:36:32 EDT
Not committed.
Comment 3 Ben Aveling CLA 2005-01-20 14:58:45 EST
More than just leaving the old code behind, I'd rather have the option to leave
behind a stub.

For eg, suppose I refactor 

    OldF old_f(X x,Y y) {...} 

to

    NewF new_f(Y y,Z z) {...}

Then I'd like to be able to automatically leave behind a stub, something like

    /** @deprecated Use new_f(Y y,Z z) instead */
    OldF old_f(X unused,Y y){
         return (OldF) new_f(y, null);
    }

I'm working on this at the moment, though this is my first eclipse bug so no
promises.

The options I'm thinking of adding are

[X] Update Known References
[ ] Leave behind 
    ( ) - Stub
    (O) - Original Code
    [X] - Deprecated Comment

This change would be made both in Change Signature and Rename Method.
Comment 4 Dirk Baeumer CLA 2005-01-21 04:53:37 EST
Markus, just to keep you in the loop.
Comment 5 Ben Aveling CLA 2005-03-07 06:49:42 EST
OK.  This is done.  

I'm not an eclipse contributor, how do I submit the fix for review and comment?
Comment 6 Dirk Baeumer CLA 2005-03-07 08:58:26 EST
Add patches to this bug report and we will have a look at them.
Comment 7 Ben Aveling CLA 2005-03-12 05:28:40 EST
Created attachment 18738 [details]
Extra code added to ChangeSignatureRefactoring and ChangeSignatureWizard to implement requested functionality

It works.  I've added some unit tests and modified others and they work.

I've been using an earlier version of it at work for a weeks and it works.

Things can always be improved, but it's time for someone else to have a look at
it.
Comment 8 Ben Aveling CLA 2005-03-12 05:32:06 EST
Created attachment 18739 [details]
updated junit test file ChangeSignatureTests and new and changed in and out files for same

There are now 124 junit tests in this class.
Comment 9 Markus Keller CLA 2005-03-21 04:15:23 EST
Thanks for the patch, Ben. Unfortunately, I won't have time to look into this
before M6, but I'll consider it for M7.

Did you also implement it for RenameMethod? We can't offer this just for change
method signature. Either both refactorings will get the functionality, or none.
Comment 10 Ben Aveling CLA 2005-03-22 01:46:48 EST
> Did you also implement it for RenameMethod?

I haven't.  I'll do so.

Comment 11 Ben Aveling CLA 2005-03-23 18:57:41 EST
Hmm.  I've had a look at RenameMethod and it seems to be a completely seperate
piece of code doing a subset of what ChangeSignature does.  Unsurprisingly,
RenameMethod is a lot simpler than ChangeSignature.

I don't want to copy code from ChangeSignature to RenameMethod.

Should I change the Rename Method wizard so that invokes the Change Signature
refactoring and deprecate the rename method refactoring?

<<My email is currently not working.  I'll check this page regularly until 
it's fixed.>>
Comment 12 Markus Keller CLA 2005-03-24 06:35:36 EST
No, rename method should not use ChangeSignature, since ChangeSignature creates
an AST for every compilation unit that references the method. This is
unnecessary for RenameMethod.

I guess we have to live with some code duplication here.
Comment 13 Markus Keller CLA 2005-04-08 06:08:26 EDT
Sorry, this won't happen for 3.1.

Offering more support for API stability will be addressed in 3.2. We will then
look into supporting this in an uniform way in all refactorings where it is
feasible (e.g. also for all kinds of moves, etc.).
Comment 14 Markus Keller CLA 2005-12-16 08:19:43 EST
Released patch from Philip with common implementation for Change Method Signature, Introduce Parameter, Rename Method, and Rename Field (constants).

Filed bug 121200 for the move refactorings.

Philip, could you please check the test cases in the attached patch and create a new patch if there are cases that your tests didn't catch? Thanks.
Comment 15 Philip Mayer CLA 2006-01-03 04:33:30 EST
Created attachment 32397 [details]
jdt.ui / jdt.ui.tests.refactoring

I had a look at the test cases - I've integrated them into the existing ones. The attached patch contains the updated test files and fixes a problem with varargs.
Comment 16 Philip Mayer CLA 2006-01-03 06:16:48 EST
Created attachment 32402 [details]
jdt.ui / jdt.ui.tests.refactoring

Updated patch - missed some test cases.
Comment 17 Markus Keller CLA 2006-01-03 10:07:35 EST
Released the patch to HEAD.