[Monodevelop-devel] Improving refactoring after 2.0

[Lluis Sanchez Gual]

El dv 07 de 11 de 2008 a les 09:31 +0100, en/na Mike Krüger va escriure:
> Hi
> 
> > Notice that change previewing for the rename operation can be done
> with
> > the current infrastructure. It's a matter of finding the references,
> > displaying them and do the replace when the user clicks on OK. The API
> > supports that, although we haven't implemented the GUI.
> > 
> 
> No not really. It's a hack. There is an operation called:
> 
> IMember RenameMember (RefactorerContext ctx, 
>                       IType cls, 
>                       IMember member, 
>                       string newName);
> 
> And that's it. Ok you've:
> 
> MemberReferenceCollection FindMemberReferences (RefactorerContext ctx,
> string fileName,
> IType cls,
> IMember member);
> MemberReferenceCollection FindVariableReferences (RefactorerContext ctx,
> string fileName, LocalVariable var);
> MemberReferenceCollection FindParameterReferences (RefactorerContext
> ctx, string fileName, IParameter param);
> 
> And with these three methods you can hack something together for this
> one refactoring case - but that's it. Thisis not really what I've in
> mind to do.
> 
> > > 
> > > I'll need to extend/change the current refactoring infrastructure
> for
> > > this. And one of the goals for the new one is: unit test support.
> The
> > > new code completion infrastructure is tested with unit tests and I
> think
> > > that this has improved the code completion correctness greatly.
> > > 
> > > Any thoughts/inputs here ?
> > 
> > I'd like to see a high level design of the API to get an idea of how
> you
> > plan to improve the refactoring infrastructure.
> > 
> 
> Currently:
> 
> Anything seems to be called "refactorer" (Even classes that don't
> implement IRefactorer) to make it more stangely we've a refactorer
> called CodeGenerator (inheriting from BaseRefactorer). It looks like
> thats currently anything is hacked together.

Before trying to rewrite any piece of code you should at least study the
current code and try to understand how it works, what are the
responsibilities of each class, and why it was designed in that way.
Only then you'll be qualified to say what's wrong and propose a better
solution. The use of words like "strangely", "hack" and "looks like"
clearly show that you didn't follow this step.

I'm not saying that the current refactoring infrastructure is perfect.
It is not. Much of it will probably have to be changed if we want to
support more advanced refactoring operations. But you can't arbitrarily
drop all current code, ignoring what has been done. We already had some
bad experiences in MD because of this.

> It starts with clicking on something in the editor window there is a
> class that builds the context menu and this class controls which
> operations are valid.
> 
> I want:
> 
> Split up refactoring a bit to make it clear what each class does.
> 
> I intend to split up the "refactoring" feature. Each refactoring
> operation should be an own class that knows it's responsibilities (based
> on the resolve result) it knows when it's valid to display in the
> refactoring context menu.
> 
> Then we would've classes like:
> 
> Rename
> ImplicitImplementInterface
> ExplicitImplementInterface
> ExtractMethod
> 
> etc.
> 
> These refactorings should be placed in the addin tree.

Will those refactoring classes be language-specific?
For example, the Rename class, there will be a RenameCSharp and a
RenameVisualBasic? if that's the case, will they share some code?

> 
> We've a RefactorerContext - that's good these context should be
> containing the find functions and some helper functions that are
> currently splitted up between IRefactorer and CodeRefactorer).
> 
> Then we need a CodeGenerator - this is a class that actually builds
> code. Not by changing it, it should produce "diffs".

Some add-ins need to perform basic code manipulation operation such as
inserting a new method to a class or adding custom attributes to class
declarations. Which class would provide those operations?

I'd like to see how the high level API will look like. That is, the API
that will allow starting refactoring operations or manipulate code.

> 
> Then we need something like a result of the refactoring operations.
> That'll give us a list of files with diffs as well as a list of the
> class tree with changes. I intend to have a dialog that looks like:
> 
> -----------------
> 
>  Positions
> 
> 
> -----------------
> 
> 
> Diffs
> 
> 
> -----------------
> 
> That's what I intend to:
> 
> class RefactoringResult {
> 	List<Change> Changes;
> }
> 
> class Change {
> 	string fileName; 
> 	IMember memberLocation;
> 
>         DomLocation position;
> 
>         string old;
> 	string new;
> }
> 
> 
> Something like this an 'positions' should be switchable between class
> view and file view (like the pads).
> 
> The Refactoring should something look like:
> 
> IRefactoring {
> 	bool IsSupported (ResolveResult result);
> 	string GetDisplaySting (ResolveResult result);
> 	
> 	void Run ();
> }
> 
> IResultRefactoring : IRefactoring {
> 	RefactoringResult DoOperation (RefactorerContext, ResolveResult
> result);
> }
> 
> I want to support maybe more than one type of refactoring, therefore
> just the "run" method. And there are more types than just the diff
> refactoring - implement/override members for example therefore not all
> refactorings require the position/diff dialogs.

Can you provide a list of all refactorings you plan to implement and
what are the requirements of each of them?

> Another example could be renaming a local variable or parameter which
> could be displayed inline in the text editor instead of having pop up
> dialogs. (Ok I admit I've worked a bit with eclipse this week :) )
> 
> To decide: On which infrastructure we'll build. Currently it's a bit
> mixed up between CodeDOM and NRefactory. I would've chosen NRefactory
> because we're already building on this and an own AST gives more control
> than a pre defined one.
> 
> Regards
> Mike
>