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

Adding support for int and bool EnumProperties #259

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 8 additions & 10 deletions strum/src/lib.rs
Expand Up @@ -136,9 +136,9 @@ pub trait EnumMessage {

/// `EnumProperty` is a trait that makes it possible to store additional information
/// with enum variants. This trait is designed to be used with the macro of the same
/// name in the `strum_macros` crate. Currently, the only string literals are supported
/// in attributes, the other methods will be implemented as additional attribute types
/// become stabilized.
/// name in the `strum_macros` crate. Currently, string, integer and boolean literals
/// are supported in attributes, the other methods will be implemented as additional
/// attribute types become stabilized.
///
/// # Example
///
Expand All @@ -152,24 +152,22 @@ pub trait EnumMessage {
/// #[strum(props(Teacher="Ms.Frizzle", Room="201"))]
/// History,
/// #[strum(props(Teacher="Mr.Smith"))]
/// #[strum(props(Room="103"))]
/// #[strum(props(Room=103))]
/// Mathematics,
/// #[strum(props(Time="2:30"))]
/// Science,
/// }
///
/// let history = Class::History;
/// assert_eq!("Ms.Frizzle", history.get_str("Teacher").unwrap());
/// let maths = Class::Mathematics;
/// assert_eq!(103, maths.get_int("Room").unwrap());
/// ```
pub trait EnumProperty {
fn get_str(&self, prop: &str) -> Option<&'static str>;
fn get_int(&self, _prop: &str) -> Option<usize> {
Option::None
}
fn get_int(&self, prop: &str) -> Option<usize>;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_usize is probably a more future-proof name

Copy link
Author

@ejmount ejmount Apr 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree and I'm happy to make that change, but I also thought it'd be worth mentioning that while it'll be more future proof, it's also technically a breaking change. I'd be immensely surprised if any real code got broken, but I just wanted to check you're ok with that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I jinxed myself by saying earlier nothing would break, because there seems to be an odd dependency between strum_macros and a previous version of the main package. I've made the change and updated the Cargo.toml to use the local version, which solved the issue.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ejmount, sorry for the delay getting back to you. I've been out of town for a few weeks. I missed that I had stubbed that method out at some point in the past. If you're still interested in completing this, here's what we can do. I can publish a new version of strum (not macros) with an updated trait definition then we can rebase your changes onto the new version.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it sounds like that's still not supported, so it'd be great if you could publish a new version to make that update.


fn get_bool(&self, _prop: &str) -> Option<bool> {
Option::None
}
fn get_bool(&self, prop: &str) -> Option<bool>;
}

/// A cheap reference-to-reference conversion. Used to convert a value to a
Expand Down
14 changes: 11 additions & 3 deletions strum_macros/src/helpers/metadata.rs
Expand Up @@ -10,6 +10,7 @@ use syn::{
};

use super::case_style::CaseStyle;
use super::PropertyValue;

pub mod kw {
use syn::custom_keyword;
Expand Down Expand Up @@ -184,7 +185,7 @@ pub enum VariantMeta {
},
Props {
kw: kw::props,
props: Vec<(LitStr, LitStr)>,
props: Vec<(LitStr, PropertyValue)>,
},
}

Expand Down Expand Up @@ -242,15 +243,22 @@ impl Parse for VariantMeta {
}
}

struct Prop(Ident, LitStr);
struct Prop(Ident, PropertyValue);

impl Parse for Prop {
fn parse(input: ParseStream) -> syn::Result<Self> {
use syn::ext::IdentExt;

let k = Ident::parse_any(input)?;
let _: Token![=] = input.parse()?;
let v = input.parse()?;
let v;
if input.peek(syn::LitBool) {
ejmount marked this conversation as resolved.
Show resolved Hide resolved
v = PropertyValue::Bool(input.parse()?);
} else if input.peek(syn::LitInt) {
v = PropertyValue::Num(input.parse()?);
} else {
v = PropertyValue::Str(input.parse()?);
};

Ok(Prop(k, v))
}
Expand Down
17 changes: 17 additions & 0 deletions strum_macros/src/helpers/mod.rs
Expand Up @@ -30,3 +30,20 @@ pub fn occurrence_error<T: ToTokens>(fst: T, snd: T, attr: &str) -> syn::Error {
e.combine(syn::Error::new_spanned(fst, "first one here"));
e
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub enum PropertyValue {
Str(syn::LitStr),
Num(syn::LitInt),
Bool(syn::LitBool),
}

impl quote::ToTokens for PropertyValue {
fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) {
match self {
PropertyValue::Str(s) => s.to_tokens(tokens),
PropertyValue::Num(n) => n.to_tokens(tokens),
PropertyValue::Bool(b) => b.to_tokens(tokens),
}
}
}
3 changes: 2 additions & 1 deletion strum_macros/src/helpers/variant_props.rs
Expand Up @@ -4,6 +4,7 @@ use syn::{Ident, LitStr, Variant};
use super::case_style::{CaseStyle, CaseStyleHelpers};
use super::metadata::{kw, VariantExt, VariantMeta};
use super::occurrence_error;
use super::PropertyValue;

pub trait HasStrumVariantProperties {
fn get_variant_properties(&self) -> syn::Result<StrumVariantProperties>;
Expand All @@ -17,7 +18,7 @@ pub struct StrumVariantProperties {
pub message: Option<LitStr>,
pub detailed_message: Option<LitStr>,
pub documentation: Vec<LitStr>,
pub string_props: Vec<(LitStr, LitStr)>,
pub string_props: Vec<(LitStr, PropertyValue)>,
serialize: Vec<LitStr>,
to_string: Option<LitStr>,
ident: Option<Ident>,
Expand Down
12 changes: 6 additions & 6 deletions strum_macros/src/lib.rs
Expand Up @@ -560,12 +560,12 @@ pub fn enum_messages(input: proc_macro::TokenStream) -> proc_macro::TokenStream
/// Add custom properties to enum variants.
///
/// Enables the encoding of arbitary constants into enum variants. This method
/// currently only supports adding additional string values. Other types of literals are still
/// experimental in the rustc compiler. The generated code works by nesting match statements.
/// The first match statement matches on the type of the enum, and the inner match statement
/// matches on the name of the property requested. This design works well for enums with a small
/// number of variants and properties, but scales linearly with the number of variants so may not
/// be the best choice in all situations.
/// currently only supports adding additional string, integer and boolean values. Other types
/// of literals are still experimental in the rustc compiler. The generated code works by
/// nesting match statements. The first match statement matches on the type of the enum,
/// and the inner match statement matches on the name of the property requested. This design
/// works well for enums with a small number of variants and properties, but scales linearly
/// with the number of variants so may not be the best choice in all situations.
///
/// ```
///
Expand Down
55 changes: 49 additions & 6 deletions strum_macros/src/macros/enum_properties.rs
Expand Up @@ -2,6 +2,7 @@ use proc_macro2::TokenStream;
use quote::quote;
use syn::{Data, DeriveInput, Fields};

use crate::helpers::PropertyValue;
use crate::helpers::{non_enum_error, HasStrumVariantProperties, HasTypeProperties};

pub fn enum_properties_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
Expand All @@ -14,7 +15,9 @@ pub fn enum_properties_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
let type_properties = ast.get_type_properties()?;
let strum_module_path = type_properties.crate_module_path();

let mut arms = Vec::new();
let mut string_props = Vec::new();
let mut num_props = Vec::new();
let mut bool_props = Vec::new();
for variant in variants {
let ident = &variant.ident;
let variant_properties = variant.get_variant_properties()?;
Expand All @@ -33,31 +36,71 @@ pub fn enum_properties_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
};

for (key, value) in variant_properties.string_props {
string_arms.push(quote! { #key => ::core::option::Option::Some( #value )});
match value {
PropertyValue::Str(s) => {
string_arms.push(quote! { #key => ::core::option::Option::Some( #s )})
}
PropertyValue::Num(n) => {
num_arms.push(quote! { #key => ::core::option::Option::Some( #n )})
}
PropertyValue::Bool(b) => {
bool_arms.push(quote! { #key => ::core::option::Option::Some( #b )})
}
}
}

string_arms.push(quote! { _ => ::core::option::Option::None });
bool_arms.push(quote! { _ => ::core::option::Option::None });
num_arms.push(quote! { _ => ::core::option::Option::None });

arms.push(quote! {
string_props.push(quote! {
&#name::#ident #params => {
match prop {
#(#string_arms),*
}
}
});
bool_props.push(quote! {
&#name::#ident #params => {
match prop {
#(#bool_arms),*
}
}
});
num_props.push(quote! {
&#name::#ident #params => {
match prop {
#(#num_arms),*
}
}
});
}

if arms.len() < variants.len() {
arms.push(quote! { _ => ::core::option::Option::None });
if string_props.len() < variants.len() {
string_props.push(quote! { _ => ::core::option::Option::None });
}
if bool_props.len() < variants.len() {
bool_props.push(quote! { _ => ::core::option::Option::None });
}
if num_props.len() < variants.len() {
num_props.push(quote! { _ => ::core::option::Option::None });
}

Ok(quote! {
impl #impl_generics #strum_module_path::EnumProperty for #name #ty_generics #where_clause {
fn get_str(&self, prop: &str) -> ::core::option::Option<&'static str> {
match self {
#(#arms),*
#(#string_props),*
}
}
fn get_bool(&self, prop: &str) -> ::core::option::Option<bool> {
match self {
#(#bool_props),*
}
}
fn get_int(&self, prop: &str) -> ::core::option::Option<usize> {
match self {
#(#num_props),*
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions strum_tests/tests/enum_props.rs
Expand Up @@ -39,9 +39,16 @@ fn crate_module_path_test() {
enum Test {
#[strum(props(key = "value"))]
A,
#[strum(props(answer = 42))]
B,
#[strum(props(to_be = false))]
C,
}

let a = Test::A;
assert_eq!("value", a.get_str("key").unwrap());
let b = Test::B;
assert_eq!(42, b.get_int("answer").unwrap());
let c = Test::C;
assert_eq!(false, c.get_bool("to_be").unwrap());
}