Make use of a setter/getter with skip invalid

jbaublitz · Jul 25, 2021 · 4ded087 · 4ded087
1 parent 1181b1a
commit 4ded087
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 7 deletions.
diff --git a/src/lib.rs b/src/lib.rs
@@ -235,7 +235,8 @@ fn parse_attr(attr: &syn::Attribute, mode: GenMode) -> Option<Meta> {
     use syn::{punctuated::Punctuated, Token};
 
     if attr.path.is_ident("getset") {
-        attr.parse_args_with(Punctuated::<Meta, Token![,]>::parse_terminated)
+        let (last, skip, mut collected) = attr
+            .parse_args_with(Punctuated::<Meta, Token![,]>::parse_terminated)
             .unwrap_or_abort()
             .into_iter()
             .inspect(|meta| {
@@ -248,8 +249,38 @@ fn parse_attr(attr: &syn::Attribute, mode: GenMode) -> Option<Meta> {
                     abort!(meta.path().span(), "unknown setter or getter")
                 }
             })
-            .filter(|meta| meta.path().is_ident(mode.name()) || meta.path().is_ident("skip"))
-            .last()
+            .fold(
+                (None, None, Vec::new()),
+                |(last, skip, mut collected), meta| {
+                    if meta.path().is_ident(mode.name()) {
+                        (Some(meta), skip, collected)
+                    } else if meta.path().is_ident("skip") {
+                        (last, Some(meta), collected)
+                    } else {
+                        // Store inapplicable item for potential error message
+                        // if used with skip.
+                        collected.push(meta);
+                        (last, skip, collected)
+                    }
+                },
+            );
+
+        if skip.is_some() {
+            // Check if there is any setter or getter used with skip, which is
+            // forbidden.
+            if last.is_none() && collected.is_empty() {
+                skip
+            } else {
+                abort!(
+                    last.or(collected.pop()).unwrap().path().span(),
+                    "use of setters and getters with skip is invalid"
+                );
+            }
+        } else {
+            // If skip is not used, return the last occurrence of matching
+            // setter/getter, if there is any.
+            last
+        }
     } else {
         attr.parse_meta()
             .ok()

diff --git a/tests/skip.rs b/tests/skip.rs
@@ -1,20 +1,36 @@
 #[macro_use]
 extern crate getset;
 
-#[derive(CopyGetters)]
-#[get_copy]
+#[derive(CopyGetters, Setters)]
+#[getset(get_copy, set)]
 pub struct Plain {
-    /// A doc comment.
+    // If the field was not skipped, the compiler would complain about moving a
+    // non-copyable type.
     #[getset(skip)]
     non_copyable: String,
 
     copyable: usize,
+
+    // Invalid use of skip -- compilation error.
+    // #[getset(skip, get_copy)]
+    // non_copyable2: String,
+
+    // Invalid use of skip -- compilation error.
+    // #[getset(get_copy, skip)]
+    // non_copyable2: String,
 }
 
 impl Plain {
     fn custom_non_copyable(&self) -> &str {
         &self.non_copyable
     }
+
+    // If the field was not skipped, the compiler would complain about duplicate
+    // definitions of `set_non_copyable`.
+    fn set_non_copyable(&mut self, val: String) -> &mut Self {
+        self.non_copyable = val;
+        self
+    }
 }
 
 impl Default for Plain {
@@ -28,7 +44,8 @@ impl Default for Plain {
 
 #[test]
 fn test_plain() {
-    let val = Plain::default();
+    let mut val = Plain::default();
     val.copyable();
     val.custom_non_copyable();
+    val.set_non_copyable("bar".to_string());
 }