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

Add derive macro to strip enum of data #39

Merged
merged 34 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions quork-proc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@ proc-macro = true
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
heck = "0.5"
jewlexx marked this conversation as resolved.
Show resolved Hide resolved
proc-macro-crate = "3.1"
proc-macro-error2 = "2.0"
proc-macro2 = "1.0"
quote = "1.0"
syn = { version = "2.0", features = ["full"] }

[dev-dependencies]
strum = { version = "0.26", features = ["derive"] }
29 changes: 18 additions & 11 deletions quork-proc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,34 @@
#![warn(clippy::pedantic)]
#![warn(missing_docs)]

use proc_macro::TokenStream;
use proc_macro_error2::proc_macro_error;
use syn::{parse_macro_input, DeriveInput, LitStr};

mod const_str;
mod enum_list;
mod from_tuple;
mod new;
mod strip_enum;
mod time_fn;
mod trim_lines;

#[macro_use]
extern crate quote;

/// Create an additional enum with all values stripped
#[proc_macro_derive(Strip, attributes(stripped_meta, stripped))]
#[proc_macro_error]
pub fn strip_enum(input: TokenStream) -> TokenStream {
let mut ast = parse_macro_input!(input as DeriveInput);

strip_enum::strip_enum(&mut ast).into()
}

