Fix error field names in 'FromForm' derive.

Prior to this commit, the `FromForm` derive could pair the incorrect
field name with a failing validation. The bug was caused by using two
mismatched iterators in a `quote!()` invocation. Specifically, the first
iterator emitted validation calls for all fields that had validation
applied, while the second emitted field names for all fields,
irrespective of whether the field had any validation applied. The two
iterators were effectively zipped to create the final error, creating
the bug.

This commit fixes the issue by correctly matching field names with their
validators at the expense of an additional allocation, necessitated by
the `quote` crate's inability to access subfields in a repetition.

Fixes #2394.
This commit is contained in:
Sergio Benitez 2022-11-11 18:13:42 -08:00
parent 440a88ad27
commit 8166ad0c7c
2 changed files with 13 additions and 7 deletions

View File

@ -1,5 +1,5 @@
use proc_macro2::TokenStream;
use devise::ext::{TypeExt, SpanDiagnosticExt, GenericsExt, quote_respanned};
use devise::ext::{TypeExt, SpanDiagnosticExt, GenericsExt, Split2, quote_respanned};
use syn::parse::Parser;
use devise::*;
@ -223,8 +223,13 @@ pub fn derive_from_form(input: proc_macro::TokenStream) -> TokenStream {
let o = syn::Ident::new("__o", fields.span());
let (_ok, _some, _err, _none) = (_Ok, _Some, _Err, _None);
let validate = fields.iter().flat_map(|f| validators(f, &o, false).unwrap());
let name_buf_opt = fields.iter().map(|f| f.name_buf_opt().unwrap());
let (validate, name_buf_opt) = fields.iter()
.flat_map(|f| {
let validate = validators(f, &o, false).unwrap();
let name_buf = std::iter::repeat_with(move || f.name_buf_opt().unwrap());
validate.zip(name_buf)
})
.split2();
let ident: Vec<_> = fields.iter()
.map(|f| f.context_ident())

View File

@ -410,7 +410,7 @@ fn form_errors() {
}
#[test]
fn form_error_return_wrong_field_name() {
fn form_error_return_correct_field_name() {
fn evaluate_other<'v>(_other: &String, _check: &bool) -> form::Result<'v, ()> {
Err(form::Error::validation(""))?
}
@ -426,11 +426,11 @@ fn form_error_return_wrong_field_name() {
}
let errors = strict::<WhoopsForm>("name=test&check=true&other=").unwrap_err();
assert!(errors.iter().any(|e| {e.name.as_ref().unwrap() == "other"}));
assert!(errors.iter().any(|e| e.name.as_ref().unwrap() == "other"));
}
#[test]
fn form_validate_missing_error() {
fn form_validate_contains_all_errors() {
fn evaluate<'v>(_value: &String) -> form::Result<'v, ()> {
Err(form::Error::validation(""))?
}
@ -450,7 +450,8 @@ fn form_validate_missing_error() {
}
let errors = strict::<WhoopsForm>("firstname=&check=true&lastname=").unwrap_err();
assert!(errors.iter().any(|e| {e.name.as_ref().unwrap() == "lastname"}));
assert!(errors.iter().any(|e| e.name.as_ref().unwrap() == "lastname"));
assert!(errors.iter().any(|e| e.name.as_ref().unwrap() == "firstname"));
}
#[test]