prost icon indicating copy to clipboard operation
prost copied to clipboard

oneof generates Option + Enum rather than only Enum?

Open vedantroy opened this issue 3 years ago • 6 comments

This is a question rather than a issue. I notice that using a oneof will generate an Option that contains an Enum. Why doesn't prost generate an Enum directly?

Example (I'm surprised that the result field doesn't have the type Result directly):

#[derive(Clone, PartialEq, ::prost::Message)]
pub struct CrawlResult {
    #[prost(oneof = "crawl_result::Result", tags = "1, 2")]
    pub result: ::core::option::Option<crawl_result::Result>,
}
/// Nested message and enum types in `CrawlResult`.
pub mod crawl_result {
    #[derive(Clone, PartialEq, ::prost::Oneof)]
    pub enum Result {
        #[prost(message, tag = "1")]
        Page(super::Page),
        #[prost(string, tag = "2")]
        Error(::prost::alloc::string::String),
    }
}

vedantroy avatar Feb 11 '21 17:02 vedantroy

Because the result oneof can be entirely unset.

sfackler avatar Apr 09 '21 20:04 sfackler

Every ::prost::Message is Default. What would be the default value of the variable if it isn't Option?

Jasperav avatar Apr 15 '21 17:04 Jasperav

I'd love an option to have parsing fail if a oneof field is not set.

My code is littered with unwrapping these options and manually failing if it is None.

benma avatar Oct 02 '21 00:10 benma

I understand that the protobuf "spirit" is that everything should be optional to allow protocol evolution, and presence should be checked at the application level. So, I understand that evety oneof is optional, and it could be missing entirely.

However, for ergonomics, I'd like to be able to instruct prost to generate an enum with one more branch, named None (or Absent, or Missing, but IMHO the name None would be usable and appropriate). This branch would have the semantics "the whole oneof is absent".

It would be like "flattening" the rust Option and Enum types that are used now. The optionality semantics would still be there, but the ergonomics (especially for value literals) would be quite different. Sometimes the explicit Option would still be desirable, but often I would find the flattened type easier to use. Ideally this should be an opt-in choice, using an annotation in the proto file.

Would this be acceptable? If yes, given directions, I would be willing to implement it!

massimiliano-mantione avatar Nov 26 '21 07:11 massimiliano-mantione

@massimiliano-mantione I think this sounds like an excellent idea, though I would love to also be able to get rid of the optionality entirely. This is useful for a server that only ever produces a response type for instance but never needs to read such a type from memory. It makes it more ergonomic as you can't accidentally set None.

Victor-N-Suadicani avatar Jun 21 '22 08:06 Victor-N-Suadicani

Removing Option would also be very appreciated for submessages, not just enums. In my project, unset submessage or oneof fields are always translated into errors manually, which adds a lot of noise to the code. It would be awesome if there was an option to make it a decode error instead.

Would there be interest in such an option?

benma avatar Aug 09 '22 22:08 benma

For now prost will continue to use the Option as that was the original design and I believe its still the correct way even if its not the most ergonomic.

LucioFranco avatar Nov 04 '22 15:11 LucioFranco