issues-721 Add impl IdenStatic and AsRef<str> to #[enum_def] attr#769
Conversation
|
Hi, as I said in another comment, for long term maintenance I would like to deprecate |
- added extended version of `#[enum_def]` in `sea-query-derive` - upgraded `darling` crate from 0.14 to 0.20
AndreiOrmanji
force-pushed
the
feature/issues-721_enum_def_impl_iden_static
branch
from
7f996cc to
d75bf0a
Compare
I've done it in d75bf0a. |
sea-query-derive/src/lib.rs
Outdated
| fields: Fields::Named(fields), | ||
| .. | ||
| }) => &fields.named, | ||
| _ => panic!("#[enum_def] can only be used on structs"), |
There was a problem hiding this comment.
This should emit a compile_error! instead of panicking in the proc-macro
There was a problem hiding this comment.
Do you mean something like this:
_ => return quote_spanned! {
input.span() => compile_error!("you can only derive Iden on enums or unit structs");
}
.into(),There was a problem hiding this comment.
Yep! The user feedback is better when emitting compiler errors instead of panicking
There was a problem hiding this comment.
changed in my last commit
sandhose
left a comment
sandhose
left a comment
There was a problem hiding this comment.
I was looking at upgrading syn/darling in sea-query-attr, and did not see your PR first.
So, I still opened a draft PR for visibility: #799
There are some improvements there with proper compilation errors, and snapshot tests for those errors that you may want to integrate in this PR. I'd be happy to do it if you don't have the time to do so
sea-query-derive/src/lib.rs
Outdated
| let ident = &field.ident; | ||
| let string = ident | ||
| .as_ref() | ||
| .expect("#[enum_def] can only be used on structs with named fields") |
There was a problem hiding this comment.
Same here, this should not panic
sea-query-derive/src/lib.rs
Outdated
| .to_string(); | ||
| let as_pascal = string.to_pascal_case(); | ||
| NamingHolder { | ||
| default: ident.as_ref().unwrap().clone(), |
There was a problem hiding this comment.
That's an unnecessary unwrap, the unwrapped option should be stored before
There was a problem hiding this comment.
My final variant is the following:
let fields =
match &input.data {
Data::Struct(DataStruct {
fields: Fields::Named(fields),
..
}) => &fields.named,
_ => return quote_spanned! {
input.span() => compile_error!("you can only derive Iden on enums or unit structs");
}
.into(),
};
let field_names: Vec<NamingHolder> = fields
.iter()
.map(|field| {
let ident = field.ident.as_ref().unwrap();
NamingHolder {
default: ident.clone(),
pascal: Ident::new(ident.to_string().to_pascal_case().as_str(), ident.span()),
}
})
.collect();I'm not sure, but Fields::Named means there are only named fields, so I've kept unwrap with slight changes for ident.
…dhose/sea-query into feature/issues-721_enum_def_impl_iden_static
|
Umm... how can I get the CI to run? |
| @@ -0,0 +1,10 @@ | |||
| use sea_query_attr::enum_def; | |||
There was a problem hiding this comment.
I think we're meant to import sea_query_derive and test it's implementation?
🎉 Released In 0.32.0-rc.2 🎉Thank you everyone for the contribution! |
🎉 Released In 0.32.0 🎉Thank you everyone for the contribution! |
3 participants
PR Info
New Features
#[enum_def]tosea-query-derive(and marked#[enum_def]as deprecated atsea-query-attr)IdenStaticto#[enum_def]ofsea-query-deriveAsRef<str>to#[enum_def]ofsea-query-derivesyncrate from (1->2) (by merging sea-query-attr: update syn, heck and darling #799, created by @sandhose)heckcrate from (0.4->0.5) (by merging sea-query-attr: update syn, heck and darling #799, created by @sandhose)darlingcrate from (0.14->0.20) (by merging sea-query-attr: update syn, heck and darling #799, created by @sandhose)