feat: Added derive macro `SelectAsFormField` #397

kumarUjjawal · 2025-09-07T18:20:04Z

As per the requirements of the #356 I implemented the SelectAsFormField derive macro. There were two directions for this either creating a derive macro or create a wrapper struct. Since I love derive macro and comfortable implementing it, I went with that.

After this change the impl_as_form_field_mult macro is now unused, I would like to know what is the preference for that?

codecov · 2025-09-07T19:02:56Z

Codecov Report

❌ Patch coverage is 99.59016% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
cot/src/form/fields/select.rs	99.56%	0 Missing and 1 partial ⚠️

Flag	Coverage Δ
rust	`90.33% <99.59%> (+0.13%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
cot-macros/src/lib.rs	`97.40% <100.00%> (+0.18%)`	⬆️
cot-macros/src/select_as_form_field.rs	`100.00% <100.00%> (ø)`
cot/src/form/fields.rs	`87.97% <ø> (ø)`
cot/src/form/fields/chrono.rs	`93.10% <ø> (-0.05%)`	⬇️
cot/src/form/fields/select.rs	`98.10% <99.56%> (+1.80%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

kumarUjjawal · 2025-09-19T11:19:27Z

@m4tx can you take a look whenever you get the time.

m4tx

Thanks for your patience waiting for my review! As usual, the change looks good, but I have some minor comments about stuff that needs to be fixed before merging this.

Also, please make sure to add additional tests if needed so that the patch has at least 85% code coverage.

About the impl_as_form_field_mult macro—since it's private and is now unused, I don't see a value in keeping it, and so it should be removed.

cot/src/form/fields/select.rs

cot/src/private.rs

m4tx · 2025-10-03T05:59:38Z

cot/src/form/fields/select.rs

+impl<T: SelectChoice + Send> crate::form::AsFormField for ::std::vec::Vec<T> {
+    type Type = SelectMultipleField<T>;
+
+    fn clean_value(field: &Self::Type) -> Result<Self, FormFieldValidationError> {
+        let values = check_required_multiple(field)?;
+        values.iter().map(|id| T::from_str(id)).collect()
+    }
+
+    fn to_field_value(&self) -> String {
+        String::new()
+    }
+}
+
+impl<T: SelectChoice + Send> crate::form::AsFormField for ::std::collections::VecDeque<T> {
+    type Type = SelectMultipleField<T>;
+
+    fn clean_value(field: &Self::Type) -> Result<Self, FormFieldValidationError> {
+        let values = check_required_multiple(field)?;
+        let mut out = ::std::collections::VecDeque::new();
+        for id in values {
+            out.push_back(T::from_str(id)?);
+        }
+        Ok(out)
+    }
+
+    fn to_field_value(&self) -> String {
+        String::new()
+    }
+}
+
+impl<T: SelectChoice + Send> crate::form::AsFormField for ::std::collections::LinkedList<T> {
+    type Type = SelectMultipleField<T>;
+
+    fn clean_value(field: &Self::Type) -> Result<Self, FormFieldValidationError> {
+        let values = check_required_multiple(field)?;
+        let mut out = ::std::collections::LinkedList::new();
+        for id in values {
+            out.push_back(T::from_str(id)?);
+        }
+        Ok(out)
+    }
+
+    fn to_field_value(&self) -> String {
+        String::new()
+    }
+}
+
+impl<T: SelectChoice + Eq + ::std::hash::Hash + Send, S: ::std::hash::BuildHasher + Default>
+    crate::form::AsFormField for ::std::collections::HashSet<T, S>
+{
+    type Type = SelectMultipleField<T>;
+
+    fn clean_value(field: &Self::Type) -> Result<Self, FormFieldValidationError> {
+        let values = check_required_multiple(field)?;
+        let mut out = ::std::collections::HashSet::default();
+        for id in values {
+            out.insert(T::from_str(id)?);
+        }
+        Ok(out)
+    }
+
+    fn to_field_value(&self) -> String {
+        String::new()
+    }
+}
+
+impl<T: SelectChoice + Eq + ::std::hash::Hash + Send> crate::form::AsFormField
+    for ::indexmap::IndexSet<T>
+{
+    type Type = SelectMultipleField<T>;
+
+    fn clean_value(field: &Self::Type) -> Result<Self, FormFieldValidationError> {
+        let values = check_required_multiple(field)?;
+        let mut out = ::indexmap::IndexSet::new();
+        for id in values {
+            out.insert(T::from_str(id)?);
+        }
+        Ok(out)
+    }
+
+    fn to_field_value(&self) -> String {
+        String::new()
+    }
+}
+


Why can't we use the impl_as_form_field_mult_collection macro for these implementations? They are all essentially the same, and as such, prone to mistakes when you want to modify something in one impl and forget to change all the others. We just need to another variant of the macro so that it supports generic bounds.

As an added bonus, Iterator::collect() might be slightly faster than <Collection>::push_back because of preallocation, although admittedly, this likely doesn't matter for the typical element counts in case of HTML form values.

I have made the changes. Please take a look if this is what you have in mind.

Co-authored-by: Mateusz Maćkowski <mateusz@mackowski.org>

m4tx

Hey! This looks good - please fix the only remaining issue and we'll merge this.

m4tx · 2025-10-28T18:07:08Z

cot/src/form/fields/select.rs

+    #[cot::test]
+    async fn vec_as_form_field_clean_value() {
+        let mut field = SelectMultipleField::<TestChoice>::with_options(
+            FormFieldOptions {
+                id: "choices".to_owned(),
+                name: "choices".to_owned(),
+                required: true,
+            },
+            SelectMultipleFieldOptions::default(),
+        );
+
+        field
+            .set_value(FormFieldValue::new_text("opt1"))
+            .await
+            .unwrap();
+        field
+            .set_value(FormFieldValue::new_text("opt3"))
+            .await
+            .unwrap();
+
+        let values = Vec::<TestChoice>::clean_value(&field).unwrap();
+        assert_eq!(values, vec![TestChoice::Option1, TestChoice::Option3]);
+    }
+
+    #[cot::test]
+    async fn vec_as_form_field_required_empty() {
+        let field = SelectMultipleField::<TestChoice>::with_options(
+            FormFieldOptions {
+                id: "choices".to_owned(),
+                name: "choices".to_owned(),
+                required: true,
+            },
+            SelectMultipleFieldOptions::default(),
+        );
+
+        let result = Vec::<TestChoice>::clean_value(&field);
+        assert_eq!(result, Err(FormFieldValidationError::Required));
+    }
+
+    #[cot::test]
+    async fn vec_as_form_field_invalid_value() {
+        let mut field = SelectMultipleField::<TestChoice>::with_options(
+            FormFieldOptions {
+                id: "choices".to_owned(),
+                name: "choices".to_owned(),
+                required: false,
+            },
+            SelectMultipleFieldOptions::default(),
+        );
+
+        field
+            .set_value(FormFieldValue::new_text("opt1"))
+            .await
+            .unwrap();
+        field
+            .set_value(FormFieldValue::new_text("bad"))
+            .await
+            .unwrap();
+
+        let result = Vec::<TestChoice>::clean_value(&field);
+        assert!(matches!(
+            result,
+            Err(FormFieldValidationError::InvalidValue(value)) if value == "bad"
+        ));
+    }
+
+    #[test]
+    fn vec_as_form_field_to_field_value() {
+        let items = vec![TestChoice::Option1, TestChoice::Option2];
+        assert_eq!(items.to_field_value(), "");
+    }
+
+    #[cot::test]
+    async fn vec_deque_as_form_field_clean_value() {
+        let mut field = SelectMultipleField::<TestChoice>::with_options(
+            FormFieldOptions {
+                id: "choices".to_owned(),
+                name: "choices".to_owned(),
+                required: false,
+            },
+            SelectMultipleFieldOptions::default(),
+        );
+
+        field
+            .set_value(FormFieldValue::new_text("opt2"))
+            .await
+            .unwrap();
+        field
+            .set_value(FormFieldValue::new_text("opt1"))
+            .await
+            .unwrap();
+
+        let mut values = VecDeque::<TestChoice>::clean_value(&field).unwrap();
+        assert_eq!(values.pop_front(), Some(TestChoice::Option2));
+        assert_eq!(values.pop_back(), Some(TestChoice::Option1));
+    }
+
+    #[cot::test]
+    async fn linked_list_as_form_field_clean_value() {
+        let mut field = SelectMultipleField::<TestChoice>::with_options(
+            FormFieldOptions {
+                id: "choices".to_owned(),
+                name: "choices".to_owned(),
+                required: false,
+            },
+            SelectMultipleFieldOptions::default(),
+        );
+
+        field
+            .set_value(FormFieldValue::new_text("opt3"))
+            .await
+            .unwrap();
+
+        let mut values = LinkedList::<TestChoice>::clean_value(&field).unwrap();
+        assert_eq!(values.pop_front(), Some(TestChoice::Option3));
+        assert!(values.is_empty());
+    }
+
+    #[cot::test]
+    async fn hash_set_as_form_field_clean_value() {
+        let mut field = SelectMultipleField::<TestChoice>::with_options(
+            FormFieldOptions {
+                id: "choices".to_owned(),
+                name: "choices".to_owned(),
+                required: false,
+            },
+            SelectMultipleFieldOptions::default(),
+        );
+
+        field
+            .set_value(FormFieldValue::new_text("opt1"))
+            .await
+            .unwrap();
+        field
+            .set_value(FormFieldValue::new_text("opt1"))
+            .await
+            .unwrap();
+        field
+            .set_value(FormFieldValue::new_text("opt2"))
+            .await
+            .unwrap();
+
+        let values = HashSet::<TestChoice>::clean_value(&field).unwrap();
+        assert_eq!(values.len(), 2);
+        assert!(values.contains(&TestChoice::Option1));
+        assert!(values.contains(&TestChoice::Option2));
+    }
+
+    #[cot::test]
+    async fn index_set_as_form_field_preserves_order() {
+        let mut field = SelectMultipleField::<TestChoice>::with_options(
+            FormFieldOptions {
+                id: "choices".to_owned(),
+                name: "choices".to_owned(),
+                required: false,
+            },
+            SelectMultipleFieldOptions::default(),
+        );
+
+        field
+            .set_value(FormFieldValue::new_text("opt2"))
+            .await
+            .unwrap();
+        field
+            .set_value(FormFieldValue::new_text("opt3"))
+            .await
+            .unwrap();
+        field
+            .set_value(FormFieldValue::new_text("opt2"))
+            .await
+            .unwrap();
+
+        let values = IndexSet::<TestChoice>::clean_value(&field).unwrap();
+        let mut iter = values.iter();
+        assert_eq!(iter.next(), Some(&TestChoice::Option2));
+        assert_eq!(iter.next(), Some(&TestChoice::Option3));
+        assert_eq!(iter.next(), None);
+    }


These tests do not actually test the SelectAsFormField macro, do they? TestChoice has a custom SelectChoice implementation. I'd suggest adding another enum that actually uses SelectAsFormField and use it in the tests.

Can you check if the new test are accurate?

feat: Added derive macro SelectAsFormField

8987ddd

github-actions bot added C-lib Crate: cot (main library crate) C-macros Crate: cot-macros labels Sep 7, 2025

pre-commit-ci bot and others added 2 commits September 7, 2025 18:21

chore(pre-commit.ci): auto fixes from pre-commit hooks

d9257c9

fixed clippy warning

96fad4b

Merge branch 'master' into feat/forms-enum

b09a070

m4tx requested changes Oct 3, 2025

View reviewed changes

m4tx and others added 5 commits October 3, 2025 08:23

Merge branch 'master' into feat/forms-enum

a178d1f

Update cot/src/form/fields/select.rs

4371916

Co-authored-by: Mateusz Maćkowski <mateusz@mackowski.org>

removed unused macrso and import

2e385bf

made the impl_as_form_field_mult_collection generic and added more tests

db8d567

Merge branch 'master' into feat/forms-enum

555d128

xerion0712 mentioned this pull request Oct 12, 2025

Allow using enums in form easily #356

Open

m4tx requested changes Oct 28, 2025

View reviewed changes

fixed test cases

ac8197f

seqre assigned kumarUjjawal Oct 29, 2025

Merge branch 'master' into feat/forms-enum

9d90d4d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat: Added derive macro `SelectAsFormField` #397

feat: Added derive macro `SelectAsFormField` #397

Uh oh!

kumarUjjawal commented Sep 7, 2025

Uh oh!

codecov bot commented Sep 7, 2025 •

edited

Loading

Uh oh!

kumarUjjawal commented Sep 19, 2025

Uh oh!

m4tx left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

m4tx Oct 3, 2025

Uh oh!

kumarUjjawal Oct 9, 2025

Uh oh!

m4tx left a comment

Uh oh!

m4tx Oct 28, 2025

Uh oh!

kumarUjjawal Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

feat: Added derive macro SelectAsFormField #397

Are you sure you want to change the base?

feat: Added derive macro SelectAsFormField #397

Uh oh!

Conversation

kumarUjjawal commented Sep 7, 2025

Uh oh!

codecov bot commented Sep 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kumarUjjawal commented Sep 19, 2025

Uh oh!

m4tx left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

m4tx Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

kumarUjjawal Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

m4tx left a comment

Choose a reason for hiding this comment

Uh oh!

m4tx Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

kumarUjjawal Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat: Added derive macro `SelectAsFormField` #397

feat: Added derive macro `SelectAsFormField` #397

codecov bot commented Sep 7, 2025 •

edited

Loading

m4tx left a comment •

edited

Loading