Option guards should return None only if the value is empty.

core/codegen/src/lib.rs

@@ -1361,13 +1361,13 @@ pub fn catchers(input: TokenStream) -> TokenStream {
 ///
 /// ```rust
 /// # #[macro_use] extern crate rocket;
-/// #[get("/person/<name>")]
-/// fn maybe(name: Option<&str>) { }
+/// // #[get("/person/<name>")]
+/// // fn maybe(name: Option<&str>) { }
 ///
-/// let bob1 = uri!(maybe(name = "Bob"));
-/// let bob2 = uri!(maybe("Bob Smith"));
-/// assert_eq!(bob1.to_string(), "/person/Bob");
-/// assert_eq!(bob2.to_string(), "/person/Bob%20Smith");
+/// // let bob1 = uri!(maybe(name = "Bob"));
+/// // let bob2 = uri!(maybe("Bob Smith"));
+/// // assert_eq!(bob1.to_string(), "/person/Bob");
+/// // assert_eq!(bob2.to_string(), "/person/Bob%20Smith");
 ///
 /// #[get("/person/<age>")]
 /// fn ok(age: Result<u8, std::num::ParseIntError>) { }

core/codegen/tests/typed-uris.rs

@@ -2,8 +2,10 @@
 
 #[macro_use] extern crate rocket;
 
+use std::num::ParseIntError;
 use std::path::PathBuf;
 
+use rocket::error::Empty;
 use rocket::http::CookieJar;
 use rocket::http::uri::fmt::{FromUriParam, Query};
 use rocket::form::{Form, error::{Errors, ErrorKind}};
@@ -474,8 +476,8 @@ struct Third<'r> {
 
 #[post("/<foo>/<bar>?<q1>&<rest..>")]
 fn optionals(
-    foo: Option<usize>,
-    bar: Option<String>,
+    foo: Result<usize, ParseIntError>,
+    bar: Result<String, Empty>,
     q1: Result<usize, Errors<'_>>,
     rest: Option<Third<'_>>
 ) { }
@@ -547,6 +549,13 @@ fn test_optional_uri_parameters() {
             q1 = _,
             rest = _,
         )) => "/10/hi%20there",
+
+        // uri!(optionals(
+        //     foo = 10,
+        //     bar = Err(Empty),
+        //     q1 = _,
+        //     rest = _,
+        // )) => "/10/hi%20there",
     }
 }
 

core/lib/src/data/from_data.rs

@@ -128,15 +128,11 @@ pub type Outcome<'r, T, E = <T as FromData<'r>>::Error>
 ///
 ///   * `Option<T>`
 ///
-///     Forwards to `T`'s `FromData` implementation, capturing the outcome.
-///
-///     - **Fails:** Never.
-///
-///     - **Succeeds:** Always. If `T`'s `FromData` implementation succeeds, the
-///     parsed value is returned in `Some`. If its implementation forwards or
-///     fails, `None` is returned.
+///     Forwards to `T`'s `FromData` implementation if there is data, capturing the outcome.
+///     If `T` returns an Error or Forward, the `Option` returns the same.
 ///
-///     - **Forwards:** Never.
+///     - **None:** If the data stream is empty.
+///     - **Some:** If `T` succeeds to parse the incoming data.
 ///
 ///   * `Result<T, T::Error>`
 ///