/// Implement [`quork::ListVariants`] for enums
#[proc_macro_derive(ListVariants)]
#[proc_macro_error]
pub fn derive_enum_list(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
pub fn derive_enum_list(input: TokenStream) -> TokenStream {
let ast = parse_macro_input!(input as DeriveInput);
enum_list::enum_list(&ast).into()
}
Expand All @@ -29,7 +40,7 @@ pub fn derive_enum_list(input: proc_macro::TokenStream) -> proc_macro::TokenStre
///
/// Converts an enum variant to a string literal, within a constant context.
#[proc_macro_derive(ConstStr)]
pub fn derive_const_str(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
pub fn derive_const_str(input: TokenStream) -> TokenStream {
let ast = parse_macro_input!(input as DeriveInput);
const_str::derive(&ast).into()
}
Expand All @@ -39,15 +50,15 @@ pub fn derive_const_str(input: proc_macro::TokenStream) -> proc_macro::TokenStre
/// Will follow the form of `new(field: Type, ...) -> Self`, where all fields are required.
#[proc_macro_derive(New)]
#[proc_macro_error]
pub fn derive_new(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
pub fn derive_new(input: TokenStream) -> TokenStream {
let ast = parse_macro_input!(input as DeriveInput);
new::derive(&ast).into()
}

/// Implement the [`std::convert::From`] trait for converting tuples into tuple structs
#[proc_macro_derive(FromTuple)]
#[proc_macro_error]
pub fn derive_from_tuple(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
pub fn derive_from_tuple(input: TokenStream) -> TokenStream {
let ast = parse_macro_input!(input as DeriveInput);
from_tuple::derive(&ast).into()
}
Expand All @@ -59,10 +70,7 @@ pub fn derive_from_tuple(input: proc_macro::TokenStream) -> proc_macro::TokenStr
/// You can pass "s", "ms", "ns"
#[proc_macro_attribute]
#[proc_macro_error]
pub fn time(
args: proc_macro::TokenStream,
input: proc_macro::TokenStream,
) -> proc_macro::TokenStream {
pub fn time(args: TokenStream, input: TokenStream) -> TokenStream {
let args_str = args.to_string();
let fmt = match args_str.as_str() {
"ms" | "milliseconds" => time_fn::TimeFormat::Milliseconds,
Expand Down Expand Up @@ -98,7 +106,6 @@ pub fn ltrim_lines(input: proc_macro::TokenStream) -> proc_macro::TokenStream {

/// Trim whitespace from the right of a string literal on each line
#[proc_macro]
#[deprecated = "Use trim_lines (renamed to avoid confusion)"]
pub fn strip_lines(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
let literal = parse_macro_input!(input as LitStr);

Expand All @@ -108,7 +115,7 @@ pub fn strip_lines(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
/// Trim whitespace from the left and right of a string literal on each line
#[proc_macro]
#[deprecated = "Use rtrim_lines (renamed to avoid confusion)"]
pub fn rstrip_lines(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
pub fn rstrip_lines(input: TokenStream) -> proc_macro::TokenStream {
let literal = parse_macro_input!(input as LitStr);

trim_lines::trim_lines(&literal, &trim_lines::Alignment::Right).into()
Expand All @@ -117,7 +124,7 @@ pub fn rstrip_lines(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
/// Trim whitespace from the left of a string literal on each line
#[proc_macro]
#[deprecated = "Use ltrim_lines (renamed to avoid confusion)"]
pub fn lstrip_lines(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
pub fn lstrip_lines(input: TokenStream) -> proc_macro::TokenStream {
let literal = parse_macro_input!(input as LitStr);

trim_lines::trim_lines(&literal, &trim_lines::Alignment::Left).into()
Expand Down
151 changes: 151 additions & 0 deletions quork-proc/src/strip_enum.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
use proc_macro2::{Ident, TokenStream};
use proc_macro_error2::{abort, abort_call_site};
use quote::{quote, ToTokens};
use syn::{spanned::Spanned, DeriveInput, Meta, MetaNameValue, Variant, Visibility};

fn ignore_variant(variant: &Variant) -> bool {
variant.attrs.iter().any(|attr| match attr.meta {
syn::Meta::List(ref list) if list.path.is_ident("stripped") => {
let mut ignored = false;

let list_parser = syn::meta::parser(|meta| {
if meta.path.is_ident("ignore") {
ignored = true;
Ok(())
} else {
Err(meta.error("unsupported stripped property"))
}
});

list.parse_args_with(list_parser).unwrap();
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle potential parsing errors instead of using unwrap()

Using unwrap() on list.parse_args_with(list_parser) may cause a panic if parsing fails. It's better to handle the error gracefully and provide a helpful error message to the user.

Apply this diff to handle the error:

-list.parse_args_with(list_parser).unwrap();
+if let Err(err) = list.parse_args_with(list_parser) {
+    abort!(list.span(), "Failed to parse 'stripped' attribute: {}", err);
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
list.parse_args_with(list_parser).unwrap();
if let Err(err) = list.parse_args_with(list_parser) {
abort!(list.span(), "Failed to parse 'stripped' attribute: {}", err);
}


ignored
}
_ => abort!(
attr.span(),
"Expected list-style (i.e #[stripped(...)]), found other style attribute macro"
),
})
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Ensure correct matching of attributes to avoid unexpected behavior

The pattern matching in the ignore_variant function may incorrectly handle attributes that are not #[stripped(...)], potentially causing unintended aborts. Ensure that only the relevant attributes are processed and others are ignored gracefully.

Consider refining the match statement to handle only the #[stripped(...)] attributes and skip others without aborting. Here's a suggested change:

-fn ignore_variant(variant: &Variant) -> bool {
-    variant.attrs.iter().any(|attr| match attr.meta {
-        syn::Meta::List(ref list) if list.path.is_ident("stripped") => {
-            // Existing code...
-        }
-        _ => abort!(
-            attr.span(),
-            "Expected list-style (i.e #[stripped(...)]), found other style attribute macro"
-        ),
-    })
+fn ignore_variant(variant: &Variant) -> bool {
+    variant.attrs.iter().any(|attr| {
+        if attr.path().is_ident("stripped") {
+            match attr.parse_meta() {
+                Ok(syn::Meta::List(list)) => {
+                    // Existing code...
+                    let mut ignored = false;
+                    let list_parser = syn::meta::parser(|meta| {
+                        if meta.path.is_ident("ignore") {
+                            ignored = true;
+                            Ok(())
+                        } else {
+                            Err(meta.error("unsupported stripped property"))
+                        }
+                    });
+                    if let Err(err) = list.parse_args_with(list_parser) {
+                        abort!(list.span(), "Failed to parse 'stripped' attribute: {}", err);
+                    }
+                    ignored
+                }
+                _ => abort!(
+                    attr.span(),
+                    "Expected list-style (i.e #[stripped(...)]), found other style attribute."
+                ),
+            }
+        } else {
+            false
+        }
+    })
 }

Committable suggestion skipped: line range outside the PR's diff.

}

struct StrippedData {
ident: Ident,
variants: Vec<TokenStream>,
meta: Vec<Meta>,
vis: Visibility,
}

// struct MetaArgs {
// meta: Vec<Expr>,
// }

// impl Parse for MetaArgs {
// fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
// input.peek3(token)
// }
// }

pub fn strip_enum(ast: &mut DeriveInput) -> TokenStream {
let data = &ast.data;
let attrs = &mut ast.attrs;

let info: StrippedData = match data {
syn::Data::Enum(ref e) => {
let variants = e
.variants
.iter()
.filter_map(|variant| {
if ignore_variant(variant) {
None
} else {
Some(variant.ident.to_token_stream())
}
})
.collect::<Vec<_>>();

let default_ident = {
let ident = ast.ident.clone();
let span = ident.span();
move || Ident::new(&format!("{ident}Stripped"), span)
};

let new_ident = if let Some(info_attr_pos) = attrs
.iter()
.position(|attr| attr.path().is_ident("stripped"))
{
let info_attr = attrs.remove(info_attr_pos);

let mut new_ident: Option<Ident> = None;

let ident_parser = syn::meta::parser(|meta| {
if meta.path.is_ident("ident") {
new_ident = Some(meta.value()?.parse()?);
Ok(())
} else {
Err(meta.error("unsupported stripped property"))
}
});

info_attr.parse_args_with(ident_parser).unwrap();
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid potential panics by handling parsing errors

The use of unwrap() on info_attr.parse_args_with(ident_parser) can lead to a panic if parsing fails. Consider handling the error to prevent unexpected panics and provide clearer error messages.

Apply this diff:

-info_attr.parse_args_with(ident_parser).unwrap();
+if let Err(err) = info_attr.parse_args_with(ident_parser) {
+    abort!(info_attr.span(), "Failed to parse 'ident' in 'stripped' attribute: {}", err);
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
info_attr.parse_args_with(ident_parser).unwrap();
if let Err(err) = info_attr.parse_args_with(ident_parser) {
abort!(info_attr.span(), "Failed to parse 'ident' in 'stripped' attribute: {}", err);
}


new_ident.unwrap_or_else(default_ident)
} else {
default_ident()
};
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consistent attribute parsing and error handling

In the strip_enum function, the parsing of the #[stripped] attribute should handle errors gracefully and consistently, similar to the previous suggestions.

Apply this diff:

-let new_ident = if let Some(info_attr_pos) = attrs
-    .iter()
-    .position(|attr| attr.path().is_ident("stripped"))
-{
-    let info_attr = attrs.remove(info_attr_pos);
-
-    let mut new_ident: Option<Ident> = None;
-
-    let ident_parser = syn::meta::parser(|meta| {
-        if meta.path.is_ident("ident") {
-            new_ident = Some(meta.value()?.parse()?);
-            Ok(())
-        } else {
-            Err(meta.error("unsupported stripped property"))
-        }
-    });
-
-    info_attr.parse_args_with(ident_parser).unwrap();
-
-    new_ident.unwrap_or_else(default_ident)
-} else {
-    default_ident()
-};
+let new_ident = if let Some(info_attr_pos) = attrs
+    .iter()
+    .position(|attr| attr.path().is_ident("stripped"))
+{
+    let info_attr = attrs.remove(info_attr_pos);
+
+    let mut new_ident: Option<Ident> = None;
+
+    let ident_parser = syn::meta::parser(|meta| {
+        if meta.path.is_ident("ident") {
+            new_ident = Some(meta.value()?.parse()?);
+            Ok(())
+        } else {
+            Err(meta.error("unsupported stripped property"))
+        }
+    });
+
+    if let Err(err) = info_attr.parse_args_with(ident_parser) {
+        abort!(info_attr.span(), "Failed to parse 'stripped' attribute: {}", err);
+    }
+
+    new_ident.unwrap_or_else(default_ident)
+} else {
+    default_ident()
+};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let new_ident = if let Some(info_attr_pos) = attrs
.iter()
.position(|attr| attr.path().is_ident("stripped"))
{
let info_attr = attrs.remove(info_attr_pos);
let mut new_ident: Option<Ident> = None;
let ident_parser = syn::meta::parser(|meta| {
if meta.path.is_ident("ident") {
new_ident = Some(meta.value()?.parse()?);
Ok(())
} else {
Err(meta.error("unsupported stripped property"))
}
});
info_attr.parse_args_with(ident_parser).unwrap();
new_ident.unwrap_or_else(default_ident)
} else {
default_ident()
};
let new_ident = if let Some(info_attr_pos) = attrs
.iter()
.position(|attr| attr.path().is_ident("stripped"))
{
let info_attr = attrs.remove(info_attr_pos);
let mut new_ident: Option<Ident> = None;
let ident_parser = syn::meta::parser(|meta| {
if meta.path.is_ident("ident") {
new_ident = Some(meta.value()?.parse()?);
Ok(())
} else {
Err(meta.error("unsupported stripped property"))
}
});
if let Err(err) = info_attr.parse_args_with(ident_parser) {
abort!(info_attr.span(), "Failed to parse 'stripped' attribute: {}", err);
}
new_ident.unwrap_or_else(default_ident)
} else {
default_ident()
};


let meta_list: Vec<syn::Meta> = attrs
.iter()
.filter(|attr| attr.path().is_ident("stripped_meta"))
.flat_map(|meta_attr| match &meta_attr.meta {
Meta::List(meta_data) => vec![meta_data.parse_args::<syn::Meta>().unwrap()],
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Replace unwrap() with error handling to prevent panics

Calling parse_args::<syn::Meta>().unwrap() can cause a panic if the parsing fails. It's advisable to handle the error and provide a meaningful message.

Apply this diff:

-Meta::List(meta_data) => vec![meta_data.parse_args::<syn::Meta>().unwrap()],
+Meta::List(meta_data) => {
+    match meta_data.parse_args::<syn::Meta>() {
+        Ok(meta) => vec![meta],
+        Err(err) => abort!(meta_data.span(), "Failed to parse 'stripped_meta' attribute: {}", err),
+    }
+},
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Meta::List(meta_data) => vec![meta_data.parse_args::<syn::Meta>().unwrap()],
Meta::List(meta_data) => {
match meta_data.parse_args::<syn::Meta>() {
Ok(meta) => vec![meta],
Err(err) => abort!(meta_data.span(), "Failed to parse 'stripped_meta' attribute: {}", err),
}
},

// Meta::NameValue(MetaNameValue {
// value:
// syn::Expr::Lit(syn::ExprLit {
// lit: syn::Lit::Str(path),
// ..
// }),
// ..
// }) => {
// if &path.value() == "inherit" {
// attrs
// .iter()
// .filter(|attr| !attr.path().is_ident("stripped_meta"))
// .map(|attr| attr.meta.clone())
// .collect()
// } else {
// abort!(path.span(), "Expected `inherit`");
// }
// }
_ => abort!(
meta_attr.span(),
"Expected #[stripped_meta(...)]. Found other style attribute."
),
})
.collect();
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve parsing of stripped_meta attributes and error handling

The current implementation may not handle multiple #[stripped_meta(...)] attributes correctly and uses unwrap() which can panic. Consider parsing all attributes properly and handling errors.

Here's a suggested change:

-let meta_list: Vec<syn::Meta> = attrs
-    .iter()
-    .filter(|attr| attr.path().is_ident("stripped_meta"))
-    .flat_map(|meta_attr| match &meta_attr.meta {
-        Meta::List(meta_data) => vec![meta_data.parse_args::<syn::Meta>().unwrap()],
-        _ => abort!(
-            meta_attr.span(),
-            "Expected #[stripped_meta(...)]. Found other style attribute."
-        ),
-    })
-    .collect();
+let mut meta_list: Vec<syn::Meta> = Vec::new();
+for attr in attrs.iter().filter(|attr| attr.path().is_ident("stripped_meta")) {
+    match attr.parse_meta() {
+        Ok(Meta::List(meta_data)) => {
+            for nested_meta in meta_data.nested.iter() {
+                if let syn::NestedMeta::Meta(meta) = nested_meta {
+                    meta_list.push(meta.clone());
+                } else {
+                    abort!(
+                        nested_meta.span(),
+                        "Invalid format in 'stripped_meta' attribute."
+                    );
+                }
+            }
+        }
+        _ => abort!(
+            attr.span(),
+            "Expected #[stripped_meta(...)] with meta list."
+        ),
+    }
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let meta_list: Vec<syn::Meta> = attrs
.iter()
.filter(|attr| attr.path().is_ident("stripped_meta"))
.flat_map(|meta_attr| match &meta_attr.meta {
Meta::List(meta_data) => vec![meta_data.parse_args::<syn::Meta>().unwrap()],
// Meta::NameValue(MetaNameValue {
// value:
// syn::Expr::Lit(syn::ExprLit {
// lit: syn::Lit::Str(path),
// ..
// }),
// ..
// }) => {
// if &path.value() == "inherit" {
// attrs
// .iter()
// .filter(|attr| !attr.path().is_ident("stripped_meta"))
// .map(|attr| attr.meta.clone())
// .collect()
// } else {
// abort!(path.span(), "Expected `inherit`");
// }
// }
_ => abort!(
meta_attr.span(),
"Expected #[stripped_meta(...)]. Found other style attribute."
),
})
.collect();
let mut meta_list: Vec<syn::Meta> = Vec::new();
for attr in attrs.iter().filter(|attr| attr.path().is_ident("stripped_meta")) {
match attr.parse_meta() {
Ok(Meta::List(meta_data)) => {
for nested_meta in meta_data.nested.iter() {
if let syn::NestedMeta::Meta(meta) = nested_meta {
meta_list.push(meta.clone());
} else {
abort!(
nested_meta.span(),
"Invalid format in 'stripped_meta' attribute."
);
}
}
}
_ => abort!(
attr.span(),
"Expected #[stripped_meta(...)] with meta list."
),
}
}


StrippedData {
ident: new_ident,
variants,
meta: meta_list,
vis: ast.vis.clone(),
}
}
_ => abort_call_site!("`Strip` can only be derived for enums"),
};

let StrippedData {
ident,
variants,
meta,
vis,
} = info;

// panic!("{:?}", meta);

quote! {
#(#[#meta])*
#vis enum #ident {
#(#variants),*
}
}
}
52 changes: 52 additions & 0 deletions quork-proc/tests/strip_enum.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#![allow(dead_code)]

use std::fmt::Display;

use strum::{Display, EnumIter, IntoEnumIterator};

use quork_proc::Strip;

pub fn enum_to_string<T: IntoEnumIterator + Display>() -> String {
T::iter().map(|v| v.to_string()).collect::<String>()
}

struct DummyStruct;

#[derive(Strip)]
#[stripped_meta(derive(EnumIter, Display))]
#[stripped_meta(strum(serialize_all = "kebab-case"))]
enum EnumWithData {
Test1(DummyStruct),
Test2(DummyStruct),
}

#[test]
fn has_all_variants() {
let variants = enum_to_string::<EnumWithDataStripped>();

assert_eq!(variants, "test1test2");
}

#[derive(Strip)]
#[stripped_meta(derive(EnumIter, Display))]
#[stripped_meta(strum(serialize_all = "kebab-case"))]
enum EnumExclude {
Test1(DummyStruct),
#[stripped(ignore)]
Test2(DummyStruct),
Test3(DummyStruct),
}

#[derive(Strip, Display)]
#[stripped_meta(derive(EnumIter))]
#[stripped_meta(strum(serialize_all = "kebab-case"))]
enum EnumWithInherit {
Test1(DummyStruct),
}

#[test]
fn excludes_no_hook_variant() {
let variants = enum_to_string::<EnumExcludeStripped>();

assert_eq!(variants, "test1test3");
}
Loading