[API Proposal]: Add missing overloads to "Flow System.Text.Rune through more APIs" proposal

[Joy-less]

Background and motivation

In the #27912 (Flow System.Text.Rune through more APIs) proposal, which was approved and implemented in a work-in-progress pull request in #117168 by myself, a number of APIs to expand the usage of System.Text.Rune were drafted, for example bool string.Contains(Rune). This proposal is to suggest a few amendments to that proposal before the implementation is submitted.

Here are the amendments in perspective ("PROPOSED" means suggested in the original approved proposal, "AMENDMENT" means suggested in this extra amendment proposal):

int string.IndexOf(char);
int string.IndexOf(char, int);
int string.IndexOf(char, int, int);
int string.IndexOf(char, StringComparison);
int string.IndexOf(char, int, StringComparison); // AMENDMENT
int string.IndexOf(char, int, int, StringComparison); // AMENDMENT

int string.LastIndexOf(char);
int string.LastIndexOf(char, int);
int string.LastIndexOf(char, int, int);
int string.LastIndexOf(char, StringComparison);
int string.LastIndexOf(char, int, StringComparison); // AMENDMENT
int string.LastIndexOf(char, int, int, StringComparison); // AMENDMENT

int string.IndexOf(Rune); // PROPOSED
int string.IndexOf(Rune, int); // PROPOSED
int string.IndexOf(Rune, int, int); // PROPOSED
int string.IndexOf(Rune, StringComparison); // PROPOSED
int string.IndexOf(Rune, int, StringComparison); // AMENDMENT
int string.IndexOf(Rune, int, int, StringComparison); // AMENDMENT

int string.LastIndexOf(Rune); // PROPOSED
int string.LastIndexOf(Rune, int); // PROPOSED
int string.LastIndexOf(Rune, int, int); // PROPOSED
int string.LastIndexOf(Rune, StringComparison); // PROPOSED
int string.LastIndexOf(Rune, int, StringComparison); // AMENDMENT
int string.LastIndexOf(Rune, int, int, StringComparison); // AMENDMENT

int string.IndexOf(string);
int string.IndexOf(string, int);
int string.IndexOf(string, int, int);
int string.IndexOf(string, StringComparison);
int string.IndexOf(string, int, StringComparison);
int string.IndexOf(string, int, int, StringComparison);

int string.LastIndexOf(string);
int string.LastIndexOf(string, int);
int string.LastIndexOf(string, int, int);
int string.LastIndexOf(string, StringComparison);
int string.LastIndexOf(string, int, StringComparison);
int string.LastIndexOf(string, int, int, StringComparison);

bool char.Equals(object?);
bool char.Equals(char);
bool char.Equals(char, StringComparison); // AMENDMENT

bool Rune.Equals(object?);
bool Rune.Equals(Rune);
bool Rune.Equals(Rune, StringComparison); // PROPOSED

bool string.Equals(object?);
bool string.Equals(string?);
bool string.Equals(string?, StringComparison);

API Proposal

namespace System
{
    public partial sealed class String
    {
        public int IndexOf(Rune value, int startIndex, StringComparison comparisonType);
        public int IndexOf(Rune value, int startIndex, int count, StringComparison comparisonType);
        public int IndexOf(char value, int startIndex, StringComparison comparisonType);
        public int IndexOf(char value, int startIndex, int count, StringComparison comparisonType);

        public int LastIndexOf(Rune value, int startIndex, StringComparison comparisonType);
        public int LastIndexOf(Rune value, int startIndex, int count, StringComparison comparisonType);
        public int LastIndexOf(char value, int startIndex, StringComparison comparisonType);
        public int LastIndexOf(char value, int startIndex, int count, StringComparison comparisonType);
    }

    public readonly struct Char
    {
        public bool Equals(char right, StringComparison comparisonType);
    }
}

API Usage

