improve macro compile errors

actix-multipart-derive/src/lib.rs

@@ -10,17 +10,11 @@ use std::{collections::HashSet, convert::TryFrom as _};
 
 use darling::{FromDeriveInput, FromField, FromMeta};
 use parse_size::parse_size;
-use proc_macro2::Ident;
+use proc_macro::TokenStream;
+use proc_macro2::{Ident, Span};
 use quote::quote;
 use syn::{parse_macro_input, Type};
 
-#[derive(FromDeriveInput, Default)]
-#[darling(attributes(multipart), default)]
-struct MultipartFormAttrs {
-    deny_unknown_fields: bool,
-    duplicate_field: DuplicateField,
-}
-
 #[derive(FromMeta)]
 enum DuplicateField {
     Ignore,
@@ -34,6 +28,13 @@ impl Default for DuplicateField {
     }
 }
 
+#[derive(FromDeriveInput, Default)]
+#[darling(attributes(multipart), default)]
+struct MultipartFormAttrs {
+    deny_unknown_fields: bool,
+    duplicate_field: DuplicateField,
+}
+
 #[derive(FromField, Default)]
 #[darling(attributes(multipart), default)]
 struct FieldAttrs {
@@ -51,16 +52,13 @@ struct ParsedField<'t> {
 /// Implements the `MultipartFormTrait` for a struct so that it can be used with the `MultipartForm`
 /// extractor.
 ///
-/// # Examples
+/// # Basic Use
-///
-/// ## Basic Use
 ///
 /// Each field type should implement the `FieldReader` trait:
 ///
 /// ```
-/// # use actix_multipart::form::tempfile::Tempfile;
+/// use actix_multipart::form::{tempfile::Tempfile, text::Text, MultipartForm};
-/// # use actix_multipart::form::text::Text;
+///
-/// # use actix_multipart::form::MultipartForm;
 /// #[derive(MultipartForm)]
 /// struct ImageUpload {
 ///     description: Text<String>,
@@ -69,17 +67,16 @@ struct ParsedField<'t> {
 /// }
 /// ```
 ///
-/// ## Optional and List Fields
+/// # Optional and List Fields
 ///
 /// You can also use `Vec<T>` and `Option<T>` provided that `T: FieldReader`.
 ///
-/// A [`Vec`] field corresponds to an upload with multiple parts under the
+/// A [`Vec`] field corresponds to an upload with multiple parts under the [same field
-/// [same field name](https://www.rfc-editor.org/rfc/rfc7578#section-4.3).
+/// name](https://www.rfc-editor.org/rfc/rfc7578#section-4.3).
 ///
 /// ```
-/// # use actix_multipart::form::tempfile::Tempfile;
+/// use actix_multipart::form::{tempfile::Tempfile, text::Text, MultipartForm};
-/// # use actix_multipart::form::text::Text;
+///
-/// # use actix_multipart::form::MultipartForm;
 /// #[derive(MultipartForm)]
 /// struct Form {
 ///     category: Option<Text<String>>,
@@ -87,13 +84,13 @@ struct ParsedField<'t> {
 /// }
 /// ```
 ///
-/// ## Field Renaming
+/// # Field Renaming
 ///
 /// You can use the `#[multipart(rename = "foo")]` attribute to receive a field by a different name.
 ///
 /// ```
-/// # use actix_multipart::form::tempfile::Tempfile;
+/// use actix_multipart::form::{tempfile::Tempfile, MultipartForm};
-/// # use actix_multipart::form::MultipartForm;
+///
 /// #[derive(MultipartForm)]
 /// struct Form {
 ///     #[multipart(rename = "files[]")]
@@ -101,7 +98,7 @@ struct ParsedField<'t> {
 /// }
 /// ```
 ///
-/// ## Field Limits
+/// # Field Limits
 ///
 /// You can use the `#[multipart(limit = "<size>")]` attribute to set field level limits. The limit
 /// string is parsed using [parse_size].
@@ -109,40 +106,41 @@ struct ParsedField<'t> {
 /// Note: the form is also subject to the global limits configured using `MultipartFormConfig`.
 ///
 /// ```
-/// # use actix_multipart::form::tempfile::Tempfile;
+/// use actix_multipart::form::{tempfile::Tempfile, text::Text, MultipartForm};
-/// # use actix_multipart::form::text::Text;
+///
-/// # use actix_multipart::form::MultipartForm;
 /// #[derive(MultipartForm)]
 /// struct Form {
-///     #[multipart(limit = "2KiB")]
+///     #[multipart(limit = "2 KiB")]
 ///     description: Text<String>,
-///     #[multipart(limit = "512MiB")]
+///
+///     #[multipart(limit = "512 MiB")]
 ///     files: Vec<Tempfile>,
 /// }
 /// ```
 ///
-/// ## Unknown Fields
+/// # Unknown Fields
 ///
-/// By default fields with an unknown name are ignored. You can change this using the
+/// By default fields with an unknown name are ignored. They can be rejected using the
 /// `#[multipart(deny_unknown_fields)]` attribute:
 ///
 /// ```
-/// # use actix_multipart::form::MultipartForm;
+/// use actix_multipart::form::MultipartForm;
+///
 /// #[derive(MultipartForm)]
 /// #[multipart(deny_unknown_fields)]
 /// struct Form { }
 /// ```
 ///
-/// ## Duplicate Fields
+/// # Duplicate Fields
 ///
-/// You can change the behaviour for when multiple fields are received with the same name using the
+/// The behaviour for when multiple fields with the same name are received can be changed using the
-/// `#[multipart(duplicate_field = "")]` attribute:
+/// `#[multipart(duplicate_field = "<behavior>")]` attribute:
 ///
-/// - "ignore": Extra fields are ignored (default).
+/// - "ignore": (default) Extra fields are ignored. I.e., the first one is persisted.
+/// - "deny": A `MultipartError::UnsupportedField` error response is returned.
 /// - "replace": Each field is processed, but only the last one is persisted.
-/// - "deny": A `MultipartError::UnsupportedField` error is returned.
 ///
-/// (Note this option does not apply to `Vec` fields)
+/// Note that `Vec` fields will ignore this option.
 ///
 /// ```
 /// # use actix_multipart::form::MultipartForm;
@@ -157,16 +155,34 @@ pub fn impl_multipart_form(input: proc_macro::TokenStream) -> proc_macro::TokenS
     let input: syn::DeriveInput = parse_macro_input!(input);
 
     let name = &input.ident;
-    let str = match &input.data {
+
-        syn::Data::Struct(s) => s,
+    let data_struct = match &input.data {
-        _ => panic!("This trait can only be derived for a struct"),
+        syn::Data::Struct(data_struct) => data_struct,
-    };
+        _ => {
-    let fields = match &str.fields {
+            return TokenStream::from(
-        syn::Fields::Named(n) => n,
+                syn::Error::new(
-        _ => panic!("This trait can only be derived for a struct"),
+                    Span::call_site(),
+                    "This trait can only be derived for a struct",
+                )
+                .to_compile_error(),
+            )
+        }
     };
 
-    let attrs: MultipartFormAttrs = match MultipartFormAttrs::from_derive_input(&input) {
+    let fields = match &data_struct.fields {
+        syn::Fields::Named(fields_named) => fields_named,
+        _ => {
+            return TokenStream::from(
+                syn::Error::new(
+                    Span::call_site(),
+                    "This trait can only be derived for a struct with named fields",
+                )
+                .to_compile_error(),
+            )
+        }
+    };
+
+    let attrs = match MultipartFormAttrs::from_derive_input(&input) {
         Ok(attrs) => attrs,
         Err(err) => return err.write_errors().into(),
     };
@@ -177,16 +193,19 @@ pub fn impl_multipart_form(input: proc_macro::TokenStream) -> proc_macro::TokenS
         .iter()
         .map(|field| {
             let rust_name = field.ident.as_ref().unwrap();
-            let attrs: FieldAttrs = FieldAttrs::from_field(field)?;
+            let attrs = FieldAttrs::from_field(field).map_err(|err| err.write_errors())?;
             let serialization_name = attrs.rename.unwrap_or_else(|| rust_name.to_string());
 
-            let limit = attrs.limit.map(|limit| {
+            let limit = match attrs.limit.map(|limit| match parse_size(&limit) {
-                usize::try_from(
+                Ok(size) => Ok(usize::try_from(size).unwrap()),
-                    parse_size(&limit)
+                Err(err) => Err(syn::Error::new(
-                        .unwrap_or_else(|_| panic!("Unable to parse limit `{}`", limit)),
+                    field.ident.as_ref().unwrap().span(),
-                )
+                    format!("Unable to parse limit `{}`: {}", limit, err),
-                .unwrap()
+                )),
-            });
+            }) {
+                Some(Err(err)) => return Err(TokenStream::from(err.to_compile_error())),
+                limit => limit.map(Result::unwrap),
+            };
 
             Ok(ParsedField {
                 serialization_name,
@@ -195,17 +214,23 @@ pub fn impl_multipart_form(input: proc_macro::TokenStream) -> proc_macro::TokenS
                 ty: &field.ty,
             })
         })
-        .collect::<Result<Vec<_>, darling::Error>>()
+        .collect::<Result<Vec<_>, TokenStream>>()
     {
         Ok(attrs) => attrs,
-        Err(err) => return err.write_errors().into(),
+        Err(err) => return err,
     };
 
     // Check that field names are unique
     let mut set = HashSet::new();
-    for f in &parsed {
+    for field in &parsed {
-        if !set.insert(f.serialization_name.clone()) {
+        if !set.insert(field.serialization_name.clone()) {
-            panic!("Multiple fields named: `{}`", f.serialization_name);
+            return TokenStream::from(
+                syn::Error::new(
+                    field.rust_name.span(),
+                    format!("Multiple fields named: `{}`", field.serialization_name),
+                )
+                .to_compile_error(),
+            );
         }
     }
 
@@ -230,6 +255,7 @@ pub fn impl_multipart_form(input: proc_macro::TokenStream) -> proc_macro::TokenS
     for field in &parsed {
         let name = &field.serialization_name;
         let ty = &field.ty;
+
         read_field_impl.extend(quote!(
             #name => ::std::boxed::Box::pin(
                 <#ty as ::actix_multipart::form::FieldGroupReader>::handle_field(req, field, limits, state, #duplicate_field)

actix-multipart/CHANGES.md

@@ -1,13 +1,14 @@
 # Changes
 
 ## Unreleased - 2022-xx-xx
+- Added `MultipartForm` typed data extractor. [#2883]
+- `Field::content_type()` now returns `Option<&mime::Mime>`. [#2880]
 - Minimum supported Rust version (MSRV) is now 1.59 due to transitive `time` dependency.
-- `Field::content_type()` now returns `Option<&mime::Mime>` [#2880].
-- Added `MultipartForm` typed data extractor [#2883].
 
 [#2880]: https://github.com/actix/actix-web/pull/2880
 [#2883]: https://github.com/actix/actix-web/pull/2883
 
+
 ## 0.4.0 - 2022-02-25
 - No significant changes since `0.4.0-beta.13`.
 

actix-multipart/Cargo.toml

@@ -24,7 +24,6 @@ path = "src/lib.rs"
 
 [dependencies]
 actix-multipart-derive = { version = "=0.4.0", optional = true }
-actix-http = "3"
 actix-utils = "3"
 actix-web = { version = "4", default-features = false }
 
@@ -37,14 +36,15 @@ local-waker = "0.1"
 log = "0.4"
 memchr = "2.5"
 mime = "0.3"
-serde = "1.0"
+serde = "1"
-serde_json = "1.0"
+serde_json = "1"
-serde_plain = "1.0"
+serde_plain = "1"
 # TODO(MSRV 1.60): replace with dep: prefix
 tempfile-dep = { package = "tempfile", version = "3.3.0", optional = true }
 tokio = { version = "1.13.1", features = ["sync"] }
 
 [dev-dependencies]
+actix-http = "3"
 actix-multipart-rfc7578 = "0.10"
 actix-rt = "2.2"
 actix-test = "0.1.0"

actix-multipart/src/form/text.rs

@@ -26,7 +26,10 @@ impl<T: DeserializeOwned> Text<T> {
     }
 }
 
-impl<'t, T: DeserializeOwned + 'static> FieldReader<'t> for Text<T> {
+impl<'t, T> FieldReader<'t> for Text<T>
+where
+    T: DeserializeOwned + 'static,
+{
     type Future = LocalBoxFuture<'t, Result<Self, MultipartError>>;
 
     fn read_field(req: &'t HttpRequest, field: Field, limits: &'t mut Limits) -> Self::Future {
@@ -43,7 +46,7 @@ impl<'t, T: DeserializeOwned + 'static> FieldReader<'t> for Text<T> {
                     true
                 };
 
-                if !valid && config.validate_content_type {
+                if !valid {
                     return Err(MultipartError::Field {
                         field_name,
                         source: config.map_error(req, TextError::ContentType),

actix-multipart/src/server.rs

@@ -864,7 +864,7 @@ impl PayloadBuffer {
 mod tests {
     use std::time::Duration;
 
-    use actix_http::h1::Payload;
+    use actix_http::h1;
     use actix_web::{
         http::header::{DispositionParam, DispositionType},
         rt,
@@ -1124,7 +1124,7 @@ mod tests {
 
     #[actix_rt::test]
     async fn test_basic() {
-        let (_, payload) = Payload::create(false);
+        let (_, payload) = h1::Payload::create(false);
         let mut payload = PayloadBuffer::new(payload);
 
         assert_eq!(payload.buf.len(), 0);
@@ -1134,7 +1134,7 @@ mod tests {
 
     #[actix_rt::test]
     async fn test_eof() {
-        let (mut sender, payload) = Payload::create(false);
+        let (mut sender, payload) = h1::Payload::create(false);
         let mut payload = PayloadBuffer::new(payload);
 
         assert_eq!(None, payload.read_max(4).unwrap());
@@ -1150,7 +1150,7 @@ mod tests {
 
     #[actix_rt::test]
     async fn test_err() {
-        let (mut sender, payload) = Payload::create(false);
+        let (mut sender, payload) = h1::Payload::create(false);
         let mut payload = PayloadBuffer::new(payload);
         assert_eq!(None, payload.read_max(1).unwrap());
         sender.set_error(PayloadError::Incomplete(None));
@@ -1159,7 +1159,7 @@ mod tests {
 
     #[actix_rt::test]
     async fn test_readmax() {
-        let (mut sender, payload) = Payload::create(false);
+        let (mut sender, payload) = h1::Payload::create(false);
         let mut payload = PayloadBuffer::new(payload);
 
         sender.feed_data(Bytes::from("line1"));
@@ -1176,7 +1176,7 @@ mod tests {
 
     #[actix_rt::test]
     async fn test_readexactly() {
-        let (mut sender, payload) = Payload::create(false);
+        let (mut sender, payload) = h1::Payload::create(false);
         let mut payload = PayloadBuffer::new(payload);
 
         assert_eq!(None, payload.read_exact(2));
@@ -1194,7 +1194,7 @@ mod tests {
 
     #[actix_rt::test]
     async fn test_readuntil() {
-        let (mut sender, payload) = Payload::create(false);
+        let (mut sender, payload) = h1::Payload::create(false);
         let mut payload = PayloadBuffer::new(payload);
 
         assert_eq!(None, payload.read_until(b"ne").unwrap());
@@ -1235,7 +1235,7 @@ mod tests {
     #[actix_rt::test]
     async fn test_multipart_payload_consumption() {
         // with sample payload and HttpRequest with no headers
-        let (_, inner_payload) = Payload::create(false);
+        let (_, inner_payload) = h1::Payload::create(false);
         let mut payload = actix_web::dev::Payload::from(inner_payload);
         let req = TestRequest::default().to_http_request();