add additional tests

actix-web-codegen/Cargo.toml

@@ -18,7 +18,7 @@ proc-macro = true
 actix-router = "0.5.0"
 proc-macro2 = "1"
 quote = "1"
-syn = { version = "1", features = ["full", "parsing"] }
+syn = { version = "1", features = ["full", "extra-traits"] }
 
 [dev-dependencies]
 actix-macros = "0.2.3"

actix-web-codegen/src/route.rs

@@ -4,7 +4,7 @@ use actix_router::ResourceDef;
 use proc_macro::TokenStream;
 use proc_macro2::{Span, TokenStream as TokenStream2};
 use quote::{format_ident, quote, ToTokens, TokenStreamExt};
-use syn::{parse_macro_input, Attribute, AttributeArgs, Ident, LitStr, Meta, NestedMeta, Path};
+use syn::{parse_macro_input, AttributeArgs, Ident, LitStr, Meta, NestedMeta, Path};
 
 enum ResourceType {
     Async,
@@ -101,7 +101,7 @@ impl Args {
             return Err(syn::Error::new(
                 Span::call_site(),
                 format!(
-                    r#"invalid service definition, expected #[{}("<some path>")]"#,
+                    r#"invalid service definition, expected #[{}("<path>")]"#,
                     method
                         .map_or("route", |it| it.as_str())
                         .to_ascii_lowercase()
@@ -202,9 +202,18 @@ impl Args {
 }
 
 pub struct Route {
+    /// Name of the handler function being annotated.
     name: syn::Ident,
+
+    /// Args passed to routing macro.
+    ///
+    /// When using `#[routes]`, this will contain args for each specific routing macro.
     args: Vec<Args>,
+
+    /// AST of the handler function being annotated.
     ast: syn::ItemFn,
+
+    /// TODO: remove
     resource_type: ResourceType,
 
     /// The doc comment attributes to copy to generated struct, if any.
@@ -251,6 +260,7 @@ impl Route {
             .collect();
 
         let args = Args::new(args, method)?;
+
         if args.methods.is_empty() {
             return Err(syn::Error::new(
                 Span::call_site(),
@@ -329,19 +339,22 @@ impl ToTokens for Route {
 
         let registrations: TokenStream2 = args
             .iter()
-            .map(
-                |Args {
+            .map(|args| {
+                let Args {
                     path,
                     resource_name,
                     guards,
                     wrappers,
                     methods,
-                 }| {
+                } = args;
+
                 let resource_name = resource_name
                     .as_ref()
                     .map_or_else(|| name.to_string(), LitStr::value);
+
                 let method_guards = {
                     let mut others = methods.iter();
+
                     // unwrapping since length is checked to be at least one
                     let first = others.next().unwrap();
 
@@ -358,6 +371,7 @@ impl ToTokens for Route {
                         }
                     }
                 };
+
                 quote! {
                     let __resource = ::actix_web::Resource::new(#path)
                         .name(#resource_name)
@@ -368,8 +382,7 @@ impl ToTokens for Route {
 
                     ::actix_web::dev::HttpServiceFactory::register(__resource, __config);
                 }
-                },
-            )
+            })
             .collect();
 
         let stream = quote! {
@@ -416,14 +429,14 @@ pub(crate) fn with_methods(input: TokenStream) -> TokenStream {
         Err(err) => return input_and_compile_error(input, err),
     };
 
-    let (methods, others): (Vec<Result<(MethodType, Attribute), Attribute>>, _) = ast
+    let (methods, others) = ast
         .attrs
         .into_iter()
         .map(|attr| match MethodType::from_path(&attr.path) {
             Ok(method) => Ok((method, attr)),
             Err(_) => Err(attr),
         })
-        .partition(Result::is_ok);
+        .partition::<Vec<_>, _>(Result::is_ok);
 
     ast.attrs = others.into_iter().map(Result::unwrap_err).collect();
 

actix-web-codegen/tests/test_macro.rs

@@ -8,7 +8,7 @@ use actix_web::{
         header::{HeaderName, HeaderValue},
         StatusCode,
     },
-    web, App, Error, HttpResponse, Responder,
+    web, App, Error, HttpRequest, HttpResponse, Responder,
 };
 use actix_web_codegen::{
     connect, delete, get, head, options, patch, post, put, route, routes, trace,
@@ -99,8 +99,33 @@ async fn routes_test() -> impl Responder {
     HttpResponse::Ok()
 }
 
+// routes overlap with the more specific route first, therefore accessible
+#[routes]
+#[get("/routes/overlap/test")]
+#[get("/routes/overlap/{foo}")]
+async fn routes_overlapping_test(req: HttpRequest) -> impl Responder {
+    // foo is only populated when route is not /routes/overlap/test
+    match req.match_info().get("foo") {
+        None => assert!(req.uri() == "/routes/overlap/test"),
+        Some(_) => assert!(req.uri() != "/routes/overlap/test"),
+    }
+
+    HttpResponse::Ok()
+}
+
+// routes overlap with the more specific route last, therefore inaccessible
+#[routes]
+#[get("/routes/overlap2/{foo}")]
+#[get("/routes/overlap2/test")]
+async fn routes_overlapping_inaccessible_test(req: HttpRequest) -> impl Responder {
+    // foo is always populated even when path is /routes/overlap2/test
+    assert!(req.match_info().get("foo").is_some());
+
+    HttpResponse::Ok()
+}
+
 #[get("/custom_resource_name", name = "custom")]
-async fn custom_resource_name_test<'a>(req: actix_web::HttpRequest) -> impl Responder {
+async fn custom_resource_name_test<'a>(req: HttpRequest) -> impl Responder {
     assert!(req.url_for_static("custom").is_ok());
     assert!(req.url_for_static("custom_resource_name_test").is_err());
     HttpResponse::Ok()
@@ -211,6 +236,8 @@ async fn test_body() {
             .service(patch_test)
             .service(test_handler)
             .service(route_test)
+            .service(routes_overlapping_test)
+            .service(routes_overlapping_inaccessible_test)
             .service(routes_test)
             .service(custom_resource_name_test)
             .service(guard_test)
@@ -281,6 +308,26 @@ async fn test_body() {
     let response = request.send().await.unwrap();
     assert!(response.status().is_success());
 
+    let request = srv.request(http::Method::GET, srv.url("/routes/not-set"));
+    let response = request.send().await.unwrap();
+    assert!(response.status().is_client_error());
+
+    let request = srv.request(http::Method::GET, srv.url("/routes/overlap/test"));
+    let response = request.send().await.unwrap();
+    assert!(response.status().is_success());
+
+    let request = srv.request(http::Method::GET, srv.url("/routes/overlap/bar"));
+    let response = request.send().await.unwrap();
+    assert!(response.status().is_success());
+
+    let request = srv.request(http::Method::GET, srv.url("/routes/overlap2/test"));
+    let response = request.send().await.unwrap();
+    assert!(response.status().is_success());
+
+    let request = srv.request(http::Method::GET, srv.url("/routes/overlap2/bar"));
+    let response = request.send().await.unwrap();
+    assert!(response.status().is_success());
+
     let request = srv.request(http::Method::GET, srv.url("/custom_resource_name"));
     let response = request.send().await.unwrap();
     assert!(response.status().is_success());

actix-web-codegen/tests/trybuild.rs

@@ -14,6 +14,8 @@ fn compile_macros() {
 
     t.pass("tests/trybuild/routes-ok.rs");
     t.compile_fail("tests/trybuild/routes-missing-method-fail.rs");
+    t.compile_fail("tests/trybuild/routes-invalid-method-fail.rs");
+    t.compile_fail("tests/trybuild/routes-missing-args-fail.rs");
 
     t.pass("tests/trybuild/docstring-ok.rs");
 

actix-web-codegen/tests/trybuild/routes-invalid-method-fail.rs

@@ -0,0 +1,14 @@
+use actix_web_codegen::*;
+
+#[routes]
+#[get]
+async fn index() -> String {
+    "Hello World!".to_owned()
+}
+
+#[actix_web::main]
+async fn main() {
+    use actix_web::App;
+
+    let srv = actix_test::start(|| App::new().service(index));
+}

actix-web-codegen/tests/trybuild/routes-missing-args-fail.rs

@@ -0,0 +1,14 @@
+use actix_web_codegen::*;
+
+#[routes]
+#[get]
+async fn index() -> String {
+    "Hello World!".to_owned()
+}
+
+#[actix_web::main]
+async fn main() {
+    use actix_web::App;
+
+    let srv = actix_test::start(|| App::new().service(index));
+}

actix-web/CHANGES.md

@@ -9,6 +9,7 @@
 ### Changed
 - Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency.
 
+[#2718]: https://github.com/actix/actix-web/pull/2718
 [#2752]: https://github.com/actix/actix-web/pull/2752
 [#2786]: https://github.com/actix/actix-web/pull/2786