From 39d1e11b9d894801a503972b044085840359c051 Mon Sep 17 00:00:00 2001
From: borngraced <samuelonoja970@gmail.com>
Date: Tue, 19 Dec 2023 00:25:17 +0100
Subject: [PATCH] improve skip_serializing_none_add_attr_to_field

Signed-off-by: borngraced <samuelonoja970@gmail.com>
---
 .../derives/skip_serializing_none/src/lib.rs  | 77 ++++++++-----------
 1 file changed, 34 insertions(+), 43 deletions(-)

diff --git a/mm2src/derives/skip_serializing_none/src/lib.rs b/mm2src/derives/skip_serializing_none/src/lib.rs
index 9f7b5781b2..dfb3fec55f 100644
--- a/mm2src/derives/skip_serializing_none/src/lib.rs
+++ b/mm2src/derives/skip_serializing_none/src/lib.rs
@@ -72,55 +72,46 @@ where
     }
 }
 
-fn skip_serializing_none_add_attr_to_field(field: &mut Field) -> Result<(), String> {
-    if is_std_option(&field.ty) {
-        let has_skip_serializing_if = field_has_attribute(field, "serde", "skip_serializing_if");
-
-        let mut has_always_attr = false;
-        for attr in field.clone().attrs {
-            let has_attr = attr
-                .parse_meta()
-                .map_err(|e| e.to_string())?
-                .path()
-                .is_ident("serialize_always");
-
-            has_always_attr |= has_attr;
+fn is_serialize_always(attr: &syn::Attribute) -> Result<bool, String> {
+    attr.parse_meta()
+        .map_err(|e| e.to_string())
+        .map(|meta| meta.path().is_ident("serialize_always"))
+}
 
-            if !has_attr {
-                field.attrs.retain(|ele| *ele == attr);
+fn skip_serializing_none_add_attr_to_field(field: &mut Field) -> Result<(), String> {
+    if !is_std_option(&field.ty) {
+        for attr in field.attrs.iter() {
+            if is_serialize_always(attr)? {
+                return Err("`serialize_always` may only be used on fields of type `Option`.".into());
             }
         }
-
-        if has_always_attr && has_skip_serializing_if {
-            let mut msg = r#"The attributes `serialize_always` and `serde(skip_serializing_if = "...")` cannot be used on the same field"#.to_string();
-            if let Some(ident) = &field.ident {
-                msg += ": `";
-                msg += &ident.to_string();
-                msg += "`";
-            }
-            msg += ".";
-            return Err(msg);
+        return Ok(());
+    }
+    let has_skip_serializing_if = field_has_attribute(field, "serde", "skip_serializing_if");
+    let mut has_always_attr = false;
+    let mut attrs_to_retain = Vec::new();
+    for attr in &field.attrs {
+        let has_attr = is_serialize_always(attr)?;
+        has_always_attr |= has_attr;
+        if !has_attr {
+            attrs_to_retain.push(attr.clone());
         }
-
-        if has_skip_serializing_if || has_always_attr {
-            return Ok(());
+    }
+    field.attrs = attrs_to_retain;
+    if has_always_attr && has_skip_serializing_if {
+        let mut msg = r#"The attributes `serialize_always` and `serde(skip_serializing_if = "...")` cannot be used on the same field"#.to_string();
+        if let Some(ident) = &field.ident {
+            msg += ": `";
+            msg += &ident.to_string();
+            msg += "`";
         }
-
-        let attr = parse_quote!(
+        msg += ".";
+        return Err(msg);
+    }
+    if !has_skip_serializing_if && !has_always_attr {
+        field.attrs.push(parse_quote!(
             #[serde(skip_serializing_if = "Option::is_none")]
-        );
-        field.attrs.push(attr);
-    } else {
-        for attr in field.attrs.iter() {
-            if attr
-                .parse_meta()
-                .map_err(|e| e.to_string())?
-                .path()
-                .is_ident("serialize_always")
-            {
-                return Err("`serialize_always` may only be used on fields of type `Option`.".into());
-            }
-        }
+        ));
     }
     Ok(())
 }