dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.85k stars 4.01k forks source link

Sync namespace to folder reformats the entire document #34707

Open davkean opened 5 years ago

davkean commented 5 years ago

Version Used: 2019 RTM

Steps to Reproduce:

  1. At $ run Sync namespace to folder
using System;

namespace Namespace
{
    class Program
    {
        //          Column1               Column2
        [InlineData("FooBar",             "Bar")]
        [InlineData("FooBarFooBar",       "Bar")]
        [InlineData("FooBarFooBarFooBar", "Bar")]
        static void Main(string[] args)
        {
            Console.WriteLine("Hello World!");
        }
    }
}

Expected Behavior:

using System;

namespace Namespace.NewFolder
{
    class Program
    {
        //          Column1               Column2
        [InlineData("FooBar",             "Bar")]
        [InlineData("FooBarFooBar",       "Bar")]
        [InlineData("FooBarFooBarFooBar", "Bar")]
        static void Main(string[] args)
        {
            Console.WriteLine("Hello World!");
        }
    }
}

Actual Behavior:

using System;

namespace Namespace.NewFolder
{
    class Program
    {
        //          Column1               Column2
        [InlineData("FooBar", "Bar")]
        [InlineData("FooBarFooBar", "Bar")]
        [InlineData("FooBarFooBarFooBar", "Bar")]
        static void Main(string[] args)
        {
            Console.WriteLine("Hello World!");
        }
    }
}

I cannot use this feature in any test project as it breaks all tables. Why does it need to reformat the document to fix the namespace?

felixhirt commented 5 years ago

I noticed that (at least for me), it even formats based on rules which are marked as silent in .editorconfig. To be precise, i have: csharp_prefer_braces = false:silent in my editorconfig, and fixing the namespace removes all braces for single line statements. This breaks the feature for me

sharwell commented 5 years ago

To be precise, i have: csharp_prefer_braces = false:silent in my editorconfig, and fixing the namespace removes all braces for single line statements.

Roslyn does not have a feature to remove braces (see #29606 for a request to add it). I have no idea how/what/why you observed this.

felixhirt commented 5 years ago

Your right, i guess its a VisualStudio feature. Im talking about this: prefer_braces_setting And its respective .editorconfig setting. The reason why i mention it here (and not in the VisualStudio uservoice) is because it happens for me on the namespace refactoring move_namespace_braces Im sorry if this is the wrong place for this issue, im happy to move this wherever it is supposed to go

felixhirt commented 5 years ago

Also, i dont think i have any code cleanup or formatting options defined which do this for me automatically (on save or by hotkey or whatever), which is why it confuses me even more. The only way to do this normally in my setup is to move the cursor to the braces and use the "remove braces" refactoring it offers there.

CyrusNajmabadi commented 5 years ago

Roslyn does not have a feature to remove braces (see #29606 for a request to add it). I have no idea how/what/why you observed this.

@sharwell if someone puts ont he simplifaction annotation on code, this is a simplification that can happen. That happens so that someone can use the normal SyntaxGenerator codepaths but end up with blocks that match their "allow preference" here:

image

If its the case that sync-namespace is adding teh simplification annotation (for example, to shorten type names post-move) then this might fall out because of it.

felixhirt commented 5 years ago

Ah, i understand. Thanks for the info! Sorry to bother again, but could you give me any hints at how i can disable this feature for this specific option? The option is allready set to "silent", which seems to be the lowest severity? I also would like to keep all the other code style options on and also apply them on code generation if possible edit: pretty sure the original issue creator would prefer to completely disable these simplifications for this specific refactoring. I would be fine with that as well

felixhirt commented 5 years ago

I just realized that if i set the option to "csharp_prefer_braces = when_multiline:silent" it is not automatically applied, even if it would be the case. No idea why, but that works for me...

felixhirt commented 5 years ago

I have to revoke my last comment. The brace removal started again. I cannot figure out why it happens sometimes and sometimes not.

JDoucette commented 11 months ago

I opened a bug that was de-duped against this one. https://developercommunity.visualstudio.com/t/VS2022:-Moving-files-from-one-folder-to-/10474008

In my case, I drag-n-drop a source file to a new folder, which updates the C# namespaces to match the folders - in doing so, it re-formats the file, and changes all of my tabs into spaces, even though that's not the setting.

This feels similar to:

felixhirt - "...it even formats based on rules which are marked as silent..."

sharwell commented 7 months ago

Note that the desired behavior here is the Sync Namespaces feature should not make any alterations to the formatting within the document. The only changes that should be made are the introduction or removal of qualified names, e.g. AA.B.C or vice versa).

ygoe commented 5 months ago

My VS feedback was also duped against this. The feature is unusable for me and I must be careful not to accept the offer to adjust namespaces when moving files to another folder. It will do that, but it also does other offensive things, like removing all braces with single instructions content. So this is open for years?! Guess the namespace update has only recently been added to VS so I only noticed this now. I'm not sure what preferences I've set in Visual Studio, hopefully none. Everything should be controlled exclusively through .editorconfig files, they're way more manageable. And if it says I have no preference about braces, it means that I will decide individually. The editor then has no right to change this. Even more so when it claims it will only change the single namespace line. Why does it even have to touch any other line of the document that has nothing to do with that? The whole feature seems to be out of control here.

CyrusNajmabadi commented 5 months ago

Why does it even have to touch any other line of the document that has nothing to do with that? The whole feature seems to be out of control here.

@ygoe The feature needs to update code to keep the semantics when it moves from one namespace to another (you can imagine how certain names would now mean something else if it didn't do that). However, the primary mechanism it uses to perform that work is to 'expand' the code to have it's fully-qualified meaning, and then 'simplify' it back to the reduced form people normally type.

Unfortunately the subsystem it uses for that makes a lot more changes along the way beyond just the name expansion/simplification.

If you're interested in helping us improve this, we'd happily work with you and we'd accept community PRs here. Thanks!

ygoe commented 5 months ago

Understood. I will make sure I'm not using this trap feature until I'm able to repair it myself. Which may take a long time.