Changes to our Code Style Guidelines

[siegfriedpammer]

The current code formatting of the ILSpy code base has some problems:

1. Often lines get very long, due to the following:

   • All our transforms use either pattern matching or out var declarations in combination with long &&/|| chains
   • Developers often use very wide monitors, implicitly encouraging use of the horizontal space
   • We have no maximum line length defined in our style
   • Visual Studio does not support automatically breaking up long lines

2. Manually breaking up conditions into multiple lines comes with putting the opening brace on the next line for readability - as opposed to all the other expressions where we put the opening brace on the same line.

3. Historically ILSpy has used the SharpDevelop code formatting style. However, point 2 mentioned above causes problems with it and its use in Visual Studio. The IDE will always put the opening brace on the previous line when trying to reformat the code. This makes using the auto formatter a frustrating experience.

In order to partially solve this problem, we decided to use "opening brace on the next line" for all statements.
In order to make indentation less of a problem, we also decided to stop indenting namespace blocks. However, because this is currently not supported by the Roslyn formatter, we will have to make this change at a later point.

We also discussed adding dotnet-format --check to our build pipeline and we will extend our indentation checker script ("tidy.py") to allow enforcing a maximum line length.

The maximum line length will be set at 110 columns. The reasoning behind this is that with 110 columns it is still possible to view two files side-by-side on an HD screen, which comes in handy when reviewing code.

We also tried clang-format and its Visual Studio extension. The results of the line-breaking algorithm were quite pleasing - as it is highly customizable - however, as it does not support all C# features (for example, LINQ clauses and named arguments are not interpreted properly), it is impossible for us to rely on clang-format, as we make use of named arguments to improve readability in case of bool flags.

Tools to be used:

• dotnet-format: https://github.com/dotnet/format
• editor guidelines: https://github.com/pharring/EditorGuidelines
• Format document on save in Power Commands for Visual Studio, included also via Productivity Power Tools 2017/2019 - to be recommended to contributors

The reformatting is to be done in a single commit as GIT_AUTHOR="dotnet format <[email protected]>". We will add that commit to .git-blame-ignore-revs.

References:

• Issue regarding enforcing max line-length: Feature request: Allow me to specify a desired max-column that code should be wrapped against. dotnet/roslyn#15406
• Issue regarding namespace indentation: Add "IndentNamespace" formatting option. dotnet/roslyn#24129

[sharwell]

The maximum line length will be set at 110 columns. The reasoning behind this is that with 110 columns it is still possible to view two files side-by-side on an HD screen, which comes in handy when reviewing code.

💡 Roslyn's line wrapper is hard-coded to 120 columns, so if you want to use the available refactorings then this will need alteration.

[siegfriedpammer]

The maximum line length will be set at 110 columns. The reasoning behind this is that with 110 columns it is still possible to view two files side-by-side on an HD screen, which comes in handy when reviewing code.

💡 Roslyn's line wrapper is hard-coded to 120 columns, so if you want to use the available refactorings then this will need alteration.

Thank you for pointing this out. Enforcing formatting and line length during build is the next step. However, we have a large amount of lines that violate this new rule. I fear we will have to manually adjust all these places, so it will take some time until we can actively enforce it. I am hoping that Visual Studio will have a more advanced and customizable line breaking / aligning algorithm built-in before we reach that point. 😉

@sharwell I am not sure, whether this is a bug or a feature:
when using dotnet-format I have one file of test cases, where some tuple expressions combined with delegates are formatted in an unexpected way:

diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/TupleTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/TupleTests.cs
index a5581f48a..4f4c19dac 100644
--- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/TupleTests.cs
+++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/TupleTests.cs
@@ -92,10 +92,12 @@ public struct GenericStruct<T>
 		public (int, int) AccessRest => (1, 2, 3, 4, 5, 6, 7, 8, 9).Rest;
 
 		public (string, object, Action) TargetTyping => (null, 1, delegate {
-		});
+		}
+		);
 
 		public object NotTargetTyping => ((string)null, (object)1, (Action)delegate {
-		});
+		}
+		);
 
 		public void UnnamedTupleOut(out (int, string, Action, dynamic) tuple)
 		{

Looking at our https://github.com/icsharpcode/ILSpy/blob/master/.editorconfig, would you happen to know the setting that causes this? Should I file a bug report with Roslyn?

Thank you very much!

[sharwell]

some tuple expressions combined with delegates are formatted in an unexpected way

I'm guessing this is a Roslyn bug.

[christophwille]

Notes for build server integration - those links might help:

• AppVeyor https://github.com/stripe/stripe-dotnet/pull/2079/files
• Azure DevOps https://www.meziantou.net/enforce-dotnet-code-style-in-ci-with-dotnet-format.htm