-
-
Notifications
You must be signed in to change notification settings - Fork 1
Refactor: Improve macro hygiene, code quality, and maintainability #17
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
Open
jhg
wants to merge
7
commits into
main
Choose a base branch
from
claude/refactor-macro-optimization-011CUmaUpXD9ePrSnxWvqoC4
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
ad3de5c
Refactor: Improve macro hygiene, code quality, and maintainability
claude da53164
Optimize: Major performance and code quality improvements
claude 4226356
Fix: Remove unnecessary structure name repetition (clippy::nursery)
claude c7083b4
Fix: Add explicit semicolons after assignment in generated code
claude 0b29c8b
Refactor: Rename 'assignment' to 'assignment_stmt' for clarity
claude bd19c21
Docs: Clarify semicolon handling in assignment_stmt generation
claude 69be234
Format Rust code using rustfmt
github-actions[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -9,7 +9,7 @@ use syn::{Expr, Ident, LitStr, spanned::Spanned}; | |||||
|
|
||||||
| /// Generate parsing code from tokens. | ||||||
| /// | ||||||
| /// Returns `(code, anon_count)` or error for consecutive placeholders / missing args. | ||||||
| /// Returns `(code, anon_count)` or error for consecutive placeholders/missing args. | ||||||
| fn generate_parsing_code( | ||||||
| tokens: &[FormatToken], | ||||||
| explicit_args: &[&Expr], | ||||||
|
|
@@ -87,157 +87,126 @@ fn generate_parsing_code( | |||||
| Ok((generated, anon_index)) | ||||||
| } | ||||||
|
|
||||||
| /// Generate code for named placeholder with separator. | ||||||
| fn generate_named_placeholder_with_separator( | ||||||
| name: &str, | ||||||
| /// Generate code for placeholder with separator (named or anonymous). | ||||||
| fn generate_placeholder_with_separator( | ||||||
| assignment_stmt: &proc_macro2::TokenStream, | ||||||
| var_desc: &str, | ||||||
| separator: &LitStr, | ||||||
| ) -> proc_macro2::TokenStream { | ||||||
| let ident = Ident::new(name, Span::call_site()); | ||||||
|
|
||||||
| quote! { | ||||||
| if let Some(pos) = remaining.find(#separator) { | ||||||
| { | ||||||
| let pos = remaining.find(#separator).ok_or_else(|| { | ||||||
| std::io::Error::new( | ||||||
| std::io::ErrorKind::InvalidInput, | ||||||
| format!( | ||||||
| "Expected separator {:?} for {} not found in remaining input: {:?}", | ||||||
| #separator, | ||||||
| #var_desc, | ||||||
| remaining | ||||||
| ) | ||||||
| ) | ||||||
| })?; | ||||||
| let slice = &remaining[..pos]; | ||||||
| match slice.parse() { | ||||||
| Ok(parsed) => { | ||||||
| #ident = parsed; | ||||||
| } | ||||||
| Err(error) => { | ||||||
| result = result.and(Err(std::io::Error::new( | ||||||
| std::io::ErrorKind::InvalidInput, | ||||||
| format!("Failed to parse variable '{}' from {:?}: {}", #name, slice, error) | ||||||
| ))); | ||||||
| } | ||||||
| } | ||||||
| remaining = &remaining[pos + #separator.len()..]; | ||||||
| } else { | ||||||
| result = result.and(Err(std::io::Error::new( | ||||||
| std::io::ErrorKind::InvalidInput, | ||||||
| format!( | ||||||
| "Expected separator {:?} for variable '{}' not found in remaining input: {:?}", | ||||||
| #separator, | ||||||
| #name, | ||||||
| remaining | ||||||
| let parsed = slice.parse().map_err(|error| { | ||||||
| std::io::Error::new( | ||||||
| std::io::ErrorKind::InvalidInput, | ||||||
| format!("Failed to parse {} from {:?}: {}", #var_desc, slice, error) | ||||||
| ) | ||||||
| ))); | ||||||
| })?; | ||||||
| #assignment_stmt; | ||||||
| remaining = &remaining[pos + #separator.len()..]; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /// Generate code for named placeholder with separator. | ||||||
| /// | ||||||
| /// Note: `assignment_stmt` contains the expression `#ident = parsed` WITHOUT a trailing | ||||||
| /// semicolon. The semicolon is added explicitly at the insertion point to form a complete | ||||||
| /// statement within the generated block. | ||||||
| fn generate_named_placeholder_with_separator( | ||||||
| name: &str, | ||||||
| separator: &LitStr, | ||||||
| ) -> proc_macro2::TokenStream { | ||||||
| let ident = Ident::new(name, Span::call_site()); | ||||||
| let assignment_stmt = quote! { #ident = parsed }; // No trailing semicolon | ||||||
| let var_desc = format!("variable '{name}'"); | ||||||
| generate_placeholder_with_separator(&assignment_stmt, &var_desc, separator) | ||||||
| } | ||||||
|
|
||||||
| /// Generate code for anonymous placeholder with separator. | ||||||
| /// | ||||||
| /// Note: `assignment_stmt` contains the expression `*#arg_expr = parsed` WITHOUT a trailing | ||||||
| /// semicolon. The semicolon is added explicitly at the insertion point to form a complete | ||||||
| /// statement within the generated block. | ||||||
| fn generate_anonymous_placeholder_with_separator( | ||||||
| arg_expr: &Expr, | ||||||
| placeholder_num: usize, | ||||||
| separator: &LitStr, | ||||||
| ) -> proc_macro2::TokenStream { | ||||||
| quote! { | ||||||
| if let Some(pos) = remaining.find(#separator) { | ||||||
| let slice = &remaining[..pos]; | ||||||
| match slice.parse() { | ||||||
| Ok(parsed) => { | ||||||
| *#arg_expr = parsed; | ||||||
| } | ||||||
| Err(error) => { | ||||||
| result = result.and(Err(std::io::Error::new( | ||||||
| std::io::ErrorKind::InvalidInput, | ||||||
| format!( | ||||||
| "Failed to parse anonymous placeholder #{} from {:?}: {}", | ||||||
| #placeholder_num, | ||||||
| slice, | ||||||
| error | ||||||
| ) | ||||||
| ))); | ||||||
| } | ||||||
| } | ||||||
| remaining = &remaining[pos + #separator.len()..]; | ||||||
| } else { | ||||||
| result = result.and(Err(std::io::Error::new( | ||||||
| std::io::ErrorKind::InvalidInput, | ||||||
| format!( | ||||||
| "Expected separator {:?} for anonymous placeholder #{} not found in remaining input: {:?}", | ||||||
| #separator, | ||||||
| #placeholder_num, | ||||||
| remaining | ||||||
| ) | ||||||
| ))); | ||||||
| } | ||||||
| } | ||||||
| let assignment_stmt = quote! { *#arg_expr = parsed }; // No trailing semicolon | ||||||
| let var_desc = format!("anonymous placeholder #{placeholder_num}"); | ||||||
| generate_placeholder_with_separator(&assignment_stmt, &var_desc, separator) | ||||||
| } | ||||||
|
|
||||||
| /// Generate code for fixed text matching at current position. | ||||||
| fn generate_fixed_text_match(text: &LitStr) -> proc_macro2::TokenStream { | ||||||
| quote! { | ||||||
| if let Some(pos) = remaining.find(#text) { | ||||||
| if pos == 0 { | ||||||
| remaining = &remaining[#text.len()..]; | ||||||
| } else { | ||||||
| result = result.and(Err(std::io::Error::new( | ||||||
| { | ||||||
| if !remaining.starts_with(#text) { | ||||||
| return Err(std::io::Error::new( | ||||||
| std::io::ErrorKind::InvalidInput, | ||||||
| format!( | ||||||
| "Expected text {:?} at current position, but found it at offset {}. \ | ||||||
| Remaining input: {:?}", | ||||||
| "Expected text {:?} at current position. Remaining input: {:?}", | ||||||
| #text, | ||||||
| pos, | ||||||
| remaining | ||||||
| ) | ||||||
| ))); | ||||||
| )); | ||||||
| } | ||||||
| } else { | ||||||
| result = result.and(Err(std::io::Error::new( | ||||||
| std::io::ErrorKind::InvalidInput, | ||||||
| format!( | ||||||
| "Required text separator {:?} not found. Remaining input: {:?}", | ||||||
| #text, | ||||||
| remaining | ||||||
| ) | ||||||
| ))); | ||||||
| remaining = &remaining[#text.len()..]; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /// Generate code for final named placeholder (consumes rest of input). | ||||||
| fn generate_final_named_placeholder(name: &str) -> proc_macro2::TokenStream { | ||||||
| let ident = Ident::new(name, Span::call_site()); | ||||||
|
|
||||||
| /// Generate code for final placeholder (consumes rest of input). | ||||||
| fn generate_final_placeholder( | ||||||
| assignment_stmt: &proc_macro2::TokenStream, | ||||||
| var_desc: &str, | ||||||
| ) -> proc_macro2::TokenStream { | ||||||
| quote! { | ||||||
| match remaining.parse() { | ||||||
| Ok(parsed) => { | ||||||
| #ident = parsed; | ||||||
| } | ||||||
| Err(error) => { | ||||||
| result = result.and(Err(std::io::Error::new( | ||||||
| { | ||||||
| let parsed = remaining.parse().map_err(|error| { | ||||||
| std::io::Error::new( | ||||||
| std::io::ErrorKind::InvalidInput, | ||||||
| format!("Failed to parse variable '{}' from remaining input {:?}: {}", #name, remaining, error) | ||||||
| ))); | ||||||
| } | ||||||
| format!("Failed to parse {} from remaining input {:?}: {}", #var_desc, remaining, error) | ||||||
| ) | ||||||
| })?; | ||||||
| #assignment_stmt; | ||||||
|
||||||
| #assignment_stmt; | |
| #assignment_stmt |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The semicolon after
#assignment_stmtshould be removed. This line expands a token stream that already includes proper statement termination (either#ident = parsedor*#arg_expr = parsed), and adding a semicolon creates= parsed;;which is a syntax error.