Azure / azure-sdk-for-rust

This repository is for the active development of the Azure SDK for Rust. For consumers of the SDK we recommend visiting Docs.rs and looking up the docs for any of libraries in the SDK.
MIT License
714 stars 247 forks source link

Create derive macro for variants (enums) #1846

Open heaths opened 1 month ago

heaths commented 1 month ago

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:

See https://github.com/Azure/azure-sdk-for-rust/pull/1690

Question(s) on top of mind:

heaths commented 1 month ago

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

analogrelay commented 1 month ago

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.

heaths commented 1 month ago

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 commented 1 month ago

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

That's correct.

analogrelay commented 1 month ago

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.