mypy-protobuf icon indicating copy to clipboard operation
mypy-protobuf copied to clipboard

Manage optional field in Message body

Open Dufgui opened this issue 1 year ago • 1 comments

Optional field are well manage in constructor but not in the class field declaration. I think it"s a quick fix to do in writeMessage as I can see but not sure. Today generation is:

class MyRequest(google.protobuf.message.Message):
    DESCRIPTOR: google.protobuf.descriptor.Descriptor

    field1: builtins.str
    optionalfield1: builtins.str
    optionalfield2: builtins.str
    def __init__(
        self,
        *,
        field1: builtins.str = ...,
        optionalfield1: builtins.str | None = ...,
        optionalfield2: builtins.str | None = ...,
    ) -> None: ...

But it must be:

class MyRequest(google.protobuf.message.Message):
    DESCRIPTOR: google.protobuf.descriptor.Descriptor

    field1: builtins.str
    optionalfield1: builtins.str | None
    optionalfield2: builtins.str | None
    def __init__(
        self,
        *,
        field1: builtins.str = ...,
        optionalfield1: builtins.str | None = ...,
        optionalfield2: builtins.str | None = ...,
    ) -> None: ...

Dufgui avatar Mar 05 '24 13:03 Dufgui

my understanding is that for proto3, scalar non-repeated fields are not actually optional on the wire, nor on the generated class. IE - if you construct it with None, my understanding is that you will get the 0-value.

https://github.com/nipunn1313/mypy-protobuf/blob/7506de3f39469e1a6665f1bf388f506a40571996/mypy_protobuf/main.py#L439

It's possible my understanding is wrong. I'd be welcome to a fix - as long as it includes a unit test that proves that this behavior is expected - eg https://github.com/nipunn1313/mypy-protobuf/blob/7506de3f39469e1a6665f1bf388f506a40571996/test/test_generated_mypy.py#L421

nipunn1313 avatar Apr 23 '24 17:04 nipunn1313