Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support refactoring to Sync Namespace and Folder Name #14341

Closed
kuhlenh opened this issue Oct 6, 2016 · 19 comments
Closed

Support refactoring to Sync Namespace and Folder Name #14341

kuhlenh opened this issue Oct 6, 2016 · 19 comments

Comments

@kuhlenh
Copy link

kuhlenh commented Oct 6, 2016

From comment in P5 Blog Post:

"Does the “Sync File and Type Name” refactoring also change the namespace of a type to match its folder name? And can it be performed in batch? This would make mass renames/moves of namespaces much easier!"
https://blogs.msdn.microsoft.com/visualstudio/2016/10/05/announcing-visual-studio-15-preview-5/#comments

@kuhlenh
Copy link
Author

kuhlenh commented Mar 24, 2017

see comments here: #18147

@Pilchie Pilchie modified the milestones: 15.1, 15.3 Mar 29, 2017
@Pilchie Pilchie modified the milestones: 15.later, 15.3 May 23, 2017
@jinujoseph jinujoseph modified the milestones: 15.6, Unknown Nov 1, 2017
@genlu
Copy link
Member

genlu commented Sep 27, 2018

Proposed design for V1 (C#)

This will be triggered when cursor is moved to the namespace declaration, or first member declaration if everything in the document is declared in global namespace. It tries to match current declared namespace with the document's path, based on rootnamespace property defined the project file. If no such property is defined, global namespace is then used as root namespace.

Two kind of actions might be provided: Move file to a new directory to match namespace and rename namespace to match directory hierarchy.

  1. Don't provide any refactoring on linked files.
  2. No "move file" action if rootnamespace isn't the containing namespace of current declared namespace.
  3. Offers (additional) "move file" action to use existing directory if it fits the target namespace. e.g. if the target namespace is RootNamespace.A.B.C and there a directory A.B at project root, then we offer to move file to <ProjectRoot>\A.B\C\ as well.
  4. No refactoring available if multiple namespace declarations are in the document. This includes nested namespace declaration and for document with members declared inside both global namespace and namespace declaration. We might want to provide a namespace folding codefix for simple nested namespace declaration later.
  5. For perf reason, we don't check for potential conflicts that might be caused by renaming namespace.
  6. Don't provide "move file" action if target path to move file to already exists on disk. (or should we just change the file name when this is detected?)
  7. Triggers the analysis on the first member declaration if all declarations are in global namespace.
  8. Don't provide "rename namespace" refactoring when there's partial class definition in the document that has definitions in other documents. (Or we might want to provide a refactoring regardless if checking this is too expensive to do during the analysis phase.)
  9. Don't provide "rename namespace" action if we can't construct a valid namespace name from directory hierarchy.
  10. Make it clear to user the rename action doesn't attempt to fix everything, so the code might end up in a bunch of errors, e.g. duplicated definitions, ambiguities, and in worst case, the semantic might change w/o causing errors.

Examples

  1. rootnamespace: Microsoft.VisualStudio

    Path: <projectRoot>\ProjectSystem\LanguageServices\SomeTests.cs

    Declared namespace: Microsoft.VisualStudio.ProjectSystem.LanguageServices

    Available options:

    • None
  2. rootnamespace: Microsoft.VisualStudio

    Path: <projectRoot>\ProjectSystem\LanguageServices\SomeTests.cs

    Declared namespace: Microsoft.VisualStudio.ProjectSystem.LanguageServices.UnitTests

    Available options:

    • Move file to <projectRoot>\ProjectSystem\LanguageServices\UnitTests\ folder.
    • Rename namespace to Microsoft.VisualStudio.ProjectSystem.LanguageServices
  3. rootnamespace: ``

    Path: <projectRoot>\ProjectSystem\LanguageServices\SomeTests.cs

    Declared namespace: Microsoft.VisualStudio.ProjectSystem.LanguageServices.UnitTests

    Available options:

    • Move file to <projectRoot>\Microsoft\VisualStudio\ProjectSystem\LanguageServices\UnitTests\ folder.
    • Rename namespace to ProjectSystem.LanguageServices
  4. rootnamespace: Microsoft.CodeAnalysis

    Path: <projectRoot>\ProjectSystem\LanguageServices\SomeTests.cs

    Declared namespace: Microsoft.VisualStudio.ProjectSystem.LanguageServices.UnitTests

    Available options:

    • No "Move to folder option" since the rootnamespace isn't the containing namespace of declared namespace.
    • Rename namespace to Microsoft.CodeAnalysis.ProjectSystem.LanguageServices

@CyrusNajmabadi
Copy link
Member

Can you clarify what you mean by "rename namespace"? Is that a true rename (where references to that namespace are updated? Or do you just mean "the namespace name declaration in that file is update to have the new name?

@genlu
Copy link
Member

genlu commented Oct 1, 2018

It's the latter - the namespace name declaration in that file is updated to have the new name

@KathleenDollard
Copy link

How is the root namespace determined?

I got lost in some of the comments - is this a new feature or a change to move type to file that already exists?

@genlu
Copy link
Member

genlu commented Oct 25, 2018

@KathleenDollard Sorry, the description above might be confusing, you can find a better explaination of root namespace vs. default namespace here. A quick recap, what it is actually called is "default namespace" in C#, which is defined in csproj by rootnamespace property. Concept wise root namespace is a competely different thing in VB, which is a compiler argument, whereas C# project's default namespace is merely used by VS.

This is a new feature, since the file move is decided by namespace declaration instead of type name, and it also involves moving to a different folder. But logically it would make sense to have a single MoveFileService in workspace to handle both "move to folder" and "move type to file" refactoring as @heejaechang suggested in the PR.

@KathleenDollard
Copy link

I asked the VB community in VBLang.

My first reaction is that the relation with namespaces in VB tends to be different because of the difference in root namespace - I think there are more VB programs with no explicit namespace in the file, and more cases where the namespaces do not align with the file structure.

But those cases might not be offered the code fix, so are there enough VB programmers that do use a parallel file and namespace structure to justify.

@jinujoseph jinujoseph modified the milestones: Unknown, 16.0.P2 Nov 1, 2018
@jinujoseph
Copy link
Contributor

Available from 16.0.Preview2 via #30920
Thanks @genlu

@btastic
Copy link

btastic commented Feb 6, 2019

I am currently on the VS 2019 Preview 2.2 and it does not seem to be available to me.

I changed my classes namespace to something else: from CompanyName.ProductName.Domain.Commands to CompanyName.ProductName.Domain.dasdas.

I expected to have the refactoring available when hitting ctrl+. on the now wrong namespace. But I don't. Do I miss anything?

Projects default namespace is CompanyName.ProductName.Domain
Projects target framework is .NET Core 2.2
Projects lang version is 7.2

If you need anything else, let me know.

@genlu
Copy link
Member

genlu commented Feb 6, 2019

@btastic Thanks for trying out this feature and providing feedbacks!

This refactoring isn't supported for .NET core projects in preview 2. The required change to support this was implemented in project-system very recently (dotnet/project-system#4432, which I believe will ship with preview 3), and there was a small fix needed on Roslyn side to make it work (#32920), which will ship with preview 4.

To try this today with VS 2019 preview 2 bits, you will need to build and deploy both project-system and roslyn from source.

@btastic
Copy link

btastic commented Feb 6, 2019

@genlu Thanks for the response! Can't wait to have this in .NET core. Quick question though: will it be possible to get warnings for inconsistent namespaces?

@genlu
Copy link
Member

genlu commented Feb 6, 2019

Right now this is just a code refactoring, but we are contemplating the idea of converting it into an analyzer/codefix combo (although, just want to point it out, because of perf, it's highly unlikely a "fix-all" fixer will be provided).

FYI @jinujoseph @mavasani

@btastic
Copy link

btastic commented Feb 6, 2019

I love it. Thank you!

@btastic
Copy link

btastic commented Feb 27, 2019

Just eagerly installed the latest Preview 4.0 (RC) and it works! Thank you again

@felixhirt
Copy link

I would love to see this become an analyzer (as you suggest above) and show warnings, even if it would only work in visual studio

@genlu
Copy link
Member

genlu commented Mar 5, 2019

I have created #33877 for converting the refactoring to diagnostic/codefix. Please leave a comment if you have any feedback/suggestion. Thanks!

@bugproof
Copy link

bugproof commented May 7, 2019

@genlu Is it possible to make this refactoring globally available? I mean, in ReSharper and Rider you can right click on your project and you can apply this refactoring to all files in your project with a single click.

@genlu
Copy link
Member

genlu commented May 7, 2019

@BackDoorManUC We are considering to make this refactoring a analyzer/codefix pair, it will provide warnings for out-of-sync namespaces, which might help to identify those cases. That said, we are not sure a fix-all will be provided for this, because the perf on a large solution might be an issue.

@dagood
Copy link
Member

dagood commented Sep 17, 2019

I mean, in ReSharper and Rider you can right click on your project and you can apply this refactoring to all files in your project with a single click.

FYI for anyone else who wants this and might want to 👍 and/or follow it, I filed #37453 to specifically request it because I frequently use this ReSharper feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests