Getters for nested messages, and oneof fields

[dae]

Hi there,

Firstly, thank you for your work on Prost - I have been finding it very useful.

I'd like to propose two changes to make dealing with nested messages and oneof fields more ergonomic:

1. adding an accessor for nested messages, like in the C++ bindings
2. adding an accessor for oneof fields, unlike the existing bindings.

A brief example of what I mean:

syntax = "proto3";

package Example;

message One { 
    // ....
}

message Two { 
    // ....
}

message Union {
  oneof value {
    One one = 1;
    Two two = 2;
  }
}

enum Choice {
  FIRST_OR_DEFAULT = 0;
  SECOND = 1;
}

message TopLevel {
  Union tagged_union = 1;
  Choice choice = 2;
}

For point 1:

    let top = TopLevel::default();

    // for a standard enum, we get a convenient accessor
    assert_eq!(top.choice(), Choice::FirstOrDefault);

It would be nice if a similar accessor existed for nested messages, like in the C++ bindings:

    let tagged_union = top.tagged_union();

Currently, we need to manually handle it instead:

    let tagged_union = top.tagged_union.unwrap_or_default();

For point 2:

Matching on the inner tagged union is even more awkward, since we always need to account for the missing case, either by matching on the outer optional, or by manually specifying the default value. eg:

    match top
        .tagged_union
        .unwrap_or_default()
        .value
        .unwrap_or_else(|| Value::One(Default::default()))
    {
        Value::One(_one) => todo!(),
        Value::Two(_two) => todo!(),
    };

Unlike standard enumerations which default to the first value when missing/invalid, the C++ & Python bindings appear to expose oneof as an optional with no default. If Prost provided an accessor that behaved like the standard enum accessor, returning the first variant when missing/unset, it would mean the above code could be simplified to:

    match top.tagged_union().value() {
        Value::One(one) => todo!(),
        Value::Two(two) => todo!(),
    };

Users would be able to continue to inspect&use the field directly, for cases where they want to handle the missing/invalid case. But as I don't see precedent for this in the C++ or Python rules, I can understand how you might be more reluctant about part 2.

If I'm able to figure out how to implement these, would you be interested in accepting a PR that adds one or both of these changes?

[danburkert]

For point 1, what would the function signature be? I don’t see how that can be defined without cloning the nested message or modifying the outer message, neither of which seem like a good idea to me.

[dae]

I fear I might be missing something - couldn't the getter return a Cow?

[dae]

eg:

pub mod pb {
    include!(concat!(env!("OUT_DIR"), "/example.rs"));
}

use pb::{union::Value, Choice, One, TopLevel, Union};
use std::borrow::Cow;

impl TopLevel {
    fn tagged_union(&self) -> Cow<Union> {
        if let Some(inner) = self.tagged_union.as_ref() {
            Cow::Borrowed(inner)
        } else {
            Cow::Owned(Union::default())
        }
    }
}

impl Union {
    fn value(&self) -> Cow<Value> {
        if let Some(inner) = self.value.as_ref() {
            Cow::Borrowed(inner)
        } else {
            Cow::Owned(Value::One(Default::default()))
        }
    }
}

fn main() {
    let top = TopLevel::default();

    match top.tagged_union().value().as_ref() {
        Value::One(_one) => todo!(),
        Value::Two(_two) => todo!(),
    };
}

Though to be fair, I hadn't actually thought through the implementation when I made the original post, and it's not quite as trivial as I was hoping it would be.