Rewrite derive macro implementation #262

pintariching · 2023-07-26T17:24:30Z

A proposal on rewriting the derive macro implementation using the darling crate. It massively simplifies the macro. The only two validators I got up an working are the length and email just for show. Also there is no error handling for now.

Also I had to upgrade syn version to 2 as darling would work with version 1.

Keats

Veeery nice

Keats · 2023-07-26T21:28:17Z

validator_derive/src/lib.rs

+    min: Option<u64>,
+    max: Option<u64>,
+    equal: Option<u64>,
+    message: Option<String>,


It's missing code as well. It is much nicer and waaaay less code, very nice.

Random thought: is it possible to parse all the attributes into a hashmap rather than a struct? I'm wondering if there would be a way for a user to register functions with validator and use them on the derive as if they were built-in. Maybe it's not worth it though.

There is a map attribute for the macro that let's you specify a function to run after parsing the attributes like

#[derive(FromMeta)] #[darling(map = "to_hashmap")] struct Custom { par_1: Option<Expr>, par_2: Option<Expr>, par_3: Option<Expr>, #[darling(skip)] hashmap: HashMap<String, Expr> } fn to_hashmap(custom: Custom) -> HashMap<String, Expr> {}

I have to try it out, I'm not sure if this will work

It's only worth it if we have some kind of register of fns, and I'm not sure how to configure where to look that up... Maybe let's not worry about it for now?

Very well. I'll implement the rest of the macro and get it up and running and possibly passing the tests and then worry about everything else.

Keats · 2023-07-28T21:18:25Z

validator/src/validation/length.rs

@@ -12,7 +12,7 @@ use indexmap::{IndexMap, IndexSet};
 /// If you apply it on String, don't forget that the length can be different
 /// from the number of visual characters for Unicode
 #[must_use]
-pub fn validate_length<T: ValidateLength>(
+pub fn validate_length<T: ValidateLength<u64>>(


The function is kind of useless now that there's the trait I think. Also casting the min, max and equal values to u64 might lead to issues when you for example cast a negative i32 to u64. So I thought to add a generic, so that the length and all the parameters have to be the same type to compare them without runtime errors or weird casting issues.

We can remove it then I think?

None of the functions will be used in the macro from the validator crate so they can be removed unless they're used anywhere else?

Yep they can be removed now that we have traits

pintariching · 2023-08-03T19:14:00Z

I've rewritten the custom validation a bit. If you want arguments for your function you can specify them like:

#[derive(Validate)]
struct TestStruct {
    #[validate(custom(
        function = "valid_reference_with_lifetime",
        arg = "len: i64",
        arg = "something: bool",
        arg = "name: String"
    ))]
    value: String,
}

let test = TestStruct { name: "asdfg".to_string() };
let res = test.validate(TestStructArgs { len: 123, something: false, name: "Something".into() });

Also the TestStructArgs could be given a builder pattern if we wanna be fancy, so you get

TestStructArgs::build().len(123).something(false).name("Something").build()

How does this look? I'm now wrangling with lifetimes so for now you can only pass values and not references. Should I continue with this or go back to tuples like it's now?

pintariching · 2023-09-07T20:21:28Z

The UI tests now pass. I had to put them behind a feature to avoid running them on anything other than stable due to the errors that keep changing and reordering. Also for some reason the error produced by compile-fail::range::wrong_type test was different in the CI and locally, even though both use stable, so I removed it.

Keats

I admit i went a little fast on the review but i left some comments

Keats · 2023-09-08T20:49:06Z

validator/src/types.rs

@@ -58,6 +58,22 @@ impl ValidationErrors {
        }
    }

+    pub fn merge_self(


what is that? I am thinking we probably also want to maybe re-think the errors interface in another PR to be easier to use/more intuitive

I had some trouble adding errors on nested validations so I whipped up this function. And I agree, the errors API should be redone.

validator/src/validation/regex.rs

validator/src/validation/urls.rs

validator_derive/src/tokens/ip.rs

Keats · 2023-09-09T20:44:02Z

validator_derive_tests/tests/compile-fail/length/unknown_arg.stderr

@@ -1,5 +1,5 @@
-error: Invalid attribute #[validate] on field `s`: unknown argument `eq` for validator `length` (it only has `min`, `max`, `equal`)
- --> $DIR/unknown_arg.rs:5:23
+error: Unknown field: `eq`. Did you mean `equal`?


it's an unknown attribute/argument, not an unknown field

This is an error from darling. I think it defaults to field for everything?

Keats · 2023-09-09T20:47:14Z

validator_derive_tests/tests/complex.rs

-        panic!("Expected list validation errors");
-    }
-}
+// Skip this test for now


