"Move type to file" can bring along #endregion directive, resulting in non-compiling code

[drewnoakes]

Version Used: Version 17.1.0 Preview 1.0 [31810.13.main]

Steps to Reproduce:

namespace ConsoleApp340;

class C
{
    #region Foo

    string S { get; }

    #endregion

    class I { }
}

Caret on class I, invoke lightbulb, select "Move type to C.I.cs".

Expected Behavior:

C.I.cs contains:

namespace ConsoleApp340;

partial class C
{
    class I { }
}

Actual Behavior:

C.I.cs contains the following non-compiling code:

namespace ConsoleApp340;

partial class C
{
    #endregion

    class I { }
}

The #endregion in the original file is also pulled into column 0 (leading white space removed).

[yvef]

Hi. I see that the problem is with trivia handling. Because the leading and the trailing trivia(s) have a different processing approach.
In fact, that poses a common problem. How to preserve the scope of preprocessor directives correctly. (Keep open and close directives and only refer to the target (movable) SyntaxNode). Just examples of the problem:

namespace N1
{
    public class A
    {

#if DEBUG // the leading trivia of "Value" property
        public string Value { get; set; }

        #region // the leading trivia of "B" class

        public class B { }

        #endregion // the leading trivia of "Value2" property
#endif // the same as "#endregion"

        public string Value2 { get; set; }
    }
}

Regarding to the comments it will introduce (file A.B.cs):

namespace N1
{
    public partial class A
    {
        #region // the trivia of "B" class

        public class B { }
    }
}

If we remove #region/endregion directives from the source file, the code without compile error will be generated as file A.B.cs

namespace N1
{
    public partial class A
    {
        public class B { }
    }
}

As expected - #if/#endif directives are missing, but this is the wrong compilation unit right now.

The next example:

namespace N1
{
    public class A
    {
#if DEBUG
        //public string Value { get; set; }
#endif
        public class B { }
    }
}

Introduce the A.B.cs file:

namespace N1
{
    public partial class A
    {
#if DEBUG
        //public string Value { get; set; }
#endif // redundant scope
        public class B { }
    }
}

So, I can suggest during the MoveType process, which is going in AbstractMoveTypeService.MoveTypeEditor ignore directives while member definitions subtracted from the target node. and only then resolve the directive that have correct scope and only related to the movable things.
For example:

namespace N1
{
    public class A
    {

#if DEBUG
        public string Value { get; set; }

        #region
#if Dir2
#endif
        public class B { }

        #endregion
#endif

        public string Value2 { get; set; }
    }
}

Step 1:

namespace N1
{
    public partial class A
    {

#if DEBUG
        #region
#if Dir2 // to remove
#endif // to remove
        public class B { }
        #endregion
#endif
    }
}

namespace N1
{
    public partial class A
    {

#if DEBUG
        #region
        public class B { }
        #endregion
#endif
    }
}

Any other ideas?

[sharwell]

Duplicate of #19613