[RFC] Flatten PartialX Types with X

[laralove143]

discord doesn't always end us the full data, which is handled by creating partially duplicate types such as https://api.twilight.rs/twilight_model/guild/struct.PartialMember.html and https://api.twilight.rs/twilight_model/guild/struct.Member.html but this comes with the obvious problem of duplicate fields

it also makes generalizing things more involved (implementing X trait on members means implementing it on Member and PartialMember, most likely duplicating much of the function body because of type-safety), this came up in the now-stale cdn endpoints creator module and happens in twilight-mention (for example Mention isnt implemented for PartialMember or PartialUser and it's implemented twice for User and CurrentUser)

proposal

essentially rename PartialX to X, add an Option<Enum> field to X, where Enum has variants for each of the specialized fields

struct MemberMessage {
  permissions: Option<Permissions>,
  // other fields specific to `PartialMember`
}

struct MemberUpdate {
  flags: MemberFlags,
  // other fields specific to `MemberUpdate`
}

enum MemberData {
  Message(MemberMessage), // what's currently named `PartialMember`
  Update(MemberUpdate),
}

struct Member {
  id: Id<UserMarker>,
  // common fields across all `Member` types
  data: Option<MemberData>, // can be non-option if there's always some variant that's returned
}

this would also improve documentation, currently we have many duplicate types that aren't put in the same module

drawbacks

• sometimes we might not know which variant to use, meaning we want to use serde's untagged representation
• sometimes the main struct might not be enough for some methods, meaning those methods fail if the data's variant isn't expected, see alternative for a solution
• there will still be duplicate fields across different enum variants, unless we nest enums inside enums which i think is going too far
• adding new variants is a breaking change unless we make the enums #[non_exhaustive] which might be inconvenient to work with for the users, see alternative for a solution

alternative

we could also make the types have a generic parameter which is used in the data field, this solves some of the drawbacks but introduces others, see below for them

struct MemberMessage {
  permissions: Option<Permissions>,
  // other fields specific to `PartialMember`
}

struct MemberUpdate {
  flags: MemberFlags,
  // other fields specific to `MemberUpdate`
}

struct Member<T> {
  id: Id<UserMarker>,
  // common fields across all `Member` types
  data: T,
}

// `Member` will be used like
Member<NoData> // akin to `Member.data = None`
Member<MemberMessage>
Member<MemberUpdate>

drawbacks

• not applicable when we don't know the type of the data field
• might make it harder for users to know what T can be, we could implement a marker trait for possible T types

[itohatweb]

I don't think that we should implement this.

This increases the complexity to work with normal - non partial - structs. I don't think that this is good DX:

let member = get_member();
let flags = match member {
    MemberData::Message(data) => data.flags,
    _ => unreachable!(),
}

vs.

let member = get_member();
let flags = member.flags;

This is imho the biggest drawback. You know for 100% which variant it is - e.g. because the member comes from a GuildMemberAdd event - but you still need to match the enum, in order to get the data out.

Additionally we would also need to implement custom deserializers for every struct which has a partial representation, which is also not really feasible.

Generally imho there are too many drawbacks compared to the few benefits this would bring.

Keeping code DRY is definitely something we should keep in mind, but overdoing it is also not good, we need to find the middle way.

[vilgotf]

Drive-by contribution.

This is imho the biggest drawback. You know for 100% which variant it is - e.g. because the member comes from a GuildMemberAdd event - but you still need to match the enum, in order to get the data out.

Just want to mention let-else as a better alternative to matching:

let member = get_member();
let MemberData::Message(data) = member else { unreachable!() };
let flags = data.flags;