Allow ignore spaces around declaration to work when preserve single line statements is set to false

[chborl]

Fixes #31868

Who is impacted by this bug?
Users who use Format Document and have these two settings in the .editorconfig
csharp_space_around_declaration_statements = ignore
csharp_preserve_single_line_statements = false

What is the customer scenario and impact of the bug?
If a user has these two settings in their .editorconfig:
csharp_space_around_declaration_statements = ignore
csharp_preserve_single_line_statements = false

When they Format Document the extra spaces around their declaration statements are removed when they should have been ignored.

Expected
Original spacing around the declaration statements should be left alone since user specified
csharp_space_around_declaration_statements = ignore.

Example,

    public void FixMyType()
    {
        var     myint       = 0;
        var     myint2      = 1;
    }

Actual
The original spacing is not maintained. It becomes,

    public void FixMyType()
    {
        var myint = 0;
        var myint2 = 1;
    }

What is the workaround?
After using Format Document, manually redo spacing around declaration statements.

How was the bug found?
Developer Community issue

If this fix is for a regression - what had regressed, when was the regression introduced, and why was the regression originally missed?
Not a regression. Repros in 15.9.11

[CyrusNajmabadi]

this is a pretty core method. Can you add comments explaining why this behavior is correct?

[chborl]

@CyrusNajmabadi ,

When setting these options:

changingOptions.Add(CSharpFormattingOptions.SpacesIgnoreAroundVariableDeclaration, true);
changingOptions.Add(CSharpFormattingOptions.WrappingKeepStatementsOnSingleLine, false);

and running this code:

class Program
{
    public void FixMyType()
    {
        var    myint    =    0;
    }
}

This line of code is removing all three of these entries:

If I leave the NoSpacing entry in the list, it keeps the spacing intact as desired.

There might be another place that is more appropriate to fix it. Any ideas where?

[chborl]

The fix could also be this, if (list[i] != null && list[i].TextSpan.Start >= span.Start && list[i].TextSpan.End <= span.End && list[i].Option != SuppressOption.NoSpacing). I'm not familiar enough with this code to know if other features are affected by the same setting.

[CyrusNajmabadi]

I would definitely ask @heejaechang about this. I think he's the only one qualified to state the best place for this fix.

[heejaechang]

I am not sure why code is written this way, but it looks like we are removing suppression operation based on option rather than add suppression operation based on option.

because it is blindly add all suppression rule first and then later try to remove those suppression added, we have this strange code flow.

I think we should have an issue opened saying we should do thing in opposite way where we add suppression rules based on option rather than remove rule based on options

[heejaechang]

that being said, in current form, what @chborl did is right.

[chborl]

I think we should have an issue opened saying we should do thing in opposite way where we add suppression rules based on option rather than remove rule based on options

I filed this follow-up issue: #35377

[dpoeschl]

Thanks for the explanations.

[jinujoseph]

@ManishJayaswal for approval