Update FxCop analyzers

[ThomasGoulet73]

Description

Update Microsoft.CodeAnalysis.FxCopAnalyzers to 3.3.1 and activate valid rules. The last version before the deprecated warning. I plan on doing a follow-up PR on chaning FxCopAnalyzers for NetAnalyzers.

Customer Impact

None, there are no behavior changes. Except here: #4800 (comment).

Regression

None.

Testing

Local build.

Risk

None.

[ThomasGoulet73]

This is the only behavior change to use Ordinal by instead of the default current culture.

It is more inline with the usage of Contains, which defaults to Ordinal comparison here:
https://github.com/dotnet/wpf/blob/78e93de6c2450b4f7b1e3adb5a8b357ab8823d41/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs#L94
and here:
https://github.com/dotnet/wpf/blob/78e93de6c2450b4f7b1e3adb5a8b357ab8823d41/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs#L99

[ThomasGoulet73]

Disable CA1062 because there are false positives, when a public method is invoking another method that already does the null check.

[dipeshmsft]

@ThomasGoulet73 There are only two such cases where this warning is raised. Shall we suppress only for those two locations, instead of globally suppressing it?

[ThomasGoulet73]

@dipeshmsft You're right, I thought about it more and I think we should enable it to make sure that we do null checks in the public method, even if there is a null check in a private method. This helps make sure that the argument name in the exception is the same as the public one, in case the argument name in the private method is not the same as the one in the public method. This additional check probably won't even be noticeable performance-wise.

I'll enable it in a separate commit and I'll leave the decision to the reviewers.

[ThomasGoulet73]

This shouldn't be needed and it conflicts with other PackageReference when enabling analyzers in any other project than System.Xaml.

[ThomasGoulet73]

This one would succeed in XamlLanguage.TypeAlias(type) but throw in GetXamlType. By adding an explicit throw here, we make sure to throw early, with the right stacktrace and the right parameter name.

[ThomasGoulet73]

This one would already throw a NullReferenceException destinationType.FullName. By adding an explicit throw here, we make sure to throw early, with the right stacktrace and the right parameter name.

This one is technically a breaking change, the type of exception returned is not the same as before. Let me know if you would prefer to revert this change.

[RussKie]

Whilst this is a breaking change indeed, these types of changes have been generally considered acceptable across the .NET ecosystem.
https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/breaking-changes.md#bucket-2-reasonable-grey-area