Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create derive macro for variants (enums) #1846

Open
3 tasks
heaths opened this issue Oct 11, 2024 · 5 comments
Open
3 tasks

Create derive macro for variants (enums) #1846

heaths opened this issue Oct 11, 2024 · 5 comments
Labels
Azure.Core The azure_core crate design-discussion An area of design currently under discussion and open to team and community feedback.

Comments

@heaths
Copy link
Member

heaths commented Oct 11, 2024

Because Azure services define a lot of enums and, despite generating most of our clients, hand-authored enums could benefit from consistency with our guidelines, we should create a derive macro for enums called, tentatively, Variant e.g.,

#[derive(Variant)]
#[variant(fixed, rename_all = "camelCase")] // optional; default is extensible per <https://aka.ms/azapi/guidelines>
pub enum DaysOfWeek {
    Monday,
    Tuesday,
    // ...
}

#[derive(Variant)]
#[variant(rename_all = "camelCase")]
pub enum Metasyntactic {
    Foo,
    Bar,
    #[variant(other)] // UnknownValue(String) is used by default, actually
    UnknownValue(String),
}

The Variant derive macro would:

  • Derive Clone, Debug, PartialEq, and Eq
  • Implement serde::Serialize and serde::Deserialize
  • Implement FromStr. Extensible enums will define FromStr::Err as std::convert::Infallible but fixed enums using azure_core::ErrorKind::DataConversion. This will be used in serde::Deserialize.
  • Implement Display. This will be used in serde::Serialize.

See #1690

Question(s) on top of mind:

  • Should we try to duplicate necessary serde attributes like rename_all? Off hand, that may be all we need.
  • Should we define a Variant trait for which we can add blanket implementations that are easier to update than a derive macro? Is that even practical? Seems we'd need a Variant trait to return tuples of tokens to values which might make the whole thing inefficient.
  • #[variant(other)] is meant to be equivalent to #[serde(untagged)] but I feel is more intuitive; or, should we make these the same for familiarity?
@heaths
Copy link
Member Author

heaths commented Oct 14, 2024

In the meantime, I created a create_extensible_enum!() macro in #1852 as a stopgap solution for @jhendrixMSFT for the TypeSpec emitter.

@analogrelay
Copy link
Member

I like #[variant(other)] and had a similar thought that we might want a way to support unknown values from the server.

Should we try to duplicate necessary serde attributes like rename_all? Off hand, that may be all we need.

If we're hand-implementing the serde traits, I think we would have to duplicate those attributes, since the #[serde] attribute is only in scope when using their derive macro. I believe rename_all would be sufficient, though we might need rename as well to handle irregular cases.


Since we're not actually deriving a trait, it might also be reasonable to use a general attribute macro, rather than a #[derive] macro. I don't mind using a #[derive] one if that's what feels easiest, but it might be a little confusing to someone reading the code.

@RickWinter RickWinter added design-discussion An area of design currently under discussion and open to team and community feedback. Azure.Core The azure_core crate labels Oct 14, 2024
@heaths
Copy link
Member Author

heaths commented Oct 14, 2024

Since we're not actually deriving a trait, it might also be reasonable to use a general attribute macro, rather than a #[derive] macro. I don't mind using a #[derive] one if that's what feels easiest, but it might be a little confusing to someone reading the code.

The goal is to create a derive macro, though, and specifically so we could have helper attributes; though, if we could easily enough still handle helper attributes to customize the enum or individual variants I'd still be open to that. Some earlier prototypes I couldn't make work but - especially back then - hadn't gotten my hands quite so dirty into TokenStreams as more recently.

And, yes, I'd want to support individual renames as well. I expect the emitter would use rename, in fact, because it's easier to just repeat a pattern; rename_all would be better for what little hand-written code I'd expect. Am I right, @jhendrixMSFT?

@jhendrixMSFT
Copy link
Member

I expect the emitter would use rename, in fact, because it's easier to just repeat a pattern;

That's correct.

@analogrelay
Copy link
Member

The goal is to create a derive macro, though, and specifically so we could have helper attributes; though, if we could easily enough still handle helper attributes to customize the enum or individual variants I'd still be open to that.

Ah yeah, if we need helper attributes, I agree a derive macro is likely the best option. With a plain attribute macro we could still support helper attributes, but we're entirely on our own to extract them from the TokenStream, which would be a lot of work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core The azure_core crate design-discussion An area of design currently under discussion and open to team and community feedback.
Projects
None yet
Development

No branches or pull requests

4 participants