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
19.07k stars 4.04k forks source link

Extract method over-indents second line of expression #21453

Open sharwell opened 7 years ago

sharwell commented 7 years ago

Version Used: 15.3 Preview 7

:link: Originally revealed by ExtractMethodTests.TestUseExpressionWhenOnSingleLine_AndNotIsOnSingleLine in #21439.

Steps to Reproduce:

  1. Use the following code

    using System;
    
    namespace ConsoleApp5
    {
        class Program
        {
            static void Main(string[] args)
            {
                bool b = true;
                Console.WriteLine(b !=
                    true ? b = true : b = false);
            }
        }
    }
  2. Select b != true

  3. Apply the Extract Method refactoring

Expected Behavior:

using System;

namespace ConsoleApp5
{
    class Program
    {
        static void Main(string[] args)
        {
            bool b = true;
            Console.WriteLine(NewMethod(b) ? b = true : b = false);
        }

        private static bool NewMethod(bool b)
        {
            return b !=
                true;
        }
    }
}

Actual Behavior:

using System;

namespace ConsoleApp5
{
    class Program
    {
        static void Main(string[] args)
        {
            bool b = true;
            Console.WriteLine(NewMethod(b) ? b = true : b = false);
        }

        private static bool NewMethod(bool b)
        {
            return b !=
                            true;
        }
    }
}
CyrusNajmabadi commented 7 years ago

Tagging @heejaechang . This is a confluence of extract method + formatting. Can you let us know the best way to handle this?

heejaechang commented 7 years ago

formatter probably see this as code that is asked to be formatted

private static bool NewMethod(bool b)
{
returnb !=
[same as original code]true;
}

and then

        private static bool NewMethod(bool b)
        {
             return b !=
[same as original code + return + indentation added to return]true;
        }

this is happening since formatting tries to keep delta between "true" and "return".

I am not sure whether formatter can fix it themselves in general way, feature probably need to recognize the case and explicitly tell formatter what to do either through rule or explicitly playing with trivia.