@@ -415,12 +411,14 @@ impl<'r, T: FromData<'r> + 'r> FromData<'r> for Result<T, T::Error> {
 
 #[crate::async_trait]
 impl<'r, T: FromData<'r>> FromData<'r> for Option<T> {
-    type Error = std::convert::Infallible;
+    type Error = T::Error;
 
-    async fn from_data(req: &'r Request<'_>, data: Data<'r>) -> Outcome<'r, Self> {
-        match T::from_data(req, data).await {
-            Success(v) => Success(Some(v)),
-            Error(..) | Forward(..) => Success(None),
+    async fn from_data(req: &'r Request<'_>, mut data: Data<'r>) -> Outcome<'r, Self> {
+        // Ask for at least one byte of data, if it's empty, there is no body.
+        if data.peek(1).await.is_empty() {
+            Outcome::Success(None)
+        } else {
+            T::from_data(req, data).await.map(Some)
         }
     }
 }

core/lib/src/form/from_form.rs

@@ -106,7 +106,7 @@ use crate::http::uncased::AsUncased;
 /// |------------------------|-------------|-------------------|--------|--------|----------------------------------------------------|
 /// | [`Strict<T>`]          | **strict**  | if `strict` `T`   | if `T` | if `T` | `T: FromForm`                                      |
 /// | [`Lenient<T>`]         | **lenient** | if `lenient` `T`  | if `T` | if `T` | `T: FromForm`                                      |
-/// | `Option<T>`            | **strict**  | `None`            | if `T` | if `T` | Infallible, `T: FromForm`                          |
+/// | `Option<T>`            | **strict**  | `None`            | if `T` | if `T` | `T: FromForm`                          |
 /// | [`Result<T>`]          | _inherit_   | `T::finalize()`   | if `T` | if `T` | Infallible, `T: FromForm`                          |
 /// | `Vec<T>`               | _inherit_   | `vec![]`          | if `T` | if `T` | `T: FromForm`                                      |
 /// | [`HashMap<K, V>`]      | _inherit_   | `HashMap::new()`  | if `V` | if `V` | `K: FromForm + Eq + Hash`, `V: FromForm`           |
@@ -151,6 +151,12 @@ use crate::http::uncased::AsUncased;
 ///
 /// ## Additional Notes
 ///
+///   * **`Option<T>` where `T: FromForm`**
+///
+///     `Option` is no longer Infallible. It now checks (depending on the strictness)
+///     for whether any values were passed in, and returns an error if any values were passed in,
+///     and the inner `T` failed to parse.
+///
 ///   * **`Vec<T>` where `T: FromForm`**
 ///
 ///     Parses a sequence of `T`'s. A new `T` is created whenever the field
@@ -814,24 +820,51 @@ impl<'v, K, V> FromForm<'v> for BTreeMap<K, V>
     }
 }
 
+pub struct OptionFormCtx<'v, T: FromForm<'v>> {
+    strict: bool,
+    has_field: bool,
+    inner: T::Context,
+}
+
 #[crate::async_trait]
 impl<'v, T: FromForm<'v>> FromForm<'v> for Option<T> {
-    type Context = <T as FromForm<'v>>::Context;
+    type Context = OptionFormCtx<'v, T>;
 
-    fn init(_: Options) -> Self::Context {
-        T::init(Options { strict: true })
+    fn init(opts: Options) -> Self::Context {
+        OptionFormCtx {
+            strict: opts.strict,
+            has_field: false,
+            inner: T::init(Options { strict: true }),
+        }
     }
 
     fn push_value(ctxt: &mut Self::Context, field: ValueField<'v>) {
-        T::push_value(ctxt, field)
+        ctxt.has_field = true;
+        T::push_value(&mut ctxt.inner, field)
     }
 
     async fn push_data(ctxt: &mut Self::Context, field: DataField<'v, '_>) {
-        T::push_data(ctxt, field).await
+        ctxt.has_field = true;
+        T::push_data(&mut ctxt.inner, field).await
     }
 
     fn finalize(this: Self::Context) -> Result<'v, Self> {
-        Ok(T::finalize(this).ok())
+        if this.has_field {
+            match T::finalize(this.inner) {
+                Ok(v) => Ok(Some(v)),
+                Err(errors) => {
+                    if this.strict ||
+                        errors.iter().any(|e| e.kind != ErrorKind::Missing)
+                    {
+                        Err(errors)
+                    } else {
+                        Ok(None)
+                    }
+                }
+            }
+        } else {
+            Ok(None)
+        }
     }
 }
 

core/lib/src/form/tests.rs

@@ -105,18 +105,20 @@ fn defaults() {
         &[] => Result<'_, bool> = Ok(false),
         &[] => Result<'_, Strict<bool>> = Err(error::ErrorKind::Missing.into()),
 
-        &["=unknown"] => Option<bool> = None,
-        &["=unknown"] => Option<Strict<bool>> = None,
-        &["=unknown"] => Option<Lenient<bool>> = None,
-
-        &[] => Option<Lenient<bool>> = Some(false.into()),
-        &["=123"] => Option<time::Date> = None,
+        &[] => Option<Lenient<bool>> = None,
 
         &["=no"] => Option<bool> = Some(false),
         &["=yes"] => Option<bool> = Some(true),
         &["=yes"] => Option<Lenient<bool>> = Some(true.into()),
         &["=yes"] => Option<Strict<bool>> = Some(true.into()),
     }
+
+    assert_parses_fail! {
+        &["=unknown"] => Option<bool>,
+        &["=unknown"] => Option<Strict<bool>>,
+        &["=unknown"] => Option<Lenient<bool>>,
+        &["=123"] => Option<time::Date>,
+    }
 }
 
 #[test]

core/lib/src/outcome.rs

@@ -50,7 +50,7 @@
 //! ```rust
 //! # use rocket::post;
 //! # type S = Option<String>;
-//! # type E = std::convert::Infallible;
+//! # type E = std::io::Error;
 //! #[post("/", data = "<my_val>")]
 //! fn hello(my_val: Result<S, E>) { /* ... */ }
 //! ```

core/lib/src/request/from_param.rs

@@ -46,16 +46,9 @@ use crate::http::uri::{Segments, error::PathError, fmt::Path};
 ///
 /// Sometimes, a forward is not desired, and instead, we simply want to know
 /// that the dynamic path segment could not be parsed into some desired type
-/// `T`. In these cases, types of `Option<T>`, `Result<T, T::Error>`, or
+/// `T`. In these cases, types of `Result<T, T::Error>`, or
 /// `Either<A, B>` can be used, which implement `FromParam` themselves.
 ///
-///   * **`Option<T>`** _where_ **`T: FromParam`**
-///
-///     Always returns successfully.
-///
-///     If the conversion to `T` fails, `None` is returned. If the conversion
-///     succeeds, `Some(value)` is returned.
-///
 ///   * **`Result<T, T::Error>`** _where_ **`T: FromParam`**
 ///
 ///     Always returns successfully.
@@ -121,14 +114,6 @@ use crate::http::uri::{Segments, error::PathError, fmt::Path};
 ///     Returns the percent-decoded path segment with invalid UTF-8 byte
 ///     sequences replaced by � U+FFFD.
 ///
-///   * **Option&lt;T>** _where_ **T: FromParam**
-///
-///     _This implementation always returns successfully._
-///
-///     The path segment is parsed by `T`'s `FromParam` implementation. If the
-///     parse succeeds, a `Some(parsed_value)` is returned. Otherwise, a `None`
-///     is returned.
-///
 ///   * **Result&lt;T, T::Error>** _where_ **T: FromParam**
 ///
 ///     _This implementation always returns successfully._
@@ -299,17 +284,17 @@ impl<'a, T: FromParam<'a>> FromParam<'a> for Result<T, T::Error> {
     }
 }
 
-impl<'a, T: FromParam<'a>> FromParam<'a> for Option<T> {
-    type Error = std::convert::Infallible;
+// impl<'a, T: FromParam<'a>> FromParam<'a> for Option<T> {
+//     type Error = std::convert::Infallible;
 
-    #[inline]
-    fn from_param(param: &'a str) -> Result<Self, Self::Error> {
-        match T::from_param(param) {
-            Ok(val) => Ok(Some(val)),
-            Err(_) => Ok(None)
-        }
-    }
-}
+//     #[inline]
+//     fn from_param(param: &'a str) -> Result<Self, Self::Error> {
+//         match T::from_param(param) {
+//             Ok(val) => Ok(Some(val)),
+//             Err(_) => Ok(None)
+//         }
+//     }
+// }
 
 /// Trait to convert _many_ dynamic path segment strings to a concrete value.
 ///
@@ -329,6 +314,11 @@ impl<'a, T: FromParam<'a>> FromParam<'a> for Option<T> {
 /// any other segments that begin with "*" or "." are ignored.  If a
 /// percent-decoded segment results in invalid UTF8, an `Err` is returned with
 /// the `Utf8Error`.
+///
+/// **`Option&lt;T>` where `T: FromSegments`**
+///
+/// Succeeds as `None` if the this segments matched nothing, otherwise invokes
+/// `T`'s `FromSegments` implementation.
 pub trait FromSegments<'r>: Sized {
     /// The associated error to be returned when parsing fails.
     type Error: std::fmt::Debug;
