Investigate support for nullable reference types in generated code

[JamesNK]

Code generated from .proto files does not include nullable reference type metadata.

Investigate:

• [ ] Does the status-quo lead to undesirable warnings
• [ ] Could Grpc.Tools be updated to detect when it is in a project with nullable reference types enabled and generate metadata

[AlgorithmsAreCool]

Just noticed the lack of nullable annotations in the generated client.

I think this is pretty important since the semantics of proto3 allow for any property to be null.

[AlgorithmsAreCool]

See also: grpc/grpc#20729 protocolbuffers/protobuf#6632

[bastianeicher]

The google.api.FieldBehavior option provides a somewhat standardized way to express the intent of whether a field should be nullable or not:

string id = 1 [(google.api.field_behavior) = REQUIRED]; // not nullable
string name = 2 [(google.api.field_behavior) = OPTIONAL]; // nullable

If you do end up supporting nullable reference types it would be great if the protoc plugin could also take these options into account if they are present, rather than assuming that all fields are nullable.

[Falco20019]

@bastianeicher The FieldBehavior fulfills another need. It designated to give hints about the input-output-usage of the fields. Using it for two purposes might lead to problems.

A string field in proto3 must never become null, since the value is not transmittable on the wire and would therefore lead to unstable state (memory state != transmission state). The way to correctly specify a nullable string in proto3 (and proto2) is to use google.protobuf.StringValue instead from the google/protobuf/wrappers.proto. This is a well-known type in all languages and will use system-native types over generated ones.

Using nullability, I would expect the code generator to generate string for string and string? for google.protobuf.StringValue. Just like int32 is generating int and google.protobuf.Int32Value generated int?. Right now, both are generated to string since without adding nullability support, there is no good way to do this.

[bastianeicher]

@Falco20019 Ah, you're right of course. string was a bad example for this kind of nullability.

However, for fields with a message type I still think google.api.field_behavior could be helpful.

For example:

google.protobuf.Timestamp a = 1 [(google.api.field_behavior) = REQUIRED];
google.protobuf.Timestamp b = 2 [(google.api.field_behavior) = OPTIONAL];

could map to:

public Google.Protobuf.WellKnownTypes.Timestamp { get; set; }
public Google.Protobuf.WellKnownTypes.Timestamp? { get; set; }

The presence of a REQUIRED option of course does not actually guarantee the field cannot be 'null on the wire. But if for example a gRPC client trusts a gRPC server to stick to its own interface definition, this could make null-related much easier to reason about.

[SonicGD]

I think support of google.api.field_behavior would be great for implementing nullability and for indication of required fields.

[tonydnewell]

See

• https://github.com/grpc/grpc/pull/33878
• https://github.com/grpc/grpc/pull/33879
• https://github.com/protocolbuffers/protobuf/pull/13218

[ytimenkov]

Great to see that you're making progress on this.

Because current behavior is quite misleading and I would say dangerous. proto3 in principle doesn't allow required fields and therefore all are optional / nullable. However the code is generated and injected into the project as a source code and if project has nullable enabled this tricks static analyzer into believing that properties can't be null and instead of getting a compile-time error one can get a runtime exception at a quite inappropriate moment.

(at least this is unusual to me who skipped decade(s) of dark C# ages and returning when things got nice and shiny :D)

[tonydnewell]

A gRFC has been published to discuss: https://github.com/grpc/proposal/blob/master/L110-csharp-nullable-reference-types.md

Discussion: https://groups.google.com/g/grpc-io/c/R1agzzcmKiE/m/4-HQgggrAgAJ