refactor: use more ideomatic practices

sonicfromnewyoke · sonicfromnewyoke · commit 23496409d800 · 2026-06-14T17:51:25.000+03:00
diff --git a/codama-attributes/src/attributes.rs b/codama-attributes/src/attributes.rs
@@ -18,8 +18,9 @@ impl<'a> Attributes<'a> {
                     let inners = ast.unfeatured_all();
                     if inners.len() <= 1 {
                         // Not a multi-attr cfg_attr - use standard parsing
-                        let unfeatured = ast.unfeatured();
-                        let effective = unfeatured.unwrap_or_else(|| (*ast).clone());
+                        // Fall back to the attribute itself when it is not a
+                        // `cfg_attr` (the only case that needs an owned clone).
+                        let effective = ast.unfeatured().unwrap_or_else(|| ast.clone());
                         vec![(ast, effective)]
                     } else {
                         // Multi-attr cfg_attr - expand each inner attribute
diff --git a/codama-errors/src/combine_errors.rs b/codama-errors/src/combine_errors.rs
@@ -66,7 +66,7 @@ macro_rules! combine_errors {
             (Ok(value1), Ok(value2)) => Ok((value1, value2)),
             (Err(err1), Err(err2)) => {
                 let mut combined = err1;
-                codama_errors::CombineErrors::combine(&mut combined, err2);
+                $crate::CombineErrors::combine(&mut combined, err2);
                 Err(combined)
             }
             (Err(err), _) => Err(err),
@@ -76,11 +76,11 @@ macro_rules! combine_errors {
 
     // 3 results.
     ($first:expr, $second:expr, $third:expr $(,)?) => {{
-        match ($first, combine_errors!($second, $third)) {
+        match ($first, $crate::combine_errors!($second, $third)) {
             (Ok(value1), Ok((value2, value3))) => Ok((value1, value2, value3)),
             (Err(err1), Err(err2)) => {
                 let mut combined = err1;
-                codama_errors::CombineErrors::combine(&mut combined, err2);
+                $crate::CombineErrors::combine(&mut combined, err2);
                 Err(combined)
             }
             (Err(err), _) => Err(err),
diff --git a/codama-errors/src/errors.rs b/codama-errors/src/errors.rs
@@ -46,14 +46,10 @@ pub enum CodamaError {
 pub type CodamaResult<T> = Result<T, CodamaError>;
 
 impl CodamaError {
-    pub fn to_compile_error(&self) -> TokenStream {
+    pub fn into_compile_error(self) -> TokenStream {
         match self {
             CodamaError::Compilation(error) => error.to_compile_error(),
             _ => TokenStream::new(),
         }
     }
-
-    pub fn into_compile_error(self) -> TokenStream {
-        self.to_compile_error()
-    }
 }
diff --git a/codama-korok-visitors/src/combine_types_visitor.rs b/codama-korok-visitors/src/combine_types_visitor.rs
@@ -15,7 +15,7 @@ pub struct CombineTypesVisitor {
     pub r#override: bool,
     pub get_enum_variant:
         fn(korok: &EnumVariantKorok, parent: &str) -> Option<CodamaResult<EnumVariantTypeNode>>,
-    pub get_nammed_field:
+    pub get_named_field:
         fn(korok: &FieldKorok, parent: &str) -> Option<CodamaResult<StructFieldTypeNode>>,
     pub get_unnammed_field:
         fn(korok: &FieldKorok, parent: &str, index: usize) -> Option<CodamaResult<TypeNode>>,
@@ -27,7 +27,7 @@ impl Default for CombineTypesVisitor {
         Self {
             r#override: false,
             get_enum_variant: |x, _| Self::get_default_enum_variant(x),
-            get_nammed_field: |x, _| Self::get_default_named_field(x),
+            get_named_field: |x, _| Self::get_default_named_field(x),
             get_unnammed_field: |x, _, _| Self::get_default_unnamed_field(x),
             parent_enum: String::new(),
         }
@@ -41,7 +41,7 @@ impl CombineTypesVisitor {
     pub fn strict() -> Self {
         Self {
             get_enum_variant: Self::get_strict_enum_variant,
-            get_nammed_field: Self::get_strict_named_field,
+            get_named_field: Self::get_strict_named_field,
             get_unnammed_field: Self::get_strict_unnamed_field,
             ..Self::default()
         }
@@ -124,7 +124,7 @@ impl CombineTypesVisitor {
     ) -> CodamaResult<Vec<StructFieldTypeNode>> {
         let fields = fields
             .iter()
-            .filter_map(|field| (self.get_nammed_field)(field, parent))
+            .filter_map(|field| (self.get_named_field)(field, parent))
             .collect_and_combine_errors()?;
 
         let (before, after): (Vec<_>, Vec<_>) = attributes
@@ -173,9 +173,9 @@ impl KorokVisitor for CombineTypesVisitor {
                 StructTypeNode::new(fields).into()
             }
             syn::Fields::Unnamed(_) => {
-                let items = self.parse_unnamed_fields(&korok.fields, &parent)?;
+                let mut items = self.parse_unnamed_fields(&korok.fields, &parent)?;
                 if items.len() == 1 {
-                    items.first().unwrap().clone()
+                    items.remove(0)
                 } else {
                     TupleTypeNode::new(items).into()
                 }
diff --git a/codama-korok-visitors/src/set_instructions_visitors.rs b/codama-korok-visitors/src/set_instructions_visitors.rs
@@ -25,7 +25,7 @@ impl Default for SetInstructionsVisitor {
         Self {
             combine_types: CombineTypesVisitor {
                 // Skip fields with the `account` codama directive.
-                get_nammed_field: |korok, parent| {
+                get_named_field: |korok, parent| {
                     if korok.attributes.has_codama_attribute("account") {
                         return None;
                     }
diff --git a/codama-nodes/derive/src/nestable_type_node.rs b/codama-nodes/derive/src/nestable_type_node.rs
@@ -17,9 +17,18 @@ pub fn expand_derive_nestable_type_node(input: &syn::DeriveInput) -> CodamaResul
     input.as_struct()?;
     let item_name = &input.ident;
 
-    // The item name ident without the last 8 characters (for "TypeNode" or "LinkNode").
-    let variant_name = item_name.to_string();
-    let variant_name = syn::Ident::new(&variant_name[..variant_name.len() - 8], item_name.span());
+    // The item name without its `TypeNode`/`LinkNode` suffix names the `TypeNode` variant.
+    let item_name_string = item_name.to_string();
+    let variant_name = item_name_string
+        .strip_suffix("TypeNode")
+        .or_else(|| item_name_string.strip_suffix("LinkNode"))
+        .ok_or_else(|| {
+            syn::Error::new(
+                item_name.span(),
+                "expected a name ending in `TypeNode` or `LinkNode`",
+            )
+        })?;
+    let variant_name = syn::Ident::new(variant_name, item_name.span());
 
     Ok(quote! {
         impl crate::TypeNodeTrait for #item_name<TypeNode> {
diff --git a/codama-nodes/derive/src/node_union.rs b/codama-nodes/derive/src/node_union.rs
@@ -113,7 +113,7 @@ pub fn expand_derive_node_union(input: &syn::DeriveInput) -> CodamaResult<TokenS
                 let value = serde_json::Value::deserialize(deserializer)?;
                 let kind = value["kind"].as_str().ok_or_else(|| serde::de::Error::custom("missing kind"))?;
                 let to_serde_error = |e: serde_json::Error| -> D::Error {
-                    serde::de::Error::custom(format!("failed to deserialize AmountTypeNode: {}", e))
+                    serde::de::Error::custom(format!("failed to deserialize {}: {}", stringify!(#item_name), e))
                 };
                 match kind {
                     #(#deserialize_patterns)*
diff --git a/codama-nodes/derive/src/type_node.rs b/codama-nodes/derive/src/type_node.rs
@@ -18,9 +18,18 @@ pub fn expand_derive_type_node(input: &syn::DeriveInput) -> CodamaResult<TokenSt
     let item_name = &input.ident;
     let (pre_generics, post_generics) = &input.generics.block_wrappers();
 
-    // The item name ident without the last 8 characters (for "TypeNode" or "LinkNode").
-    let variant_name = item_name.to_string();
-    let variant_name = syn::Ident::new(&variant_name[..variant_name.len() - 8], item_name.span());
+    // The item name without its `TypeNode`/`LinkNode` suffix names the `TypeNode` variant.
+    let item_name_string = item_name.to_string();
+    let variant_name = item_name_string
+        .strip_suffix("TypeNode")
+        .or_else(|| item_name_string.strip_suffix("LinkNode"))
+        .ok_or_else(|| {
+            syn::Error::new(
+                item_name.span(),
+                "expected a name ending in `TypeNode` or `LinkNode`",
+            )
+        })?;
+    let variant_name = syn::Ident::new(variant_name, item_name.span());
 
     Ok(quote! {
         impl #pre_generics crate::TypeNodeTrait for #item_name #post_generics{
diff --git a/codama-plugin-core/src/plugin.rs b/codama-plugin-core/src/plugin.rs
@@ -89,6 +89,7 @@ impl<'a> DirectiveResolver for CompositeDirectiveResolver<'a> {
 pub type ResolvePluginsResult<'a> = Box<dyn Fn(&mut dyn KorokVisitable) -> CodamaResult<()> + 'a>;
 
 /// Combine all plugins into a single function that runs them in sequence.
+#[must_use = "the returned function must be called on a visitable to run the plugins"]
 pub fn resolve_plugins<'a>(plugins: &'a [Box<dyn KorokPlugin + 'a>]) -> ResolvePluginsResult<'a> {
     Box::new(move |visitable: &mut dyn KorokVisitable| {
         // Phase 0: Resolve all resolvable directives.
diff --git a/codama-visitors-core/src/bottom_up_transformer.rs b/codama-visitors-core/src/bottom_up_transformer.rs
@@ -113,12 +113,12 @@ impl BottomUpTransformer {
 
     /// Applies every rule whose selector matches the current path, in order.
     fn run_rules(&mut self, mut node: Node) -> Node {
-        // Clone the path so the rule closures (which borrow `&mut self.rules`)
-        // can also read it without a borrow conflict.
-        let path = self.path.clone();
-        for rule in self.rules.iter_mut() {
-            if rule.selector.matches(&path) {
-                node = (rule.transform)(node, &path);
+        // Disjoint field borrows let the rule closures read the path while
+        // `rules` is borrowed mutably -- no need to clone the path per node.
+        let Self { rules, path, .. } = self;
+        for rule in rules.iter_mut() {
+            if rule.selector.matches(path) {
+                node = (rule.transform)(node, path);
             }
         }
         node
diff --git a/codama-visitors-core/src/byte_size.rs b/codama-visitors-core/src/byte_size.rs
@@ -261,19 +261,18 @@ impl ByteSize<'_> {
     fn link_size(&mut self, link: &DefinedTypeLinkNode, program: Option<&str>) -> Option<usize> {
         // Copy the dictionary reference so the lookup borrows it, not `self`.
         let linkables = self.linkables;
-        let (name, body, link_program) = {
-            let defined_type = linkables.get_defined_type(link, program)?;
-            (
-                defined_type.name.to_string(),
-                (*defined_type.r#type).clone(),
-                link.program.as_ref().map(|p| p.name.to_string()),
-            )
-        };
-        if self.in_progress.iter().any(|n| n == &name) {
+        let defined_type = linkables.get_defined_type(link, program)?;
+        let name = defined_type.name.as_ref();
+        if self.in_progress.iter().any(|n| n == name) {
             return None; // cyclic defined type
         }
-        let new_program = link_program.or_else(|| program.map(String::from));
-        self.in_progress.push(name);
+        let body = (*defined_type.r#type).clone();
+        let new_program = link
+            .program
+            .as_ref()
+            .map(|p| p.name.to_string())
+            .or_else(|| program.map(String::from));
+        self.in_progress.push(name.to_string());
         let size = self.type_size(&body, new_program.as_deref());
         self.in_progress.pop();
         size
diff --git a/codama-visitors/src/flatten_instruction_data_arguments.rs b/codama-visitors/src/flatten_instruction_data_arguments.rs
@@ -33,26 +33,26 @@ pub fn flatten_instruction_data_arguments() -> BottomUpTransformer {
 pub fn flatten_instruction_arguments(
     arguments: Vec<InstructionArgumentNode>,
 ) -> Vec<InstructionArgumentNode> {
-    let original = arguments.clone();
-    let mut inlined: Vec<InstructionArgumentNode> = Vec::with_capacity(arguments.len());
+    // First, check (by borrowing) whether inlining would collide -- so we can
+    // bail out and return the arguments untouched without cloning them.
+    let would_collide = {
+        let mut seen = HashSet::new();
+        !arguments.iter().all(|argument| match &*argument.r#type {
+            TypeNode::Struct(inner) => inner.fields.iter().all(|field| seen.insert(&field.name)),
+            _ => seen.insert(&argument.name),
+        })
+    };
+    if would_collide {
+        return arguments;
+    }
+
+    let mut inlined = Vec::with_capacity(arguments.len());
     for argument in arguments {
-        if matches!(&*argument.r#type, TypeNode::Struct(_)) {
-            if let TypeNode::Struct(inner) = *argument.r#type {
-                inlined.extend(inner.fields.into_iter().map(InstructionArgumentNode::from));
-            }
+        if let TypeNode::Struct(inner) = *argument.r#type {
+            inlined.extend(inner.fields.into_iter().map(InstructionArgumentNode::from));
         } else {
             inlined.push(argument);
         }
     }
-
-    if has_duplicate_names(&inlined) {
-        original
-    } else {
-        inlined
-    }
-}
-
-fn has_duplicate_names(arguments: &[InstructionArgumentNode]) -> bool {
-    let mut seen = HashSet::with_capacity(arguments.len());
-    !arguments.iter().all(|a| seen.insert(&a.name))
+    inlined
 }
diff --git a/codama/src/codama.rs b/codama/src/codama.rs
@@ -63,7 +63,7 @@ impl Codama {
 
     pub fn get_node(&self) -> CodamaResult<Node> {
         let korok = self.get_visited_korok()?;
-        korok.node.clone().ok_or(CodamaError::NodeNotFound)
+        korok.node.ok_or(CodamaError::NodeNotFound)
     }
 
     pub fn get_idl(&self) -> CodamaResult<RootNode> {

Original file line number	Diff line number	Diff line change
`@@ -46,14 +46,10 @@ pub enum CodamaError {`
`46`	`46`	`pub type CodamaResult<T> = Result<T, CodamaError>;`
`47`	`47`
`48`	`48`	`impl CodamaError {`
`49`		`- pub fn to_compile_error(&self) -> TokenStream {`
	`49`	`+ pub fn into_compile_error(self) -> TokenStream {`
`50`	`50`	`match self {`
`51`	`51`	`CodamaError::Compilation(error) => error.to_compile_error(),`
`52`	`52`	`_ => TokenStream::new(),`
`53`	`53`	`}`
`54`	`54`	`}`
`55`		`-`
`56`		`- pub fn into_compile_error(self) -> TokenStream {`
`57`		`- self.to_compile_error()`
`58`		`- }`
`59`	`55`	`}`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ impl Default for SetInstructionsVisitor {`
`25`	`25`	`Self {`
`26`	`26`	`combine_types: CombineTypesVisitor {`
`27`	`27`	// Skip fields with the `account` codama directive.
`28`		`- get_nammed_field: \|korok, parent\| {`
	`28`	`+ get_named_field: \|korok, parent\| {`
`29`	`29`	`if korok.attributes.has_codama_attribute("account") {`
`30`	`30`	`return None;`
`31`	`31`	`}`