@@ -384,13 +374,14 @@ impl<'r, T: FromSegments<'r>> FromSegments<'r> for Result<T, T::Error> {
 }
 
 impl<'r, T: FromSegments<'r>> FromSegments<'r> for Option<T> {
-    type Error = std::convert::Infallible;
+    type Error = T::Error;
 
     #[inline]
     fn from_segments(segments: Segments<'r, Path>) -> Result<Option<T>, Self::Error> {
-        match T::from_segments(segments) {
-            Ok(val) => Ok(Some(val)),
-            Err(_) => Ok(None)
+        if segments.is_empty() {
+            Ok(None)
+        } else {
+            T::from_segments(segments).map(Some)
         }
     }
 }

core/lib/src/request/from_request.rs

@@ -84,8 +84,7 @@ pub type Outcome<S, E> = outcome::Outcome<S, (Status, E), Status>;
 ///   If the `Outcome` is [`Error`], the request will fail with the given
 ///   status code and error. The designated error [`Catcher`](crate::Catcher)
 ///   will be used to respond to the request. Note that users can request types
-///   of `Result<S, E>` and `Option<S>` to catch `Error`s and retrieve the
-///   error value.
+///   of `Result<S, E>` to catch `Error`s and retrieve the error value.
 ///
 /// * **Forward**(Status)
 ///
@@ -177,9 +176,9 @@ pub type Outcome<S, E> = outcome::Outcome<S, (Status, E), Status>;
 ///
 ///     The type `T` is derived from the incoming request using `T`'s
 ///     `FromRequest` implementation. If the derivation is a `Success`, the
-///     derived value is returned in `Some`. Otherwise, a `None` is returned.
-///
-///     _This implementation always returns successfully._
+///     derived value is returned in `Some`. If the derivation is a `Forward`,
+///     the result is `None`, and if the derivation is an `Error`, the `Error`
+///     is preserved.
 ///
 ///   * **Result&lt;T, T::Error>** _where_ **T: FromRequest**
 ///
@@ -189,6 +188,14 @@ pub type Outcome<S, E> = outcome::Outcome<S, (Status, E), Status>;
 ///     returned in `Err`. If the derivation is a `Forward`, the request is
 ///     forwarded with the same status code as the original forward.
 ///
+///   * **Outcome&lt;T, T::Error>** _where_ **T: FromRequest**
+///
+///     The type `T` is derived from the incoming request using `T`'s
+///     `FromRequest` implementation. The `Outcome` is then provided to the handler,
+///     reguardless of what it returned.
+///
+///     _This guard **always** succeeds_
+///
 /// [`Config`]: crate::config::Config
 ///
 /// # Example
@@ -521,12 +528,13 @@ impl<'r, T: FromRequest<'r>> FromRequest<'r> for Result<T, T::Error> {
 
 #[crate::async_trait]
 impl<'r, T: FromRequest<'r>> FromRequest<'r> for Option<T> {
-    type Error = Infallible;
+    type Error = T::Error;
 
-    async fn from_request(request: &'r Request<'_>) -> Outcome<Self, Infallible> {
+    async fn from_request(request: &'r Request<'_>) -> Outcome<Self, Self::Error> {
         match T::from_request(request).await {
             Success(val) => Success(Some(val)),
-            Error(_) | Forward(_) => Success(None),
+            Forward(_) => Success(None),
+            Error(e) => Error(e)
         }
     }
 }

core/lib/tests/flash-lazy-removes-issue-466.rs

@@ -11,13 +11,13 @@ fn set() -> Flash<&'static str> {
 }
 
 #[get("/unused")]
-fn unused(flash: Option<FlashMessage<'_>>) -> Option<()> {
-    flash.map(|_| ())
+fn unused(flash: Result<FlashMessage<'_>, ()>) -> Option<()> {
+    flash.ok().map(|_| ())
 }
 
 #[get("/use")]
-fn used(flash: Option<FlashMessage<'_>>) -> Option<String> {
-    flash.map(|f| f.message().into())
+fn used(flash: Result<FlashMessage<'_>, ()>) -> Option<String> {
+    flash.ok().map(|f| f.message().into())
 }
 
 mod flash_lazy_remove_tests {