Provide attribute macro for multiple HTTP methods (actix#1674)

Co-authored-by: Rob Ede <[email protected]>

actix-web-codegen/CHANGES.md

@@ -2,8 +2,10 @@
 
 ## Unreleased - 2020-xx-xx
 * Added compile success and failure testing. [#1677]
+* Add `route` macro for supporting multiple HTTP methods guards.
 
 [#1677]: https://github.com/actix/actix-web/pull/1677
+[#1674]: https://github.com/actix/actix-web/pull/1674
 
 
 ## 0.3.0 - 2020-09-11

actix-web-codegen/Cargo.toml

@@ -23,3 +23,4 @@ actix-rt = "1.0.0"
 actix-web = "3.0.0"
 futures-util = { version = "0.3.5", default-features = false }
 trybuild = "1"
+rustversion = "1"

actix-web-codegen/src/lib.rs

@@ -141,6 +141,23 @@ pub fn patch(args: TokenStream, input: TokenStream) -> TokenStream {
     route::generate(args, input, route::GuardType::Patch)
 }
 
+/// Creates resource handler, allowing multiple HTTP method guards.
+///
+/// Syntax: `#[route("path"[, attributes])]`
+///
+/// Example: `#[route("/", method="GET", method="HEAD")]`
+///
+/// ## Attributes
+///
+/// - `"path"` - Raw literal string with path for which to register handler. Mandatory.
+/// - `method="HTTP_METHOD"` - Registers HTTP method to provide guard for.
+/// - `guard="function_name"` - Registers function as guard using `actix_web::guard::fn_guard`
+/// - `wrap="Middleware"` - Registers a resource middleware.
+#[proc_macro_attribute]
+pub fn route(args: TokenStream, input: TokenStream) -> TokenStream {
+    route::generate(args, input, route::GuardType::Multi)
+}
+
 /// Marks async main function as the actix system entry-point.
 ///
 /// ## Usage

actix-web-codegen/src/route.rs

@@ -1,5 +1,8 @@
 extern crate proc_macro;
 
+use std::collections::HashSet;
+use std::convert::TryFrom;
+
 use proc_macro::TokenStream;
 use proc_macro2::{Span, TokenStream as TokenStream2};
 use quote::{format_ident, quote, ToTokens, TokenStreamExt};
@@ -17,7 +20,7 @@ impl ToTokens for ResourceType {
     }
 }
 
-#[derive(PartialEq)]
+#[derive(Debug, PartialEq, Eq, Hash)]
 pub enum GuardType {
     Get,
     Post,
@@ -28,6 +31,7 @@ pub enum GuardType {
     Options,
     Trace,
     Patch,
+    Multi,
 }
 
 impl GuardType {
@@ -42,6 +46,7 @@ impl GuardType {
             GuardType::Options => "Options",
             GuardType::Trace => "Trace",
             GuardType::Patch => "Patch",
+            GuardType::Multi => "Multi",
         }
     }
 }
@@ -53,17 +58,41 @@ impl ToTokens for GuardType {
     }
 }
 
+impl TryFrom<&syn::LitStr> for GuardType {
+    type Error = syn::Error;
+
+    fn try_from(value: &syn::LitStr) -> Result<Self, Self::Error> {
+        match value.value().as_str() {
+            "CONNECT" => Ok(GuardType::Connect),
+            "DELETE" => Ok(GuardType::Delete),
+            "GET" => Ok(GuardType::Get),
+            "HEAD" => Ok(GuardType::Head),
+            "OPTIONS" => Ok(GuardType::Options),
+            "PATCH" => Ok(GuardType::Patch),
+            "POST" => Ok(GuardType::Post),
+            "PUT" => Ok(GuardType::Put),
+            "TRACE" => Ok(GuardType::Trace),
+            _ => Err(syn::Error::new_spanned(
+                value,
+                &format!("Unexpected HTTP Method: `{}`", value.value()),
+            )),
+        }
+    }
+}
+
 struct Args {
     path: syn::LitStr,
     guards: Vec<Ident>,
     wrappers: Vec<syn::Type>,
+    methods: HashSet<GuardType>,
 }
 
 impl Args {
     fn new(args: AttributeArgs) -> syn::Result<Self> {
         let mut path = None;
         let mut guards = Vec::new();
         let mut wrappers = Vec::new();
+        let mut methods = HashSet::new();
         for arg in args {
             match arg {
                 NestedMeta::Lit(syn::Lit::Str(lit)) => match path {
@@ -96,10 +125,28 @@ impl Args {
                                 "Attribute wrap expects type",
                             ));
                         }
+                    } else if nv.path.is_ident("method") {
+                        if let syn::Lit::Str(ref lit) = nv.lit {
+                            let guard = GuardType::try_from(lit)?;
+                            if !methods.insert(guard) {
+                                return Err(syn::Error::new_spanned(
+                                    &nv.lit,
+                                    &format!(
+                                        "HTTP Method defined more than once: `{}`",
+                                        lit.value()
+                                    ),
+                                ));
+                            }
+                        } else {
+                            return Err(syn::Error::new_spanned(
+                                nv.lit,
+                                "Attribute method expects literal string!",
+                            ));
+                        }
                     } else {
                         return Err(syn::Error::new_spanned(
                             nv.path,
-                            "Unknown attribute key is specified. Allowed: guard and wrap",
+                            "Unknown attribute key is specified. Allowed: guard, method and wrap",
                         ));
                     }
                 }
@@ -112,6 +159,7 @@ impl Args {
             path: path.unwrap(),
             guards,
             wrappers,
+            methods,
         })
     }
 }
