Copy trait function docs to overridden functions that lack their own

When using #[contractimpl(contracttrait)], overridden trait functions
that lack their own doc comments now inherit docs from the trait
function definition. This ensures contract specs are fully documented
when using well-documented traits.

Closes #1653

Author: leighmcculloch

Cargo.lock

@@ -1,4 +1,5 @@
 use crate::{
+    attribute::is_attr_doc,
     default_crate_path,
     derive_args::derive_args_impl,
     derive_client::derive_client_impl,
@@ -9,7 +10,7 @@ use crate::{
 use darling::{ast::NestedMeta, Error, FromMeta};
 use proc_macro2::{Ident, TokenStream as TokenStream2};
 use quote::quote;
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 use syn::{LitStr, Path, Type};
 
 // See soroban-sdk/docs/contracttrait.md for documentation on how this works.
@@ -20,8 +21,10 @@ struct Args {
     crate_path: Path,
     trait_ident: Path,
     trait_default_fns: Vec<LitStr>,
+    trait_all_fns: Vec<LitStr>,
     impl_ident: Ident,
     impl_fns: Vec<LitStr>,
+    impl_fns_with_docs: Vec<LitStr>,
     client_name: String,
     args_name: String,
     spec_name: Type,
@@ -84,5 +87,33 @@ fn derive(args: &Args) -> Result<TokenStream2, Error> {
         fns.iter().map(|f| &f.ident),
     ));
 
+    // Generate spec for overridden functions, inheriting trait docs for
+    // functions that lack their own doc comments.
+    let trait_all_fns = syn_ext::strs_to_fns(&args.trait_all_fns)?;
+    let trait_fn_docs: HashMap<String, Vec<syn::Attribute>> = trait_all_fns
+        .into_iter()
+        .map(|f| (f.ident.to_string(), f.attrs))
+        .collect();
+
+    let impl_fns_parsed = syn_ext::strs_to_fns(&args.impl_fns_with_docs)?;
+    let impl_fns_for_spec: Vec<syn_ext::Fn> = impl_fns_parsed
+        .into_iter()
+        .map(|f| {
+            let has_docs = f.attrs.iter().any(|a| is_attr_doc(a));
+            if !has_docs {
+                if let Some(trait_attrs) = trait_fn_docs.get(&f.ident.to_string()) {
+                    return syn_ext::Fn {
+                        ident: f.ident,
+                        attrs: trait_attrs.clone(),
+                        inputs: f.inputs,
+                        output: f.output,
+                    };
+                }
+            }
+            f
+        })
+        .collect();
+    output.extend(derive_fns_spec(&args.spec_name, &impl_fns_for_spec, spec_export));
+
     Ok(output)
 }

soroban-sdk-macros/src/derive_contractimpl_trait_default_fns_not_overridden.rs

@@ -56,6 +56,16 @@ fn derive(args: &Args, input: &ItemTrait) -> Result<TokenStream2, Error> {
         _ => None,
     });
 
