Move Type to Namespace refactoring

[jinujoseph]

Moved from link

This should replace Namespace declarations in the original files and make sure that usages are updated accordingly.

[ryzngard]

I've been looking into how we can do this, and I think we have a few options:

Option 1

Invocation would be refactor suggestion on left side when cursor is in a type declaration. On invocation, would bring up a dialog that allows selection of any "top level" declaration (directly inside a namespace, not nested in another type) to be selected. A new namespace name is given via a textbox, and resulting code is moved within the same file but encapsulated in the new namespace. If a namespace in the file would be left empty, it is removed.

Option 2
Similar to Option 1, but we could also allow specifying a file destination. This could be the current file, an existing file, or a new file. Moved code would always be inserted at the end of the target scope

Option 3
Limit the refactor option to only files with a single type declaration inside a namespace. Refactoring only asks for new name, and then changes the encapsulating namespace in that file. This is simple and straight forward, but lacks the flexibility of the other options.

I believe Option 1 is the current best target, but we should discuss.

[CyrusNajmabadi]

Having to specify the target namespace manually, without anything to help out, seems pretty arduous.

[ryzngard]

Having to specify the target namespace manually, without anything to help out, seems pretty arduous.

I'm not opposed to having something like autocomplete for helping decide a namespace if it's already existing. That seems like a good value add. We still want to allow a new namespace to be specified as well.

[CyrusNajmabadi]

Yeah. Intellisense both supports suggesting namespaces when in a namespace name. And we have precidence for intellisense inside a text-box (i.e. the debugger windows).

[ryzngard]

Current Proposal

Two ways to move types to a new namespace.

1. Move all types within a given namespace declaration scope.
2. Move a specific type to a new namespace

Both will be refactor suggesions on namespace declaration (1) or type declaration (2). All references will be updated same as "sync to namespace" feature.

UI

UI will be the same for both cases, since all that will be needed is a new namespace. If the namespace is the same as the original, no action will be taken. A single input field with fully qualified namespaces from the current project as suggestions.

Examples

For each example, the $$ indicates cursor placement where the refactor will be suggested. Upon selecting the refactor the UI, shown above, will be presented. This assumes that Chunky.Bacon is the new namespace chosen by the user and submitted.

All types within a declaration

Example 1

Before

namespace $$Some.Existing.Name
{
    class Foo { }
    class Bar { }
}

After

namespace Chunky.Bacon
{ 
    class Foo { } 
    class Bar { }
}

Example 2

Before

$$namespace Some.Existing.Name
{
    interface IBar { }
    class Bar : IBar { }
}

After

namespace Chunky.Bacon
{ 
    interface IBar { } 
    class Bar : IBar { }
}

Example 3

Before

Foo.cs

namespace Some.Existing.Name$$
{
    class Foo { } 
}

Bar.cs

namespace Some.Existing.Name
{
    class Bar : Foo { }
}

After

Foo.cs

namespace Chunky.Bacon
{
    class Foo { } 
}

Bar.cs

using Chunky.Bacon;

namespace Some.Existing.Name
{
    class Bar : Foo { }
}

Move individual type

Only apply to top level types in a namespace; no nested types.

Move to own namespace declaration scope in same file if more than one declaration in the current scope. Namespaces will be split to accomodate the new declaration.

Example 1

Before

namespace Some.Existing.Name
{
    class $$Baz { }
    class Foo { }
    class Bar { }
}

After

namespace Chunky.Bacon
{
    class Baz { }
}

namespace Some.Existing.Name
{
    class Foo { }
    class Bar { }
}

Example 2

Before

namespace Some.Existing.Name
{
    class Baz { }
    $$class Foo { }
    class Bar { }
}

After

namespace Some.Existing.Name
{ 
    class Baz { }
}

namespace Chunky.Bacon
{
    class Foo { }
}

namespace Some.Existing.Name
{
    class Bar { }
}

Example 3

Before

namespace Some.Existing.Name
{
    class Baz { }
    class Foo { }
    class Bar$$ { }
}

After

namespace Some.Existing.Name
{
    class Baz { }
    class Foo { }
}

namespace Chunky.Bacon
{
    class Bar { }
}

Future Improvements

Should this be a shortcut and added to Edit > Refactor? Ctrl + R, N is currently available in default bindings.

More places this gets suggested as a refactoring? Current scope may not have enough visibility.

Select items from list to move multiple at same time.

Consolidation of move refactorings provided.

Sync file name or location to match new namespace

[dpoeschl]

UI will be the same for both cases, since all that will be needed is a new namespace.

@ryzngard I understand the dialog being the same (mostly), but can the lightbulb entry text be distinct for the two cases? The behavior difference being the caret position (between type and namespace declarations) makes sense to me intuitively, but I'd still want the text the be clear about what's going to happen, especially if one or more types in a namespace declaration are scrolled offscreen (Move 'Foo' to namespace... from a type versus something like Move 3 types to namespace... from a namespace declaration). Perhaps then the dialog title could reflect this as well so you know what you're doing there if you clicked the lightbulb item too fast.

Should this be a shortcut and added to Edit > Refactor? Ctrl + R, N is currently available in default bindings.

I personally don't think this is necessary/desirable.

More places this gets suggested as a refactoring? Current scope may not have enough visibility.

I assume you were using the [||] notation as an example invocation location instead of indicating the full invocation span. I think having them offered on the full declaration part ([| namespace Foo.Bar |]{ ... } or [| public class Foo |]{ } would be a good start.

[ryzngard]

I assume you were using the [||] notation as an example invocation location instead of indicating the full invocation span. I think having them offered on the full declaration part ([| namespace Foo.Bar |]{ ... } or [| public class Foo |]{ } would be a good start.

I'll update the examples to use $$ to be consistent with a single caret position.

I understand the dialog being the same (mostly), but can the lightbulb entry text be distinct for the two cases?

I think that's a great way to help distinguish intent. It also reaffirms the context of the action. The suggestions seem to make sense, but I'll consider it more.

I personally don't think this is necessary/desirable.

@dpoeschl interesting... why is that? Movement as a whole is personally something I do a lot as I write code. It also works in almost any context. Maybe if we consolidate our move operations then that would make sense to promote to top level.

[dpoeschl]

Maybe if we consolidate our move operations then that would make sense to promote to top level.

@ryzngard Yes. I'd prefer we figure out what we want this suite of operations to look like before assigning a keyboard shortcut to one specific action that we might later want to take back for a broader operation of some sort.

EDIT: And then have another conversation about the merits of a shortcut, at least partially informed by telemetry etc.

[ryzngard]

The work for this is done. Any further feedback should be filed as new issues