False-negative for S1449 on "IndexOf" and "LastIndexOf" overloads with "char" parameter

  • Language: C# (and I guess Visual Basic)
  • Rule: S1449
  • Product: SonarLint 8.6.0.10679
  • IDE: Visual Studio Community 2022 17.11.5
  • Connected mode: No

The rule S1449 is not applied to the overloads of the methods String.IndexOf and String.LastIndexOf that take a char for the value parameter. It seems to me that the same issues that would happen when using a string parameter would apply when using a char one, so the rule should apply to them as well.

Example C# code:

"asdf".IndexOf('a'); // False-negative
"asdf".LastIndexOf('a'); // False-negative
"asdf".IndexOf('a', 0); // False-negative
"asdf".LastIndexOf('a', 0); // False-negative
"asdf".IndexOf('a', 0, 1); // False-negative
"asdf".LastIndexOf('a', 0, 1); // False-negative

The assumption is incorrect: the IndexOf and LastIndexOf overloads that take char default to ordinal comparisons.

It looks like S1449 is for people who think defaulting to ordinal comparisons is OK and S4058 is for people who want to be explicit in any case.

Note, though, that the IndexOf(char, StringComparison) overload is not in .NET Framework and that there are no overloads for searching for a char in a substring using a StringComparison. So insisting on a StringComparison may affect performance (only slightly, but each decision like this adds up).

By the way, using IndexOf or LastIndexOf on a ReadOnlySpan<char> (from MemoryExtensions) will also default to an ordinal comparison (clearly, since these overloads are generic for all T: IEquatable<T>).

Indeed, I didn’t realize that, it’s not confusing at all… :face_with_spiral_eyes:

The description of S1449 seems to assume that problems would only happen because the methods are culture-dependent; however, I’d argue that problems can also happen for the method overloads that aren’t culture-dependent, and it’d be nice to have a rule to always specify the culture or StringComparison. Unfortunately, S4058 doesn’t seem to do that, as it doesn’t flag text.IndexOf('a') for me, only text.IndexOf("a").

Hey there,

This is an interesting discussion.
I added a ticket in our internal backlog to track this FN.

Thanks for raising this!