Setup common Roslyn analyzer rules #9620
Comments
Eilon
added
the
area-infrastructure
|
Build Team Triage: putting on the backlog for now. We're open to adding analyzers to greenfield code. Adding FxCop to existing code takes a lot more work. I tried adding a few analyzers like FxCop and NETCoreAnalyzers just to see what would happened, and found thousands of issues. Addressing the warnings/errors requires careful inspection of the code so that don't break existing behaviors just to fix an analyzer recommendation. If we get more time dedicated to improving code quality, we'll like to do this. @JamesNK - you mentioned a few specific analyzer rules that might have high value. Can you open individual issues for these recommendations, if they aren't already open? Existing issues that I'm aware of:
|
|
@dougbu to add PublicAPI analyzers and some baseline for adding additional analyzers here, then people can propose individual. |
|
Work on another analyzer in parallel to work on PublicAPI analyzer e.g. |
All rules are currently disabled, except for one that I enabled for src/Http via a new ".editorconfig" file I added there. Other changes: * Allow editorconfigs in MVC and Razor to flow to the root * Consolidate a few editorconfig settings * Tweak Ruleset config in Azure/AzureAD where it clashed. Addresses the beginning of dotnet#9620, but it's a fair chunk of work to enable most rules through the whole repo. That can be done directory by directory and rule by rule by dropping .editorconfig files though.
All rules are currently disabled, except for one that I enabled for src/Http via a new ".editorconfig" file I added there. Other changes: * Allow editorconfigs in MVC and Razor to flow to the root * Consolidate a few editorconfig settings * Tweak Ruleset config in Azure/AzureAD where it clashed. Addresses the beginning of #9620, but it's a fair chunk of work to enable most rules through the whole repo. That can be done directory by directory and rule by rule by dropping .editorconfig files though.
|
Closing this since @Pilchie enabled it on the repository. I've filed a separate issue to go through categories of rules and discuss and decide which ones to enable |
ghost
locked as resolved and limited conversation to collaborators
Microsoft.CodeAnalysis.FxCopAnalyzersprovides code analysis of rules ASP.NET Core code should always follow. For example, we should always consider culture when comparing strings, and converting values to and from strings. That would help avoid culture related bugs, e.g. #9537, #6059And in aspnet/Extensions we should also always require ConfigureAwait to avoid sync over async deadlocks in apps that have a sync context, e.g. .NET Core WPF. e.g. dotnet/extensions#999 Related: https://github.com/aspnet/Extensions/issues/1440
I think the number of rules enabled should be small, and only be rules that should always be followed. We don't want source code covered with suppressions.
I've just enabled the FxCop analyzers on Newtonsoft.Json and I think this ruleset is a good starting point - https://github.com/JamesNK/Newtonsoft.Json/blob/cc2ef36f5533d1ab21518a8e9ec5c2f74eb2fecc/Src/Newtonsoft.Json/Newtonsoft.Json.ruleset
The text was updated successfully, but these errors were encountered: