Precondition enforcement using 'requires' keyword

theunrepentantgeek commented 4 years ago

Motivation

There has been a significant amount of discussion across this repo recently on parameter and precondition enforcement.

See, for example, Primary Constructors (#2691), Simplified parameter null validation code (#2145), and Records as a collection of features (#3137)

Issue #2145 particularly concerns me because it burns syntax purely to handle one very specific (though very common) precondition check without doing anything to address other likely preconditions.

Syntax

The new keyword requires would allow a required precondition to be specified as a suffix to an existing parameter-declaration or expression. This syntax inspired by both the existing when keyword used for exception filtering, and by the common use of ?? for throwing exceptions when null values are encountered.

For a parameter declaration, the requires keyword would allow the precondition to be declared inline with the parameters of the method:

public IEnumerable<Qualifications> FindQualifications(
    Student student requires student != null,
    SearchFilter filter requires filter != null) 
{
    // elided 
}

This would be directly translated by the compiler into

public IEnumerable<Qualifications> FindQualifications(
    Student student,
    SearchFilter filter) 
{
   if (!(student != null)) 
   {
        throw new ArgumentException(
            "Precondition 'student != null' not satisfied", nameof(student));
   }

   if (!(filter != null)) 
   {
        throw new ArgumentException(
            "Precondition 'filter != null' not satisfied", nameof(filter));
   }

    // elided 
}

Aside - using the existing ArgumentException here may not be the best approach; I chose it to keep the translation simple.

All method parameters would be in scope for the checks, allowing conditions like this:

public void SpecifyLifetime(
    DateTimeOffset start,
    DateTimeOffset finish requires start < finish)
{
    // elided
}

I'd also expect that instance members (fields, properties, & methods, including privates) would be in scope (assuming a non-static method) to allow for enforcement of state specific validation:

public void ExtendLifetime(
    DateTimeOffset newFinish requires newFinish > Finish)
{
    // elided
}

For a static method, it would make sense that only static members were in scope.

For the more general case, it may be useful to allow requires as a suffix to any existing expression.

Among other uses, this would allow a different form for method parameter validation that may suit developers who object to nesting it with in the method signature. To restate the example from above:

public IEnumerable<Qualifications> FindQualifications(
    Student student, SearchFilter filter) 
{
    var query = CreateQuery(
        student requires student != null,
        filter requires filter != null);
    // elided
}

This would be translated into the following:

public IEnumerable<Qualifications> FindQualifications(
    Student student, SearchFilter filter) 
{
    if (!(student != null)) 
    {
        throw new PreconditionException(
            "Precondition 'student != null' not satisfied");
    }

    if (!(filter != null))
    {
        throw new PreconditionException(
            "Precondition 'filter != null' not satisfied");
    }

    var query = CreateQuery( student, filter);

    // elided
}

Comparision with other approaches

Using `??`

For code currently using ?? to do null argument value checks, switching to requires would both reduce code size and increase the declarative nature of the code; the check is explicitly stated, instead of being inferred from the use of an exception expression.

To illustrate, compare this

public IEnumerable<Qualifications> FindQualifications(
    Student student, SearchFilter filter) 
{
    var query = CreateQuery(
        student requires student != null,
        filter requires filter != null);
    // elided
}

with the way we'd currently write this check using ??:

public IEnumerable<Qualifications> FindQualifications(
    Student student, SearchFilter filter) 
{
    var query = CreateQuery(
        student ?? throw new ArgumentNullException(nameof(student)),
        filter ?? throw new ArgumentNullException(nameof(filter)));
    // elided
}

Simplified parameter null validation code

Issue #2145 would allow null checks to be specified by suffixing the argument name with ! so the example above would become:

public IEnumerable<Qualifications> FindQualifications(
    Student student!, SearchFilter filter!) 
{
    var query = CreateQuery(student, filter);
    // elided
}

This is considerably more concise than the syntax proposed here, but has two distinct disadvantages.

The syntax needs to be looked up; there's no way for a reader to discern or guess the intent of the ! suffix from context alone.
Lack of flexibility - only null checks are supported; every other check needs to be stated in the existing long form.

dsaf commented 4 years ago

https://github.com/dotnet/csharplang/issues/105

theunrepentantgeek commented 4 years ago

Method contracts are a very broad feature that includes flow analysis and a whole bunch of other things. Plus, there doesn't seem to be any great appetite for it, particularly since NRT has made it into the language.

My suggestion here is for a very simple piece of additional syntax, that perhaps could be generalised into contracts later, but which has a straightforward translation for now.

I'm still concerned about the proposed ! suffix syntax for null enforcement, for all the reasons I enumerated originally.

dotnet / csharplang