Prefer string.Equals over string.Compare(string, string) == 0 or != 0

[AArnott]

As a carryover from native code habits, I see folks use string.Compare(a, b) == 0 or != 0 a lot where string.Equals(a, b) is more semantically meaningful.

But more importantly, Compare is for sorting strings whereas Equals is for determining equality. In particular cultures two strings may have sort equivalence without being equal. So in some cases it may even make a security difference (when StringComparison isn't specified).

Category: Performance

[AArnott]

@terrajobst to consider for inclusion in the Roslyn Analyzers project.

[danmoseley]

cc @GrabYourPitchforks who I think has commented on this before.

[GrabYourPitchforks]

string.Equals(...) and string.Compare(...) == 0 are semantically equivalent as long as the same StringComparison is used.

The thing that messes people up is that if you don't specify an explicit comparison, string.Equals defaults to Ordinal, while string.Compare defaults to CurrentCulture.

IMO this particular issue should be performance rather than correctness. Let existing issue dotnet/roslyn-analyzers#2581 (comment) be the "correctness / security" issue.

[GrabYourPitchforks]

Shouldn't we keep this issue open? There is some benefit to having devs prefer string.Equals(...) over the equivalent string.Compare(...) == 0 (or != 0).

[AArnott]

Oh, I misunderstood. I thought you were suggesting we close it in favor of the other issue. But now I see that you simply suggested I revise the category.

[EgorBo]

There are a few hits in dotnet/runtime itself:

cc @stephentoub

[stephentoub]

We have https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca2251. This issue should have been closed with that.

Regarding "There are a few hits in dotnet/runtime itself", those are all for string.CompareOrdinal rather than string.Compare. We should open an issue in dotnet/roslyn-analyzers to update CA2251 to also look for that method, at least the two-argument one... string.Equals doesn't provide an equivalent overload for the five-argument one.
dotnet/roslyn-analyzers#6498