描述
As a team, we'd like to get a core set of Roslyn code analyzers fully integrated into our build to help ensure the long term health of the code base. Much of WPF was written 10+ years ago and doesn't necessarily follow the best practices. It's important to understand what can and can't change, no change as part of this work should change the behavior.
Scope of work
- Determine which rules should and shouldn't be enabled.
- If a rule should be suppressed entirely, add the suppression to the code analysis ruleset in eng\WpfArcadeSdk\tools\CodeAnalysis\WpfCodeAnalysis.ruleset
- If a rule is to be fixed, use the associated code-fixer to make the changes, instead of by hand.
Projects to enable
Here are the assemblies that we should focus on. Ideally done one at a time so that PR's are easy to read and understand. They can follow what we did with System.Xaml for an example. It's recommended to start with one of the smaller projects first :)
- System.Windows.Controls.Ribbon
- ReachFramework
- UIAutomationClient
- UIAutomationProvider
- UIAutomationTypes
- PresentationBuildTasks
- PresentationCore
- PresentationFramework
- WindowsBase
Note: this work isn't necessary for test projects
Methodology
- To make this work easier to understand for the maintainers, it should be easy to differentiate at the commit/pr level what was a suppression versus what was a fix. This is not a hard requirement, but it will make the life of anyone reviewing the code much easier.
- Add
<EnableAnalyzers>true</EnableAnalyzers>to the .csproj file and build. There will most likely be a lot of errors, this is where the fun begins :) - Part of determining the proper rules is based on the current state of the code, and not necessarily what is ideal. Too many changes to the code will result in PR's that are too lengthy and cumbersome to review, which isn't ideal for anyone. Separate issues can be filed if there are certain rules we want to enable in the future.
- There will be times where we'd like a rule to be enabled for most projects, but occasionally suppressed.
- If the suppression will never be fixed (i.e. for compat reasons) put the suppression in a GlobalSuppressions.cs file.
- If the suppression should be addressed a later time, put the suppression at the callsite.
Code-fixers
We should run the code-fixers for each analyzer with the enabled rules and auto-update the source code, rather than making the edits by hand. Some of the fixers may not be implemented, and it would be extra valuable to go and add the fixer so that everyone in the .NET community can benefit from it. I did this for one of the fixers early on, and here is an example PR: https://github.com/dotnet/roslyn-analyzers/pull/2166
All the analyzers and associated code-fixers are in this repo: https://github.com/dotnet/roslyn-analyzers
Running the code-fixers can be done when the solution is loaded in Visual Studio, or by using this tool (I don't know what state this tool is in or if it works, there may be bugs) https://github.com/stevenbrix/AnalyzerRunner
This is work that we've had on our backlog for a while, but isn't the highest priority at the moment for us. There have been a lot of code cleanup/beautification PR's to the WPF code base since it's been open-sourced (thank you everyone!), and so I'm hoping to formalize this work a bit and open it up to anyone that is interested in defining the best-practices in this code base, as well as learn a bit about Roslyn analyzers while doing it!
Please reply if interested and with the project (or projects) you will be working on and I will assign this to you!
/cc @YoshihiroIto who expressed interest in learning more about Roslyn analyzers.