IDE0025 strangely indents when a comment follows the property name

[stephentoub]

Version Used:
3.3.0-beta3-19413-06+ac06df1bffb7b7fd0e5bf63cc91921af76b21e03

Steps to Reproduce:

using System;

class Program
{
    static void Main() => Console.WriteLine(Foo);

    private static string Foo // cool prop
    {
        get { return "blah"; }
    }
}

Run IDE0025.

Expected Behavior:

using System;

class Program
{
    static void Main() => Console.WriteLine(Foo);

    private static string Foo => // cool prop
        "blah";
}

or

using System;

class Program
{
    static void Main() => Console.WriteLine(Foo);

    private static string Foo =>
         // cool prop
        "blah";
}

or

using System;

class Program
{
    static void Main() => Console.WriteLine(Foo);

    // cool prop
    private static string Foo => "blah";
}

Actual Behavior:

using System;

class Program
{
    static void Main() => Console.WriteLine(Foo);

    private static string Foo // cool prop
=> "blah";
}

[CyrusNajmabadi]

This appears to be a formatting issue. We are property putting an elastic trivia at the start of the =>. the formatter is deciding it needs to put the => on the next line (because of the comment between it and the =>, but it's not properly indenting the token.

the formatting engine is so complex here, and i have no idea what portion is even responsible for handling this, or how it would need to be updated to take caer of this.

@heejaechang can you shed some light here?

[jnm2]

Investigating a little, there appear to be two hurdles.

1. Elasticity is ignored when reformatting whitespace between non-whitespace trivia and a comment.
   • Exploration here: main...jnm2:38057_ide_strangely_indents
2. The formatter doesn't apply additional wrapping indentation.

To demonstrate the latter point, with or without adding elastic whitespace before test, the following:

public class C
{
int // comment
test;
}

Formats as:

public class C
{
    int // comment
    test;
}

It does not apply further indentation to test the way it would wrap if you placed your cursor after // comment and pressed enter. This is something that could potentially be incorporated into the formatter.

The reason why an 'enter' keypress indents the way you expect, four spaces further in than the indentation level of the property itself, is that it falls through the big https://github.com/dotnet/roslyn/blob/main/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Indentation/CSharpIndentationService.Indenter.cs#L74 method, finds nothing special to do, and does the default indentation. The extra level of indentation is added at https://github.com/dotnet/roslyn/blob/main/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Formatting/BottomUpBaseIndentationFinder.cs#L149 and is determined by https://github.com/dotnet/roslyn/blob/main/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Indentation/CSharpIndentationService.Indenter.cs#L466.

So an enter keypress will add an additional increment of indentation by default. When it does not add this additional increment of indentation, such as after a semicolon or closing brace of a class member, this is because it no longer falls through the big indentation service syntax switch, and instead hits lines that override the int extraSpace parameter to 0. https://github.com/dotnet/roslyn/blob/main/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Indentation/CSharpIndentationService.Indenter.cs#L238-L261

[jnm2]

My proof of concept worked! It's one approach. It adds more indentation operations within single-line constructs which represent the wrapping that they should have if they were to break across multiple lines. main...jnm2:38057_wrapping_indentation_rules

[jnm2]

Unfortunately, following this up is a lot of work. We wouldn't want to just capture this "extra level of indentation" choice when formatting elastic trivia, we'd want to follow all the logic in https://github.com/dotnet/roslyn/blob/main/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Indentation/CSharpIndentationService.Indenter.cs which special-cases dozens of scenarios to indent properly when you press Enter.

That logic itself calls into the formatter to make decisions. So the formatter can't easily call into this indentation-on-new-line logic.

Longterm, perhaps all this logic could be reimplemented by adding IndentBlockOperations computed by the formatting rules, making the formatting rules the source of truth. This would position the formatting rules to be able to codify things like how to wrap long lines, which is a community request: #15406

In this approach, the logic would change form. Instead of introspecting and deciding on the fly, spans would be computed up front along with the other indentation blocks. These spans would start one character in from the start of a syntactic construct, because pressing Enter there should indent on the next line, and likewise end one character in from the end of the construct. When braces and brackets and embedded statements are introduced, they will have to mesh well. I started experimenting with this and it seems like a great direction to go in, but it isn't a small investment.

In the meantime, fixing this bug probably requires the code fix to manually compute the indentation for the added => token, perhaps transferring it from the get token.