Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty lines not removed? #1074

Closed
wondering639 opened this issue Apr 4, 2021 · 19 comments
Closed

Empty lines not removed? #1074

wondering639 opened this issue Apr 4, 2021 · 19 comments
Labels
Question This issue is requesting information about dotnet-format Resolved-External This issue is fixed by changes outside dotnet/format

Comments

@wondering639
Copy link

Just installed dotnet-format and tested with the following code (empty lines intended). Used command was
dotnet format Test.sln --fix-whitespace --fix-style. I expected the empty lines to be removed, but they remained. How to apply a more strict formatting that removes unnecessary empty lines?

Tested on this code, which is input and output at the sae time (meaning output remains unchanged)

using System;

namespace Test
{
    using System;
    using System.Collections.Generic;
    using System.Threading;
    using System.Threading.Tasks;

    class Program
    {
        static void Main()
        {
            DemoAsync().Wait();
        }

        static async Task DemoAsync()

        {

            var d = new Dictionary<int, int>();

            for (int i = 0; i < 10000; i++)

            {

                int id = Thread.CurrentThread.ManagedThreadId;

                int count;

                d[id] = d.TryGetValue(id, out count) ? count + 1 : 1;



                await Task.Yield();

            }

            foreach (var pair in d) Console.WriteLine(pair);

        }

    }
}
@jmarolf
Copy link
Contributor

jmarolf commented Apr 5, 2021

@sharwell is there a fixall in Stylecop analyzers for SA1507?

@wondering639
Copy link
Author

wondering639 commented Apr 5, 2021

@sharwell is there a fixall in Stylecop analyzers for SA1507?

