Discover sentinels in known type macros.

Resolves #1657.

core/codegen/src/attribute/route/mod.rs

@@ -274,10 +274,11 @@ fn sentinels_expr(route: &Route) -> TokenStream {
     //      * returns `true` for the parent, and so the type has a parent, and
     //      the theorem holds.
     //    3. these are all the cases. QED.
+    const TYPE_MACROS: &[&str] = &["ReaderStream", "TextStream", "ByteStream", "EventStream"];
     let eligible_types = route.guards()
         .map(|guard| &guard.ty)
         .chain(ret_ty.as_ref().into_iter())
-        .flat_map(|ty| ty.unfold())
+        .flat_map(|ty| ty.unfold_with_known_macros(TYPE_MACROS))
         .filter(|ty| ty.is_concrete(&generic_idents))
         .map(|child| (child.parent, child.ty));
 

core/codegen/src/syn_ext.rs

@@ -2,9 +2,12 @@
 
 use std::ops::Deref;
 use std::hash::{Hash, Hasher};
+use std::borrow::Cow;
 
 use syn::{self, Ident, ext::IdentExt as _, visit::Visit};
 use proc_macro2::{Span, TokenStream};
+use devise::ext::PathExt;
+use rocket_http::ext::IntoOwned;
 
 pub trait IdentExt {
     fn prepend(&self, string: &str) -> syn::Ident;
@@ -29,8 +32,8 @@ pub trait FnArgExt {
 
 #[derive(Debug)]
 pub struct Child<'a> {
-    pub parent: Option<&'a syn::Type>,
+    pub parent: Option<Cow<'a, syn::Type>>,
-    pub ty: &'a syn::Type,
+    pub ty: Cow<'a, syn::Type>,
 }
 
 impl Deref for Child<'_> {
@@ -41,8 +44,20 @@ impl Deref for Child<'_> {
     }
 }
 
+impl IntoOwned for Child<'_> {
+    type Owned = Child<'static>;
+
+    fn into_owned(self) -> Self::Owned {
+        Child {
+            parent: self.parent.into_owned(),
+            ty: Cow::Owned(self.ty.into_owned()),
+        }
+    }
+}
+
 pub trait TypeExt {
     fn unfold(&self) -> Vec<Child<'_>>;
+    fn unfold_with_known_macros(&self, known_macros: &[&str]) -> Vec<Child<'_>>;
     fn is_concrete(&self, generic_ident: &[&Ident]) -> bool;
 }
 
@@ -122,24 +137,58 @@ impl FnArgExt for syn::FnArg {
     }
 }
 
-impl TypeExt for syn::Type {
+fn known_macro_inner_ty(t: &syn::TypeMacro, known: &[&str]) -> Option<syn::Type> {
-    fn unfold(&self) -> Vec<Child<'_>> {
+    if !known.iter().any(|k| t.mac.path.last_ident().map_or(false, |i| i == k)) {
-        #[derive(Default)]
+        return None;
-        struct Visitor<'a> {
-            parents: Vec<&'a syn::Type>,
-            children: Vec<Child<'a>>,
     }
 
-        impl<'a> Visit<'a> for Visitor<'a> {
+    syn::parse2(t.mac.tokens.clone()).ok()
+}
+
+impl TypeExt for syn::Type {
+    fn unfold(&self) -> Vec<Child<'_>> {
+        self.unfold_with_known_macros(&[])
+    }
+
+    fn unfold_with_known_macros<'a>(&'a self, known_macros: &[&str]) -> Vec<Child<'a>> {
+        struct Visitor<'a, 'm> {
+            parents: Vec<Cow<'a, syn::Type>>,
+            children: Vec<Child<'a>>,
+            known_macros: &'m [&'m str],
+        }
+
+        impl<'m> Visitor<'_, 'm> {
+            fn new(known_macros: &'m [&'m str]) -> Self {
+                Visitor { parents: vec![], children: vec![], known_macros }
+            }
+        }
+
+        impl<'a> Visit<'a> for Visitor<'a, '_> {
             fn visit_type(&mut self, ty: &'a syn::Type) {
-                self.children.push(Child { parent: self.parents.last().cloned(), ty });
+                let parent = self.parents.last().cloned();
-                self.parents.push(ty);
+
+                if let syn::Type::Macro(t) = ty {
+                    if let Some(inner_ty) = known_macro_inner_ty(t, self.known_macros) {
+                        let mut visitor = Visitor::new(self.known_macros);
+                        if let Some(parent) = parent.clone().into_owned() {
+                            visitor.parents.push(parent);
+                        }
+
+                        visitor.visit_type(&inner_ty);
+                        let mut children = visitor.children.into_owned();
+                        self.children.append(&mut children);
+                        return;
+                    }
+                }
+
+                self.children.push(Child { parent, ty: Cow::Borrowed(ty) });
+                self.parents.push(Cow::Borrowed(ty));
                 syn::visit::visit_type(self, ty);
                 self.parents.pop();
             }
         }
 
-        let mut visitor = Visitor::default();
+        let mut visitor = Visitor::new(known_macros);
         visitor.visit_type(self);
         visitor.children
     }

core/lib/src/sentinel.rs

@@ -114,32 +114,35 @@ use crate::{Rocket, Ignite};
 /// A type, whether embedded or not, is queried if it is a `Sentinel` _and_ none
 /// of its parent types are sentinels. Said a different way, if every _directly_
 /// eligible type is viewed as the root of an acyclic graph with edges between a
-/// type and its type parameters, the _first_ `Sentinel` in each graph, in
+/// type and its type parameters, the _first_ `Sentinel` in breadth-first order
-/// breadth-first order, is queried:
+/// is queried:
 ///
 /// ```text
-///   Option<&State<String>>           Either<Foo, Inner<Bar>>
+/// 1.     Option<&State<String>>        Either<Foo, Inner<Bar>>
 ///                 |                           /         \
-///      &State<String>                      Foo     Inner<Bar>
+/// 2.        &State<String>                   Foo     Inner<Bar>
 ///                 |                                     |
-///       State<String>                                 Bar
+/// 3.         State<String>                              Bar
 ///                 |
-///          String
+/// 4.            String
 /// ```
 ///
-/// Neither `Option` nor `Either` are sentinels, so they won't be queried. In
+/// In each graph above, types are queried from top to bottom, level 1 to 4.
-/// the next level, `&State` is a `Sentinel`, so it _is_ queried. If `Foo` is a
+/// Querying continues down paths where the parents were _not_ sentinels. For
-/// sentinel, it is queried as well. If `Inner` is a sentinel, it is queried,
+/// example, if `Option` is a sentinel but `Either` is not, then querying stops
-/// and traversal stops without considering `Bar`. If `Inner` is _not_ a
+/// for the left subgraph (`Option`) but continues for the right subgraph
-/// `Sentinel`, `Bar` is considered and queried if it is a sentinel.
+/// `Either`.
 ///
 /// # Limitations
 ///
 /// Because Rocket must know which `Sentinel` implementation to query based on
-/// its _written_ type, only explicitly written, resolved, concrete types are
+/// its _written_ type, generally only explicitly written, resolved, concrete
-/// eligible to be sentinels. Most application will only work with such types,
+/// types are eligible to be sentinels. A typical application will only work
-/// but occasionally an existential `impl Trait` may find its way into return
+/// with such types, but there are several common cases to be aware of.
-/// types:
+///
+/// ## `impl Trait`
+///
+/// Occasionally an existential `impl Trait` may find its way into return types:
 ///
 /// ```rust
 /// # use rocket::*;
@@ -162,8 +165,10 @@ use crate::{Rocket, Ignite};
 /// it is not possible to name the underlying concrete type of an `impl Trait`
 /// at compile-time and thus not possible to determine if it implements
 /// `Sentinel`. As such, existentials _are not_ eligible to be sentinels.
-/// However, this limitation applies per embedding, so the inner, directly named
+///
-/// `AnotherSentinel` type continues to be eligible to be a sentinel.
+/// That being said, this limitation only applies _per embedding_: types
+/// embedded inside of an `impl Trait` _are_ eligible. As such, in the example
+/// above, the named `AnotherSentinel` type continues to be eligible.
 ///
 /// When possible, prefer to name all types:
 ///
@@ -181,11 +186,10 @@ use crate::{Rocket, Ignite};
 ///
 /// ## Aliases
 ///
-/// Embedded discovery of sentinels is syntactic in nature: an embedded sentinel
+/// _Embedded_ sentinels made opaque by a type alias will fail to be considered;
-/// is only discovered if its named in the type. As such, embedded sentinels
+/// the aliased type itself _is_ considered. In the example below, only
-/// made opaque by a type alias will fail to be considered. In the example
+/// `Result<Foo, Bar>` will be considered, while the embedded `Foo` and `Bar`
-/// below, only `Result<Foo, Bar>` will be considered, while the embedded `Foo`
+/// will not.
-/// and `Bar` will not.
 ///
 /// ```rust
 /// # use rocket::get;
