Add StyleCop.Analyzers and enable some basic suggestions #14074
Conversation
.editorconfig
Outdated
| dotnet_diagnostic.SA1114.severity = none # Parameter list should follow declaration. | ||
| dotnet_diagnostic.SA1115.severity = none # Parameter should follow comma . | ||
| dotnet_diagnostic.SA1116.severity = none # Split parameters should start on line after declaration. | ||
| dotnet_diagnostic.SA1117.severity = suggestion # Parameters should be on same line or separate lines. |
There was a problem hiding this comment.
Maybe not use tabs for the indentation.
Do we need rules for the .editorconfig itself ;)
|
Yes, I thought about this but the first goal was just to follow our current editor rules and the default analyzers, because it was already implying many updates. Updated Analyzers and new ones will come with net8.0, trying dotnet 8.0 I could saw new code analysis suggestions. Otherwise looks good, we will need more time to check the rules we want, at least the ones that we already follow implicitly, didn't try to define them without having to use |
|
@jtkech I cleaned up the The idea with this PR here is to include the StyleCop analyzer and then use it in other PR along with applying rules. |
|
@jtkech you recently did lots of clean up. Should we merge this one? |
|
Refer to #14074 (comment)
Yes, to follow our current rules and default analyzers, already implying many updates.
I would like too to have at least the rules that we already follow implicitly, so I'm not against. Just need to ask @sebastienros if he agrees to use StyleCop, knowing that:
Yes, if we use StyleCop I will take the time to try some rules, not sure about For info about our current rules, we have remaining violations, it looks like that when we run the code analysis it doesn't check the whole files, but I think we fixed most of the violations. |
.editorconfig
Outdated
| dotnet_diagnostic.SA1113.severity = none # Comma should be on the same line as previous parameter. | ||
| dotnet_diagnostic.SA1114.severity = none # Parameter list should follow declaration. | ||
| dotnet_diagnostic.SA1115.severity = none # Parameter should follow comma . | ||
| dotnet_diagnostic.SA1116.severity = none # Split parameters should start on line after declaration. |
There was a problem hiding this comment.
Looks like there are some remaining tab indentations
| dotnet_diagnostic.SA1505.severity = none # A closing brace should not be preceded by a blank line. | ||
| dotnet_diagnostic.SA1506.severity = none # Element documentation headers should not be followed by blank line. | ||
| dotnet_diagnostic.SA1507.severity = none # Code should not contain multiple blank lines in a row. | ||
| dotnet_diagnostic.SA1508.severity = none # Closing braces should not be preceded by blank line. |
There was a problem hiding this comment.
@jtkech
yes we could suggest more than just this one. But, I think in this PR, we should merge it as is if agreed. Once we merge it, we can use follow up PR to enable and apply which every rules we want. this way, it's easy to clean up the project using smaller PRs with one/two rules at a time.
| dotnet_diagnostic.SA1510.severity = none # Chained statement blocks should not be preceded by blank line. | ||
| dotnet_diagnostic.SA1511.severity = none # While-do footer should not be preceded by blank line. | ||
| dotnet_diagnostic.SA1512.severity = none # Single-line comments should not be followed by blank line. | ||
| dotnet_diagnostic.SA1513.severity = none # Closing brace should be followed by blank line. |
There was a problem hiding this comment.
Idem, would need a little more time to check all of them ;)
Okay then, so I think it is too soon to suggest Then I will approve and merge, unless you prefer to wait for @sebastienros, if he prefers to not use StyleCop, hmm but anyway easy to revert. |
|
@jtkech okay. I switched SA1122 to none for now. We always prefer |
|
Yes I agree but for now to not add 500 new sugestions in the current code. |
Adding
StyleCop.Analyzersto help us enforce some good practices. We should be able to apply theSA1122rule globally using thedotnet formatto replace all""withstring.Empty.Also, at some point I think almost all the rules that are defined as
Warningin the link below should be defined at least assuggestionin OC.https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/rulesets/StyleCopAnalyzersDefault.ruleset