Sorting of using directives

[NonSpicyBurrito]

I would like using directives to be sorted with these rules:

• System on top.
• The rest sorted alphabetically.
• All using static at the end which follows the same rules.

Example:

using System;
using Bar;
using Baz;
using Foo;
using static System.Math;
using static Bar.BarHelper;
using static Foo.FooHelper;

[belav]

I personally also like sorted imports, but I don't know if csharpier should be concerned with sorting them.

So far csharpier has followed a lot of the rationale of prettier. Part of that is avoiding transforming any code and sticking to just printing code (adding/removing whitespace). This makes it fairly straightforward for csharpier to validate that changes it made do not change the behavior of the code.

There are some other options for sorting usings. At work we use a combination of stylecop + csharpier.
It looks like dotnet format can also sort usings with https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/formatting-rules?view=vs-2019#organize-using-directives

[NonSpicyBurrito]

Related Prettier issue.
The big point is that import sorting in JS can potentially mess up your code because imports can have side effects, which is not the case for C#.

I do understand not wanting to do it because of the added complexity, but having to use multiple tools to handle formatting is not exactly ideal. I hope you can consider it more.

[belav]

I think I am starting to feel less and less married to the idea that csharpier will never transform code. There are a lot of useful things it can't do if the only thing it worries about is whitespace. The challenge will be figuring out how to get the validation code to understand the transform. Although if validation is opt in, maybe that is less important.

Another argument for sorting usings is that it can help avoid merge conflicts. I don't know that I've run into it often for c# (maybe because most IDEs auto add usings sorted) but I remember searching for an eslint plugin to do it specifically because of how often we ran into merge conflicts on imports in typescript files at work.

[belav]

Conditional compilation is going to cause some headaches on this. An example

using System;
using System.Collections.Generic;
using Insite.Common.Dependencies;
using Insite.Common.Providers;
using Insite.WebFramework.Templating;

#if NET6_0_OR_GREATER
using Microsoft.AspNetCore.Html;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Razor;
using Microsoft.AspNetCore.Mvc.Rendering;
using Microsoft.AspNetCore.Mvc.Routing;
using Microsoft.AspNetCore.Mvc.ViewFeatures;
using Microsoft.Extensions.Caching.Memory;
#else
using System.Web;
using System.Web.Caching;
using System.Web.Hosting;
using System.Web.Mvc;
using System.Web.Mvc.Html;
using System.Web.Optimization;
using System.Web.WebPages;
#endif

[NonSpicyBurrito]

Wouldn't it to be simply sorting these 3 blocks of usings separately?

[belav]

I think that's the proper way to handle ii. I don't think it will be too difficult to handle but wanted to make sure I didn't forget about it. There is the potential to introduce compilation errors if the usings are moved outside of the scope of an #if.

[snebjorn]

While a solution is found for sorting import with CSharpier could an option to trigger the import formatter from OmniSharp be added to the VSCode extension?

omnisharp.organizeImportsOnFormat

The issue is that only 1 formatter can be set in VSCode. So I have to do CTRL + SHIFT + P then select "Format Document With..." then select "C#" and then save the file to trigger formatting with CSharpier.

[Sti2nd]

I think one should be careful with increasing the scope of CSharpier. I greatly appreciate that CSharpier only formats white spaces. One tool with one job is easy to understand; "Csharpier formats white space". I think this simplicity made it easier for me to convince my team to use CSharpier as they were initially not convinced to use it in favor of dotnet format whitespace.

CSharpier brings something new that dotnet format doesn't have; wrapping long lines 😍. Sorting usings is something there exists tools for already so it will be yet another implementation.

If this feature is introduced please make sure that it plays nice with the other two dotnet format commands; dotnet format style and dotnet format analyzers.

[Sti2nd]

A colleague said yesterday that it would be nice if Csharpier removed unused usings. So if sorting is implemented, it makes sense to implement that as well. (Mind the feature creep)

[viniciuschiele]

Prettier for Java sorts imports and personally I find it great.