I just read SA15007 and it's about multiple empty lines following each other directly. In my case it's only one blank line followed by code (well, there's both in my code; expectation would be too remove most of them like e.g. Prettifier in the JavaScript world does it).

@sharwell
Copy link
Member

sharwell commented Apr 5, 2021

@jmarolf SA1507 does have a code fix (as listed in the status table), but it's not the only rule that applies here. The blank lines before and after most { and } chararacters would be removed by SA1012 and SA1013, respectively.

With that said, none of these rules would apply unless StyleCop Analyzers is installed. 16.10 is the first release where Roslyn's built-in analyzers consider blank line removal, and only in a small set of cases.

@wondering639
Copy link
Author

@sharwell Being new to these things, do I understand correctly that I could add https://github.com/DotNetAnalyzers/StyleCopAnalyzers to my project and dotnet-format would than have more rules/functionality?

@JoeRobich
Copy link
Member

do I understand correctly that I could add https://github.com/DotNetAnalyzers/StyleCopAnalyzers to my project and dotnet-format would than have more rules/functionality?

@wondering639 That is right. You can add a package reference to the StyleCopAnalyzers in your projects and fix issues by using the --fix-analyzers option. In fact, we test the StyleCop empty line fixers in our unit tests.

public async Task TestStyleCopBlankLineFixer_RemovesUnnecessaryBlankLines()
{
var analyzerReferences = GetAnalyzerReferences("StyleCop");
var testCode = @"
class C
{
void M()
{
object obj = new object();
int count = 5;
}
}
";
var expectedCode = @"
class C
{
void M()
{
object obj = new object();
int count = 5;
}
}
";
var editorConfig = new Dictionary<string, string>()
{
// Turn off all diagnostics analyzers
["dotnet_analyzer_diagnostic.severity"] = "none",
// Two or more consecutive blank lines: Remove down to one blank line. SA1507
["dotnet_diagnostic.SA1507.severity"] = "error",
// Blank line immediately before or after a { line: remove it. SA1505, SA1509
["dotnet_diagnostic.SA1505.severity"] = "error",
["dotnet_diagnostic.SA1509.severity"] = "error",
// Blank line immediately before a } line: remove it. SA1508
["dotnet_diagnostic.SA1508.severity"] = "error",
};
await AssertCodeChangedAsync(testCode, expectedCode, editorConfig, fixCategory: FixCategory.Analyzers, analyzerReferences: analyzerReferences);
}

@JoeRobich JoeRobich added the Question This issue is requesting information about dotnet-format label Apr 6, 2021
@dzmitry-lahoda
Copy link

similar request #67

@JoeRobich
Copy link
Member

Closing this issue as StyleCopAnalyzers can be used to remove blank lines.

@anthony-steele-cko
Copy link

anthony-steele-cko commented Apr 16, 2021

Not working for me. What I did was

  • introduce a lot of blank lines in SomeClass.cs
  • add to the .csproj file this line: <PackageReference Include="StyleCop.Analyzers" Version="1.1.118">
  • run dotnet format --fix-analyzers --fix-style and variations on that.

Result: SomeClass.cs still has the blank lines afterwards.
All I get is Warnings were encountered while loading the workspace.

@JoeRobich
Copy link
Member

@anthony-steele-cko You will need to configure the analyzers to report a sufficient severity. You will need to either add or update your editorconfig to include the following configuration. If you do not want them to be errors in your IDE, you can configure them as warnings and invoke dotnet-format with --fix-analyzers warn instead.

.editorconfig

root = true

[*.cs]

# Two or more consecutive blank lines: Remove down to one blank line. SA1507 
dotnet_diagnostic.SA1507.severity = error
  
# Blank line immediately before or after a { line: remove it. SA1505, SA1509 
dotnet_diagnostic.SA1505.severity = error
dotnet_diagnostic.SA1509.severity = error
  
# Blank line immediately before a } line: remove it. SA1508 
dotnet_diagnostic.SA1508.severity = error

@anthony-steele-cko
Copy link

anthony-steele-cko commented Apr 17, 2021

I confirm that this works for me. This time I have a Directory.Build.Props file containing:

<Project>
  <ItemGroup>
    <PackageReference Include="StyleCop.Analyzers" Version="1.1.118" />
  </ItemGroup>
</Project>

In the .editorconfig I have file markup as given above.

Note that when opening the soltution in VS, I have errors for e.g.

Error	SA1508	A closing brace should not be preceded by a blank line.	
Error	SA1505	An opening brace should not be followed by a blank line.	
Error	SA1507	Code should not contain multiple blank lines in a row	

I run dotnet format --fix-analyzers

These are all fixed! 🎉 Blank lines are gone.

More conventional spacing, issues e.g. public SomeClass ( string pk , string sk ) was not fixed by this run, so I run dotnet format a second time with no args, and they are fixed, I get public SomeClass(string pk, string sk)

I can also confirm that without the .editorconfig markup to make these errors not warnings, dotnet format --fix-analyzers warn will remove the blank lines.

Thank you.

@sharwell
Copy link
Member

@anthony-steele-cko Note that I would not recommend ever setting analyzer diagnostics to error. This setting will impair unit testing and debugging experiences, and make it difficult to distinguish build warnings (like these) from true errors (e.g. forgetting a ;). You can set Treat Warnings as Errors to true specifically for CI builds to avoid letting warnings get merged to the product.

@anthony-steele-cko
Copy link

@sharwell I agree: as proof of concept, it works; as a way to remove blank lines using dotnet format, it comes with a huge overhead.

@JoeRobich
Copy link
Member

@anthony-steele-cko Glad things are working for you now. To fix both whitespace formatting and analyzer reported issues in the same run, you can use the command dotnet format --fix-whitespace --fix-analyzers warn.

@niemyjski
Copy link

This seems like a good thing to have without having to bring in style cop.

@ThijmenDam
Copy link

Agree with @niemyjski. This seems like such a basic feature, why rely on StyleCop?

@niemyjski
Copy link

@JoeRobich can you please reopen

@JoeRobich
Copy link
Member

@niemyjski If the request is to have these empty line formatting options in the box, then it would be best to open a feature request on Roslyn (https://github.com/dotnet/roslyn/issues). I know Roslyn has experimental analyzers for consecutive empty line and some other empty line scenarios (see dotnet/roslyn#50358).

@nhuethmayr
Copy link

Shouldn't this be part of the dotnet format whitespace option?
Especially considering that dotnet format is quite slow on large solutions. The only way for it to be acceptable as a pre-commit hook is to use the whitespace mode.

@sharwell
Copy link
Member

@norberthuethmayr whitespace options for dotnet format are handled by https://github.com/dotnet/roslyn, so this would still be considered external.

@sharwell sharwell closed this as not planned Won't fix, can't repro, duplicate, stale Mar 18, 2024
@sharwell sharwell added the Resolved-External This issue is fixed by changes outside dotnet/format label Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question This issue is requesting information about dotnet-format Resolved-External This issue is fixed by changes outside dotnet/format
Projects
None yet
Development

No branches or pull requests

9 participants