nipunn1313 / mypy-protobuf

open source tools to generate mypy stubs from protobufs
Apache License 2.0
647 stars 80 forks source link

Manage optional field in Message body #579

Open Dufgui opened 7 months ago

Dufgui commented 7 months ago

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: ...
nipunn1313 commented 6 months ago

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