Automated OCD with Stylecop
I never really got into Stylecop or FXCop. I did make use of some of the Resharper autoformatting configuration, and static analysis features, and have in the past automated and configured this.
But since starting to use Visual Studio Code, I have been looking for more and more ways to get the benefits of the Visual Studio Gui tools, but as command-line alternatives. This leads me to the Roslyn Analyzers.
I'm gonna preempt typo considerations here. The technology is Rosyln Analyzers, with a Zed (Z), which while hurts my spelling sensibilities, it is a name, so I am going to respect that spelling. Much like using libraries that use color instead of colour, most of us non-US English speakers are used to this anyway. But I digress.
A gentle reminder
This is the cool new latest-and-greatest technology of choice. This means two things: If you are not using the latest dotnet core project structure (1.1 or greater), this might not be for you; and you may come across bugs and errors that require bug reports to be posted, or custom rule configuration to disable some rules you don't like.
As of Visual Studio 2015, A new concept of Code Analyzers was introduced. 2017 has taken these further with analyzer APIs supporting automated code fixes as well.
A great example of this coming to fruition is the latest version of the xUnit NuGet packages. Since xUnit version 2.3.0 the xUnit.analyzers has been bundled as a dependency with xUnit. When you compile, you will see warnings and errors on how you use their library. Basically, as well as the interfaces of what you can do with their library, you also get style guidance on how you use them. For example, if you write a test with an Assert.Equal with boolean parameters, you get an Analyzer error:
The neat thing about this error is that it is actually a compiler error. You cannot compile and run your code unless you fix it! Amazing!
(You can find documentation of xUnit's rules here: https://xunit.github.io/xunit.analyzers/rules/)
How does this work? Well, NuGet packages are required to be restored before building, or the build fails. Packages result in assembly references, as well as loading MSBuild target files, and also Rosyln Analyzers. All of this is fed into the Roslyn compiler. The compiler executes the Analyzers, and they come out with other Compiler Info/Warning/Error messages. If there are any errors, the build fails. No need for anything more than your compiler to make the most of these checks, which means they not only work with Visual Studio, but also works when using the dotnet cli only as well.
As you can see above, the analyzers also get loaded by the IDE, and can give red underlining of errors, and, if available, automatic refactoring, too!
The best bit, because they do not require any extra tools or processes to be run on your code, you fix the issues as you make them, not later on when you run a special manual post-processing script before checking in, or on your Pull Request Builds. No chance of creating code bugs from restyling your code, if you have to fix all before it compiles, and you have to compile before you test your work.
There are a ship-load of analyzers already out there, and more NuGet packages are likely to ship with Analyzers going forward. (As well as the xUnit Analyzers, I also found one distributed with FluentAssertions, and there are many other custom ones.)
I found an article that pulled out a few key ones from 18 months ago that are still going strong and have found they provide good coverage to tune your code by.
Microsoft.CodeAnalysis.FxCopAnalyzers(an aggregate over several Rosyln Analyzers from Microsft)
codecracker.CSharp(The first OpenSource Analyzer project I heard about)
SonarAnalyzer.CSharp(If you've used SonarCube in the past, this is their modern approach so analyzers)
StyleCop.Analyzers(StyleCop new and shiny using the .NET Compiler Platform)
I've gone whole-hog and added all four to some of my projects, and so far so good. Each project is individually tunable, so you can disable rules as needed, and some give better support for the automated refactorings that others do.
Microsoft.CodeAnalysis.FxCopAnalyzers is in beta, and chasing the tails of the Roslyn Compiler's newest features, so have a look at https://github.com/dotnet/roslyn-analyzers#recommended-version-of-analyzer-packages to make sure you pick the version most compatible with your minimum compiler version your team/project is using.
Because it is so cutting edge, there is an advanced compiler feature you have to enable to get the rules working with 2.3.0-beta1:
<ItemGroup> <Features>IOperation</Features> </ItemGroup>
The latest on this is:
IOperation API shipped in Visual Studio 2017 15.5 Preview5, and we have released fully supported version 2.6.0 analyzer packages that should work on all future compiler/Visual Studio versions.
First Helpful Trick:
If you are producing NuGet packages, you might not want everyone using your project to be forced into using these all themselves.
PrivateAssets="All" Solves this!
If you use this on your Package Reference in the
csproj file, then it will not inherit these dependencies into your built package.
<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.3.0-beta1" PrivateAssets="All"/>
Second Helpful Trick:
You have a lot of projects. I don't mean 2-3, I mean a lot. You have so many
csproj files, that trying to add this to your projects is daunting.
Enter MSBuild. There is an under-utilised feature of MSBuild that allows you to add Directory.Build.Props files to your parent directories, and share common settings across your projects. Now that man-handling
csproj files is much calmer and simpler, this is super useful for adding common settings to one place that should be the same across projects. (You can also replace/remove/override these in child folders and
csproj files as well.)
<?xml version="1.0" encoding="utf-8"?> <Project> <PropertyGroup> <Authors>csMACnz</Authors> <DebugType>full</DebugType> <NeutralLanguage>en-NZ</NeutralLanguage> <VersionPrefix>0.0.1</VersionPrefix> <TreatWarningsAsErrors>true</TreatWarningsAsErrors> <Company>csMACnz</Company> <Product>MyWidgetFizzBuzz</Product> <PackageTags>Fizz; Buzz</PackageTags> <Copyright>Copyright © csMACnz 2018</Copyright> <LangVersion>7.1</LangVersion> <Features>IOperation</Features> <CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)/ruleset.ruleset</CodeAnalysisRuleSet> </PropertyGroup> <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'"> <DebugSymbols>True</DebugSymbols> </PropertyGroup> <ItemGroup> <PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.3.0-beta1" PrivateAssets="All" /> <PackageReference Include="SonarAnalyzer.CSharp" Version="18.104.22.16869" PrivateAssets="All" /> <PackageReference Include="codecracker.CSharp" Version="1.0.3" PrivateAssets="All" /> <PackageReference Include="StyleCop.Analyzers" Version="1.1.0-beta004" PrivateAssets="All" /> </ItemGroup> </Project>
As well as adding the selected analyzers to every project, this can handle common metadata like
Copyright and compiler options such as
VersionPrefix. You are actually removing more clutter from your
csproj files, making them super slimmed down.
Exception to the Rule
You have added the Analyzers, and you now have thousands of failing errors in your project. Some of these you will want to actually fix now, others fix later, and some just don't matter and the rule needs to be turned off.
You can use a
.ruleset xml file to configure rules. This is done using the
<CodeAnalysisRuleSet> section tag in your project file. Alternatively (as above) you can use
<CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)/ruleset.ruleset</CodeAnalysisRuleSet> in your
Directory.Build.Props file, and have a shared
ruleset.ruleset xml file next to it at the root of your repository.
Ruleset files can also include other ruleset files so you can make these modular, or heirarchical. (This is useful if you want slightly different rules for one project. Override
<CodeAnalysisRuleSet> in your project to use it's own file, and include the main
.ruleset file from the project's own version.)
Great advice is to copy someone else's configuration file as a starting point, then disable anything you still don't like and then schedule time to review the existing exclusions possibly to turn them on again. (If the other project is yours, it probably doesn't need much tweaking.)
A large number of the errors you will see will be "Fix spacing". The default rules seem perfectly reasonable so I recommend using the fix all across Solution auto-fix for these first, to find where the real problems lie. You could also go through and disable all the rules that are failing and come back one or two a sprint to slowly improve things, too.
I guarantee that you will find at least one issue that will make you say "ooh, yip, that's an exception waiting to happen".
Fixing some of these issues will break existing contracts. If this is a library that is shared with others, be sure to follow SemVer for breaking changes, or add exceptions around areas that could break existing contracts, with a comment explaining why you have violated the rule, so it can be fixed in your next lot of breaking changes. (You can do this by wrapping your code in
#pragma warning disable CA1034
#pragma warning enable CA1034)
I've done a bunch of analysis on the Analyzers. I can tell you that they do not drastically increase your build times enough to not use them. Yes, there is an increase, but it is not enough in practice to slow down your build/test development cycle. It respects the existing build caching and only runs if you actually need to do a rebuild of a particular project.
It might cause you to spend more time on the code formatting as you write it, but that is going to reduce errors you might have added if you came back later and tried to fix them out of context.
Given the four Analyzers above, here are some rough results (on an application that takes about 1 minute to build a project one of my projects.)
Some quick stats: 24019 lines of code, 497 C# code files, 33 projects, (incl. 6 application component projects, 7 Test projects)
|Configuration||Number of Analyzers||Analyzer Execution Time|
|All 4 Analyzers||277/729||~71.9 seconds|
Basically, if you are worried about performance, don't use SonarAnalyzer. Another reason to not use SonarAnalyzer is that it was the one I had to disable the most analyzers from. If you are really really worried, I recommend just using
StyleCop.Analyzers for the best coverage vs time ratio.
You can also see from these stats that there are more Analyzers available in these packs than are enabled to run by default. It might be worth looking into what you are missing that you want to enforce.
Do you use editor config? You should use editor config.
Once you start using Code Analyzers, specifically the StyleCop Analyzer, you may find the default code generation rules in Visual Studio and VSCode (or even Resharper) might fight against you. Luckily you can use EditorConfig to tell Visual Studio, VSCode, and even Resharper the specific way you want your code generation to be formatted as. The
.editorconfig file checks into your repository, so you can make this configurable per repository.
Getting it working on your Build Servers
This is short and sweet. Your build server will just fail to build on analyzer errors, with build errors. Because its all part of the compile pipeline in Roslyn now.
Some StyleCop Rules that I turn off
This contains the opinions of Me, the Environment I've learned C# in, and possibly that of my Team. Possible controversy ahead.
- SA0001 - Xml Comment Analysis Disabled
- I don't use XML documentation, so this rule is disabled
- SA1101 - Prefix Local Calls With This
- This is unnecessary since I prefer the underscore prefix of private fields. Never use
this, it is redundant.
- This is unnecessary since I prefer the underscore prefix of private fields. Never use
- SA1118 - Parameter Must Not Span Multiple Lines
- You probably don't need this, but for syntactic reasons, some fluent code building reads better allowing this and I use it a lot.
- SA1200 - Using Directives Must Be Placed Correctly
- Stylecop seems to want them inside namespaces, despite all tooling I've ever used has always placed them outside. Turning this on will confuse more than assist.
- SA1201 - Elements Must Appear In The Correct Order
- If you are starting a new project, then this might appeal, but the code churn in git of turning this on is worse than the benefits. Also, automating to match this ordering is non-obvious
- SA1202 - Elements Must Be Ordered By Access
- Similar to the above, ordering is hard on an existing project, and I like private fields at the top for some reason (History). You might still find it gets in the way of your preferred ordering, anyway.
- SA1204 - Static Elements Must Appear Before Instance Elements
- As with SA1201 and SA1204 above.
- SA1309 - Field Names Must Not Begin With Underscore
- Related to SA1101, I have a preference for using underscores here.
- SA1413 - Use Trailing Commas In Multi Line Initializers
- o rly? I can see the appeal for merging actually, but the compiler will catch it, and that's what Build servers and PRs are for. My pedantic nature wants to remove it, so I do, and this rule is off.
- SA1600 - Elements Must Be Documented
- In an ideal world of a pristine reference code base, this must be amazing to have. For an application where comments get out of sync faster then the build can run, Nope.
- SA1602 - Enumeration Items Must Be Documented
- Docs again, as above.
- SA1633 - File Must Have Header
- Same opinion as SA1600 above
There are probably other (related) rules, but I haven't hit them yet, so haven't turned them off.
Go forth and automate the improved quality of your Repository! You have the tools, instructions, and if you have read this far the inclination to make it happen.