LDM Notes for April 13th, 2022

[333fred]

https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-04-13.md

Agenda

• Parameter null checking
• File-scoped types

[chrisoverzero]

Parameter null checking is removed from C# 11.

😿

A tremendous shame.

[pinkfloydx33]

Eh... I've got mixed emotions. I liked that I wasn't going to have to write validator methods wrapping a task or iterator returning method.... But I was never sold on the syntax or its limitations (ie. vs contracts)... Its a shame they already updated the BCL to use it (though I see a PR already to revert)

[RikkiGibson]

As a person who wants the best for C#, I feel good that the feature was removed (at least in its current form in this release). But as someone who will be responsible for deleting the feature implementation and corresponding editor features from Roslyn, I feel a tiny bit sad. 😇

[saint4eva]

Its a shame they already updated the BCL to use it (though I see a PR already to revert)

Reverting it will bring back millions of lines of if null ckecks. What a disappointment.

Noise makers always have their way. The same people were against Top-level program and records, and even pattern matching.

[iam3yal]

The general concept of parameter null checking is great but the syntax although short always felt to me out of place so I think you made the right decision. personally I prefer something like contracts.

[theunrepentantgeek]

I agree that something to simplify parameter checking would be great - but I don't know that it needs to go as far as full code contract support. In the earlier proof-of-concept work, the flow analysis verifying the code contracts was super slow.

In the LDM notes it says

void M(string s) where s is not null;
This looks like a promising start for a full contracts feature, but not for a feature only doing null checking of parameters.

I'm curious how much this syntax got scuppered by a superficial resemblance to prior code contract work.
(Admittedly, I explicitly called this similarity out with some of my prior comments.)

Is there a sweet spot nearby for a feature that does guard clauses only?

where expr

• expr must be a Boolean expression consuming exactly one parameter
• If expr returns false, an ArgumentException is thrown

A couple of examples

public void Example(User user)
    where user != null
{
    // ... elided ...
}

public void MinimumRate(int rate)
    where rate > 0
{
    // ... elided ...
}

I'm aware this doesn't have the conciseness previously championed by many - but perhaps this is a habitable compromise?

One more observation:

Having !! on the parameter name made it seem that it was a part of the method signature - but it was not. Null checks are an implementation detail of the method itself. It's already well established that type constraints (our current use of the where keyword) are not part of the method signature, so the where clause is a natural destination for pulling things out of the method body.

[CyrusNajmabadi]

Constraints are definitely part of the signature. :-)

[theunrepentantgeek]

I must have used the wrong terminology - apologies.

I was referring to this: If you have two methods that differ only in the generic constraints, they have to be given different names, else you get a compiler error:

public class Demo
{
    public void Write<T>(T t) 
        where T : class 
        {}
    
    // error CS0111: Type 'Demo' already defines a member called 'Write' with the same parameter types
    public void Write<T>(T t)
        where T: struct
        {}
}

[HaloFour]

This is private protected all over again. As someone who actually cares about null checking I was looking forward to a shorthand for it. So much of the negative feedback seemed to be more annoyed that NRTs weren't enforcing this for them. But that change isn't going to happen either, so we're all worse off for it. Might as well pull required properties, too, for the same exact reason.

As for contracts, they are an incredibly heavy solution, one mired in tons of policy. The flow analysis would be many, many orders of magnitude more complicated (and expensive) than NRTs. Contracts were one of the first proposals on CodePlex, and I have severe doubts that they'll go anywhere.

[TonyValenti]

I am sad to see parameter null checking go but I agree it is the right move. It really seemed to be a very specialized case of a contract/validation framework and I'd much rather see that fully fleshed out.

[333fred]

Private protected was actually name checked several times in the meeting, but our concern is that the private protected feature is a relatively niche thing. Null checks on parameters are not.

[iam3yal]

@HaloFour contracts is heavy solution but what can I say I was one of the early adopters of CodeContracts and I liked it and like I said the concept of parameter null checking is good but I think they can do better with the syntax. My issue with NRT is that they initially sold it out as lightweight solution and it is for the most part but then we started to get many attributes to tweak the behaviour of the static analysis to the point you need a dictionary not to mention verbose.

[alrz]

As for contracts

I recall one of the principles that the team voiced in the past was "don't put it in the method signature if it doesn't affect the method signature"; apply it to parameter null checking and this will be reduced to method contracts, or a trimmed down version of it, mind you.

edit: @HaloFour I'd agree this is bikeshedding as far as the choice of the syntax goes, but I think the distinction is about how you define this feature:

• if this is about code generation, use an attribute
• if this is about extending the type system, use T!
• if this is about context, follow checked/unchecked route and a cli flag
• if this is about runtime/compile-time verification, use method contracts
• if this is only about having a specific syntax for null checking, use !! parameters and !! expressions

From there, the choice of syntax is constrained within the area that this is a part of.

[qrli]

I'd prefer doing null check in the direction of NRT, e.g.:

[GenerateNullCheck]
void M1(string a, string? b, string c)
{
    // generate null checks for a & c
}

void M(string a, string? b, [GenerateNullCheck] string c)
{
    // generate null checks for c only
}

[TonyValenti]

I really like the idea of it being attribute based!

Here is an idea:
What if there was a ParameterValidorBase attribute. When applied to a parameter, Validate method on the attribute is called with the parameter and the name of the parameter. This method would be responsible for throwing any exceptions.

[TonyValenti]

@qrli - Here is a little bit more around what I was thinking.

