[API Proposal]: Change the nullable annotation of IParsable.TryParse / ISpanParsable.TryParse

[meziantou]

Background and motivation

It's common for TryParse methods to set the out parameter to null when returning false. The current IParsable / ISpanParsable doesn't allow null values. I think the interface should add [MaybeNullWhen(false)] to the out parameter.

API Proposal

namespace System;

public interface ISpanParsable<T>
{
    // existing
    static abstract bool TryParse(ReadOnlySpan<char> s, IFormatProvider? provider, out TSelf result);

    // suggested
    static abstract bool TryParse(ReadOnlySpan<char> s, IFormatProvider? provider, [MaybeNullWhen(false)]out TSelf result);
}

public interface IParsable<T>
{
    // existing
    static abstract bool TryParse([NotNullWhen(true)] string? s, IFormatProvider? provider, out TSelf result);

    // suggested
    static abstract bool TryParse([NotNullWhen(true)] string? s, IFormatProvider? provider, [MaybeNullWhen(false)]out TSelf result);
}

API Usage

public class Sample : ISpanParsable<Sample>
{
    public static Sample Parse(string s, IFormatProvider? provider) => new Sample();
    public static Sample Parse(ReadOnlySpan<char> s, IFormatProvider? provider) => new Sample();

    public static bool TryParse([NotNullWhen(true)] string? s, IFormatProvider? provider, out Sample result)
    {
        result = default; // should be valid
        return false;
    }

    public static bool TryParse(ReadOnlySpan<char> s, IFormatProvider? provider, out Sample result)
    {
        result = default; // should be valid
        return false;
    }
}

Alternative Designs

No response

Risks

No response

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

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

Issue Details

---

Background and motivation

It's common for TryParse methods to set the out parameter to null when returning false. The current IParsable / ISpanParsable doesn't allow null values. I think the interface should add [MaybeNullWhen(false)] to the out parameter.

API Proposal

namespace System;

public interface ISpanParsable<T>
{
    // existing
    static abstract bool TryParse(ReadOnlySpan<char> s, IFormatProvider? provider, out TSelf result);

    // suggested
    static abstract bool TryParse(ReadOnlySpan<char> s, IFormatProvider? provider, [MaybeNullWhen(false)]out TSelf result);
}

public interface IParsable<T>
{
    // existing
    static abstract bool TryParse([NotNullWhen(true)] string? s, IFormatProvider? provider, out TSelf result);

    // suggested
    static abstract bool TryParse([NotNullWhen(true)] string? s, IFormatProvider? provider, [MaybeNullWhen(false)]out TSelf result);
}

API Usage

public class Sample : ISpanParsable<Sample>
{
    public static Sample Parse(string s, IFormatProvider? provider) => new Sample();
    public static Sample Parse(ReadOnlySpan<char> s, IFormatProvider? provider) => new Sample();

    public static bool TryParse([NotNullWhen(true)] string? s, IFormatProvider? provider, out Sample result)
    {
        result = default; // should be valid
        return false;
    }

    public static bool TryParse(ReadOnlySpan<char> s, IFormatProvider? provider, out Sample result)
    {
        result = default; // should be valid
        return false;
    }
}

Alternative Designs

No response

Risks

No response

Author:	meziantou
Assignees:	-
Labels:	area-System.Runtime, api-ready-for-review
Milestone:	7.0.0

[stephentoub]

This makes sense, and we should just go ahead and add the attribute. I believe all of our implementations of these interfaces are on structs, so this only impacts the interfaces; we shouldn't need to touch any implementations.