Remove ProtoPreconditions.CheckNotNull from setters C# .NET

[marqustd]

In proto3 all non-basic fields are optional and should treated as nullable. Nullability is not supported #6632 and probably won't be, but right now working with generated files is a mess.

Generated types project does not support <Nullable>enable</Nullable>, so compiler allows assigning nulls

new FooType 
{
FooField = null // no warning in IDE
}

But there is null checking in setter
https://github.dev/protocolbuffers/protobuf/blob/3becebb82a745b1a50e4cd2c5d44b4d22c15d4be/src/google/protobuf/compiler/csharp/csharp_primitive_field.cc#L143
which will throw an exception if we try to set value to null.

Current solution to make it working is to NOT SET THE FIELD.

new FooType 
{
//FooField = null // do not assign null!!1
}

I would like to see either nullability which was introduced in C#8 in 2019 or at least being able to consciously and purposely assign a null.

[jskeet]

No, just ignoring invalid values instead of failing on them seems like a very bad idea to me. Just don't set the properties to null. (Note that message type properties can be set to null.)

[jskeet]

Ah, I think I misunderstood the proposal. If you want setting the value to null to be equivalent to Clear, I think that would be confusing - because calling the getter would then still return an empty string or byte string. (Note that this really only applies to string and bytestring fields anyway. As I said before, message type fields can already be set to null.)

[marqustd]

Ok, I now see my problem. Thanks :) So now I only wait for #6632, because this will protect guys like me from assigning a null to a string.

If you want setting the value to null to be equivalent to Clear

What do you mean?

[jskeet]

If you want setting the value to null to be equivalent to Clear

What do you mean?

I mean that if the field is marked as optional in proto3, there will be a Clear method, e.g. ClearName(). I had thought that you'd wanted message.Name = null; to be equivalent to message.ClearName(); - but message.Name will be an empty string (not a null reference) after that. (If we were designing everything from scratch, we might take a different approach to that, but I'm not sure. At the moment changing it would be a massively breaking change.)

[marqustd]

Yeah, I would expect that any optional field will be mapped to sth like

public string? Name { get; set;}

and then being able to assign a value

new FooType 
{
   Name = input.Any() ? Translate(input.First) : null
}

Ok, I can set string.Empty though in my mind optional == nullable

[jskeet]

Ok, I can set string.Empty though in my mind optional == nullable

I can see why, but that's not how it's generally represented - and in particular, it's definitely not how it's represented for primitive numeric fields; even if we started from scratch, we'd need to very carefully consider whether we wanted to do that... because it means adding field presence (by making an existing field optional) becomes a breaking change, where it's not at the moment.

I'm going to close this issue now as we're definitely not going to make the change given where we are today. While it might be interesting to speculate what we'd do in a completely green-field environment, unfortunately it's also time-consuming :(

[felschr]

Regarding the issue of null checking in the setter & fallback to a default value in the getter, there is issue:
#9352

If you want to be able to directly get & set null values on a property, the best solution seems to be replacing optional scalar types with well known wrapper types, e.g. optional string -> google.protobuf.StringValue.