[StyleCleanUp] Addressing CA warnings Part 2

[himgoyalmicro]

Fixes #10294

Description

This work is a part of our initiative to set code-style guidelines, align WPF and WinForms, and ensure PR standards with respect to styles. This in turn will help us in better maintainability and readability of the repo overall. The changes follow up from the PR #10080 and references to the issue #10017.

The current changes address the following Errors/Warnings in the src folder of WPF:

• CA1507: Use nameof in place of string

A good way to go about reviewing this is to go commit by commit which pans over individual errors/warnings and hence easing out the overall understanding.

Customer Impact

None

Regression

None

Testing

Local Build Pass
The current set of changes looks fairly mechanical and probably don't need a Test Pass, but these set of PRs can be clubbed to do so at a later stage.

Risk

Low

Microsoft Reviewers: Open in CodeFlow

[harshit7962]

LGTM

[franchyd]

Is there a reason all of the Format arguments that were string versions of type names, method names or other parameters/variables were not converted to nameof as well? Tons of examples on just the edited lines in this PR.

Examples:
https://github.com/dotnet/wpf/pull/10186/files#diff-9548ccd136bd9496ec1e3988fbde1bb4bb157edca85c98db2ccab0231a5f6606R141 "array" and "arrayIndex"
https://github.com/dotnet/wpf/pull/10186/files#diff-f14bd869705deb8af6905a835978658884bb9e2a6154168a21c726a68c62e1e2R66 "ConvertFrom"
https://github.com/dotnet/wpf/pull/10186/files#diff-3665389550074b6e45c276975d5865702b3697658193afd41ede2922895af436R493 "int"
https://github.com/dotnet/wpf/pull/10186/files#diff-b852c8a4cb730d7c8d936b0c75a60bada11119276207ea687a5180937815254eR121 "FontFamily"

And many more.

[h3xds1nz]

@DanFTRX Read the implementation notes on https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1507 to understand what is the reason. Especially the note about parameter names required for the analyzer to fire.

[franchyd]

@h3xds1nz It seems a lot like following the word of the rules rather than the spirit of the rules only correct the strings that caused this warning. The point of the diagnostic is that using nameof improves code maintainability in cases where the code element may be renamed in the future, but the string literal is mistakenly not renamed. That motivation applies equally to these other string constants even if the particular analyzer didn't happen to highlight them. I would posit that a pass on these other string constants should also be done otherwise this particular rule is just theatre.

[h3xds1nz]

@DanFTRX Analyzers don't catch everything, likely won't for a while as its a static analysis too, or they catch too much.

Of course I agree with you that a pass on the occurences that the analyzer does not catch should be done as well, but does not necessarily need to be done in this PR, where the clear scope was to compile with CA1507 enabled ("addressing CA warnings", not "making sure we're using nameof on every symbol where it makes sense", which will require a bit of manual work.)

Sorry if you've misunderstood my reply but given you've asked for a reason behind "why those weren't converted as well", I merely answered that part.

[franchyd]

@DanFTRX Analyzers don't catch everything, likely won't for a while as its a static analysis too, or they catch too much.

Of course I agree with you that a pass on the occurences that the analyzer does not catch should be done as well, but does not necessarily need to be done in this PR, where the clear scope was to compile with CA1507 enabled ("addressing CA warnings", not "making sure we're using nameof on every symbol where it makes sense", which will require a bit of manual work.)

Sorry if you've misunderstood my reply but given you've asked for a reason behind "why those weren't converted as well", I merely answered that part.

Ah understood, the answer wasn't "we don't want to" but rather "the scope of this PR was limited". That makes sense.