[RFC] Is ```System.StringComparer.Compare(object? x, object? y)``` Implemented Correctly?

[praeceptum]

The implementation of System.StringComparer member int Compare(object? x, object? y) may not be correct.

public int Compare(object? x, object? y)
{
    if (x == y) return 0;
    if (x == null) return -1;
    if (y == null) return 1;

    if (x is string sa)
    {
        if (y is string sb)
        {
            return Compare(sa, sb);
        }
    }

    if (x is IComparable ia)
    {
        return ia.CompareTo(y);
    }

    throw new ArgumentException(SR.Argument_ImplementIComparable);
}

If the parameters x and y are non-null, non-equal, one isn't type string, and x isn't type IComparable, then an exception is raised advising that "At least one object must implement IComparable." However, y is never tested for, or leveraged as, type IComparable. It seems that the behavior and/or the message may be incorrect.

For comparison, the System.Collections.Generic.Comparer<T> member int IComparer.Compare(object? x, object? y) doesn't leverage IComparable for its parameters. ( It simply raises an exception if they can't be cast to T. ) Because the StringComparer implementation does, subtypes that rely on it will compare non-string types. This seems to exceed the IComparer contract, suppressing semantic errors. For example:

Assert( StringComparer.Ordinal.Compare(1.0, 2.0) == -1 );

[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 implementation of `System.StringComparer` member `int Compare(object? x, object? y)` may not be correct. ``` public int Compare(object? x, object? y) { if (x == y) return 0; if (x == null) return -1; if (y == null) return 1; if (x is string sa) { if (y is string sb) { return Compare(sa, sb); } } if (x is IComparable ia) { return ia.CompareTo(y); } throw new ArgumentException(SR.Argument_ImplementIComparable); } ``` If the parameters `x` and `y` are non-null, non-equal, one isn't type `string`, and `x` isn't type `IComparable`, then an exception is raised advising that "At least one object must implement IComparable." However, `y` is never tested for, or leveraged as, type `IComparable`. It seems that the behavior and/or the message may be incorrect. For comparison, the `System.Collections.Generic.Comparer` member `int IComparer.Compare(object? x, object? y)` doesn't leverage `IComparable` for its parameters. ( It simply raises an exception if they can't be cast to `T`. ) Because the `StringComparer` implementation does, subtypes that rely on it will compare non-string types. This seems to exceed the `IComparer` contract, suppressing semantic errors. For example: ``` Assert( StringComparer.Ordinal.Compare(1.0, 2.0) == -1 ); ```

Author:	praeceptum
Assignees:	-
Labels:	`area-System.Runtime`, `untriaged`, `needs-area-label`
Milestone:	-