+    // Serialize all trait functions' docs+sigs (including non-default ones) so
+    // that overridden functions can inherit trait docs when they lack their own.
+    let all_fns = input.items.iter().filter_map(|i| match i {
+        TraitItem::Fn(TraitItemFn { sig, attrs, .. }) => {
+            let doc_attrs: Vec<_> = attrs.iter().filter(|a| is_attr_doc(a)).collect();
+            Some(quote!(#(#doc_attrs)* #sig).to_token_stream().to_string())
+        }
+        _ => None,
+    });
+
     let macro_ident = macro_ident(&input.ident);
 
     let output = quote! {
@@ -67,15 +77,18 @@ fn derive(args: &Args, input: &ItemTrait) -> Result<TokenStream2, Error> {
                 $trait_ident:path,
                 $impl_ident:ty,
                 $impl_fns:expr,
+                $impl_fns_with_docs:expr,
                 $client_name:literal,
                 $args_name:literal,
                 $spec_name:literal $(,)?
             ) => {
                 #path::contractimpl_trait_default_fns_not_overridden!(
                     trait_ident = $trait_ident,
                     trait_default_fns = [#(#fns),*],
+                    trait_all_fns = [#(#all_fns),*],
                     impl_ident = $impl_ident,
                     impl_fns = $impl_fns,
+                    impl_fns_with_docs = $impl_fns_with_docs,
                     client_name = $client_name,
                     args_name = $args_name,
                     spec_name = $spec_name,
@@ -99,11 +112,23 @@ pub fn generate_call_to_contractimpl_for_trait(
     spec_ident: &str,
 ) -> TokenStream2 {
     let impl_fn_idents = pub_methods.iter().map(|f| f.sig.ident.to_string());
+    // Serialize impl functions with their doc attrs so the macro bridge can
+    // generate spec entries, falling back to trait docs when the impl function
+    // lacks its own.
+    let impl_fn_strs: Vec<String> = pub_methods
+        .iter()
+        .map(|f| {
+            let doc_attrs: Vec<_> = f.attrs.iter().filter(|a| is_attr_doc(a)).collect();
+            let sig = &f.sig;
+            quote!(#(#doc_attrs)* #sig).to_token_stream().to_string()
+        })
+        .collect();
     quote! {
         #trait_ident!(
             #trait_ident,
             #impl_ident,
             [#(#impl_fn_idents),*],
+            [#(#impl_fn_strs),*],
             #client_ident,
             #args_ident,
             #spec_ident,

soroban-sdk-macros/src/derive_contractimpl_trait_macro.rs

@@ -259,12 +259,24 @@ pub fn contractimpl(metadata: TokenStream, input: TokenStream) -> TokenStream {
 
     match derived {
         Ok(derived_ok) => {
-            let mut output = quote! {
-                #[#crate_path::contractargs(name = #args_ident, impl_only = true)]
-                #[#crate_path::contractclient(crate_path = #crate_path_str, name = #client_ident, impl_only = true)]
-                #[#crate_path::contractspecfn(name = #ty_str)]
-                #imp
-                #derived_ok
+            // When contracttrait is true, spec generation for overridden
+            // functions is handled by the macro bridge (so that trait docs can
+            // be used as fallback), so contractspecfn is not applied here.
+            let mut output = if args.contracttrait {
+                quote! {
+                    #[#crate_path::contractargs(name = #args_ident, impl_only = true)]
+                    #[#crate_path::contractclient(crate_path = #crate_path_str, name = #client_ident, impl_only = true)]
+                    #imp
+                    #derived_ok
+                }
+            } else {
+                quote! {
+                    #[#crate_path::contractargs(name = #args_ident, impl_only = true)]
+                    #[#crate_path::contractclient(crate_path = #crate_path_str, name = #client_ident, impl_only = true)]
+                    #[#crate_path::contractspecfn(name = #ty_str)]
+                    #imp
+                    #derived_ok
+                }
             };
 
             // See soroban-sdk/docs/contracttrait.md for documentation on how

soroban-sdk-macros/src/lib.rs

@@ -18,3 +18,4 @@ test_contracttrait_trait = {path = "../contracttrait_trait"}
 [dev-dependencies]
 soroban-sdk = {path = "../../soroban-sdk", features = ["testutils"]}
 test_contracttrait_trait = {path = "../contracttrait_trait"}
+stellar-xdr.workspace = true

tests/contracttrait_impl_partial/Cargo.toml

@@ -20,6 +20,7 @@ impl AllTypes for Contract {
         v
     }
 
+    /// Custom env param docs.
     fn test_env_param(_env: &Env) -> u32 {
         100
     }
@@ -37,6 +38,47 @@ mod test {
     use super::*;
     use soroban_sdk::{map, symbol_short, testutils::Address as _, vec, Env};
 
+    #[test]
+    fn test_spec_docs_inherited_from_trait() {
+        use stellar_xdr::curr as stellar_xdr;
+        use stellar_xdr::{Limits, ReadXdr, ScSpecEntry};
+
+        // An overridden function without its own docs should inherit trait docs.
+        let entry = ScSpecEntry::from_xdr(Contract::spec_xdr_test_u32(), Limits::none()).unwrap();
+        let ScSpecEntry::FunctionV0(func) = entry else {
+            panic!("expected FunctionV0");
+        };
+        assert_eq!(
+            func.doc.to_utf8_string().unwrap(),
+            "Test u32 values.\nReturns the input unchanged."
+        );
+
+        // A non-overridden default function should also have trait docs.
+        let entry = ScSpecEntry::from_xdr(Contract::spec_xdr_test_i32(), Limits::none()).unwrap();
+        let ScSpecEntry::FunctionV0(func) = entry else {
+            panic!("expected FunctionV0");
+        };
+        assert_eq!(func.doc.to_utf8_string().unwrap(), "Test i32 values.");
+
+        // An overridden function without docs and a trait function without docs
+        // should have empty docs.
+        let entry =
+            ScSpecEntry::from_xdr(Contract::spec_xdr_test_string(), Limits::none()).unwrap();
+        let ScSpecEntry::FunctionV0(func) = entry else {
+            panic!("expected FunctionV0");
+        };
+        assert_eq!(func.doc.to_utf8_string().unwrap(), "");
+
+        // An overridden function with its own docs should use its own docs,
+        // not the trait docs.
+        let entry =
+            ScSpecEntry::from_xdr(Contract::spec_xdr_test_env_param(), Limits::none()).unwrap();
+        let ScSpecEntry::FunctionV0(func) = entry else {
+            panic!("expected FunctionV0");
+        };
+        assert_eq!(func.doc.to_utf8_string().unwrap(), "Custom env param docs.");
+    }
+
     #[test]
     fn test_partial_override() {
         let e = Env::default();