runtime: Proposal: Convenience API for throwing exceptions

It would likely need to be something like NuGet “source” package when used; rather than a built in to runtime as assembly, so the tests will inline across assembly boundaries when compiled AoT/R2R

So, I’ve had time to think about it. We already have Debug and Trace, so the concern about strictly following the FDG with regards to using verbs for class names is probably not a concern here. So Require is probably an “OK” name. However, I still believe Argument to be superior.

Verify.NotNull(list, nameof(list)); // Probably confusing
Require.NotNull(list, nameof(list)); // Probably not confusing
Argument.NotNull(list, nameof(list)); // Absolutely not confusing

I like the fact that with Argument as the name the assertions read exactly the same as the exceptions they throw: Argument.NotNull throws ArgumentNullException.

I also just noticed in your sample implementation you have overloads of InRange for validating indexes, but they throw ArgumentException. I’ve always thrown ArgumentOutOfRange exception which made more sense to me. However, looking at List<T>.BinarySearch it looks like precedent says otherwise? List<T>.BinarySearch also throws ArgumentOutOfRangeException but that’s for validation that index and count are non-negative, while it just throws ArgumentException for range violations (confusing to me). You also don’t assign a ParamName to the exception, which is something that really bothers me, but, again, this is the behavior of List<T>.BinarySearch as well. So, while you’ve followed precedent, maybe we should consider changing some of this behavior?

Also you only have overloads for string and arrays when you should probably just have a single generic overload constrained for IList<T> types. Also, it might be nice to have an overload that doesn’t take the count parameter and instead just validates index is in the range of valid indexes for the collection.

Argument.InRange(index, array, nameof(array), nameof(index));

BTW, while I’d like to see something like this regardless, these helpers really don’t shine until we get your language proposal. I vote +1 for both.

“Chaining” is problematic. The example given of value.VerifyNotNull().VerifyNotEmpty() requires these verifiers to be extension methods that work on all types. I find that repugnant, to be honest. It pollutes IntelliSense. There is a solution to this, which I use in my Testify unit testing library. The above can be coded as Argument.Verify(value).NotNull().NotEmpty(), where Argument.Verify returns some unique wrapper type, such as an ArgumentValue<T> type, and the extension methods are based on that type. This also has the benefit of making it easy to add assertions and even to limit IntelliSense to valid types (so, for instance, NotNull won’t be in IntelliSense for value types). The downside is in performance and possibly memory/gc overhead if ArgumentValue<T> is a reference type. Also, for this to work with the concept of returning the value the API is more complex with the need to chain to a Value property, or to rely on implicit conversions.

All that said, however, you could always take the functional POV to chaining and code it as NotNull(NotEmpty(value)) (with lots more syntactical noise without the compiler feature to get at the caller expression and without a static using).

While these verification methods are handy (I use similar often myself), there is something to be said about the benefit of a throw within a method for static analysis.

public bool SomeMethod(string someArg)
{
    if (someArg == null) throw new ArgumentNullException();
    // Clearly someArg is not null at this point.
    if (someArg.length < 3)
    {
        if (someArg.length == 0)
        {
            throw new ArgumentException("Empty");
        }
        else
        {
            throw new ArgumentException("Too short");
        }
        // Clearly unreachable. A helper method that threw the exception itself
        // above would produce CS0161 here.
    }
    return true
}

Changing ErrorContext.Error in Microsoft.CSharp from throwing to returning an exception to throw, and passing that change up to any method that always threw, made quite a few chunks of code not only more clearly unreachable, but demonstrably unreachable so the compiler would complain about their being there. It was a big jump in how easy it was to reason about a lot of the methods.

This is going to become even more pertinent when C#8 introduces nullability checks as promised:

public bool IsEmpty(string? someArg)
{
    if (someArg == null) throw new ArgumentNullException();
    return someArg.length  == 0;
}

or even

public bool IsEmpty(string? someArg)
{
    if (someArg == null) throw ErrorHelper.ArgumentNull();
    return someArg.length  == 0;
}

vs

public bool IsEmpty(string? someArg)
{
    Requires.NotNull(someArg);
    return someArg.length  == 0; // Compiler warning someArg could be null.
}

Unless we get a way to flag to the compiler that Requres.NotNull() will never succeed with with a null argument, and hence that someArg is demonstrably not null after it, using it will lose the nullability checking of the language.

How do we get to have both?

There is a need for APIs like that in corfxlab and in the platfrom. For example, System.Slices already duplicates this: https://github.com/dotnet/corefxlab/blob/master/src/System.Slices/System/Runtime/Contract.cs

I never turned the Contract.cs APIs into a package as there are several interesting trade-offs here, and I did not have time to think how to resolve them.

Also, I think this is related to a feature that @justinvp was pushing at some point (and I liked a lot), i.e. the ability to throw various common exceptions conveniently without the need to deal with message resources.

Lastly, I would love to have a set of attributes for identifying exceptions that propagate out of APIs. The attributes would divide errors into usage errors (roughly preconditions) and runtime errors (errors that need to be handled). Usage errors would show in VS tooltips and runtime error attributes would be used by analyzers to detect code that does not handle them properly.