[Analyzer] string.ToLower() == otherString.ToLower()

[stephentoub]

The right way to compare two strings in a case-insensitive manner is via a StringComparer/Comparison that specifies case-insensitivity. But there's are many cases where code is using ToLower() on each string and then comparing those for equality, e.g.

• https://grep.app/search?q=%5C.ToLower%5C%28%5C%29%20%3D%3D%20%5Cw%2B%5C.ToLower%5C%28%5C%29&regexp=true&case=true&filter[lang][0]=C%23
• https://grep.app/search?q=%5C.ToUpper%5C%28%5C%29%20%3D%3D%20%5Cw%2B%5C.ToUpper%5C%28&regexp=true&case=true&filter[lang][0]=C%23
• https://grep.app/search?q=%5C.ToLower%5C%28%5C%29%5C.Equals%5C%28%5Cw%2B%5C.ToLower%5C%28%5C%29&regexp=true&case=true&filter[lang][0]=C%23
• https://grep.app/search?q=%5C.ToUpper%5C%28%5C%29%5C.Equals%5C%28%5Cw%2B%5C.ToUpper%5C%28%5C%29&regexp=true&case=true&filter[lang][0]=C%23

We should write an analyzer that flags such uses and recommends fixing them to use Equals with the appropriate comparison.

This analyzer should cover ToUpper too even if the usage is low for that.

Such analyzers will help avoid the allocations caused by ToLower/ToUpper calls.

Performance rules Category
Severity = suggestion

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[MichalPetryka]

Should the invariant variants be also changed?

[stephentoub]

The intent is all variations of this that make sense. That includes ToLower/UpperInvariant.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

---

The right way to compare two strings in a case-insensitive manner is via a StringComparer/Comparison that specifies case-insensitivity. But there's are many cases where code is using ToLower() on each string and then comparing those for equality, e.g.

• https://grep.app/search?q=%5C.ToLower%5C%28%5C%29%20%3D%3D%20%5Cw%2B%5C.ToLower%5C%28%5C%29&regexp=true&case=true&filter[lang][0]=C%23
• https://grep.app/search?q=%5C.ToUpper%5C%28%5C%29%20%3D%3D%20%5Cw%2B%5C.ToUpper%5C%28&regexp=true&case=true&filter[lang][0]=C%23
• https://grep.app/search?q=%5C.ToLower%5C%28%5C%29%5C.Equals%5C%28%5Cw%2B%5C.ToLower%5C%28%5C%29&regexp=true&case=true&filter[lang][0]=C%23
• https://grep.app/search?q=%5C.ToUpper%5C%28%5C%29%5C.Equals%5C%28%5Cw%2B%5C.ToUpper%5C%28%5C%29&regexp=true&case=true&filter[lang][0]=C%23

We should write an analyzer that flags such uses and recommends fixing them to use Equals with the appropriate comparison.

Author:	stephentoub
Assignees:	-
Labels:	area-System.Runtime, code-analyzer, code-fixer
Milestone:	8.0.0

[ghost]

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

Issue Details

---

The right way to compare two strings in a case-insensitive manner is via a StringComparer/Comparison that specifies case-insensitivity. But there's are many cases where code is using ToLower() on each string and then comparing those for equality, e.g.

• https://grep.app/search?q=%5C.ToLower%5C%28%5C%29%20%3D%3D%20%5Cw%2B%5C.ToLower%5C%28%5C%29&regexp=true&case=true&filter[lang][0]=C%23
• https://grep.app/search?q=%5C.ToUpper%5C%28%5C%29%20%3D%3D%20%5Cw%2B%5C.ToUpper%5C%28&regexp=true&case=true&filter[lang][0]=C%23
• https://grep.app/search?q=%5C.ToLower%5C%28%5C%29%5C.Equals%5C%28%5Cw%2B%5C.ToLower%5C%28%5C%29&regexp=true&case=true&filter[lang][0]=C%23
• https://grep.app/search?q=%5C.ToUpper%5C%28%5C%29%5C.Equals%5C%28%5Cw%2B%5C.ToUpper%5C%28%5C%29&regexp=true&case=true&filter[lang][0]=C%23

We should write an analyzer that flags such uses and recommends fixing them to use Equals with the appropriate comparison.

This analyzer should cover ToUpper too even if the usage is low for that.

Such analyzers will help avoid the allocations caused by ToLower/ToUpper calls.

Performance rules Category
Severity = suggestion

Author:	stephentoub
Assignees:	-
Labels:	api-suggestion, area-System.Globalization, code-analyzer, code-fixer
Milestone:	8.0.0

[tarekgh]

CC @Youssef1313

[bartonjs]

We should identify any of the equality cases with a string-case transformation

• a.ToLower() == b.ToLower()
• a.ToLower() == b.ToLowerInvariant()
• a.ToLower() == b
• And all of the above in reverse order, !=, and .Equals; as well as the ToUpper versions.

And replace them with the equivalent call to .Equals(b, StringComparison.IgnoreCase)

This should be merged into the same diagnostic ID as #78606 (if possible).

[MichalPetryka]

a.ToLower() == b is not safe to replace since that'd be false when b contains any upper case chars.

[tarekgh]

a.ToLower() == b is not safe to replace since that'd be false when b contains any upper case chars.

I think it will be safe to do it if b is literal string.

[carlossanlop]

I'll work on this one next. It is closely related to my other one that's almost done: #78606

[carlossanlop]

@tarekgh -

For this analyzer, should I continue using the same rules I used in my previous PR?:

• StringComparison.InvariantCultureIgnoreCase for ToLowerInvariant/ToUpperInvariant
• StringComparison.CurrentCultureIgnoreCase for ToLower/ToUpper

I ask because Jeremy's approval comment mentioned a non-existent comparison:

And replace them with the equivalent call to .Equals(b, StringComparison.IgnoreCase)

But there's also OrdinalIgnoreCase, so I'm unsure.

[tarekgh]

You will use StringComparison.InvariantCultureIgnoreCase and StringComparison.CurrentCultureIgnoreCase as you indicated. ToUpper/ToLower is always linguistic operation. We don't provide any API does ToUpper/ToLower for ordinal cases.

[carlossanlop]

I submitted a PR to fix this: dotnet/roslyn-analyzers#6720

@tarekgh question: If I have this comparison:

a.ToLower() == a.ToLowerInvariant()`

Am I right to assume that the substitution should be this?:

a.Equals(b, StringComparison.CurrentCultureIgnorecase)

My assumption is that CurrentCulture takes precedence over InvariantCulture. Is this correct?

[tarekgh]

@carlossanlop in such case we shouldn't provide a fix but only diagnostic telling this is can be optimized by using string comparison with ignore casing feature. We cannot know the intention of the caller if want to have the current culture or invariant casing at that time.