Skip to content

Add a repr option for enums instead of relying on a feature#1539

Open
zackyancey wants to merge 1 commit into
juhaku:masterfrom
serialtek:configurable-enum-repr
Open

Add a repr option for enums instead of relying on a feature#1539
zackyancey wants to merge 1 commit into
juhaku:masterfrom
serialtek:configurable-enum-repr

Conversation

@zackyancey
Copy link
Copy Markdown
Contributor

This is to address this issue:

The #[repr()] attribute is a language-level attribute that affects the internal representation of the enum. It doesn't on its own say anything about how the type is serialized, so we need some option to tell us whether to use the #[repr()]

The current way of doing that, using the repr crate feature to control this functionality, has a problem. If I have an enum definition like this:

// Note serde::Serialize, not serde_repr::Serialize, so I need strings, not
// ints, in the schema
#[derive(serde::Serialize, ToSchema)] 
#[repr(u8)] // repr is specified, but nothing to do with serialization
enum StringEnum {
    A = 0,
    B = 1
}

What I need to do is not enable the repr feature. However, because features are addative and unified across all crates, if any other dependency of mine enables the repr feature, then the schema for StringEnum will be wrong.

This is something that needs to be specified per-schema, eg

#[derive(serde::Serialize, ToSchema)]
#[schema(repr)]
#[repr(u8)]
enum IntEnum {
    A = 0,
    B = 1
}

// To avoid the ambiguity caused by the feature, you can specify repr = false
// to ignore the `repr` feature if it's enabled.
#[derive(serde::Serialize, ToSchema)]
#[schema(repr = false)]
#[repr(u8)]
enum StringEnum {
    A = 0,
    B = 1
}

This change leaves the repr crate feature unchanged if no #[schema(repr)] is specified, but if it is then that value will be used regardless of the state of the repr feature. This lets new code use the unambiguous syntax without breaking anything that depends on the feature. I think the repr crate feature should probably be removed in the next major release.

The `#[repr()]` attribute is a language-level attribute that affects the
internal representation of the enum. It doesn't on its own say anything about
how the type is serialized, so we need some option to tell us whether to use
the `#[repr()]`

The current way of doing that, using the `repr` crate feature to control this
functionality, has a problem. If I have an enum definition like this:

```rust
// Note serde::Serialize, not serde_repr::Serialize, so I need strings, not
// ints, in the schema
#[derive(serde::Serialize, ToSchema)] 
#[repr(u8)] // repr is specified, but nothing to do with serialization
enum StringEnum {
    A = 0,
    B = 1
}
```

What I need to do is not enable the `repr` feature. However, because features
are addative and unified across all crates, if *any other* dependency of mine
enables the repr feature, then the schema for `StringEnum` will be wrong.

This is something that needs to be specified per-schema, eg

```rust
#[derive(serde::Serialize, ToSchema)]
#[schema(repr)]
#[repr(u8)]
enum IntEnum {
    A = 0,
    B = 1
}

// To avoid the ambiguity caused by the feature, you can specify repr = false
// to ignore the `repr` feature if it's enabled.
#[derive(serde::Serialize, ToSchema)]
#[schema(repr = false)]
#[repr(u8)]
enum StringEnum {
    A = 0,
    B = 1
}
```

This change leaves the `repr` crate feature unchanged if no `#[schema(repr)]`
is specified, but if it is then that value will be used regardless of the state
of the `repr` feature. This lets new code use the unambiguous syntax without
breaking anything that depends on the feature. I think the `repr` crate feature
should probably be removed in the next major release.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant