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

Refactoring "Move type to ..." breaks preprocessor directives #19613

Closed
josefpihrt opened this issue May 18, 2017 · 6 comments · Fixed by #76275
Closed

Refactoring "Move type to ..." breaks preprocessor directives #19613

josefpihrt opened this issue May 18, 2017 · 6 comments · Fixed by #76275
Labels
Area-IDE Bug help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Milestone

Comments

@josefpihrt
Copy link
Contributor

Version Used: Visual Studio 2017 15.2.0+26430.6

Code to Reproduce:

public class Foo
{
    #region Region
    public class Bar // Move type to 'Bar.cs'
    {
    }
    #endregion
}

Actual Behavior:

public partial class Foo
{
    #endregion
}
@sharwell
Copy link
Member

sharwell commented May 18, 2017

📝 We had to deal with this for StyleCop Analyzers, and believe we found a rather clever solution in the end. Rather than construct a new file from a particular syntax node, we duplicate the entire tree and then selectively remove different sets of nodes from each file.

https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1402CodeFixProvider.cs

@Pilchie
Copy link
Member

Pilchie commented May 18, 2017

This was fixed already for 15.3 (I think by #16300 - @CyrusNajmabadi does that look right), though it leaves the #region and removes it's indentation, which could be better.

@Pilchie Pilchie added this to the 15.later milestone May 18, 2017
@Pilchie
Copy link
Member

Pilchie commented May 18, 2017

@sharwell That's actually how this is implemented as well :)

@Pilchie Pilchie added the help wanted The issue is "up for grabs" - add a comment if you are interested in working on it label May 18, 2017
@Pilchie Pilchie modified the milestones: 15.later, Unknown May 18, 2017
@CyrusNajmabadi
Copy link
Member

Yeah. This was fixed. But clearly the fix could use some work. If anyone wants to fix up the formatting, that would be totally great.

@CyrusNajmabadi
Copy link
Member

@yvef I would not over complicate things. We can just detect some very common cases that go awry today and improve on them. :-)

@Rekkonnect
Copy link
Contributor

Rekkonnect commented Mar 4, 2024

This still applies (17.9.2) and also carries over #endregion directives that are not bound to any #region ones. Example:

We have this file:

public class Foo
{
    #region Region1
    public class NotBar
    {
    }
    #endregion

    #region Region2
    public class Bar // Move type to 'Bar.cs'
    {
    }
    #endregion
}

After moving Foo.Bar to Bar.cs, we have (indentation is preserved as-is):

// Foo.cs
public partial class Foo
{
    #region Region1
    public class NotBar
    {
    }

#endregion
#region Region2
    #endregion
}

// Bar.cs
public partial class Foo
{
    #endregion

    #region Region2
    public class Bar // Move type to 'Bar.cs'
    {
    }
    #endregion
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Bug help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Projects
Status: Completed
Development

Successfully merging a pull request may close this issue.

6 participants