Console.WriteLine("hello world".IndexOf(new Rune('R'), 6, StringComparison.OrdinalIgnoreCase)); // 8
Console.WriteLine("hello world".IndexOf(new Rune('R'), 6, 1, StringComparison.OrdinalIgnoreCase)); // -1
Console.WriteLine("hello world".IndexOf('R', 6, StringComparison.OrdinalIgnoreCase)); // 8
Console.WriteLine("hello world".IndexOf('R', 6, 1, StringComparison.OrdinalIgnoreCase)); // -1

Console.WriteLine("hello world".LastIndexOf(new Rune('R'), 9, StringComparison.OrdinalIgnoreCase)); // 8
Console.WriteLine("hello world".LastIndexOf(new Rune('R'), 9, 1, StringComparison.OrdinalIgnoreCase)); // -1
Console.WriteLine("hello world".LastIndexOf('R', 9, StringComparison.OrdinalIgnoreCase)); // 8
Console.WriteLine("hello world".LastIndexOf('R', 9, 1, StringComparison.OrdinalIgnoreCase)); // -1

Console.WriteLine('m'.Equals('M', StringComparison.OrdinalIgnoreCase)); // True
Console.WriteLine(new Rune('m').Equals(new Rune('M'), StringComparison.OrdinalIgnoreCase)); // True

Alternative Designs

There aren't really any alternative designs, since these amendments only fill in missing overloads. The only thing to note is the possibility of ReadOnlySpan<char> versions of these overloads, which was raised in the original proposal but never reviewed. As far as I'm concerned, ReadOnlySpan<char> versions should be in another proposal if desired.

Risks

Most of the overloads suggested (except for the IndexOf/LastIndexOf overloads featuring char, which could be implemented similarly to the Rune versions) have already been implemented in my pull request in #117168 internally. Approval of this proposal would allow those overloads to be made public.

There are some concerns about handling of the \0 character on different systems, but these can be easily addressed by converting the char/Rune to a stack-allocated ReadOnlySpan<char> and letting the existing overloads handle those inconsistencies.

There are only 9 extra methods suggested in this amendment pull request, all of which are overloads of existing methods, so the impact on the size and complexity of the runtime should be small.

[dotnet-policy-service]

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

[bartonjs]

Video

Looks good as proposed.

namespace System
{
    public partial sealed class String
    {
        public int IndexOf(Rune value, int startIndex, StringComparison comparisonType);
        public int IndexOf(Rune value, int startIndex, int count, StringComparison comparisonType);
        public int IndexOf(char value, int startIndex, StringComparison comparisonType);
        public int IndexOf(char value, int startIndex, int count, StringComparison comparisonType);

        public int LastIndexOf(Rune value, int startIndex, StringComparison comparisonType);
        public int LastIndexOf(Rune value, int startIndex, int count, StringComparison comparisonType);
        public int LastIndexOf(char value, int startIndex, StringComparison comparisonType);
        public int LastIndexOf(char value, int startIndex, int count, StringComparison comparisonType);
    }

    public readonly struct Char
    {
        public bool Equals(char right, StringComparison comparisonType);
    }
}

[tarekgh]

@Joy-less this proposal is approved now, are you interested submitting a PR to support it?

[Joy-less]

@Joy-less this proposal is approved now, are you interested submitting a PR to support it?

Absolutely, I will start on it later!

[Joy-less]

@tarekgh As I was implementing this I noticed I had made two mistakes in this proposal (which I apologise for):

1. Unlike what I stated/assumed, the following overload does not exist:

int string.LastIndexOf(char, StringComparison);

Which affects these proposed overloads:

int string.LastIndexOf(char, int, StringComparison);
int string.LastIndexOf(char, int, int, StringComparison);

2. I suggested the name right for the parameter in the char.Equals overload despite it being an instance method:

public bool Equals(char right, StringComparison comparisonType);

The existing methods aren't that consistent, but they either use obj or other for this parameter. char.Equals(char) uses obj, and Rune.Equals(Rune) & Rune.Equals(Rune, StringComparison) use other.

If I were to amend this proposal I would make the following changes:

1. Add an int string.LastIndexOf(char, StringComparison); overload
2. Rename right to obj (or other)

I think this proposal probably needs to be re-reviewed. What do you think? (I can only apologise for not catching these issues earlier)