Add derive macro to strip enum of data #39
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes in this pull request involve modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Macro
participant Enum
participant Output
User->>Macro: Apply #[proc_macro_derive(Strip)]
Macro->>Enum: Process enum definition
Enum->>Macro: Return variants and attributes
Macro->>Output: Generate new enum definition
Output-->>User: Provide stripped enum
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (5)
quork-proc/src/strip_enum.rs (1)
143-150: Consider handling empty variant lists gracefullyIf all variants are ignored, the generated enum will have no variants, which might cause issues. It's advisable to check if
variantsis empty and provide a helpful error message or handle it accordingly.You can add a check before generating the output:
if variants.is_empty() { abort_call_site!("No variants remain after stripping. Enum cannot be empty."); }quork-proc/tests/strip_enum.rs (1)
13-13: Remove unused structDummyStructif unnecessaryThe
DummyStructis defined but may not be necessary if it's not providing additional functionality.If
DummyStructis only used as a placeholder, consider replacing it with a standard type like()or removing it if not needed.quork-proc/src/lib.rs (3)
Line range hint
73-79: Avoid unnecessary conversion of TokenStreamIn the
timemacro, the input is already aTokenStream, and calling.into()may be redundant.Apply this diff:
-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, - "ns" | "nanoseconds" => time_fn::TimeFormat::Nanoseconds, - _ => time_fn::TimeFormat::Seconds, - }; - time_fn::attribute(fmt, &input.into()).into() +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, + "ns" | "nanoseconds" => time_fn::TimeFormat::Nanoseconds, + _ => time_fn::TimeFormat::Seconds, + }; + time_fn::attribute(fmt, &input).into() }
Line range hint
93-105: Ensure consistency in macro functions' input and output typesSome macros use
proc_macro::TokenStreamwhile others useTokenStream. For consistency, consider unifying the types used across all macros.Review the input and output types of all macro functions and standardize them.
Line range hint
118-125: Update deprecated functions and clarify deprecation messagesThe deprecated functions
rstrip_linesandlstrip_lineshave deprecation messages that might be confusing. Ensure that the messages clearly guide the user to the new functions.Apply this diff:
#[proc_macro] -#[deprecated = "Use rtrim_lines (renamed to avoid confusion)"] +#[deprecated( + since = "0.3.2", + note = "Please use `rtrim_lines` instead. This function has been renamed to avoid confusion." +)] pub fn rstrip_lines(input: TokenStream) -> proc_macro::TokenStream { /* ... */ } #[proc_macro] -#[deprecated = "Use ltrim_lines (renamed to avoid confusion)"] +#[deprecated( + since = "0.3.2", + note = "Please use `ltrim_lines` instead. This function has been renamed to avoid confusion." +)] pub fn lstrip_lines(input: TokenStream) -> proc_macro::TokenStream { /* ... */ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
quork-proc/Cargo.toml(1 hunks)quork-proc/src/lib.rs(6 hunks)quork-proc/src/strip_enum.rs(1 hunks)quork-proc/tests/strip_enum.rs(1 hunks)
🔇 Additional comments (6)
quork-proc/Cargo.toml (1)
25-26: Confirm the usage of strum as a dev-dependency
Since strum is added as a development dependency, ensure it's only used in tests and not in the main library.
Run the following script to verify the usage:
✅ Verification successful
strum usage is correctly limited to test files
The verification confirms that strum is only used in test files (quork-proc/tests/strip_enum.rs) and is appropriately declared as a dev-dependency. There is only one other mention of strum in a TODO comment in lib.rs, which doesn't constitute actual usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Ensure 'strum' is only used in test files.
# Test: Search for imports of 'strum' outside the tests directory.
# Expect: No matches found outside tests/.
rg 'use\s+strum' src/ --glob '!tests/*'
Length of output: 41
Script:
#!/bin/bash
# Let's try a more comprehensive search approach
# 1. First, let's check if src/ directory exists
fd --type d "^src$"
# 2. Search for strum usage across all files (not just imports)
rg "strum" --type rust
# 3. Look specifically for test files and modules
fd -e rs "test"
Length of output: 486
quork-proc/tests/strip_enum.rs (4)
9-11: Consider specifying return type explicitly for clarity
The enum_to_string function could be clearer if the return type is explicitly specified.
Apply this diff:
-pub fn enum_to_string<T: IntoEnumIterator + Display>() -> String {
+pub fn enum_to_string<T: IntoEnumIterator + Display>() -> String {
T::iter().map(|v| v.to_string()).collect::<String>()
}(Actually, the return type is already specified, so this might be unnecessary.)
15-21: Ensure proper derivation of traits after stripping
When deriving Strip, make sure that the derived enum (EnumWithDataStripped) correctly implements the intended traits (EnumIter, Display).
40-45: Check for consistency in trait derivations
In EnumWithInherit, both Strip and Display are derived. Ensure that the stripped enum also derives the necessary traits.
48-52: Verify the correctness of the excludes_no_hook_variant test
Ensure that the test accurately reflects the expected behavior when a variant is ignored using #[stripped(ignore)].
quork-proc/src/lib.rs (1)
6-14: 🛠️ Refactor suggestion
Ensure consistent use of TokenStream imports
The code mixes proc_macro::TokenStream and TokenStream imports from proc_macro. For clarity and consistency, consider importing TokenStream explicitly and using it uniformly.
Apply this diff:
-use proc_macro::TokenStream;
+use proc_macro::TokenStream as TokenStream;Alternatively, you can import TokenStream from proc_macro2 if appropriate.
Likely invalid or redundant comment.
quork-proc/src/strip_enum.rs
Outdated
| } | ||
| }); | ||
|
|
||
| list.parse_args_with(list_parser).unwrap(); |
There was a problem hiding this comment.
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.
| 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); | |
| } |
quork-proc/src/strip_enum.rs
Outdated
| .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()], |
There was a problem hiding this comment.
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.
| 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), | |
| } | |
| }, |
quork-proc/src/strip_enum.rs
Outdated
| } | ||
| }); | ||
|
|
||
| info_attr.parse_args_with(ident_parser).unwrap(); |
There was a problem hiding this comment.
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.
| 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); | |
| } |
quork-proc/src/strip_enum.rs
Outdated
| 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(); |
There was a problem hiding this comment.
🛠️ 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.
| 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." | |
| ), | |
| } | |
| } |
quork-proc/src/strip_enum.rs
Outdated
| 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(); | ||
|
|
||
| ignored | ||
| } | ||
| _ => abort!( | ||
| attr.span(), | ||
| "Expected list-style (i.e #[stripped(...)]), found other style attribute macro" | ||
| ), | ||
| }) |
There was a problem hiding this comment.
🛠️ 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.
quork-proc/src/strip_enum.rs
Outdated
| 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() | ||
| }; |
There was a problem hiding this comment.
🛠️ 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.
| 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() | |
| }; |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
quork-proc/tests/strip_enum.rs (1)
15-28: Consider adding edge cases to the test suite.While the basic functionality is well-tested, consider adding test cases for:
- Empty enums
- Single variant enums
- Enums with complex data types
quork-proc/src/strip_enum.rs (2)
6-35: Consider more graceful handling of non-stripped attributes.The function currently aborts on any non-
#[stripped(...)]attribute, which might be too strict. Consider ignoring non-stripped attributes instead of aborting, as variants might have other valid attributes.- _ => abort!( - attr.span(), - "Expected list-style (i.e #[stripped(...)]), found other style attribute macro" - ), + _ => false,
44-52: Remove or document commented code.The commented-out
MetaArgsstruct and its implementation should either be:
- Removed if no longer needed
- Implemented if required for inheritance functionality
- Documented with a TODO comment if planned for future use
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
quork-proc/src/strip_enum.rs(1 hunks)quork-proc/tests/strip_enum.rs(1 hunks)
🔇 Additional comments (4)
quork-proc/tests/strip_enum.rs (2)
1-11: LGTM! Well-designed helper function.
The helper function makes good use of trait bounds to ensure type safety and provides a clean way to test enum variants.
30-60: LGTM! Good test coverage for ignore functionality.
The test effectively verifies that variants marked with #[stripped(ignore)] are excluded from the stripped enum.
quork-proc/src/strip_enum.rs (2)
166-171: LGTM! Clean token stream generation.
The quote macro usage is clean and correct, with proper interpolation and repetition syntax.
54-156:
Good error handling improvements, but inheritance needs attention.
The error handling has been improved with helpful messages, but the inheritance functionality (lines 122-139) is commented out. This should be either:
- Implemented to support the test case in
strip_enum.rs - Removed along with related test cases
✅ Verification successful
Let me check for any test cases or documentation related to inheritance functionality.
Let me check the test directory structure and content more broadly to ensure we're not missing anything.
Remove commented-out inheritance code as it's not being used
The inheritance functionality is commented out in the implementation and there are no test cases or documentation that rely on the inherit feature. The test file strip_enum.rs only shows usage of stripped_meta for derive and strum attributes, but no inheritance-related test cases. The commented-out code can be safely removed to maintain cleaner codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any usage of inheritance in tests or documentation
rg -A 5 "inherit" .
Length of output: 1025
Script:
#!/bin/bash
# Search for test cases related to inheritance
rg -l "stripped_meta.*inherit" quork-proc/tests/
# Check for any documentation mentioning inheritance
rg -l "inherit" quork-proc/README.md quork-proc/docs/ 2>/dev/null
# Look for test files related to strip_enum
fd "strip.*test" quork-proc/tests/
Length of output: 141
Script:
#!/bin/bash
# List all files in the tests directory
fd . quork-proc/tests/
# Check content of test files for any stripped_meta usage
rg -A 5 "stripped_meta" quork-proc/tests/
Length of output: 1795
| #[derive(Strip)] | ||
| #[stripped_meta(derive(EnumIter))] | ||
| #[stripped_meta(strum(serialize_all = "kebab-case"))] | ||
| enum EnumWithInherit { | ||
| Test1(DummyStruct), | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
Inheritance functionality is commented out and needs implementation
Based on the code inspection:
- The inheritance implementation is commented out in
strip_enum.rs(visible in the commented block containinginheritlogic) - There's a test case
EnumWithInheritexpecting this functionality - No alternative or active implementation of inheritance was found
This confirms the original review comment's observation. The feature is incomplete as the implementation is commented out while the test exists.
🔗 Analysis chain
Verify inheritance functionality implementation.
The test case exists for enum inheritance, but the corresponding implementation appears to be commented out in strip_enum.rs (lines 122-139). Either implement the inheritance functionality or remove the test case.
Let me check the implementation details in the strip_enum.rs file.
Let me check if there are any active implementations or tests related to inheritance functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any other tests or implementations of inheritance
rg -l "inherit" .
Length of output: 48
Script:
#!/bin/bash
# Check the implementation in strip_enum.rs
rg -A 5 -B 5 "inherit" ./quork-proc/src/strip_enum.rs
Length of output: 915
Script:
#!/bin/bash
# Search for stripped_meta and inheritance related patterns in tests
rg -A 5 "stripped_meta" ./quork-proc/tests/strip_enum.rs
Length of output: 780
Summary by CodeRabbit
strip_enumfor generating stripped enums.strumfor enhanced capabilities.