Code Style Initiative

[harshit7962]

Description

This is a tracker issue to ensure that our code style and guidelines align with the standards set by dotnet. This is a follow-up of the issue .

As in the PR we have added .editorconfig that helps us ensure these style requirements during the build. Given the current state of code, quite a few of these analyzers are disabled in src/Microsoft.DotNet.Wpf/src and src/Microsoft.DotNet.Wpf/cycle-breakers in their respective .editorconfig. The overall idea is to get rid of .editorconfig(s) in the src and have a single .editorconfig (the one in the root).

Steps to proceed with the task

Assume that we are fixing IDE0044: Make field readonly

1. Remove the analyzer's severity from .editorconfig in src folder. It is generally added as dotnet_diagnostic.IDE0044.severity = suggestion.
2. Using the script start-vs.cmd, open WPF in Visual Studio
3. Build the solution (if not already built).
4. The build is likely to fail with errors/warnings of IDE0044.
5. Click on any of the warning/error from Error List window, and it should navigate to the line that violates the rule.
6. On that line, use the shortcut Ctrl + . or right click to find Quick Actions and Refactoring
7. It should open the possible fixes, one of which would be related to the warning being fixed (as in the image below).
   
8. As in the image above, click on Fix all occurrences in Solution, which should resolve the warning in the solution.

Alternatively

After the step 1, use the command
dotnet format analyzers --diagnostics IDE0044 --severity warn .\src\Microsoft.DotNet.Wpf\src\PresentationBuildTasks\PresentationBuildTasks.csproj. This will fix occurrences of IDE0044 violations in PresentationBuildTasks.csproj. Similarly, fix the occurrences in all the csproj.

Raising the PR

• There should be no changes to the public API contract. If there are violations, instead of fixing them, suppress the warning.
• Apart from the suppressions, ensure that all the changes are automated.
• There should be no behavioral changes in these PRs; they should only comprise the fixes for these analyzer changes.
• Unless the changes in one resolution are very small (10-20 files), raise a single PR for a single fix. This will help in the easier review process.
• If fixing a single warning changes a lot of files (>=500), try creating sub-PRs for individual assemblies if that helps reduce the count.
• Name the PR with the pre-text [StyleCleanUp] for better visibility.
• As with other PRs, each PR will be taken for a test pass.
• Try branching out fixes on top of others to minimize merge conflicts. Let us know the order in which you would like the PRs to be taken.

Warnings to be removed

Feel free to create individual sub-issues and assign it to yourself before picking up any of these.

• CA1036: Override methods on comparable types #10271
• CA1052
• CA1066
• CA1067
• CA1070
• CA1310
• CA1419
• CA1507: Use nameof in place of string #10294
• CA1510
• CA1512
• CA1513
• CA1514
• CA1725
• CA1802
• CA1805
• CA1810
• CA1815
• CA1821
• CA1822
• CA1823
• CA1825
• CA1834
• CA1838
• CA1845: Use span-based 'string.Concat' #10298
• CA1846: Prefer AsSpan over Substring #10299
• CA1847: Use String.Contains(char) instead of String.Contains(string) with single characters #10297
• CA1851
• CA1852
• CA1853
• CA1854
• CA1858
• CA1859
• CA2011
• CA2019
• CA2020
• CA2201
• CA2207
• CA2208
• CA2211
• CA2219
• CA2242
• CA2249: Consider using String.Contains instead of String.IndexOf #10282
• CA2251: Use String.Equals over String.Compare #10296
• CA2300
• CA2302
• CA5350
• CA5351
• CA5362
• CA5394
• CSIsNull001
• CSIsNull002
• IDE0005
• IDE0017
• IDE0019
• IDE0020
• IDE0029
• IDE0030
• IDE0031
• IDE0034
• IDE0036
• IDE0040
• IDE0041
• IDE0044
• IDE0051
• IDE0052
• IDE0057
• IDE0059
• IDE0060
• IDE0062
• IDE0063
• IDE0065
• IDE0066
• IDE0073
• IDE0074
• IDE0075
• IDE0076
• IDE0077
• IDE0078
• IDE0082
• IDE0100
• IDE0150: Prefer 'null' check over type check #10272
• IDE0180
• IDE0251
• IDE0270
• IDE0300
• IDE1006
• SA1001
• SA1005
• SA1014
• SA1027
• SA1028
• SA1121
• SA1129
• SA1131
• SA1206
• SA1400
• SA1404
• SA1408
• SA1500
• SA1505
• SA1507
• SA1508
• SA1513
• SA1518
• SYSLIB1045

[h3xds1nz]

Think you need at least triage rights on the repo to create or link sub-issues by the way.

[harshit7962]

Ah, didn't realize it.
Would request anyone picking an issue to comment it here and then we create the sub-issue.

[h3xds1nz]

I also don't think we should enforce the ones that are just suggestions in .editorconfig

wpf/.editorconfig

Lines 260 to 261 in 2fa8170

	# IDE0057: Use range operator
	dotnet_diagnostic.IDE0057.severity = suggestion

e.g. There's no real gain in IDE0057 for example (tbh personal bias, I don't like them 😀 )

[harshit7962]

@h3xds1nz, yeah we are of the same opinion here. Not enforcing the ones which are suggestions. I did not see that IDE0057 is a suggestion even in the root config.

I believe using the dotnet format option with --severity set to warn will not enforce such analyzers, but the visual studio might show a possible fix. So to avoid the confusion, will mark this one as resolved.

Thanks for pointing this out.