feat(macros)!: change completion context parameter to owned type

BREAKING CHANGE: `#[completion]` functions must now take `ShellContext`
by value instead of `&ShellContext`. Reference parameters are reserved
for resource injection.

1 files changed, 100 insertions, 132 deletions

diff --git a/mingling_macros/src/attr/completion.rs b/mingling_macros/src/attr/completion.rs
index cbc0fef..71acaae 100644
--- a/mingling_macros/src/attr/completion.rs
+++ b/mingling_macros/src/attr/completion.rs
@@ -34,52 +34,92 @@ pub(crate) fn completion_attr(attr: TokenStream, item: TokenStream) -> TokenStre
     let inputs = &sig.inputs;
     let output = &sig.output;
 
-    // The first parameter (if any) is the completion context. It may be
-    // `&ShellContext`, an owned `ShellContext`, or any other type that
-    // implements `From<&ShellContext>`. With no parameters, the completion
-    // function simply ignores the shell context.
-    let ctx_ty: Option<Type> = match inputs.first() {
-        None => None,
-        Some(FnArg::Typed(PatType { ty, .. })) => Some((**ty).clone()),
-        Some(FnArg::Receiver(_)) => {
-            return syn::Error::new(
-                inputs.span(),
-                "Completion function cannot have self parameter",
-            )
-            .to_compile_error()
-            .into();
-        }
-    };
+    // Parameter classification:
+    // - Owned (non-reference) parameters are **Shell sources**: each is derived
+    //   from `&ShellContext` via `From<&ShellContext>` (identity `From` covers
+    //   `ShellContext` itself).
+    // - `&T` / `&mut T` parameters are resource injections (unchanged).
+    // - `&ShellContext` is rejected with a helpful message: use the owned
+    //   `ShellContext` (or any other `From<&ShellContext>` type) instead.
+    let mut derived_stmts: Vec<proc_macro2::TokenStream> = Vec::new();
+    let mut call_args: Vec<proc_macro2::TokenStream> = Vec::new();
+    let mut resources = Vec::new();
 
