OneOf fields not typed as optional in constructor

[aidandj]

Given this proto:

enum Enum {
    UNKNOWN=0;
    KNOWN=1;
}
message WithOneOfs {
    oneof optional_enum {
        Enum some_enum = 1;
    }
}

The constructor looks something like this:

    def __init__(
        self,
        *,
        some_enum: global___Enum.ValueType = ...,
    ) -> None: ...

But the OneOf block is effectively marking the some_enum field as optional, but type checkers error when you try and pass None to the constructor.

[nipunn1313]

In the mypy-protobuf generator, we have this

for field in constructor_fields:
    field_type = self.python_type(field, generic_container=True)
    if self.fd.syntax == "proto3" and is_scalar(field) and field.label != d.FieldDescriptorProto.LABEL_REPEATED and not self.relax_strict_optional_primitives and not field.proto3_optional:
        wl(f"{field.name}: {field_type} = ...,")
    else:
        wl(f"{field.name}: {field_type} | None = ...,")

so it looks pretty explicit that we don't allow passing None in when the field is scalar.

We have some tests of this here - that show that protos give TypeError when you pass in None for a scalar field. Hence only the message fields (not scalar) are optional.
https://github.com/nipunn1313/mypy-protobuf/blob/main/test/test_generated_mypy.py#L421

I don't actually know what happens for enums inside oneof. We have such a thing in the tests, but not in the runtime test cases.
https://github.com/nipunn1313/mypy-protobuf/blob/main/proto/testproto/test3.proto#L39

I think step one is figuring out what the runtime behavior is for enums inside oneof - do they operate more like messages or like scalars. Can do this by adding a test case here
https://github.com/nipunn1313/mypy-protobuf/blob/main/test/test_generated_mypy.py#L421

Once we know how it should behave, we can decide how to proceed.

Thank you for the report!

[aidandj]

So here is some exploration I did. Interestingly I was able to pass None in for all of the fields, scalar or enum, or oneof. But as expected, if you set the enum in the oneof to None, then the oneof did not get set.

    x = SimpleProto3(a_string=None, outer_enum_in_oneof=None,  # type: ignore
                     inner_enum=None)  # type: ignore
    assert x.a_string == ''
    assert x.outer_enum_in_oneof == 0
    assert x.WhichOneof('a_oneof') is None
    assert x.inner_enum == 0
    assert x.inner_enum_in_oneof == 0

    x = SimpleProto3(a_string="hello", outer_enum_in_oneof=FOO3)
    assert x.a_string == 'hello'
    assert x.outer_enum_in_oneof == FOO3
    assert x.WhichOneof('a_oneof') == 'outer_enum_in_oneof'

I think technically anything inside a oneof can be set to None which results in the oneof having nothing set.

[nipunn1313]

ok interesting!

Cool find - seems like the behavior is slightly different when the scalar field is part of a oneof vs in the message.

Would take a fix along with a unit test that proves that the types match the runtime behavior.