can it be enabled again?

validator_derive_tests/tests/complex.rs

Keats · 2023-09-09T20:48:02Z

validator_derive_tests/tests/compile-fail/schema/missing_function.stderr

@@ -1,5 +1,5 @@
-error: Invalid schema level validation: `function` is required
- --> $DIR/missing_function.rs:4:12
+error: Missing field `function`


again attribute/argument

Keats · 2023-09-09T21:15:01Z

validator_derive_tests/tests/schema.rs

@@ -114,81 +117,103 @@ fn can_fail_schema_fn_validation() {
    assert_eq!(errs["__all__"][0].code, "meh");
 }

+// #[test]
+// fn can_fail_multiple_schema_fn_validation() {


can that test be enabled?

Keats · 2023-09-25T06:53:42Z

Just a couple more tests that are currently commented out, can I merge and uncomment them or is there a reason they are commented out?

pintariching · 2023-09-27T08:55:57Z

Sorry for the wait, forgot about this. No the tests don't pass currently if you uncomment them I think. I was thinking gating the serde original name behind a feature.

Keats · 2023-09-27T16:36:20Z

ah it's all related to that feature? You can just remove it for now then

pintariching · 2023-09-28T16:10:36Z

Ok I removed the original field name from serde test and added back multiple schema validation. I think that makes everything? Also the documentation should be updated, this PR is a breaking change after all.

Keats · 2023-09-29T06:54:09Z

Let's update the docs later, we might have more things.

Thanks a lot!

* Rewrite derive macro implementation * Add better error handling * Add new types to validator macro * Add empty files in tokens module * Removed i32 tests for length * Fix email to pass tests * fix length validation trait to work with u64 only * Add credit card token generation to macro * Add url token generation to macro * Add ValidateIp trait * Add ip token generation to macro * Remove unneeded import * Export ValidateIp trait from main crate * Add tests for ValidateIp macro * Add non control character token generation to macro * Add test for range with enums * Add range token generation to macro * Add required token generation to macro * Fix ValidationErrors merge function * Move contains trait to contains.rs * Add ValidateContains trait and fix tests * Add ValidateContains and DoesNotContain traits to macro * Add must match token generation to macro * Add regex token generation * Add custom validation token generation to macro * Add custom validation with arguments to macro * Add custom validation with args with contexts * Add custom validation recommit * Fix custom validation to work without lifetimes * Change custom validation to use closures * Fix tests for custom validation * Change custom validation to implement FnOnce and add tests * Remove code used for experementing * Remove unneeded code * Remove unused imports * Add schema with arguments and fix tests * Impl ValidateLength trait for Option types * Fix impls for ValidateRange * Fix duplicate use statements * Fix tests and add Option impls for Contains * Implement ValidateUrl traits for specific types * Implement ValidateEmail traits for specific types * Fix some tests and warnings * Add regex trait for validation * Fix to pass tests in validator lib * Fix range validation trait to pass tests * Update and remove unneeded dependencies * Add ValidateNested trait * Fix custom and nested validation * Fix Regex validator to work with OnceCell and OnceLock * Fix derive macro to support all the arguments * Remove unneeded functions and fix tests * Remove validator_types crate and fix tests * Update CI workflow * Fix custom to be used in nested validations * Fix custom validation to use context * Add custom nested validation with args test * Fix validation to use Validation trait * Remove conflicting trait * Fixed tests and remove ValidateContext trait * Fix nested validation * Fix custom args test * Add `nest_all_fields` attribute * Add better error handling and start fixing UI tests * Pass almost all tests * Add skip_on_field_errors attribute to schema * Remove rust beta channel from workflow * Use proc_macro_error instead of darling diagnostics feature * Conditionally run UI tests * Fix ci.yml * Fix ci.yml * Remove UI test for wrong type on range * Change trait impls to be consistent * Run cargo clippy --fix * Replace if else with match * Remove cargo-expand leftovers * Replace underscore with `#[allow(unused)]` * Add support for multiple schema validations * Remove serde original field name test

pintariching added 2 commits July 26, 2023 19:16

Rewrite derive macro implementation

3a631c0

Add better error handling

0f87439

Keats reviewed Jul 26, 2023

View reviewed changes

pintariching added 5 commits July 27, 2023 22:19

Add new types to validator macro

0131e5b

Add empty files in tokens module

01c7195

Removed i32 tests for length

12b0444

Fix email to pass tests

a1d5b0c

fix length validation trait to work with u64 only

3639c3b

Keats reviewed Jul 28, 2023

View reviewed changes

pintariching added 19 commits July 30, 2023 20:21

Add credit card token generation to macro

2e187a0

Add url token generation to macro

c843c8d

Add ValidateIp trait

5ab6116

Add ip token generation to macro

a2bc199

Remove unneeded import

e98f172

Export ValidateIp trait from main crate

79fbc83

Add tests for ValidateIp macro

19ae0f1

Add non control character token generation to macro

b05a0d9

Add test for range with enums

dbbf836

Add range token generation to macro

96813a0

Add required token generation to macro

9513156

Fix ValidationErrors merge function

3fa7a89

Move contains trait to contains.rs

90f1d38

Add ValidateContains trait and fix tests

29768f4

Add ValidateContains and DoesNotContain traits to macro

df273f5

Add must match token generation to macro

1f893c9

Add regex token generation

70f8c4e

Add custom validation token generation to macro

9d18883

Add custom validation with arguments to macro

ac820e6

Keats mentioned this pull request Aug 31, 2023

Start implementing range support for NonZero types #270

Open

Add better error handling and start fixing UI tests

8ee8b26

pintariching mentioned this pull request Sep 6, 2023

Use chars().count() instead of len() for Cow<'a, str> #271

Closed

pintariching added 8 commits September 6, 2023 21:46

Pass almost all tests

6e12db7

Add skip_on_field_errors attribute to schema

007ad15

Remove rust beta channel from workflow

3d0a7e2

Use proc_macro_error instead of darling diagnostics feature

6a60de5

Conditionally run UI tests

edf24be

Fix ci.yml

81eff2d

Fix ci.yml

873dfb3

Remove UI test for wrong type on range

04b449b

Keats reviewed Sep 9, 2023

View reviewed changes

pintariching added 5 commits September 10, 2023 14:28

Change trait impls to be consistent

c06e93f

Run cargo clippy --fix

2cbad70

Replace if else with match

5dda688

Remove cargo-expand leftovers

88e8fd2

Replace underscore with #[allow(unused)]

00f2751

Keats mentioned this pull request Sep 24, 2023

Span-based email validation #272

Open

pintariching added 2 commits September 28, 2023 18:05

Add support for multiple schema validations

6ebc49e

Remove serde original field name test

8fb8d9e

Keats merged commit 31593cb into Keats:next Sep 29, 2023
4 checks passed

nopeNoshishi mentioned this pull request Apr 18, 2024

Update README #323

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite derive macro implementation #262

Rewrite derive macro implementation #262

pintariching commented Jul 26, 2023

Keats left a comment

Keats Jul 26, 2023

pintariching Jul 27, 2023

Keats Jul 27, 2023

pintariching Jul 27, 2023

Keats Jul 28, 2023

pintariching Jul 30, 2023

Keats Jul 30, 2023

pintariching Jul 30, 2023

Keats Jul 30, 2023

pintariching commented Aug 3, 2023

pintariching commented Sep 7, 2023

Keats left a comment

Keats Sep 8, 2023

pintariching Sep 28, 2023

Keats Sep 9, 2023

pintariching Sep 10, 2023

Keats Sep 9, 2023

Keats Sep 9, 2023

Keats Sep 9, 2023

Keats commented Sep 25, 2023

pintariching commented Sep 27, 2023

Keats commented Sep 27, 2023

pintariching commented Sep 28, 2023

Keats commented Sep 29, 2023

Rewrite derive macro implementation #262

Rewrite derive macro implementation #262

Conversation

pintariching commented Jul 26, 2023

Keats left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pintariching commented Aug 3, 2023

pintariching commented Sep 7, 2023

Keats left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Keats commented Sep 25, 2023

pintariching commented Sep 27, 2023

Keats commented Sep 27, 2023

pintariching commented Sep 28, 2023

Keats commented Sep 29, 2023