The compiler should automatically look for a static Validate method on the validator type and, if it is found, invoke it. If it is not found, it should create an instance of the attribute class and invoke the validate method on the instance.

using System.Runtime.CompilerServices;

Test("Hello", "");

static void Test([ErrorIfNull] string? Name, [ErrorIf<string>("")] string Value) {
    //Auto Generated Code
    {
        if (ErrorIfNullAttribute.TryValidate(Name) is { } Error1) {
            throw Error1;
        }
        if (new ErrorIfAttribute<string>("").TryValidate(Value) is { } Error2) {
            throw Error2;
        }
    }
    //Method body

}


[AttributeUsage(AttributeTargets.Parameter)]
public class ValidationAttribute : Attribute { 

}

public class ErrorIfAttribute<T> : ValidationAttribute {
    private T ErrorValue { get; }
    public ErrorIfAttribute(T ErrorValue) {
        this.ErrorValue = ErrorValue;
    }

    public Exception? TryValidate(T value, [CallerArgumentExpression("value")] string? ArgumentName = default) {
        var ret = default(Exception?);

        if (EqualityComparer<T>.Default.Equals(value, ErrorValue)) {
            ret = new ArgumentOutOfRangeException(ArgumentName);
        }

        return ret;
    }
}

public class ErrorIfNullAttribute : ValidationAttribute {
    public static Exception? TryValidate<T>(T value, [CallerArgumentExpression("value")] string? ArgumentName = default) {
        var ret = default(Exception?);
        
        if (value == null) {
            ret = new ArgumentNullException(ArgumentName);
        }

        return ret;
    }
}

[iam3yal]

This would increase complexity and verbosity though back then I suggested something similar, also there are some interesting suggestions there.

[huoyaoyuan]

The importance of parameter null checking actually depends on the code style. In BCL is mandatory so I can see the reason of eager adoption. In other projects people may be simply not checking nulls.
For me it's much less important than generic math.

[En3Tho]

Was there a poll of some sort? I believe such things work if you can make silent majority express their voice.

I don't see how this feature breaks something in the future. Even if roles (for example a role ensuring value is not null) or active patterns (apply active pattern checking for null on a parameter) get done they still feel like an overkill for a null check. Especially if I just need to ... check for null.

With nrt days of all over the place nullchecks are gone and this feature could help with all the remaining noise. Nor attributes nor super special keywords help with that because they still litter your code just for a null check.

I'm disappointed and I'm waiting for a poll.

[BreyerW]

This looks like a promising start for a full contracts feature, but not for a feature only doing null checking of parameters.

Just a small note nothing stops you from doing features piece by piece just like all current pattern matching works (eg list patterns didnt land with 1.0v of pattern match features) as long as it useable and extensible

[yaakov-h]

Pattern matching had a plan to go places and each of the pieces made sense on their own.

Contracts is unlikely to ever happen so I don't see much point in doing one tiny aspect of it.

[TahirAhmadov]

For the record, despite my reservations about async methods and "quickthrow", I was fully supportive of the feature in general, as it would have worked 90% of the time beautifully.
Has this syntax been considered?

void Foo(string s is not null) {} // or even
void Foo(string s not null) {}

I know this is more verbose, and I would have preferred !! more personally, but if I understand the feedback correctly, many people didn't like the !! syntax. The above 1) replaces !! with something (hopefully) more palatable, 2) it leaves the door open to expand on it with contracts (if that were to ever happen in the future), and 3) is slightly less verbose than appending a where s is not null after the parameter list.
Thoughts?
PS. Also, does it make sense to move from warnings to errors when this is present, and calling the method with a known-to-be-potentially-null value?

[CyrusNajmabadi]

Has this syntax been considered?

Yes. It's literally in the notes as option 6. :)

[CyrusNajmabadi]

The above 1) replaces !! with something (hopefully) more palatable, 2) it leaves the door open to expand on it with contracts

Here's the thing:

1. it was definitely not more palatable.
2. it doesn't really open the door for contracts. If anything, it may close doors as contracts are far too vague and ill-defined, so trying to just randomly guess at waht they will be can often be worse as we may want somethnig entirely different that this would not conflict with.

[TahirAhmadov]

Has this syntax been considered?

Yes. It's literally in the notes as option 6. :)

Oops, didn't notice it. I kind of disagree that concerns with o.5 apply to o.6. This reads very naturally to me:

void Foo(string s not null = "") { }

[TahirAhmadov]

1. it was definitely not more palatable.

Like I said, personally I was perfectly happy with !!. In fact, I still prefer it to all other options - I'm in line with LDT on this. I only brought up option 6 to try and find a solution which gets more support from others.

2. it doesn't really open the door for contracts.  If anything, it may close doors as contracts are far too vague and ill-defined, so trying to just randomly guess at waht they will be can often be worse as we may want somethnig entirely different that this would not conflict with.

I know what you are saying, but I'm more optimistic about this. To me, it seems natural to apply the pattern matching to "method contracts" (if it's ever done); I definitely don't see a new syntax created for them. Obviously, not null is already an existing pattern, so adding and > 5 and whatnot, would be very intuitive.
However, again, I'm not dying for method contracts and was perfectly happy with !!.

[CyrusNajmabadi]

Obviously, not null is already an existing pattern, so adding and > 5 and whatnot, would be very intuitive.

It's not at all clear that that's what i would want with contracts. I would be very wary about locking into that now without the first inkling of how contracts would possibly shake out.