Docs: https://github.com/jhipster/prettier-java#organize-imports
PR: jhipster/prettier-java#281

[belav]

A colleague said yesterday that it would be nice if Csharpier removed unused usings. So if sorting is implemented, it makes sense to implement that as well. (Mind the feature creep)

I'd love an automatic way to do that but don't believe CSharpier is the place for it. CSharpier only understands the syntax of the code and knows nothing of the semantics. Rider screws up removing unused usings in our multi-targeted project at work so it seems like a hard problem to solve and could be a bit too dangerous to do in an automatic way.

[Sti2nd]

Absolutely agree! I think CSharpier should format white space, and just that. Removing and sorting usings should be the job of another tool. It is already possible in all major IDEs, so it should also debated why we need yet another tool to do the same.

[NonSpicyBurrito]

Absolutely agree! I think CSharpier should format white space, and just that. Removing and sorting usings should be the job of another tool. It is already possible in all major IDEs, so it should also debated why we need yet another tool to do the same.

By this point, CSharpier presumably has already moved past "format whitespace only" philosophy, as seen with the latest feature of reordering modifiers like public override async. Lots of useful features aren't going to be possible with just formatting whitespace, not limited to this.
As for why we need "yet another tool" to format usings, imo all of the same benefits that apply to using CSharpier to format whitespace rather than IDE, still apply here.

[belav]

This ended up being a bit tricky, but I have it working this way as of now.

// comments or non #if directives that appear at the top of the file are assumed to be file headers and kept there.
// later comments stay with the using they are above
// blank lines around usings are not retained as of now

#nullable enable

global using System.Linq; // sort global first
using System; // sort System next
using NonSystem; // then sorted non-system
#if DEBUG // then any #if
using Z; // contents are not sorted as of now
using A;
#endif
using static Static; // then sorted static
using Alias = Z; // finally alias sorted by Alias, not Name
using SomeAlias = A;

Retaining blank lines would be nice, I've seen some repos that use blank lines to organize into groups. Sometimes between system and non-system but also sometimes between groups within the non-system.

using System;
using System.IO;

using Company.Feature;
using Company.MoreFeatures;

using Microsoft.Azure;
using Microsoft.Bing;

Keeping the lines when sorting often ends up with them in places you wouldn't expect. It could turn into a "format, see where the blank lines end up then move them if needed" situation.

Sorting usings within #if seems less important. Having #if's in usings doesn't appear that common, and usually there are just a couple of them when they do appear.

[Sti2nd]

@NonSpicyBurrito By this point, CSharpier presumably has already moved past "format whitespace only" philosophy, as seen with the latest feature of reordering modifiers like public override async.

Scary! 👹 In general, formatting white space in a language that doesn't care about white space seems safe. In general, reordering lines doesn't seem safe because languages do care about line order. Not saying it is impossible, and of course it can be useful.

As for why we need "yet another tool" to format usings, imo all of the same benefits that apply to using CSharpier to format whitespace rather than IDE, still apply here.

Absolutely agree on that a CLI tool or .NET plugin for code cleanup is better than using an IDE. One can use the already existing Resharper to sort usings pre-commit.

I think it can be wise for a tool to have a clear philosophy of what features it should offer. I thought CSharpier was a white space formatter, which it seems to be growing out of. So what is CSharpier (becoming) @belav? :) Maybe Csharpier will become a formatter and move code around as well but never remove any? Or will it become a code cleanup tool and format and remove code based on some (linter) rules?

And again I want to mention that CSharper is a nice replacement for dotnet format whitespace. Not sure I will use it if it disagrees with dotnet format style and dotnet format analyzers

[gabynevada]

It could be provided as an option, prettier defintely allows import sorting via plugins and it's very helpful. It makes the codebase more consistent and allows for manual code review to be more about functionality instead of styling, formatting or sorting.

On the philosophy front, not really a fan of dogmatic approaches that can't be changed later on. Csharpier as an opinionated prettier like tool for c# with most of it's functionality would be awesome.