@@ -166,6 +214,13 @@ impl Route {
 
         let args = Args::new(args)?;
 
+        if guard == GuardType::Multi && args.methods.is_empty() {
+            return Err(syn::Error::new(
+                Span::call_site(),
+                "The #[route(..)] macro requires at least one `method` attribute",
+            ));
+        }
+
         let resource_type = if ast.sig.asyncness.is_some() {
             ResourceType::Async
         } else {
@@ -201,10 +256,29 @@ impl ToTokens for Route {
                     path,
                     guards,
                     wrappers,
+                    methods,
                 },
             resource_type,
         } = self;
         let resource_name = name.to_string();
+        let mut methods = methods.iter();
+
+        let method_guards = if *guard == GuardType::Multi {
+            // unwrapping since length is checked to be at least one
+            let first = methods.next().unwrap();
+
+            quote! {
+                .guard(
+                    actix_web::guard::Any(actix_web::guard::#first())
+                        #(.or(actix_web::guard::#methods()))*
+                )
+            }
+        } else {
+            quote! {
+                .guard(actix_web::guard::#guard())
+            }
+        };
+
         let stream = quote! {
             #[allow(non_camel_case_types, missing_docs)]
             pub struct #name;
@@ -214,7 +288,7 @@ impl ToTokens for Route {
                     #ast
                     let __resource = actix_web::Resource::new(#path)
                         .name(#resource_name)
-                        .guard(actix_web::guard::#guard())
+                        #method_guards
                         #(.guard(actix_web::guard::fn_guard(#guards)))*
                         #(.wrap(#wrappers))*
                         .#resource_type(#name);

actix-web-codegen/tests/test_macro.rs

@@ -5,7 +5,9 @@ use std::task::{Context, Poll};
 use actix_web::dev::{Service, ServiceRequest, ServiceResponse, Transform};
 use actix_web::http::header::{HeaderName, HeaderValue};
 use actix_web::{http, test, web::Path, App, Error, HttpResponse, Responder};
-use actix_web_codegen::{connect, delete, get, head, options, patch, post, put, trace};
+use actix_web_codegen::{
+    connect, delete, get, head, options, patch, post, put, route, trace,
+};
 use futures_util::future;
 
 // Make sure that we can name function as 'config'
@@ -79,6 +81,11 @@ async fn get_param_test(_: Path<String>) -> impl Responder {
     HttpResponse::Ok()
 }
 
+#[route("/multi", method = "GET", method = "POST", method = "HEAD")]
+async fn route_test() -> impl Responder {
+    HttpResponse::Ok()
+}
+
 pub struct ChangeStatusCode;
 
 impl<S, B> Transform<S> for ChangeStatusCode
@@ -172,6 +179,7 @@ async fn test_body() {
             .service(trace_test)
             .service(patch_test)
             .service(test_handler)
+            .service(route_test)
     });
     let request = srv.request(http::Method::GET, srv.url("/test"));
     let response = request.send().await.unwrap();
@@ -210,6 +218,22 @@ async fn test_body() {
     let request = srv.request(http::Method::GET, srv.url("/test"));
     let response = request.send().await.unwrap();
     assert!(response.status().is_success());
+
+    let request = srv.request(http::Method::GET, srv.url("/multi"));
+    let response = request.send().await.unwrap();
+    assert!(response.status().is_success());
+
+    let request = srv.request(http::Method::POST, srv.url("/multi"));
+    let response = request.send().await.unwrap();
+    assert!(response.status().is_success());
+
+    let request = srv.request(http::Method::HEAD, srv.url("/multi"));
+    let response = request.send().await.unwrap();
+    assert!(response.status().is_success());
+
+    let request = srv.request(http::Method::PATCH, srv.url("/multi"));
+    let response = request.send().await.unwrap();
+    assert!(!response.status().is_success());
 }
 
 #[actix_rt::test]

actix-web-codegen/tests/trybuild.rs

@@ -4,4 +4,24 @@ fn compile_macros() {
 
     t.pass("tests/trybuild/simple.rs");
     t.compile_fail("tests/trybuild/simple-fail.rs");
+
+    t.pass("tests/trybuild/route-ok.rs");
+    t.compile_fail("tests/trybuild/route-duplicate-method-fail.rs");
+    t.compile_fail("tests/trybuild/route-unexpected-method-fail.rs");
+
+    test_route_missing_method(&t)
+}
+
+#[rustversion::stable(1.42)]
+fn test_route_missing_method(t: &trybuild::TestCases) {
+    t.compile_fail("tests/trybuild/route-missing-method-fail-msrv.rs");
 }
+
+#[rustversion::not(stable(1.42))]
+#[rustversion::not(nightly)]
+fn test_route_missing_method(t: &trybuild::TestCases) {
+    t.compile_fail("tests/trybuild/route-missing-method-fail.rs");
+}
+
+#[rustversion::nightly]
+fn test_route_missing_method(_t: &trybuild::TestCases) {}

actix-web-codegen/tests/trybuild/route-duplicate-method-fail.rs

@@ -0,0 +1,15 @@
+use actix_web::*;
+
+#[route("/", method="GET", method="GET")]
+async fn index() -> impl Responder {
+    HttpResponse::Ok()
+}
+
+#[actix_web::main]
+async fn main() {
+    let srv = test::start(|| App::new().service(index));
+
+    let request = srv.get("/");
+    let response = request.send().await.unwrap();
+    assert!(response.status().is_success());
+}

actix-web-codegen/tests/trybuild/route-duplicate-method-fail.stderr

@@ -0,0 +1,11 @@
+error: HTTP Method defined more than once: `GET`
+ --> $DIR/route-duplicate-method-fail.rs:3:35
+  |
+3 | #[route("/", method="GET", method="GET")]
+  |                                   ^^^^^
+
+error[E0425]: cannot find value `index` in this scope
+  --> $DIR/route-duplicate-method-fail.rs:10:49
+   |
+10 |     let srv = test::start(|| App::new().service(index));
+   |                                                 ^^^^^ not found in this scope

actix-web-codegen/tests/trybuild/route-missing-method-fail-msrv.rs

@@ -0,0 +1,15 @@
+use actix_web::*;
+
+#[route("/")]
+async fn index() -> impl Responder {
+    HttpResponse::Ok()
+}
+
+#[actix_web::main]
+async fn main() {
+    let srv = test::start(|| App::new().service(index));
+
+    let request = srv.get("/");
+    let response = request.send().await.unwrap();
+    assert!(response.status().is_success());
+}

actix-web-codegen/tests/trybuild/route-missing-method-fail-msrv.stderr

@@ -0,0 +1,11 @@
+error: The #[route(..)] macro requires at least one `method` attribute
+ --> $DIR/route-missing-method-fail-msrv.rs:3:1
+  |
+3 | #[route("/")]
+  | ^^^^^^^^^^^^^
+
+error[E0425]: cannot find value `index` in this scope
+  --> $DIR/route-missing-method-fail-msrv.rs:10:49
+   |
+10 |     let srv = test::start(|| App::new().service(index));
+   |                                                 ^^^^^ not found in this scope

actix-web-codegen/tests/trybuild/route-missing-method-fail.rs

@@ -0,0 +1,15 @@
+use actix_web::*;
+
+#[route("/")]
+async fn index() -> impl Responder {
+    HttpResponse::Ok()
+}
+
+#[actix_web::main]
+async fn main() {
+    let srv = test::start(|| App::new().service(index));
+
+    let request = srv.get("/");
+    let response = request.send().await.unwrap();
+    assert!(response.status().is_success());
+}

actix-web-codegen/tests/trybuild/route-missing-method-fail.stderr

@@ -0,0 +1,13 @@
+error: The #[route(..)] macro requires at least one `method` attribute
+ --> $DIR/route-missing-method-fail.rs:3:1
+  |
+3 | #[route("/")]
+  | ^^^^^^^^^^^^^
+  |
+  = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error[E0425]: cannot find value `index` in this scope
+  --> $DIR/route-missing-method-fail.rs:10:49
+   |
+10 |     let srv = test::start(|| App::new().service(index));
+   |                                                 ^^^^^ not found in this scope

actix-web-codegen/tests/trybuild/route-ok.rs

@@ -0,0 +1,15 @@
+use actix_web::*;
+
+#[route("/", method="GET", method="HEAD")]
+async fn index() -> impl Responder {
+    HttpResponse::Ok()
+}
+
+#[actix_web::main]
+async fn main() {
+    let srv = test::start(|| App::new().service(index));
+
+    let request = srv.get("/");
+    let response = request.send().await.unwrap();
+    assert!(response.status().is_success());
+}