@@ -230,16 +234,23 @@ use crate::{Rocket, Ignite};
 /// sentinel, the macro actually expands to `&'_ rocket::Config`, which is _not_
 /// the `State` sentinel.
 ///
-/// You should prefer not to use type macros, or if necessary, restrict your use
+/// Because Rocket knows the exact syntax expected by type macros that it
-/// to those that always expand to types without sentinels.
+/// exports, such as the [typed stream] macros, discovery in these macros works
+/// as expected. You should prefer not to use type macros aside from those
+/// exported by Rocket, or if necessary, restrict your use to those that always
+/// expand to types without sentinels.
+///
+/// [typed stream]: crate::response::stream
 ///
 /// # Custom Sentinels
 ///
 /// Any type can implement `Sentinel`, and the implementation can arbitrarily
-/// inspect the passed in instance of `Rocket`. For illustration, consider the
+/// inspect an ignited instance of `Rocket`. For illustration, consider the
 /// following implementation of `Sentinel` for a custom `Responder` which
-/// requires state for a type `T` to be managed as well as catcher for status
+/// requires:
-/// code `400` at base `/`:
+///
+///   * state for a type `T` to be managed
+///   * a catcher for status code `400` at base `/`
 ///
 /// ```rust
 /// use rocket::{Rocket, Ignite, Sentinel};
@@ -262,7 +273,8 @@ use crate::{Rocket, Ignite};
 /// ```
 ///
 /// If a `MyResponder` is returned by any mounted route, its `abort()` method
-/// will be invoked, and launch will be aborted if the method returns `true`.
+/// will be invoked. If the required conditions aren't met, signaled by
+/// returning `true` from `abort()`, Rocket aborts launch.
 pub trait Sentinel {
     /// Returns `true` if launch should be aborted and `false` otherwise.
     fn abort(rocket: &Rocket<Ignite>) -> bool;

core/lib/tests/sentinel.rs

@@ -234,3 +234,68 @@ fn inner_sentinels_detected() {
     #[get("/")] fn either_route4() -> Either<(), ()> { todo!() }
     Client::debug_with(routes![either_route4]).expect("no sentinel error");
 }
+
+#[async_test]
+async fn known_macro_sentinel_works() {
+    use rocket::response::stream::{TextStream, ByteStream, ReaderStream};
+    use rocket::local::asynchronous::Client;
+    use rocket::tokio::io::AsyncRead;
+
+    #[derive(Responder)]
+    struct TextSentinel(&'static str);
+
+    impl Sentinel for TextSentinel {
+        fn abort(_: &Rocket<Ignite>) -> bool {
+            true
+        }
+    }
+
+    impl AsRef<str> for TextSentinel {
+        fn as_ref(&self) -> &str {
+            self.0
+        }
+    }
+
+    impl AsRef<[u8]> for TextSentinel {
+        fn as_ref(&self) -> &[u8] {
+            self.0.as_bytes()
+        }
+    }
+
+    impl AsyncRead for TextSentinel {
+        fn poll_read(
+            self: std::pin::Pin<&mut Self>,
+            _: &mut futures::task::Context<'_>,
+            _: &mut tokio::io::ReadBuf<'_>,
+        ) -> futures::task::Poll<std::io::Result<()>> {
+            futures::task::Poll::Ready(Ok(()))
+        }
+    }
+
+    #[get("/text")]
+    fn text() -> TextStream![TextSentinel] {
+        TextStream!(yield TextSentinel("hi");)
+    }
+
+    #[get("/bytes")]
+    fn byte() -> ByteStream![TextSentinel] {
+        ByteStream!(yield TextSentinel("hi");)
+    }
+
+    #[get("/reader")]
+    fn reader() -> ReaderStream![TextSentinel] {
+        ReaderStream!(yield TextSentinel("hi");)
+    }
+
+    macro_rules! UnknownStream {
+        ($t:ty) => (ReaderStream![$t])
+    }
+
+    #[get("/ignore")]
+    fn ignore() -> UnknownStream![TextSentinel] {
+        ReaderStream!(yield TextSentinel("hi");)
+    }
+
+    let err = Client::debug_with(routes![text, byte, reader, ignore]).await.unwrap_err();
+    assert!(matches!(err.kind(), SentinelAborts(vec) if vec.len() == 3));
+}