New deeply nested oneof and view paths are difficult to use

[techninja1008]

The new __buffa module structure introduced in 0.4.0 results in long, difficult to read paths for even the simplest of cases. For example, a message named Event in a package named test, with a oneof named payload results in test::__buffa::oneof::event::Payload, when previously it would have been test::event::Payload, which is much clearer.

The ideal solution would be configuration options to determine how oneof types/view types/extensions get generated. This would ideally offer all of the following modes:

• Unambiguous (default): the behavior of 0.4.0
• Simple: places types alongside their message, named after their message but with a suffix. Example from above would produce test::EventPayload. If this would result in a naming collision, an error thrown during codegen.
• Simple with fallback: same as "Simple", however related/generated types that would collide with messages/enums are skipped, falling back to the default __buffa location.

Due to the need for consistent paths when paired with external code gen (eg. when a crate is being used with extern_paths), all modes other than "Unambiguous" would actually still generate the code in the same way as "Unambiguous", except the __buffa modules would be annotated with #[doc(hidden)] and the types within would be re-exported (with pub use) at their 'expected' locations. This means any code generated with a mode other than unambiguous would still be compatible with downstream codegen that expects types to be at their "unambiguous" locations.

[iainmcgin]

I had debated this with some colleagues when we were switching to the __buffa:: deconflicting approach - my concern was that the inconsistency around types being re-exported in some circumstances and not in others would be confusing, and therefore for an initial pass, it was better to be unambiguous, even if the use directives were ugly. I'd also prefer not to introduce more configuration properties unless they are strongly justified.

Having said that, I think that the conflicts we have seen are relatively rare - we needed to handle them for correctness reasons, so that we can compile all valid protobuf definitions, but we should try to avoid ergonomic sharp edges in the ~99.9% of normal cases for the ~0.1% of awkward cases. So, I think you've convinced me that we should:

• Keep the unambiguous structure to avoid conflicts - code generation in both buffa and connect-rust will always use this form.
• Re-export the non-conflicting types into the "natural" module structure.

This is your "simple with fallback" approach, and I think this should be the new default. Whether I add an option for "simple only, error on collision" is a separate question - it's not clear that this won't introduce a cascade of other related configuration, e.g. "use simple here, not there" include/exclude type lists, so I think I'd prefer to implement simple with fallback only for now.

[techninja1008]

Agree with everything you've said.

I'd also add though that it's not just the use directives being ugly. In circumstances where you have multiple oneofs with the same name (on different messages, but in the same package) it can start to become hard to even write valid sets of use directives without doing use ... as ..., which significantly worsens the consumer UX (as IDEs tend to be optimised for auto-import, without renaming/aliasing). This is particularly because you can't even just go up a single level and refer to them by the the message module (ie. as event::Payload in the original example) as that would possibly conflict with an import for the identically named view module.

I agree that "Simple" without fallback probably isn't hugely useful.

Somewhat related/unrelated: it might be worth considering whether any slightly clever usage of (generic) associated types and/or type aliases could increase usability further. For example:

/// Purposefully distinct from [`Message`], because views are optional
pub trait HasView {
    type View<'a>;
}

/// [`buffa`]-exposed helper type alias to allow clean reference to views.
pub type View<'a, T> = <T as HasView>::View<'a>;

All types that have a corresponding view type would implement HasView. This would mean normal (non-view) types could still be referred to as eg. Event, whilst the corresponding view would become View<'a, Event>. Because of rust's visibility rules, it would actually be possible to hide direct access to the view types entirely (by making their containing modules non-pub, whilst leaving the actual types themselves as pub). This would clear up the exports considerably, whilst potentially improving ergonomics, as using both views and owned forms of the same messages would only require a single extra import of View compared to just using owned forms.

Unfortunately, I don't think there's a similar solution for oneofs or nested messages, at least without something like Inherent Associated Types (rust-lang/rust#8995) or something a bit cursed (such as generating a custom trait for each message type that has nested types of any kind - even then though you would have the problem of naming the trait, and consumers would have to import it to get access to the associated types).