clean up code. fix bugs with route and method attributes with parameters

actix-web-codegen/src/lib.rs

@@ -254,70 +254,19 @@ pub fn test(_: TokenStream, item: TokenStream) -> TokenStream {
 ///
 /// # Example
 ///
-/// scope("/test")
 /// ```rust
 /// use actix_web_codegen::{scope};
 ///
-/// const mod_inner: () = {
+/// #[scope("/test")]
-/// use actix_web::{HttpResponse, HttpRequest, Responder, put, head, connect, options, trace, patch, delete, web };
+/// mod scope_module {
-/// use actix_web::web::Json;
+///     use actix_web::{get, HttpResponse, Responder};
-///
+/// 
-/// #[actix_web::get("/test")]
+///     #[get("/test")]
-/// pub async fn test() -> impl Responder {
+///     pub async fn test() -> impl Responder {
-///    mod_test2()
+///         // this has path /test/test
+///         HttpResponse::Ok().finish()
+///     }
 /// }
-///
-/// #[actix_web::get("/twicetest/{value}")]
-/// pub async fn test_twice(value: web::Path<String>) -> impl actix_web::Responder {
-///    let int_value: i32 = value.parse().unwrap_or(0);
-///    let doubled = int_value * 2;
-///    HttpResponse::Ok().body(format!("Twice value: {}", doubled))
-/// }
-///
-/// #[actix_web::post("/test")]
-/// pub async fn test_post() -> impl Responder {
-///    HttpResponse::Ok().body(format!("post works"))
-/// }    
-///
-/// #[put("/test")]
-/// pub async fn test_put() -> impl Responder {
-///    HttpResponse::Ok().body(format!("put works"))
-/// }    
-///
-/// #[head("/test")]
-/// pub async fn test_head() -> impl Responder {
-///    HttpResponse::Ok().finish()
-/// }
-///
-/// #[connect("/test")]
-/// pub async fn test_connect() -> impl Responder {
-///    HttpResponse::Ok().body("CONNECT works")
-/// }
-///
-/// #[options("/test")]
-/// pub async fn test_options() -> impl Responder {
-///    HttpResponse::Ok().body("OPTIONS works")
-/// }
-///
-/// #[trace("/test")]
-/// pub async fn test_trace(req: HttpRequest) -> impl Responder {
-///    HttpResponse::Ok().body(format!("TRACE works: {:?}", req))
-/// }
-///
-/// #[patch("/test")]
-/// pub async fn test_patch() -> impl Responder {
-///    HttpResponse::Ok().body(format!("patch works"))
-/// }
-///
-/// #[delete("/test")]
-/// pub async fn test_delete() -> impl Responder {
-///    HttpResponse::Ok().body("DELETE works")
-/// }
-///
-/// pub fn mod_test2() -> impl actix_web::Responder {
-///    actix_web::HttpResponse::Ok().finish()
-/// }
-///};
 /// ```
 ///
 #[proc_macro_attribute]

actix-web-codegen/src/route.rs

@@ -4,8 +4,7 @@ use actix_router::ResourceDef;
 use proc_macro::TokenStream;
 use proc_macro2::{Span, TokenStream as TokenStream2};
 use quote::{quote, ToTokens, TokenStreamExt};
-use syn::Item::Fn;
+use syn::{punctuated::Punctuated, Ident, LitStr, Path, Token};
-use syn::{punctuated::Punctuated, Ident, ItemFn, LitStr, Path, Token}; // todo: cleanup
 
 #[derive(Debug)]
 pub struct RouteArgs {
@@ -332,14 +331,14 @@ pub struct Route {
     args: Vec<Args>,
 
     /// AST of the handler function being annotated.
-    ast: ItemFn,
+    ast: syn::ItemFn,
 
     /// The doc comment attributes to copy to generated struct, if any.
     doc_attributes: Vec<syn::Attribute>,
 }
 
 impl Route {
-    pub fn new(args: RouteArgs, ast: ItemFn, method: Option<MethodType>) -> syn::Result<Self> {
+    pub fn new(args: RouteArgs, ast: syn::ItemFn, method: Option<MethodType>) -> syn::Result<Self> {
         let name = ast.sig.ident.clone();
 
         // Try and pull out the doc comments so that we can reapply them to the generated struct.
@@ -375,7 +374,7 @@ impl Route {
         })
     }
 
-    fn multiple(args: Vec<Args>, ast: ItemFn) -> syn::Result<Self> {
+    fn multiple(args: Vec<Args>, ast: syn::ItemFn) -> syn::Result<Self> {
         let name = ast.sig.ident.clone();
 
         // Try and pull out the doc comments so that we can reapply them to the generated struct.
@@ -484,7 +483,7 @@ pub(crate) fn with_method(
         Err(err) => return input_and_compile_error(input, err),
     };
 
-    let ast = match syn::parse::<ItemFn>(input.clone()) {
+    let ast = match syn::parse::<syn::ItemFn>(input.clone()) {
         Ok(ast) => ast,
         // on parse error, make IDEs happy; see fn docs
         Err(err) => return input_and_compile_error(input, err),
@@ -498,7 +497,7 @@ pub(crate) fn with_method(
 }
 
 pub(crate) fn with_methods(input: TokenStream) -> TokenStream {
-    let mut ast = match syn::parse::<ItemFn>(input.clone()) {
+    let mut ast = match syn::parse::<syn::ItemFn>(input.clone()) {
         Ok(ast) => ast,
         // on parse error, make IDEs happy; see fn docs
         Err(err) => return input_and_compile_error(input, err),
@@ -556,15 +555,6 @@ fn input_and_compile_error(mut item: TokenStream, err: syn::Error) -> TokenStrea
     item
 }
 
-// todo: regular new scoped path test...
-// todo: test with enums in module...
-// todo: test with different namespaces...
-// todo: test with multiple get/post methods...
-// todo: test with doc strings in attributes...
-// todo: clean up the nested tree of death, tell it about the input_and_compile_error convenience function...
-// todo: add in the group functions for convenient handling of group...
-// todo: see if can cleanup the Attribute with a clone plus one field change...
-// todo: #[actix_web::get("/test")] the namespace is messing matching up
 pub fn with_scope(args: TokenStream, input: TokenStream) -> TokenStream {
     // Attempt to parse the scope path, returning on error
     if args.is_empty() {
@@ -576,7 +566,7 @@ pub fn with_scope(args: TokenStream, input: TokenStream) -> TokenStream {
             ),
         );
     }
-    let scope_path = syn::parse::<LitStr>(args.clone().into());
+    let scope_path = syn::parse::<LitStr>(args.clone());
     if let Err(_err) = scope_path {
         return input_and_compile_error(
             args.clone(),
@@ -590,10 +580,10 @@ pub fn with_scope(args: TokenStream, input: TokenStream) -> TokenStream {
     // Expect macro to be for a module
     match syn::parse::<syn::ItemMod>(input) {
         Ok(mut ast) => {
-            // Modify the attributes of functions within the module with method with scope, if any
+            // Modify the attributes of functions with method or route(s) by adding scope argument as prefix, if any
             if let Some((_, ref mut items)) = ast.content {
                 items.iter_mut().for_each(|item| {
-                    if let Fn(fun) = item {
+                    if let syn::Item::Fn(fun) = item {
                         fun.attrs = fun
                             .attrs
                             .iter()
@@ -615,19 +605,37 @@ pub fn with_scope(args: TokenStream, input: TokenStream) -> TokenStream {
     }
 }
 
+fn has_allowed_methods_in_scope(attr: &syn::Attribute) -> bool {
+    MethodType::from_path(attr.path()).is_ok()
+        || attr.path().is_ident("route")
+        || attr.path().is_ident("ROUTE")
+}
+
 // Check if the attribute is a method type and has a route path, then modify it
 fn modify_attribute_with_scope(attr: &syn::Attribute, scope_path: &str) -> syn::Attribute {
-    match (
+    match (attr.parse_args::<RouteArgs>(), attr.clone().meta) {
-        MethodType::from_path(attr.path()),
+        (Ok(route_args), syn::Meta::List(meta_list)) if has_allowed_methods_in_scope(attr) => {
-        attr.parse_args::<RouteArgs>(),
-        attr.clone().meta,
-    ) {
-        (Ok(_method), Ok(route_args), syn::Meta::List(meta_list)) => {
             let modified_path = format!("{}{}", scope_path, route_args.path.value());
-            let modified_tokens = quote! { #modified_path };
+
-            syn::Attribute {
+            let options_tokens: Vec<TokenStream2> = route_args
+                .options
+                .iter()
+                .map(|option| {
+                    quote! { ,#option }
+                })
+                .collect();
+
+            let combined_options_tokens: TokenStream2 =
+                options_tokens
+                    .into_iter()
+                    .fold(TokenStream2::new(), |mut acc, ts| {
+                        acc.extend(std::iter::once(ts));
+                        acc
+                    });
+
+            return syn::Attribute {
                 meta: syn::Meta::List(syn::MetaList {
-                    tokens: modified_tokens,
+                    tokens: quote! { #modified_path #combined_options_tokens },
                     ..meta_list.clone()
                 }),
                 ..attr.clone()

actix-web-codegen/tests/test_macro.rs

@@ -6,7 +6,6 @@ use actix_web::{
     http::{
         self,
         header::{HeaderName, HeaderValue},
-        StatusCode,
     },
     web, App, Error, HttpRequest, HttpResponse, Responder,
 };
@@ -371,26 +370,16 @@ async fn test_auto_async() {
     let response = request.send().await.unwrap();
     assert!(response.status().is_success());
 }
-/*
-#[actix_web::test]
-async fn test_wrap() {
-    let srv = actix_test::start(|| App::new().service(get_wrap));
 
-    let request = srv.request(http::Method::GET, srv.url("/test/wrap"));
-    let mut response = request.send().await.unwrap();
-    assert_eq!(response.status(), StatusCode::NOT_FOUND);
-    assert!(response.headers().contains_key("custom-header"));
-    let body = response.body().await.unwrap();
-    let body = String::from_utf8(body.to_vec()).unwrap();
-    assert!(body.contains("wrong number of parameters"));
-}
-*/
 #[scope("/test")]
 mod scope_module {
-    use actix_web::{
+    use crate::guard_module;
-        get, post, connect, delete, head, options, patch, put, trace, web, HttpRequest,
+    use actix_web::{delete, get, post, route, routes, web, HttpResponse, Responder};
-        HttpResponse, Responder,
+
-    };
+    #[get("/test/guard", guard = "guard_module::guard")]
+    pub async fn test_guard() -> impl Responder {
+        HttpResponse::Ok()
+    }
 
     #[get("/test")]
     pub async fn test() -> impl Responder {
@@ -409,67 +398,59 @@ mod scope_module {
         HttpResponse::Ok().body(format!("post works"))
     }
 
-    #[put("/test")]
-    pub async fn test_put() -> impl Responder {
-        HttpResponse::Ok().body(format!("put works"))
-    }
-
-    #[head("/test")]
-    pub async fn test_head() -> impl Responder {
-        HttpResponse::Ok().finish()
-    }
-
-    #[connect("/test")]
-    pub async fn test_connect() -> impl Responder {
-        HttpResponse::Ok().body("connect works")
-    }
-
-    #[options("/test")]
-    pub async fn test_options() -> impl Responder {
-        HttpResponse::Ok().body("options works")
-    }
-
-    #[trace("/test")]
-    pub async fn test_trace(req: HttpRequest) -> impl Responder {
-        HttpResponse::Ok().body(format!("trace works: {:?}", req))
-    }
-
-    #[patch("/test")]
-    pub async fn test_patch() -> impl Responder {
-        HttpResponse::Ok().body(format!("patch works"))
-    }
-
     #[delete("/test")]
     pub async fn test_delete() -> impl Responder {
         HttpResponse::Ok().body("delete works")
     }
 
-    pub fn mod_test2() -> impl actix_web::Responder {
+    #[route("/test", method = "PUT", method = "PATCH", method = "CUSTOM")]
-        actix_web::HttpResponse::Ok().finish()
+    pub async fn test_multiple_shared_path() -> impl Responder {
+        HttpResponse::Ok().finish()
     }
 
+    #[routes]
+    #[head("/test")]
+    #[connect("/test")]
+    #[options("/test")]
+    #[trace("/test")]
+    async fn test_multiple_separate_paths() -> impl Responder {
+        HttpResponse::Ok().finish()
+    }
+
+    // test calling this from other mod scope with scope attribute...
     pub fn mod_common(message: String) -> impl actix_web::Responder {
         HttpResponse::Ok().body(message)
     }
 }
 
 #[scope("/v1")]
-mod mod_inner_v1 {
+mod mod_scope_v1 {
     use actix_web::{get, Responder};
 
     #[get("/test")]
+    #[doc = "doc string to check in cargo expand"]
     pub async fn test() -> impl Responder {
         super::scope_module::mod_common("version1 works".to_string())
     }
 }
 
 #[scope("/v2")]
-mod mod_inner_v2 {
+mod mod_scope_v2 {
     use actix_web::{get, Responder};
 
+    // check to make sure non-function tokens in the scope block are preserved...
+    enum TestEnum {
+        Works,
+    }
+
     #[get("/test")]
     pub async fn test() -> impl Responder {
-        super::scope_module::mod_common("version2 works".to_string())
+        // make sure this type still exists...
+        let test_enum = TestEnum::Works;
+
+        match test_enum {
+            TestEnum::Works => super::scope_module::mod_common("version2 works".to_string()),
+        }
     }
 }
 
@@ -505,67 +486,37 @@ async fn test_scope_post_async() {
 }
 
 #[actix_rt::test]
-async fn test_scope_put_async() {
+async fn test_multiple_shared_path_async() {
-    let srv = actix_test::start(|| App::new().service(scope_module::test_put));
+    let srv = actix_test::start(|| App::new().service(scope_module::test_multiple_shared_path));
 
     let request = srv.request(http::Method::PUT, srv.url("/test/test"));
-    let mut response = request.send().await.unwrap();
+    let response = request.send().await.unwrap();
-    let body = response.body().await.unwrap();
+    assert!(response.status().is_success());
-    let body_str = String::from_utf8(body.to_vec()).unwrap();
-    assert_eq!(body_str, "put works");
-}
 
-#[actix_rt::test]
+    let request = srv.request(http::Method::PATCH, srv.url("/test/test"));
-async fn test_scope_head_async() {
-    let srv = actix_test::start(|| App::new().service(scope_module::test_head));
-
-    let request = srv.request(http::Method::HEAD, srv.url("/test/test"));
     let response = request.send().await.unwrap();
     assert!(response.status().is_success());
 }
 
 #[actix_rt::test]
-async fn test_scope_connect_async() {
+async fn test_multiple_multipaths_async() {
-    let srv = actix_test::start(|| App::new().service(scope_module::test_connect));
+    let srv = actix_test::start(|| App::new().service(scope_module::test_multiple_separate_paths));
 
     let request = srv.request(http::Method::CONNECT, srv.url("/test/test"));
-    let mut response = request.send().await.unwrap();
+    let response = request.send().await.unwrap();
-    let body = response.body().await.unwrap();
+    assert!(response.status().is_success());
-    let body_str = String::from_utf8(body.to_vec()).unwrap();
-    assert_eq!(body_str, "connect works");
-}
-
-#[actix_rt::test]
-async fn test_scope_options_async() {
-    let srv = actix_test::start(|| App::new().service(scope_module::test_options));
 
     let request = srv.request(http::Method::OPTIONS, srv.url("/test/test"));
-    let mut response = request.send().await.unwrap();
+    let response = request.send().await.unwrap();
-    let body = response.body().await.unwrap();
+    assert!(response.status().is_success());
-    let body_str = String::from_utf8(body.to_vec()).unwrap();
-    assert_eq!(body_str, "options works");
-}
-
-#[actix_rt::test]
-async fn test_scope_trace_async() {
-    let srv = actix_test::start(|| App::new().service(scope_module::test_trace));
 
     let request = srv.request(http::Method::TRACE, srv.url("/test/test"));
-    let mut response = request.send().await.unwrap();
+    let response = request.send().await.unwrap();
-    let body = response.body().await.unwrap();
+    assert!(response.status().is_success());
-    let body_str = String::from_utf8(body.to_vec()).unwrap();
-    assert!(body_str.contains("trace works"));
-}
 
-#[actix_rt::test]
+    let request = srv.request(http::Method::HEAD, srv.url("/test/test"));
-async fn test_scope_patch_async() {
+    let response = request.send().await.unwrap();
-    let srv = actix_test::start(|| App::new().service(scope_module::test_patch));
+    assert!(response.status().is_success());
-
-    let request = srv.request(http::Method::PATCH, srv.url("/test/test"));
-    let mut response = request.send().await.unwrap();
-    let body = response.body().await.unwrap();
-    let body_str = String::from_utf8(body.to_vec()).unwrap();
-    assert_eq!(body_str, "patch works");
 }
 
 #[actix_rt::test]
@@ -579,9 +530,24 @@ async fn test_scope_delete_async() {
     assert_eq!(body_str, "delete works");
 }
 
+#[actix_rt::test]
+async fn test_scope_get_with_guard_async() {
+    let srv = actix_test::start(|| App::new().service(scope_module::test_guard));
+
+    let request = srv
+        .request(http::Method::GET, srv.url("/test/test/guard"))
+        .insert_header(("Accept", "image/*"));
+    let response = request.send().await.unwrap();
+    assert!(response.status().is_success());
+}
+
 #[actix_rt::test]
 async fn test_scope_v1_v2_async() {
-    let srv = actix_test::start(|| App::new().service(mod_inner_v1::test).service(mod_inner_v2::test));
+    let srv = actix_test::start(|| {
+        App::new()
+            .service(mod_scope_v1::test)
+            .service(mod_scope_v2::test)
+    });
 
     let request = srv.request(http::Method::GET, srv.url("/v1/test"));
     let mut response = request.send().await.unwrap();