Consider moving all Code Analysis, Style Cop settings to .editorconfig

Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.

MIT License

5.46k stars 4.8k forks source link

Consider moving all Code Analysis, Style Cop settings to .editorconfig #35874

Open heaths opened 1 year ago

heaths commented 1 year ago

Right now we have a mix of style preferences / analysis rules in .editorconfig and _eng/CodeAnslysis.rulset. We should consider,

[ ] Moving everything to .editorconfig, which supports both styles and general StyleCop/CodeAnalysis errors e.g., dotnet_diagnostic.CA2000.severity = error.
[ ] Enabling more CodeAnslysis rules like those around IDisposable to free up any unmanaged resources immediate, especially when it comes to MemoryStream or the like which we use heavily.

See https://github.com/Azure/azure-sdk-for-net/pull/35852#discussion_r1179555190 for context.

pallavit commented 1 year ago

Looks like our codeanalysis.ruleset is in a need of a refresh so that would be goodness for sure. However, what would be the benefit of moving all into .editorconfig? I have always seen both rulesets being used side by side.

heaths commented 1 year ago

Why maintain both copies? We also have style rules in .editorconfig that have "duplicates" in the ruleset. You don't want those getting out of sync or it's very confusing, so we have to maintain a sort of duplication there e.g. regarding use of var, usings, etc. Seems better to put them all in a single place.

pallavit commented 1 year ago

I did not realize both can be coded in .editorconfig. No concerns from my side. As we update the editorconfig, should we use the .NET repo one as the baseline/starting point.

heaths commented 1 year ago

After some basic testing, the only reason I can see for keeping stylecop/code analysis rules in a .ruleset is that VS can automatically modify the file when people change settings in VS. Then again, do we even want to make that easy? I'd argue, "no."

pallavit commented 1 year ago

After some basic testing, the only reason I can see for keeping stylecop/code analysis rules in a .ruleset is that VS can automatically modify the file when people change settings in VS

Do we get a prompt for yes or no? If so that does bring back some value.

heaths commented 1 year ago

Could you clarify what you mean?

In VS, you can expect Properties -> Analyzers and change specific analyzer codes to different severities. That impacts the entire repo, hence why I don't think supporting that is desirable.