-    // Resource injection starts after the context parameter.
-    let resource_skip = usize::from(ctx_ty.is_some());
-    let resources = match extract_resources_from_args(sig, resource_skip) {
-        Ok(r) => r,
-        Err(e) => return e.to_compile_error().into(),
-    };
-    if ctx_ty.is_none() && !resources.is_empty() {
-        return syn::Error::new(
-            inputs.span(),
-            "A completion function without a context parameter cannot inject resources",
-        )
-        .to_compile_error()
-        .into();
-    }
+    for (idx, arg) in inputs.iter().enumerate() {
+        match arg {
+            FnArg::Typed(PatType { pat, ty, .. }) => {
+                if let Type::Reference(ref_type) = &**ty {
+                    // `&ShellContext` is no longer allowed: it clashes with the
+                    // resource-injection semantics of references.
+                    if is_shell_context_path(&ref_type.elem) {
+                        return syn::Error::new(
+                            ty.span(),
+                            "`&ShellContext` is not supported; use the owned `ShellContext` \
+                             (or any other type implementing `From<&ShellContext>`) as a value \
+                             parameter",
+                        )
+                        .to_compile_error()
+                        .into();
+                    }
 
-    // Bind the shell context to the declared parameter type (identity `From`
-    // covers `&ShellContext` itself).
-    let (ctx_bind_stmt, ctx_call_arg) = ctx_ty.as_ref().map_or_else(
-        || (quote! { let _ = ctx; }, quote! {}),
-        |ty| {
-            (
-                quote! {
-                    let __ctx: #ty =
-                        <#ty as ::std::convert::From<&::mingling::ShellContext>>::from(ctx);
-                },
-                quote! { __ctx },
-            )
-        },
-    );
+                    // Reference: resource injection (requires a named binding).
+                    let var_name = match &**pat {
+                        Pat::Ident(pat_ident) => pat_ident.ident.clone(),
+                        _ => {
+                            return syn::Error::new(
+                                pat.span(),
+                                "Resource injection parameter must be a simple identifier",
+                            )
+                            .to_compile_error()
+                            .into();
+                        }
+                    };
+
+                    // Reference: resource injection.
+                    let (inner_type, is_mut) = match &*ref_type.elem {
+                        Type::Path(type_path) => {
+                            let is_mut = ref_type.mutability.is_some();
+                            (type_path.clone(), is_mut)
+                        }
+                        _ => {
+                            return syn::Error::new(
+                                ty.span(),
+                                "Reference resource type must be a type path",
+                            )
+                            .to_compile_error()
+                            .into();
+                        }
+                    };
+                    resources.push(ResourceInjection {
+                        var_name: var_name.clone(),
+                        full_type: (**ty).clone(),
+                        inner_type,
+                        is_ref: true,
+                        is_mut,
+                    });
+                    call_args.push(quote! { #var_name });
+                } else {
+                    // Owned value: derive from `&ShellContext`. The parameter
+                    // name is irrelevant (anonymous `_` is fine) since the
+                    // derived binding is generated by this macro.
+                    let derived_ident = Ident::new(&format!("__ctx_derived_{idx}"), pat.span());
+                    derived_stmts.push(quote! {
+                        let #derived_ident: #ty =
+                            <#ty as ::std::convert::From<&::mingling::ShellContext>>::from(ctx);
+                    });
+                    call_args.push(quote! { #derived_ident });
+                }
+            }
+            FnArg::Receiver(_) => {
+                return syn::Error::new(
+                    arg.span(),
+                    "Completion function cannot have self parameter",
+                )
+                .to_compile_error()
+                .into();
+            }
+        }
+    }
 
     let fn_body = &input_fn.block;
 
@@ -96,26 +136,11 @@ pub(crate) fn completion_attr(attr: TokenStream, item: TokenStream) -> TokenStre
     let struct_name = Ident::new(&internal_name, fn_name.span());
 
     let program_type = crate::default_program_path();
-    let has_resources = !resources.is_empty();
     let mut_resources: Vec<_> = resources.iter().filter(|r| r.is_mut).collect();
 
     let immut_resource_stmts = generate_immut_resource_bindings(resources.iter(), &program_type);
 
-    let resource_args: Vec<_> = resources
-        .iter()
-        .map(|res| {
-            let var_name = &res.var_name;
-            quote! { #var_name }
-        })
-        .collect();
-
-    let fn_call = if has_resources {
-        quote! { #fn_name(#ctx_call_arg, #(#resource_args),*) }
-    } else if ctx_ty.is_some() {
-        quote! { #fn_name(#ctx_call_arg) }
-    } else {
-        quote! { #fn_name() }
-    };
+    let fn_call = quote! { #fn_name(#(#call_args),*) };
 
     let inner_call = if mut_resources.is_empty() {
         fn_call
@@ -133,13 +158,10 @@ pub(crate) fn completion_attr(attr: TokenStream, item: TokenStream) -> TokenStre
         wrapped
     };
 
-    let comp_body = if has_resources {
-        quote! {
-            #(#immut_resource_stmts)*
-            #inner_call
-        }
-    } else {
-        quote! { #inner_call }
+    let comp_body = quote! {
+        #(#derived_stmts)*
+        #(#immut_resource_stmts)*
+        #inner_call
     };
 
     // A `()` return (or no return type) means "no suggestions": map it to an
@@ -172,7 +194,6 @@ pub(crate) fn completion_attr(attr: TokenStream, item: TokenStream) -> TokenStre
             type Previous = #previous_type_path;
 
             fn comp(ctx: &::mingling::ShellContext) -> ::mingling::Suggest {
-                #ctx_bind_stmt
                 #return_stmt
             }
         }
@@ -208,69 +229,16 @@ pub(crate) fn completion_attr(attr: TokenStream, item: TokenStream) -> TokenStre
     expanded.into()
 }
 
-/// Extract resource injection parameters from function arguments (skipping the first N params).
-fn extract_resources_from_args(
-    sig: &syn::Signature,
-    skip: usize,
-) -> syn::Result<Vec<ResourceInjection>> {
-    let mut resources = Vec::new();
-    for arg in sig.inputs.iter().skip(skip) {
-        match arg {
-            FnArg::Typed(PatType { pat, ty, .. }) => {
-                let var_name = match &**pat {
-                    Pat::Ident(pat_ident) => pat_ident.ident.clone(),
-                    _ => {
-                        return Err(syn::Error::new(
-                            pat.span(),
-                            "Resource injection parameter must be a simple identifier",
-                        ));
-                    }
-                };
-
-                let full_type = *(*ty).clone();
-
-                let (inner_type, is_ref, is_mut) = match &full_type {
-                    Type::Reference(ref_type) => match &*ref_type.elem {
-                        Type::Path(type_path) => {
-                            let is_mut = ref_type.mutability.is_some();
-                            (type_path.clone(), true, is_mut)
-                        }
-                        _ => {
-                            return Err(syn::Error::new(
-                                ty.span(),
-                                "Reference resource type must be a type path",
-                            ));
-                        }
-                    },
-                    Type::Path(_) => {
-                        return Err(syn::Error::new(
-                            ty.span(),
-                            "Resource injection parameter must be a reference (`&T` or `&mut T`)",
-                        ));
-                    }
-                    _ => {
-                        return Err(syn::Error::new(
-                            ty.span(),
-                            "Resource injection type must be a type path or reference",
-                        ));
-                    }
-                };
-
-                resources.push(ResourceInjection {
-                    var_name,
-                    full_type,
-                    inner_type,
-                    is_ref,
-                    is_mut,
-                });
-            }
-            FnArg::Receiver(_) => {
-                return Err(syn::Error::new(
-                    arg.span(),
-                    "Resource injection parameter cannot be self",
-                ));
-            }
-        }
+/// Returns `true` when the type is a path whose last segment is `ShellContext`
+/// (e.g. `ShellContext` or `mingling::ShellContext`).
+fn is_shell_context_path(ty: &Type) -> bool {
+    if let Type::Path(type_path) = ty {
+        type_path
+            .path
+            .segments
+            .last()
+            .is_some_and(|seg| seg.ident == "ShellContext")
+    } else {
+        false
     }
-    Ok(resources)
 }