From 1f82d4bbcdd7889e6785c0d7c632dfec1ae734c7 Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Fri, 9 Aug 2024 18:44:50 -0700 Subject: [PATCH] Improve `FromParam` derive docs and error values. This commit improves the docs for the `FromParam` derive macro and exposes a new `InvalidOption` error value, which is returned when the derived `FromParam` implementation fails. --- core/codegen/src/derive/from_param.rs | 33 +++++------- core/codegen/src/lib.rs | 31 +++++++---- core/codegen/tests/from_param.rs | 19 ++++--- .../tests/ui-fail-nightly/from_param.stderr | 2 +- .../tests/ui-fail-stable/from_param.stderr | 2 +- core/lib/src/error.rs | 54 +++++++++++++++++++ core/lib/src/request/from_param.rs | 34 ++---------- core/lib/src/request/mod.rs | 3 -- 8 files changed, 106 insertions(+), 72 deletions(-) diff --git a/core/codegen/src/derive/from_param.rs b/core/codegen/src/derive/from_param.rs index 06e498a7..5f9ea464 100644 --- a/core/codegen/src/derive/from_param.rs +++ b/core/codegen/src/derive/from_param.rs @@ -1,45 +1,40 @@ -use crate::exports::*; -use devise::ext::SpanDiagnosticExt; -use devise::Support; use devise::*; -use proc_macro2::TokenStream; +use devise::ext::SpanDiagnosticExt; + use quote::quote; +use proc_macro2::TokenStream; use syn::ext::IdentExt; +use crate::exports::*; + pub fn derive_from_param(input: proc_macro::TokenStream) -> TokenStream { DeriveGenerator::build_for(input, quote!(impl<'a> #_request::FromParam<'a>)) .support(Support::Enum) .validator(ValidatorBuild::new().fields_validate(|_, fields| { if !fields.is_empty() { - return Err(fields - .span() - .error("Only enums without data fields are supported")); + return Err(fields.span().error("variants with data fields are not supported")); } + Ok(()) })) .inner_mapper(MapperBuild::new().enum_map(|_, data| { let matches = data.variants().map(|field| { let field_name = field.ident.unraw(); - quote!( - stringify!(#field_name) => Ok(Self::#field), - ) + quote!(stringify!(#field_name) => Ok(Self::#field)) }); + let names = data.variants().map(|field| { let field_name = field.ident.unraw(); - quote!( - #_Cow::Borrowed(stringify!(#field_name)), - ) + quote!(stringify!(#field_name)) }); quote! { - type Error = #_request::EnumFromParamError<'a>; + type Error = #_error::InvalidOption<'a>; + fn from_param(param: &'a str) -> Result { match param { - #(#matches)* - _ => Err(#_request::EnumFromParamError::new( - #_Cow::Borrowed(param), - #_Cow::Borrowed(&[#(#names)*]), - )), + #(#matches,)* + _ => Err(#_error::InvalidOption::new(param, &[#(#names),*])), } } } diff --git a/core/codegen/src/lib.rs b/core/codegen/src/lib.rs index 3d6b957c..07d4fc80 100644 --- a/core/codegen/src/lib.rs +++ b/core/codegen/src/lib.rs @@ -776,32 +776,41 @@ pub fn derive_from_form(input: TokenStream) -> TokenStream { /// Derive for the [`FromParam`] trait. /// -/// The [`FromParam`] derive can be applied to enums with nullary -/// (zero-length) fields. To implement FromParam, the function matches each variant -/// to its stringified field name (case sensitive): +/// This [`FromParam`] derive can be applied to C-like enums whose variants have +/// no fields. The generated implementation case-sensitively matches each +/// variant to its stringified field name. If there is no match, an error +/// of type [`InvalidOption`] is returned. +/// +/// [`FromParam`]: ../rocket/request/trait.FromParam.html +/// [`InvalidOption`]: ../rocket/error/struct.InvalidOption.html +/// +/// # Example /// /// ```rust /// # #[macro_use] extern crate rocket; -/// # /// use rocket::request::FromParam; /// /// #[derive(FromParam, Debug, PartialEq)] /// enum MyParam { /// A, -/// B, +/// Bob, /// } /// /// assert_eq!(MyParam::from_param("A").unwrap(), MyParam::A); -/// assert_eq!(MyParam::from_param("B").unwrap(), MyParam::B); +/// assert_eq!(MyParam::from_param("Bob").unwrap(), MyParam::Bob); /// assert!(MyParam::from_param("a").is_err()); -/// assert!(MyParam::from_param("b").is_err()); +/// assert!(MyParam::from_param("bob").is_err()); /// assert!(MyParam::from_param("c").is_err()); /// assert!(MyParam::from_param("C").is_err()); -/// ``` -/// -/// Now `MyParam` can be used in an endpoint and will accept either `A` or `B`. -/// [`FromParam`]: ../rocket/request/trait.FromParam.html /// +/// // Now `MyParam` can be used in an route to accept either `A` or `B`. +/// #[get("/")] +/// fn index(param: MyParam) -> &'static str { +/// match param { +/// MyParam::A => "A", +/// MyParam::Bob => "Bob", +/// } +/// } #[proc_macro_derive(FromParam)] pub fn derive_from_param(input: TokenStream) -> TokenStream { emit!(derive::from_param::derive_from_param(input)) diff --git a/core/codegen/tests/from_param.rs b/core/codegen/tests/from_param.rs index a0dc54a5..84c112e6 100644 --- a/core/codegen/tests/from_param.rs +++ b/core/codegen/tests/from_param.rs @@ -1,5 +1,6 @@ use rocket::request::FromParam; +#[allow(non_camel_case_types)] #[derive(Debug, FromParam, PartialEq)] enum Test { Test1, @@ -9,14 +10,16 @@ enum Test { #[test] fn derive_from_param() { - let test1 = Test::from_param("Test1").expect("Should be valid"); - assert_eq!(test1, Test::Test1); + assert_eq!(Test::from_param("Test1").unwrap(), Test::Test1); + assert_eq!(Test::from_param("Test2").unwrap(), Test::Test2); + assert_eq!(Test::from_param("for").unwrap(), Test::r#for); - let test2 = Test::from_param("Test2").expect("Should be valid"); - assert_eq!(test2, Test::Test2); - let test2 = Test::from_param("for").expect("Should be valid"); - assert_eq!(test2, Test::r#for); + let err = Test::from_param("For").unwrap_err(); + assert_eq!(err.value, "For"); + assert_eq!(err.options, &["Test1", "Test2", "for"]); + + let err = Test::from_param("not_test").unwrap_err(); + assert_eq!(err.value, "not_test"); + assert_eq!(err.options, &["Test1", "Test2", "for"]); - let test3 = Test::from_param("not_test"); - assert!(test3.is_err()); } diff --git a/core/codegen/tests/ui-fail-nightly/from_param.stderr b/core/codegen/tests/ui-fail-nightly/from_param.stderr index 697bb4e4..ab4c3db0 100644 --- a/core/codegen/tests/ui-fail-nightly/from_param.stderr +++ b/core/codegen/tests/ui-fail-nightly/from_param.stderr @@ -26,7 +26,7 @@ note: error occurred while deriving `FromParam` | ^^^^^^^^^ = note: this error originates in the derive macro `FromParam` (in Nightly builds, run with -Z macro-backtrace for more info) -error: Only enums without data fields are supported +error: variants with data fields are not supported --> tests/ui-fail-nightly/from_param.rs:13:6 | 13 | A(String), diff --git a/core/codegen/tests/ui-fail-stable/from_param.stderr b/core/codegen/tests/ui-fail-stable/from_param.stderr index 97a9e65d..8eafb689 100644 --- a/core/codegen/tests/ui-fail-stable/from_param.stderr +++ b/core/codegen/tests/ui-fail-stable/from_param.stderr @@ -28,7 +28,7 @@ error: [note] error occurred while deriving `FromParam` | = note: this error originates in the derive macro `FromParam` (in Nightly builds, run with -Z macro-backtrace for more info) -error: Only enums without data fields are supported +error: variants with data fields are not supported --> tests/ui-fail-stable/from_param.rs:13:6 | 13 | A(String), diff --git a/core/lib/src/error.rs b/core/lib/src/error.rs index 808b79d2..f08e94ad 100644 --- a/core/lib/src/error.rs +++ b/core/lib/src/error.rs @@ -89,6 +89,60 @@ pub enum ErrorKind { #[derive(Clone, Copy, Default, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct Empty; +/// An error that occurs when a value doesn't match one of the expected options. +/// +/// This error is returned by the [`FromParam`] trait implementation generated +/// by the [`FromParam` derive](macro@rocket::FromParam) when the value of a +/// dynamic path segment does not match one of the expected variants. The +/// `value` field will contain the value that was provided, and `options` will +/// contain each of possible stringified variants. +/// +/// [`FromParam`]: trait@rocket::request::FromParam +/// +/// # Example +/// +/// ```rust +/// # #[macro_use] extern crate rocket; +/// use rocket::error::InvalidOption; +/// +/// #[derive(FromParam)] +/// enum MyParam { +/// FirstOption, +/// SecondOption, +/// ThirdOption, +/// } +/// +/// #[get("/")] +/// fn hello(param: Result>) { +/// if let Err(e) = param { +/// assert_eq!(e.options, &["FirstOption", "SecondOption", "ThirdOption"]); +/// } +/// } +/// ``` +#[derive(Debug, Clone)] +#[non_exhaustive] +pub struct InvalidOption<'a> { + /// The value that was provided. + pub value: &'a str, + /// The expected values: a slice of strings, one for each possible value. + pub options: &'static [&'static str], +} + +impl<'a> InvalidOption<'a> { + #[doc(hidden)] + pub fn new(value: &'a str, options: &'static [&'static str]) -> Self { + Self { value, options } + } +} + +impl fmt::Display for InvalidOption<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "unexpected value {:?}, expected one of {:?}", self.value, self.options) + } +} + +impl std::error::Error for InvalidOption<'_> {} + impl Error { #[inline(always)] pub(crate) fn new(kind: ErrorKind) -> Error { diff --git a/core/lib/src/request/from_param.rs b/core/lib/src/request/from_param.rs index efecac96..f639d38e 100644 --- a/core/lib/src/request/from_param.rs +++ b/core/lib/src/request/from_param.rs @@ -1,10 +1,6 @@ -use std::borrow::Cow; -use std::fmt; use std::str::FromStr; use std::path::PathBuf; -use cookie::Display; - use crate::error::Empty; use crate::either::Either; use crate::http::uri::{Segments, error::PathError, fmt::Path}; @@ -16,6 +12,11 @@ use crate::http::uri::{Segments, error::PathError, fmt::Path}; /// a dynamic segment `` where `param` has some type `T` that implements /// `FromParam`, `T::from_param` will be called. /// +/// # Deriving +/// +/// The `FromParam` trait can be automatically derived for C-like enums. See +/// [`FromParam` derive](macro@rocket::FromParam) for more information. +/// /// # Forwarding /// /// If the conversion fails, the incoming request will be forwarded to the next @@ -310,31 +311,6 @@ impl<'a, T: FromParam<'a>> FromParam<'a> for Option { } } -/// Error type for automatically derived `FromParam` enums -#[derive(Debug, Clone)] -#[non_exhaustive] -pub struct EnumFromParamError<'a> { - pub value: Cow<'a, str>, - pub options: Cow<'static, [Cow<'static, str>]>, -} - -impl<'a> EnumFromParamError<'a> { - pub fn new(value: Cow<'a, str>, options: Cow<'static, [Cow<'static, str>]>) -> Self { - Self { - value, - options, - } - } -} - -impl fmt::Display for EnumFromParamError<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "Unexpected value {:?}, expected one of {:?}", self.value, self.options) - } -} - -impl std::error::Error for EnumFromParamError<'_> {} - /// Trait to convert _many_ dynamic path segment strings to a concrete value. /// /// This is the `..` analog to [`FromParam`], and its functionality is identical diff --git a/core/lib/src/request/mod.rs b/core/lib/src/request/mod.rs index 47ffaf20..48ac79c7 100644 --- a/core/lib/src/request/mod.rs +++ b/core/lib/src/request/mod.rs @@ -12,9 +12,6 @@ pub use self::request::Request; pub use self::from_request::{FromRequest, Outcome}; pub use self::from_param::{FromParam, FromSegments}; -#[doc(hidden)] -pub use self::from_param::EnumFromParamError; - #[doc(hidden)] pub use rocket_codegen::FromParam;