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

"Remove Unnecessary Usings" should not delete blank lines above the using declaration. #55439

Open ericmutta opened 3 years ago

ericmutta commented 3 years ago

Version Used:

Visual Studio Community 2019 v16.10.3

Steps to Reproduce:

Consider the following code:

// blank line 1 here.
using Xunit;
using System.IO;
// blank line 2 here.
namespace BlahBlah.Tests
{
  public sealed partial class Foo  {  }
}

The comments show where there are blank lines (one before the two usings, and one after them). In the editor it looks like this:

image

Now when you invoke the "Remove Unnecessary Usings" refactoring, it insists on deleting BOTH blank lines, and the red portions of the preview indicate as much:

image

Expected Behavior:

As indicated in the image above using green arrows, I believe the blank line AFTER the usings should be removed along with the usings because most people explicitly add it to separate the usings from the code that follows. However, the blank line BEFORE the usings should NOT be removed because it could be there for a reason.

Actual Behavior:

Both blanks lines are removed, including the one BEFORE the usings which I believe should be left alone. In my case all my source files start with a blank line at the top (and the end!) of the file for 2 reasons:

  1. The blank line acts as "padding" so visually the first line of code (typically a using declaration) is not too close to the edge of the editor.

  2. I use the awesome VsVim extension which allows Vim/Vi-like editing in Visual Studio and some commands like { and } can't be used easily if that blank line at the top is missing. This is why I want the blank line before the usings to remain.

This may be a case of fixing just this refactoring or perhaps adding options to insert blank line at the beginning of file and insert blank line at the end of file (handy for git) under the "New Lines" section of C# Code Style settings:

image

CyrusNajmabadi commented 3 years ago

I thnk we'd need more feedback from others that this was an issue. I'm def concerned that we might change this and impact users who like teh current behavior.

ericmutta commented 3 years ago

@CyrusNajmabadi thanks for the fast follow-up (as always!).

I definitely agree that some users may wish to keep the current behaviour. Coming from 20+ years of VB, I am utterly blown away at the incredible amount of control the editor gives for formatting C# code. I think this behaviour could be yet another knob to control code formatting. The widespread use of git nowadays makes a strong case for an add blank line at end of file setting...and if we do that, we might as well have add blank line at start of file which would resolve the problem filed in this issue, while still allowing people to choose.

CyrusNajmabadi commented 3 years ago

we need to temper the idea of just throwing options at a problem. They add real costs in development, maintenance, and especially impact and limit our ability to refactor/redesign things down the line.

Options are added when there is really a substantial amount of users that fit into multiple camps and hte only way to support both is through an option. They're not really there for any particular one-off case that a tiny subset of users (or just one :)) report.

ericmutta commented 3 years ago

They're not really there for any particular one-off case that a tiny subset of users (or just one :)) report.

I hear you :)

So what would be the best way to get feedback from others in scenarios like this, filing a suggestion on developer community or...?

CyrusNajmabadi commented 3 years ago

We keep this issue open and see if collects more community interest/votes :-)

CyrusNajmabadi commented 20 hours ago

We would take a targeted community pr here.