[Analyzer suggestion]: Usage of LastIndexOf

[Youssef1313]

LastIndexOf behavior has changed as documented in dotnet/docs#21375.
An analyzer that detects a return value of LastIndexOf that's then used as an index to access the character should report a violation.

Example

private static void M1(string s, string s2)
{
    var index = s.LastIndexOf(s2);
   Console.WriteLine(s[index]); // WARNING!! index can be -1 or the length of the string
}

private static void M2(string s, string s2)
{
    var index = s.LastIndexOf(s2);
    if (index > 0)
       Console.WriteLine(s[index]); // WARNING!! index can be the length of the string
}

private static void M3(string s, string s2)
{
    var index = s.LastIndexOf(s2);
    if (index > 0 && index < s.Length)
       Console.WriteLine(s[index]); // NO WARNING!
}

private static void M4(string s, string s2)
{
    if (string.IsNullOrEmpty(s2)) return;
    // or IsNullOrWhiteSpace, or s2 == "", or s2.Length == 0

    var index = s.LastIndexOf(s2);
    if (index > 0)
       Console.WriteLine(s[index]); // NO WARNING!
}

cc: @mavasani

[GrabYourPitchforks]

I don't think we need an analyzer for this. The return value has always been allowed to be string.Length, as given by this example.

// prints '5' across all Frameworks I tested
Console.WriteLine("Hello".LastIndexOf("\u200d", StringComparison.InvariantCulture));

Additionally, it's not really sensible to perform a linguistic search for a non-constant substring (s2, in your example), then attempt to perform an index into the string in order to retrieve a single character based on the returned index. I can't imagine anybody is ever going to run into this in practice.

[Youssef1313]

@GrabYourPitchforks I found a pattern which is similar to the one in the issue in aspnetcore repository.

https://github.com/dotnet/aspnetcore/blob/fd1a5c5a9c4812ff5fc760816c7d4d0e0dad7943/src/Tools/dotnet-watch/BrowserRefresh/src/WebSocketScriptInjection.cs#L33-L44

            var index = buffer.LastIndexOf(_bodyBytes); // if _bodyBytes is empty, the index will be equals to buffer.Length.
            if (index == -1)
            {
                baseStream.Write(buffer);
                return false;
            }

            if (index > 0)
            {
                baseStream.Write(buffer.Slice(0, index));
                buffer = buffer[index..]; // This should now throw as it tries to access buffer[buffer.Length], while it didn't in past.
            }

[GrabYourPitchforks]

That pattern is fine for two reasons: (a) it's using an ordinal search instead of a linguistic search; and (b) it's slicing instead of dereferencing.