Snafu for enums with variant types

[michiel-de-muynck]

Motivation

Currently, Snafu is designed to work with enums like

enum MyEnum {
    VariantA {
        val1: Foo,
        val2: Bar
    },
    VariantB {
        val1: Baz,
        val2: i32
    },
}

However, a somewhat common pattern in Rust (examples 1, 2) is to have enums like

struct VariantA {
    val1: Foo,
    val2: Bar
}
struct VariantB {
    val1: Baz,
    val2: i32
}
enum MyEnum {
    VariantA(VariantA),
    VariantB(VariantB)
}

While it is slightly more verbose, it has a few advantages:

1. Since enum variants are not types, it's not possible to accept a MyEnum::VariantA as a function parameter or to store it somewhere. You can only pass/store MyEnum. However, when you've made each variant its own type, VariantA can be passed around or stored.

   Note: there is an RFC which proposes to add variant types to Rust, which would remove this advantage. However, this comment doesn't give the impression that this will be implemented soon.

2. If multiple enums can have the same variant, you must repeat that variant (and all its fields) in each enum definition. With the second style, you just repeat the VariantA(VariantA) line.

3. When matching on this kind of enum, if a variant has a lot of fields, in the first style you have to match like

   match my_enum {
       VariantA{ val1, val2, val3, val4, val5, .. } => /* ... */
   }

   whereas if you use the second style, you can match like (example)

   match my_enum {
       VariantA(a) => /* use a.val1, a.val2, etc */
   }

Snafu does not support this second style of error enum, but I think support for it can be added backwards-compatibly, since #[derive(Snafu)] currently isn't implemented for regular structs (it's only implemented for single-field tuples), and it also isn't implemented for enums with single-field tuple variants (only struct-like and unit variants). So this second style of enums neatly falls outside of what Snafu currently supports.

The only issue is that there will be a naming collision between variant data structs and context selectors. Both are (according to the regular conventions) given the same name as the enum variant. So some bikeshedding will need to be done here, which I do below.

Bikeshedding

My vote for how to color the bike shed is as follows:

1. Allow #[derive(Snafu)] for regular Rust structs. If the struct is called Foo, this creates a new struct FooContext, which is the context selector. Any attributes that currently work on struct-like enum variants work on structs as well, and they do the same thing, essentially as if you had an enum with a single variant. Display and Error are implemented for Foo, IntoError<Foo> is implemented for FooContext, etc.

2. Allow #[derive(Snafu)] for enums that have single-field tuple variants. Display and Error are implemented as usual, but for these single-field tuple variants, no context selector is generated (it should've been generated in step 1 above). For an enum MyEnum with variant VariantA, I think IntoError<MyEnum> should not be implemented for VariantAContext even though it could, but instead we should implement From<VariantA> for MyEnum. I think this should allow .context(VariantAContext { ... })? to work in a function returning MyEnum (the ? operator does the .into()). Also implementing IntoError<MyEnum> for VariantAContext would probably break type inference.

3. I suppose there should also be a way to specify the name of the generated context selector. This would be part of the #[snafu(...)] attribute, e.g. #[snafu(context(MySelectorStructName), display(...)] or something. This would need to be specified both on the variant-data struct and on the enum variant.

Here's how the example from the snafu docs homepage would look with this style:

#[derive(Debug, Snafu)]
#[snafu(display("Could not open config from {}: {}", filename.display(), source))]
struct OpenConfig {
    filename: PathBuf,
    source: std::io::Error,
}

#[derive(Debug, Snafu)]
#[snafu(display("Could not save config to {}: {}", filename.display(), source))]
struct SaveConfig {
    filename: PathBuf,
    source: std::io::Error,
}

#[snafu(display("The user id {} is invalid", user_id))]
struct UserIdInvalid { user_id: i32, backtrace: Backtrace }

#[derive(Debug, Snafu)]
enum Error {
    OpenConfig(OpenConfig),
    SaveConfig(SaveConfig),
    UserIdInvalid(UserIdInvalid),
}

type Result<T, E = Error> = std::result::Result<T, E>;

fn log_in_user<P>(config_root: P, user_id: i32) -> Result<bool>
where
    P: AsRef<Path>,
{
    let config_root = config_root.as_ref();
    let filename = &config_root.join("config.toml");

    let config = fs::read(filename).context(OpenConfigContext { filename })?;
    // Perform updates to config
    fs::write(filename, config).context(SaveConfigContext { filename })?;

    ensure!(user_id == 42, UserIdInvalidContext { user_id });

    Ok(true)
}

[ExpHP]

Supporting #[derive(Snafu)] on structs is also useful for reasons beyond this use case. For error types that wrap errors from FFI, many C APIs just give integer error codes, or something like a pair of "error" and "severity" codes:

use winapi::shared::minwindef::DWORD;

#[derive(Debug, Snafu)]
#[snafu(display("Function {} failed with code {}", func_name, code))]
pub struct WindowsError {
    code: DWORD,
    func_name: &'static str,
}

In these use cases, no outer enum type is ever actually needed, because all of the different errors that may occur are represented by a single struct.

[shepmaster]

@migi thank you for the clear and well-thought-out case. I find myself basically in complete agreement. I think the only issues would be those that might come up during implementation of such functionality.

I doubt I'll be able to get around to implementing this any time in the near future, but if anyone is interested in tackling it, feel free to ask for some directions or feedback and I'll see what I can do!

[shepmaster]

A recent comment on Discord seems like it would be helped by this:

[Is there] an easy way to hide the types used in the enum struct variants. I don't really want a fully opaque type, but I don't want to expose the dependency types I'm using for the error messages in the API either.

Instead of

enum Error {
    Example { source: ApiError },
}

it could be

enum Error {
    Example(ExampleOpaque),
}

struct ExampleOpaque { source: ApiError }

[michiel-de-muynck]

Looks like it.

I had started the implementation of this (in PR #199) but looks like that PR slipped under the radar.

[shepmaster]

One thing I've talked about with people has been how to support "absorbing" subsets of errors. In code:

struct ErrorA;
struct ErrorB;
struct ErrorC;

enum ErrorAb { A(ErrorA), B(ErrorB) }

fn ab() -> Result<(), ErrorAb> { todo!() }

enum ErrorAbc { A(ErrorA), B(ErrorB), C(ErrorC) }

fn abc() -> Result<(), ErrorAbc> { todo!() }

I'd like to think about if we should make it easy to "absorb" from ErrorAb into ErrorAbc, such as by implementing From:

impl From<ErrorAb> for ErrorAbc {
    fn from(other: ErrorAb) -> Self {
        match other {
            ErrorAb::A(a) => ErrorAbc::A(a),
            ErrorAb::B(b) => ErrorAbc::B(b),
        }
    }
} 

I think that syntactically this would be annoying because there's no way for the macro to know which variants exist on the type being absorbed, so it'd have to look like:

#[derive(Snafu)]
#[snafu(absorb(ErrorAb, A, B))]
enum ErrorAbc { A(ErrorA), B(ErrorB), C(ErrorC) }

Which is fairly brittle as new variants are added.