runtime: [API Proposal]: Consider changing return types of new throw helpers

In .NET 6 we introduced:

class ArgumentNullException
{
+    public static void ThrowIfNull([NotNull] object? argument, [CallerArgumentExpression(nameof(argument))] string? paramName = null);
}

In .NET 7 we introduced:

class ArgumentNullException
{
+    public static void ThrowIfNull([NotNull] void* argument, [CallerArgumentExpression(nameof(argument))] string? paramName = null);
}
class ArgumentException
{
+    public static void ThrowIfNullOrEmpty([NotNull] string? argument, [CallerArgumentExpression(nameof(argument))] string? paramName = null);
}

Now in .NET 8, we’ve introduced:

class ArgumentException
{
+    public static void ThrowIfNullOrWhiteSpace([NotNull] string? argument, [CallerArgumentExpression(nameof(argument))] string? paramName = null);
}
class ArgumentOutOfRangeException
{
+    public static void ThrowIfEqual<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : IEquatable<T>?;
+    public static void ThrowIfGreaterThan<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : IComparable<T>;
+    public static void ThrowIfGreaterThanOrEqual<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : IComparable<T>;
+    public static void ThrowIfLessThan<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : IComparable<T>;
+    public static void ThrowIfLessThanOrEqual<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : IComparable<T>;
+    public static void ThrowIfNegative<T>(T value, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : INumberBase<T>;
+    public static void ThrowIfNegativeOrZero<T>(T value, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : INumberBase<T>;
+    public static void ThrowIfNotEqual<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : IEquatable<T>?;
+    public static void ThrowIfZero<T>(T value, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : INumberBase<T>;
}

We can’t change the 3 methods we shipped in .NET 6 and .NET 7, but we still have time to tweak the new 10 methods we’re shipping in .NET 8. In particular, we’ve had some requests for returning the input being validated, e.g. that would make them instead be:

class ArgumentException
{
+    public static string ThrowIfNullOrWhiteSpace([NotNull] string? argument, [CallerArgumentExpression(nameof(argument))] string? paramName = null);
}
class ArgumentOutOfRangeException
{
+    public static T ThrowIfEqual<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : IEquatable<T>?;
+    public static T ThrowIfGreaterThan<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : IComparable<T>;
+    public static T ThrowIfGreaterThanOrEqual<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : IComparable<T>;
+    public static T ThrowIfLessThan<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : IComparable<T>;
+    public static T ThrowIfLessThanOrEqual<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : IComparable<T>;
+    public static T ThrowIfNegative<T>(T value, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : INumberBase<T>;
+    public static T ThrowIfNegativeOrZero<T>(T value, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : INumberBase<T>;
+    public static T ThrowIfNotEqual<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : IEquatable<T>?;
+    public static T ThrowIfZero<T>(T value, [CallerArgumentExpression(nameof(value))] string? paramName = null) where T : INumberBase<T>;
}

at which point we would also do so for all new throw helpers of the same ilk.

Should we?

About this issue

Original URL
State: closed
Created a year ago
Reactions: 10
Comments: 20 (17 by maintainers)

Most upvoted comments

Maybe also worth noting (other than that assignment style Stephen already showed, which I also personally really dislike), making these APIs return a value would trigger a ton of warnings for everyone having warnings enabled for implicitly discarding return values. All those developers would have to always use _ = ThrowHelperThrowIf.Whatever(...) every single time if we made those APIs return values. Full disclosure I’m also in that group, as I’ve enabled that in all my repos, as I found that to be an extremely valuable too to help spot errors 🥲

Sergio0694 on May 16, 2023

Do you have some good examples of how we would change the code in this repo to take advantage of the return value?

I don’t think we would.

It ends up being a stylistic request for folks that prefer the style of doing things today like:

int value = input >= 0 ? input : throw new ArgumentOutOfRangeException(...);

and would like to instead do:

int value = ArgumentOutOfRangeException.ThrowIfNegative(input);

rather than:

ArgumentOutOfRangeException.ThrowIfNegative(input);
int value = input;

I’m personally not a fan of that style, but I’ve opened this to represent those who are 😃 (we discussed it briefly when adding ThrowIfNullOrWhiteSpace and said we’d have a follow-on conversation after I opened an issue for it)

The one place this currently has meaningful “I couldn’t otherwise use the API” impact is in places where only expressions are usable, such as when delegating to another this/base ctor:

public Blah(int input) : base(ArgumentOutOfRangeException.ThrowIfNegative(input)) { ... }

It’s possible we’d use that somewhere in dotnet/runtime, but I can’t think of any off the top of my head. I do know of some places where we might have done that for ArgumentNullException.ThrowIfNull, but only if ThrowIfNull returned the T input, and what we shipped neither returns anything nor is generic. For inlining/size reasons we made it non-generic, and once it’s non-generic returning the instance as object isn’t helpful. Such use might also be addressed in a future version of C#.

stephentoub on May 16, 2023

“Without this change, it is impossible to use those helpers in a switch expression.”

Worth noting, the way throw helpers are generally used in switch expressions (eg. to throw on the default case and whatnot) is to have a branch that just returns a throw helper return. That is, in this scenario you’d use a throw helper that throws unconditionally. That is not the case for the API whose signature is being proposed for change here, as they all have conditions. So I’m not sure the example of switch expressions could really be used in this case either 🤔

Sergio0694 on May 17, 2023

Changing the return type of a method would break all compiled code using it; the return type of a method is part of the IL signature.

teo-tsirpanis on May 16, 2023

Personally, I would have liked ThrowIfNull to have a return value for use in ctors (RIP !!)

public Foo(string arg)
{
    _field = ArgumentNullException.ThrowIfNull(arg);
}

But if changing ThrowIfNull is out of the question due to compat, having the option to use such a pattern for some validation, but not for null feels worse than just sticking to the current pattern.

MihaZupan on May 16, 2023

Why not just use input in the rest of the method in this example?

That was just me quickly writing something out. Often the thing being assigned to is, e.g., a field:

_value = ArgumentOutOfRangeException.ThrowIfNegative(input);

Again, I’m not actually pushing for this, just trying to represent the viewpoint.

stephentoub on May 16, 2023

I’m also not a huge fan of this style; but the other place it comes up is in places like switch patterns where the thrower can only be used if it returns something (ideally the language would improve that instead, but its hard in the case the function actually returns, since the compiler can’t “prove” it doesn’t)

tannergooding on May 16, 2023