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.04k stars 4.03k forks source link

Formatter doesn't handle indentation of wrapped expressions #40361

Open alrz opened 4 years ago

alrz commented 4 years ago

I think the formatter should be able to handle this kind of input

public class Cdsds
{
    public bool OperandExpression()
    {
        return OperandExpression() &&
OperandExpression();
    }
}

Expected Behavior: Either of these based on some user preference:

    {
        return OperandExpression() &&
            OperandExpression();
    }
    {
        return OperandExpression() &&
               OperandExpression();
    }

Actual Behavior: Code is unchanged.

alrz commented 4 years ago

I believe @CyrusNajmabadi has developed a refactoring feature for this exact purpose but I think it should be integrated into the formatting rules?

alrz commented 4 years ago

This is a problem for https://github.com/dotnet/roslyn/pull/32386 because we're producing a possibly long chain of expressions and the wrapping is handled manually which isn't ideal (for instance, we need to follow user preference on tab/space etc, practically duplicating the same logic). Perhaps the formatter should be able to handle wrapping and indentation on its own here.

/cc @sharwell

CyrusNajmabadi commented 4 years ago

Perhaps the formatter should be able to handle wrapping and indentation on its own here.

I am not opposed to this. however, the best course of action IMO would be:

  1. have the formatter do this only in the presence of elastic trivia. This is hte signal to the formatter that code was generated, and more aggressive formatting can take place.
  2. do not have the formatter do this for normal user code. People indent normal code in incredible wacky ways. interfering with that is problematic.
  3. if we want the normal formatter to change this stuff, it should likely be gated by an opt-in formatting option.