WiseTechGlobal / WTG.Analyzers

Analyzers from WiseTech Global to enforce our styles, behaviours, and prevent common mistakes.
Other
16 stars 3 forks source link

Warn against comparing the result of string.ToUpper / string.ToLower. #156

Open brian-reichle opened 3 years ago

brian-reichle commented 3 years ago

If the result of string.ToUpper or string.ToLower is simply used in a comparison, then that's a pretty good indication that a case-insensitive comparison should be used instead.

foo.ToUpper() == "FOO"
"foo" == foo.ToLower(CultureInfo.InvariantCulture)
string.Equals(foo.ToUpper(CultureInfo.InvarintCulture), "FOO")
foo.ToUpper().CompareTo("FOO")
string.Compare(foo.ToUpper(), "FOO", StringComparison.Ordinal)

Should become:

foo.Equals("FOO", StringComparison.CurrentCultureIgnoreCase)
"foo".Equals(foo, StringComparison.InvariantCultureIgnoreCase)
string.Equals(foo, "FOO", StringComparison.InvarintCultureIgnoreCase)
string.Compare(foo, "FOO", StringComparison.CurrentCultureIgnoreCase)
string.Compare(foo "FOO", StringComparison.OrdinalIgnoreCase)

Should probably report things like the following as errors on the basis that they well never match:

foo.ToUpper(CultureInfo.InvariantCulture) == "Foo"
foo.ToLower(CultureInfo.InvariantCulture) == "Foo"
yaakov-h commented 3 years ago

Should the bottom section be one of those "replace with false" code-fixes? :laughing:

brian-reichle commented 3 years ago

That's what I was thinking, or rather "replace with false and simplify".