refactor(macros): clean up clippy lints and simplify code

4 files changed, 41 insertions, 67 deletions

diff --git a/mingling_macros/src/attr/chain.rs b/mingling_macros/src/attr/chain.rs
index dc28a39..a597a39 100644
--- a/mingling_macros/src/attr/chain.rs
+++ b/mingling_macros/src/attr/chain.rs
@@ -20,22 +20,11 @@ fn is_unit_return_type(sig: &Signature) -> bool {
     }
 }
 
-/// Validates that the return type is acceptable.
-/// Accepts `()`, `Next`, `ChainProcess<...>`, or any type that can
-/// be converted to `ChainProcess` via `.into()` (i.e. any pack type).
-fn validate_return_type(sig: &Signature) -> Result<(), proc_macro2::TokenStream> {
-    // `()` or omitted is always valid
-    if is_unit_return_type(sig) {
-        return Ok(());
-    }
-
-    Ok(())
-}
-
 /// Builds the `proc` function implementation inside the generated `Chain` impl.
 ///
 /// Instead of inlining the user's body, the trait method calls the original
 /// function by name, with resources injected from the application context.
+#[allow(clippy::needless_pass_by_value)]
 fn generate_proc_fn(
     fn_name: &Ident,
     has_resources: bool,
@@ -215,6 +204,9 @@ pub(crate) fn chain_attr(attr: TokenStream, item: TokenStream) -> TokenStream {
     let is_async_fn = input_fn.sig.asyncness.is_some();
 
     #[cfg(not(feature = "async"))]
+    let is_async_fn = false;
+
+    #[cfg(not(feature = "async"))]
     {
         if let Err(err) = reject_async(&input_fn.sig) {
             return err.into();
@@ -224,11 +216,6 @@ pub(crate) fn chain_attr(attr: TokenStream, item: TokenStream) -> TokenStream {
     // Check if return type is unit
     let is_unit_return = is_unit_return_type(&input_fn.sig);
 
-    // Validate return type
-    if let Err(err) = validate_return_type(&input_fn.sig) {
-        return err.into();
-    }
-
     // Extract the previous type, parameter name, and resource injection params
     let (_, previous_type, resources) = match extract_args_info(&input_fn.sig) {
         Ok(info) => info,
@@ -259,10 +246,7 @@ pub(crate) fn chain_attr(attr: TokenStream, item: TokenStream) -> TokenStream {
         &resources,
         &program_type,
         &previous_type,
-        #[cfg(feature = "async")]
         is_async_fn,
-        #[cfg(not(feature = "async"))]
-        false,
         is_unit_return,
     );
 
@@ -343,9 +327,6 @@ pub(crate) fn register_chain(input: TokenStream) -> TokenStream {
     // Record the chain existence check
     let chain_exist_entry = build_chain_exist_arm(&previous_type);
 
-    let mut chains = crate::get_global_set(&crate::CHAINS).lock().unwrap();
-    let mut chain_exist = crate::get_global_set(&crate::CHAINS_EXIST).lock().unwrap();
-
     let chain_entry_str = chain_entry.to_string();
     let chain_exist_entry_str = chain_exist_entry.to_string();
 
@@ -357,18 +338,25 @@ pub(crate) fn register_chain(input: TokenStream) -> TokenStream {
         .unwrap()
         .ident
         .to_string();
-    if let Err(err) = crate::check_duplicate_variant(
-        &chains,
+    let value = crate::check_duplicate_variant(
+        &crate::get_global_set(&crate::CHAINS).lock().unwrap(),
         &chain_entry_str,
         &variant_name,
         "chain",
         previous_type.span(),
-    ) {
+    );
+    if let Err(err) = value {
         return err.into();
     }
 
-    chains.insert(chain_entry_str);
-    chain_exist.insert(chain_exist_entry_str);
+    crate::get_global_set(&crate::CHAINS)
+        .lock()
+        .unwrap()
+        .insert(chain_entry_str);
+    crate::get_global_set(&crate::CHAINS_EXIST)
+        .lock()
+        .unwrap()
+        .insert(chain_exist_entry_str);
 
     quote! {}.into()
 }
diff --git a/mingling_macros/src/attr/completion.rs b/mingling_macros/src/attr/completion.rs
index 3ced091..f1635a8 100644
--- a/mingling_macros/src/attr/completion.rs
+++ b/mingling_macros/src/attr/completion.rs
@@ -5,9 +5,10 @@ use syn::spanned::Spanned;
 use syn::{FnArg, Ident, ItemFn, Pat, PatType, Type, TypePath, parse_macro_input};
 
 #[cfg(feature = "comp")]
+#[allow(clippy::too_many_lines)]
 pub(crate) fn completion_attr(attr: TokenStream, item: TokenStream) -> TokenStream {
-    // Parse the attribute arguments such as HelloEntry or crate::EntryFine from #[completion(crate::EntryFine)]
     use crate::get_global_set;
+
     let previous_type_path: TypePath = if attr.is_empty() {
         return syn::Error::new(
             proc_macro2::Span::call_site(),
@@ -20,22 +21,18 @@ pub(crate) fn completion_attr(attr: TokenStream, item: TokenStream) -> TokenStre
     };
     let previous_type_ident = &previous_type_path.path.segments.last().unwrap().ident;
 
-    // Parse the function item
     let input_fn = parse_macro_input!(item as ItemFn);
 
-    // Validate the function is not async
     if input_fn.sig.asyncness.is_some() {
         return syn::Error::new(input_fn.sig.span(), "Completion function cannot be async")
             .to_compile_error()
             .into();
     }
 
-    // Get the function signature parts
     let sig = &input_fn.sig;
     let inputs = &sig.inputs;
     let output = &sig.output;
 
-    // Must have at least one parameter ctx
     if inputs.is_empty() {
         return syn::Error::new(
             inputs.span(),
@@ -45,7 +42,6 @@ pub(crate) fn completion_attr(attr: TokenStream, item: TokenStream) -> TokenStre
         .into();
     }
 
-    // Extract the first param pattern and type for the ctx parameter
     let first_arg = &inputs[0];
     let _ctx_type = match first_arg {
         FnArg::Typed(PatType { ty, .. }) => (**ty).clone(),
@@ -60,26 +56,19 @@ pub(crate) fn completion_attr(attr: TokenStream, item: TokenStream) -> TokenStre
     };
     let fixed_ctx: Pat = syn::parse_quote!(ctx);
 
-    // Extract resources from params 2 through N, skipping ctx
     let resources = match extract_resources_from_args(sig, 1) {
         Ok(r) => r,
         Err(e) => return e.to_compile_error().into(),
     };
 
-    // Get the function body
     let fn_body = &input_fn.block;
 
-    // Get function attributes excluding the completion attribute
     let mut fn_attrs = input_fn.attrs.clone();
     fn_attrs.retain(|attr| !attr.path().is_ident("completion"));
 
-    // Get function visibility
     let vis = &input_fn.vis;
-
-    // Get function name
     let fn_name = &sig.ident;
 
-    // Generate internal name from function name using snake_case
     let internal_name = format!(
         "__internal_completion_{}",
         just_fmt::snake_case!(fn_name.to_string())
@@ -90,10 +79,8 @@ pub(crate) fn completion_attr(attr: TokenStream, item: TokenStream) -> TokenStre
     let has_resources = !resources.is_empty();
     let mut_resources: Vec<_> = resources.iter().filter(|r| r.is_mut).collect();
 
-    // Generate immutable resource bindings
     let immut_resource_stmts = generate_immut_resource_bindings(resources.iter(), &program_type);
 
-    // Build the call to the original function with resource arguments injected
     let resource_args: Vec<_> = resources
         .iter()
         .map(|res| {
@@ -108,7 +95,6 @@ pub(crate) fn completion_attr(attr: TokenStream, item: TokenStream) -> TokenStre
         quote! { #fn_name(#fixed_ctx) }
     };
 
-    // Wrap the function call with modify_res for mutable resources
     let inner_call = if mut_resources.is_empty() {
         fn_call
     } else {
@@ -134,10 +120,7 @@ pub(crate) fn completion_attr(attr: TokenStream, item: TokenStream) -> TokenStre
         quote! { #inner_call }
     };
 
-    // Generate the struct and implementation
-    // The `comp` trait method only takes `ctx` as the first parameter; resources are injected internally
-
-    let expanded = quote! {
+    let expanded: proc_macro2::TokenStream = quote! {
         #(#fn_attrs)*
         #[doc(hidden)]
         #[allow(non_camel_case_types)]
@@ -162,22 +145,22 @@ pub(crate) fn completion_attr(attr: TokenStream, item: TokenStream) -> TokenStre
         Self::#previous_type_ident => <#struct_name as ::mingling::Completion>::comp(ctx),
     };
 
-    let mut completions = get_global_set(&crate::COMPLETIONS).lock().unwrap();
     let completion_str = completion_entry.to_string();
-
-    // Check for duplicate variant before inserting
     let variant_name = previous_type_ident.to_string();
+    let span = previous_type_path.span();
+
+    let mut completions = get_global_set(&crate::COMPLETIONS).lock().unwrap();
     if let Err(err) = crate::check_duplicate_variant(
         &completions,
         &completion_str,
         &variant_name,
         "completion",
-        previous_type_path.span(),
+        span,
     ) {
         return err.into();
     }
-
     completions.insert(completion_str);
+    drop(completions);
 
     expanded.into()
 }
diff --git a/mingling_macros/src/attr/help.rs b/mingling_macros/src/attr/help.rs
index b4ad989..42d81cf 100644
--- a/mingling_macros/src/attr/help.rs
+++ b/mingling_macros/src/attr/help.rs
@@ -14,6 +14,7 @@ fn extract_user_return_type(sig: &Signature) -> Option<proc_macro2::TokenStream>
     }
 }
 
+#[allow(clippy::too_many_lines)]
 pub(crate) fn help_attr(item: TokenStream) -> TokenStream {
     // Parse the function item
     let input_fn = parse_macro_input!(item as ItemFn);
@@ -50,7 +51,7 @@ pub(crate) fn help_attr(item: TokenStream) -> TokenStream {
 
     // Get original inputs to keep the original function
     let original_inputs = input_fn.sig.inputs.clone();
-    let original_return_type = user_return_type.clone().unwrap_or(quote! { () });
+    let original_return_type = user_return_type.unwrap_or_else(|| quote! { () });
 
     // Generate internal name using snake_case
     let internal_name = format!(
diff --git a/mingling_macros/src/attr/renderer.rs b/mingling_macros/src/attr/renderer.rs
index 828dc00..038edd8 100644
--- a/mingling_macros/src/attr/renderer.rs
+++ b/mingling_macros/src/attr/renderer.rs
@@ -15,9 +15,7 @@ fn extract_user_return_type(sig: &Signature) -> Option<proc_macro2::TokenStream>
 }
 
 #[allow(clippy::too_many_lines)]
-pub(crate) fn renderer_attr(attr: TokenStream, item: TokenStream) -> TokenStream {
-    // #[renderer] takes no arguments; always use the default program path
-    let _ = attr;
+pub(crate) fn renderer_attr(item: TokenStream) -> TokenStream {
     let program_path = crate::default_program_path();
     let program_type = &program_path;
 
@@ -121,7 +119,7 @@ pub(crate) fn renderer_attr(attr: TokenStream, item: TokenStream) -> TokenStream
     // The original function preserves the user's exact signature and body.
     // Resource parameters are passed directly by the caller, NOT injected from context.
     let original_inputs = input_fn.sig.inputs.clone();
-    let original_return_type = user_return_type.clone().unwrap_or(quote! { () });
+    let original_return_type = user_return_type.unwrap_or_else(|| quote! { () });
 
     let expanded = quote! {
         #(#fn_attrs)*
@@ -249,14 +247,15 @@ pub(crate) fn register_renderer(input: TokenStream) -> TokenStream {
         }
     } // renderers lock released here
 
-    let mut renderers = get_global_set(&crate::RENDERERS).lock().unwrap();
-    let mut renderer_exist = get_global_set(&crate::RENDERERS_EXIST).lock().unwrap();
-
-    #[cfg(feature = "structural_renderer")]
-    let mut structural_renderers = get_global_set(&crate::STRUCTURAL_RENDERERS).lock().unwrap();
-
-    renderers.insert(renderer_entry_str);
-    renderer_exist.insert(renderer_exist_entry_str);
+    // Insert renderer registration directly without holding a lock variable
+    get_global_set(&crate::RENDERERS)
+        .lock()
+        .unwrap()
+        .insert(renderer_entry_str);
+    get_global_set(&crate::RENDERERS_EXIST)
+        .lock()
+        .unwrap()
+        .insert(renderer_exist_entry_str);
 
     // Only register structural renderer if the type is in STRUCTURED_TYPES
     #[cfg(feature = "structural_renderer")]
@@ -266,7 +265,10 @@ pub(crate) fn register_renderer(input: TokenStream) -> TokenStream {
             .unwrap()
             .contains(&variant_name);
         if is_structured {
-            structural_renderers.insert(structural_renderer_entry_str);
+            get_global_set(&crate::STRUCTURAL_RENDERERS)
+                .lock()
+                .unwrap()
+                .insert(structural_renderer_entry_str);
         }
     }