Thanks for the great work @belav

[Sti2nd]

CSharpier has a "no option" philosophy, @gabynevada :) Plugins would be cool!

Csharpier as an opinionated prettier like tool for c# with most of it's functionality would be awesome.

Isn't this exactly what CSharpier is now?

[belav]

I thought CSharpier was a white space formatter, which it seems to be growing out of.

It was originally @Sti2nd . I liked the firm stance that black had in that regard. It was an easy line to draw to say no to requests. But there are some non-whitespace changes that I think belong and I've struggled to put into words what I think belongs, which lead me to putting off responding to this for a bit too long.

Some things I do think belong

1. Ordered Modifiers - I don't think anyone really argues about what order these should appear in, and code looks wrong when they are out of order. It is kind of style.... maybe. It was also easy to do and it is nice to have them cleaned up on save just in case they end up in the wrong order.
2. Trailing commas - I like these in theory and hate them in practice. When a rule enforces that I have to add them, I never remember to add them and spend more time going back to add them after the rule complains. But when they are there then diffs are cleaner, moving parameters around is cleaner, etc. If these are auto added by a tool then no one is disrupted by a rule saying "you forgot to do this" and you get all of the benefits
3. Sorted usings - We had so many conflicts on a big typescript project I worked on somewhat recently until we enabled the eslint rule for sorting imports. I don't think there is a good case to make for not having them sorted. It helps avoid conflicts. If you remove and then auto-add a using in the same commit you don't want it to shift to a new location. It falls into that grey area of possibly being style, much like ordered modifiers.

Things I don't think belong

1. Removing braces on ifs (and other similar blocks) with a single statement in the block. This one feels problematic. If someone deletes the second statement from a block, hits save while they are thinking, and the braces disappear. They are going to be annoyed that they need to re-add them. I don't know that there would be a way for csharpier to understand when someone wants a second statement to be included in the block.
2. Auto adding semicolons - it isn't possible because without them there is no valid syntax tree. Prettier can do it because js is valid with or without them.

Things that I haven't thought about much, but could be debated

1. Adding/removing parentheses on lambdas with a single parameter. Prettier auto adds them. I don't know that I'd love that with all the linq statements.
2. Converting methods/properties into expression bodies - this seems like a no.
3. Adding braces to ifs with a single statement - I assume there would be a lot of backlash for this.

My attempt to put the new mostly whitespace only changes philosophy into words - When there are non-whitespace changes they should help avoid friction and keep code cleaner in diffs. Not change the meaning of the code. Be done to improve readability and avoid conflicts.

I don't imagine that there will be that many non-whitespace changes implemented. They are somewhat of a pain do deal with in the SyntaxNode comparing code that is there to ensure csharpier doesn't make any unintentional semantic changes.

Plugins would be cool!

I have a branch with csproj formatting partially done. It moves the codebase in a direction where plugins for formatting new languages would be possible. I don't know that plugins to modify the existing formatting would be possible. From my knowledge of prettier plugins, they can support parsing and printing new languages, but aren't able to modify the existing formatters.

Thanks for the great work @belav

You're welcome! @gabynevada

[BenjaBobs]

Thoughts on putting the #nullable enable before or after using directives?

#nullable enable

using System;

vs

using System;

#nullable enable

As far as I can tell, Visual Studio doesn't have an opinion on the matter, but it seems that Rider prefers to put #nullable before using directives.

[gabynevada]

I usually prefer it before the using statements, makes it easier to see what files have the nullable config at first glance.

[belav]

I also prefer it before using statements at the beginning of the file. The microsoft examples seem to usually place it after the usings for some reason.

[parched]

It would be nice if it kept the space between different groups like the IDE (Visual Studio) adds. Alternatively, I'd be happy if this could be turned off and left to the IDE.

[belav]

It would be nice if it kept the space between different groups like the IDE (Visual Studio) adds. Alternatively, I'd be happy if this could be turned off and left to the IDE.

I had a similar thought while looking at one of the test cases. I created #988 to look into it.