diff --git a/actix-router/src/resource.rs b/actix-router/src/resource.rs index 860993a3786..b8aa6c2249e 100644 --- a/actix-router/src/resource.rs +++ b/actix-router/src/resource.rs @@ -252,7 +252,7 @@ impl ResourceDef { /// Multi-pattern resources can be constructed by providing a slice (or vec) of patterns. /// /// # Panics - /// Panics if path pattern is malformed. + /// Panics if any path patterns are malformed. /// /// # Examples /// ``` @@ -838,6 +838,7 @@ impl ResourceDef { fn construct(paths: T, is_prefix: bool) -> Self { let patterns = paths.patterns(); + let (pat_type, segments) = match &patterns { Patterns::Single(pattern) => ResourceDef::parse(pattern, is_prefix, false), diff --git a/actix-web-codegen/CHANGES.md b/actix-web-codegen/CHANGES.md index b9e4b0aad92..2c5f1722683 100644 --- a/actix-web-codegen/CHANGES.md +++ b/actix-web-codegen/CHANGES.md @@ -2,6 +2,7 @@ ## Unreleased - 2023-xx-xx +- Update `syn` dependency to `2`. - Minimum supported Rust version (MSRV) is now 1.65 due to transitive `time` dependency. ## 4.2.0 - 2023-02-26 diff --git a/actix-web-codegen/Cargo.toml b/actix-web-codegen/Cargo.toml index 4f2fdc56656..c202c5d6ec4 100644 --- a/actix-web-codegen/Cargo.toml +++ b/actix-web-codegen/Cargo.toml @@ -18,10 +18,10 @@ proc-macro = true actix-router = "0.5" proc-macro2 = "1" quote = "1" -syn = { version = "1", features = ["full", "extra-traits"] } +syn = { version = "2", features = ["full", "extra-traits"] } [dev-dependencies] -actix-macros = "0.2.3" +actix-macros = "0.2.4" actix-rt = "2.2" actix-test = "0.1" actix-utils = "3" diff --git a/actix-web-codegen/src/route.rs b/actix-web-codegen/src/route.rs index e87d37941ba..e992a5c4485 100644 --- a/actix-web-codegen/src/route.rs +++ b/actix-web-codegen/src/route.rs @@ -4,7 +4,44 @@ use actix_router::ResourceDef; use proc_macro::TokenStream; use proc_macro2::{Span, TokenStream as TokenStream2}; use quote::{quote, ToTokens, TokenStreamExt}; -use syn::{parse_macro_input, AttributeArgs, Ident, LitStr, Meta, NestedMeta, Path}; +use syn::{punctuated::Punctuated, Ident, LitStr, Path}; + +#[derive(Debug)] +pub struct RouteArgs { + path: syn::LitStr, + options: Punctuated, +} + +impl syn::parse::Parse for RouteArgs { + fn parse(input: syn::parse::ParseStream<'_>) -> syn::Result { + // path to match: "/foo" + let path = input.parse::().map_err(|_| { + syn::Error::new( + Span::call_site(), + r#"invalid service definition, expected #[get("")]"#, + ) + })?; + + // verify that path pattern is valid + let _ = ResourceDef::new(path.value()); + + // if there's no comma, assume that no options are provided + if !input.lookahead1().peek(syn::Token![,]) { + return Ok(Self { + path, + options: Punctuated::new(), + }); + } + + // separator + input.parse::()?; + + // zero or more options: name = "foo" + let options = input.parse_terminated(syn::MetaNameValue::parse, syn::Token![,])?; + + Ok(Self { path, options }) + } +} macro_rules! standard_method_type { ( @@ -182,111 +219,90 @@ struct Args { } impl Args { - fn new(args: AttributeArgs, method: Option) -> syn::Result { - let mut path = None; + fn new(args: RouteArgs, method: Option) -> syn::Result { let mut resource_name = None; let mut guards = Vec::new(); let mut wrappers = Vec::new(); let mut methods = HashSet::new(); - if args.is_empty() { - return Err(syn::Error::new( - Span::call_site(), - format!( - r#"invalid service definition, expected #[{}("")]"#, - method - .map_or("route", |it| it.as_str()) - .to_ascii_lowercase() - ), - )); - } - let is_route_macro = method.is_none(); if let Some(method) = method { methods.insert(MethodTypeExt::Standard(method)); } - for arg in args { - match arg { - NestedMeta::Lit(syn::Lit::Str(lit)) => match path { - None => { - let _ = ResourceDef::new(lit.value()); - path = Some(lit); - } - _ => { - return Err(syn::Error::new_spanned( - lit, - "Multiple paths specified! Should be only one!", - )); - } - }, - - NestedMeta::Meta(syn::Meta::NameValue(nv)) => { - if nv.path.is_ident("name") { - if let syn::Lit::Str(lit) = nv.lit { - resource_name = Some(lit); - } else { - return Err(syn::Error::new_spanned( - nv.lit, - "Attribute name expects literal string!", - )); - } - } else if nv.path.is_ident("guard") { - if let syn::Lit::Str(lit) = nv.lit { - guards.push(lit.parse::()?); - } else { - return Err(syn::Error::new_spanned( - nv.lit, - "Attribute guard expects literal string!", - )); - } - } else if nv.path.is_ident("wrap") { - if let syn::Lit::Str(lit) = nv.lit { - wrappers.push(lit.parse()?); - } else { - return Err(syn::Error::new_spanned( - nv.lit, - "Attribute wrap expects type", - )); - } - } else if nv.path.is_ident("method") { - if !is_route_macro { - return Err(syn::Error::new_spanned( + for nv in args.options { + if nv.path.is_ident("name") { + if let syn::Expr::Lit(syn::ExprLit { + lit: syn::Lit::Str(lit), + .. + }) = nv.value + { + resource_name = Some(lit); + } else { + return Err(syn::Error::new_spanned( + nv.value, + "Attribute name expects literal string!", + )); + } + } else if nv.path.is_ident("guard") { + if let syn::Expr::Lit(syn::ExprLit { + lit: syn::Lit::Str(lit), + .. + }) = nv.value + { + guards.push(lit.parse::()?); + } else { + return Err(syn::Error::new_spanned( + nv.value, + "Attribute guard expects literal string!", + )); + } + } else if nv.path.is_ident("wrap") { + if let syn::Expr::Lit(syn::ExprLit { + lit: syn::Lit::Str(lit), + .. + }) = nv.value + { + wrappers.push(lit.parse()?); + } else { + return Err(syn::Error::new_spanned( + nv.value, + "Attribute wrap expects type", + )); + } + } else if nv.path.is_ident("method") { + if !is_route_macro { + return Err(syn::Error::new_spanned( &nv, "HTTP method forbidden here. To handle multiple methods, use `route` instead", )); - } else if let syn::Lit::Str(ref lit) = nv.lit { - if !methods.insert(MethodTypeExt::try_from(lit)?) { - 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 { + } else if let syn::Expr::Lit(syn::ExprLit { + lit: syn::Lit::Str(lit), + .. + }) = nv.value.clone() + { + if !methods.insert(MethodTypeExt::try_from(&lit)?) { return Err(syn::Error::new_spanned( - nv.path, - "Unknown attribute key is specified. Allowed: guard, method and wrap", + nv.value, + format!("HTTP method defined more than once: `{}`", lit.value()), )); } + } else { + return Err(syn::Error::new_spanned( + nv.value, + "Attribute method expects literal string!", + )); } - - arg => { - return Err(syn::Error::new_spanned(arg, "Unknown attribute.")); - } + } else { + return Err(syn::Error::new_spanned( + nv.path, + "Unknown attribute key is specified. Allowed: guard, method and wrap", + )); } } Ok(Args { - path: path.unwrap(), + path: args.path, resource_name, guards, wrappers, @@ -312,11 +328,7 @@ pub struct Route { } impl Route { - pub fn new( - args: AttributeArgs, - ast: syn::ItemFn, - method: Option, - ) -> syn::Result { + pub fn new(args: RouteArgs, ast: syn::ItemFn, method: Option) -> syn::Result { let name = ast.sig.ident.clone(); // Try and pull out the doc comments so that we can reapply them to the generated struct. @@ -324,7 +336,7 @@ impl Route { let doc_attributes = ast .attrs .iter() - .filter(|attr| attr.path.is_ident("doc")) + .filter(|attr| attr.path().is_ident("doc")) .cloned() .collect(); @@ -360,7 +372,7 @@ impl Route { let doc_attributes = ast .attrs .iter() - .filter(|attr| attr.path.is_ident("doc")) + .filter(|attr| attr.path().is_ident("doc")) .cloned() .collect(); @@ -455,7 +467,11 @@ pub(crate) fn with_method( args: TokenStream, input: TokenStream, ) -> TokenStream { - let args = parse_macro_input!(args as syn::AttributeArgs); + let args = match syn::parse(args) { + Ok(args) => args, + // on parse error, make IDEs happy; see fn docs + Err(err) => return input_and_compile_error(input, err), + }; let ast = match syn::parse::(input.clone()) { Ok(ast) => ast, @@ -480,7 +496,7 @@ pub(crate) fn with_methods(input: TokenStream) -> TokenStream { let (methods, others) = ast .attrs .into_iter() - .map(|attr| match MethodType::from_path(&attr.path) { + .map(|attr| match MethodType::from_path(attr.path()) { Ok(method) => Ok((method, attr)), Err(_) => Err(attr), }) @@ -492,13 +508,8 @@ pub(crate) fn with_methods(input: TokenStream) -> TokenStream { .into_iter() .map(Result::unwrap) .map(|(method, attr)| { - attr.parse_meta().and_then(|args| { - if let Meta::List(args) = args { - Args::new(args.nested.into_iter().collect(), Some(method)) - } else { - Err(syn::Error::new_spanned(attr, "Invalid input for macro")) - } - }) + attr.parse_args() + .and_then(|args| Args::new(args, Some(method))) }) .collect::, _>>() { diff --git a/actix-web-codegen/tests/trybuild/routes-missing-args-fail.stderr b/actix-web-codegen/tests/trybuild/routes-missing-args-fail.stderr index e845241a4d6..2a62a97d439 100644 --- a/actix-web-codegen/tests/trybuild/routes-missing-args-fail.stderr +++ b/actix-web-codegen/tests/trybuild/routes-missing-args-fail.stderr @@ -6,11 +6,11 @@ error: invalid service definition, expected #[get("")] | = note: this error originates in the attribute macro `get` (in Nightly builds, run with -Z macro-backtrace for more info) -error: Invalid input for macro - --> tests/trybuild/routes-missing-args-fail.rs:4:1 +error: expected attribute arguments in parentheses: #[get(...)] + --> tests/trybuild/routes-missing-args-fail.rs:4:3 | 4 | #[get] - | ^^^^^^ + | ^^^ error[E0277]: the trait bound `fn() -> impl std::future::Future {index}: HttpServiceFactory` is not satisfied --> tests/trybuild/routes-missing-args-fail.rs:13:55 diff --git a/actix-web-codegen/tests/trybuild/simple-fail.stderr b/actix-web-codegen/tests/trybuild/simple-fail.stderr index cffc81ff81a..ab795d69f1a 100644 --- a/actix-web-codegen/tests/trybuild/simple-fail.stderr +++ b/actix-web-codegen/tests/trybuild/simple-fail.stderr @@ -1,22 +1,28 @@ -error: Unknown attribute. - --> $DIR/simple-fail.rs:3:15 +error: expected `=` + --> $DIR/simple-fail.rs:3:1 | 3 | #[get("/one", other)] - | ^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the attribute macro `get` (in Nightly builds, run with -Z macro-backtrace for more info) -error: expected identifier or literal - --> $DIR/simple-fail.rs:8:8 +error: invalid service definition, expected #[get("")] + --> $DIR/simple-fail.rs:8:1 | 8 | #[post(/two)] - | ^ + | ^^^^^^^^^^^^^ + | + = note: this error originates in the attribute macro `post` (in Nightly builds, run with -Z macro-backtrace for more info) -error: Unknown attribute. - --> $DIR/simple-fail.rs:15:9 +error: invalid service definition, expected #[get("")] + --> $DIR/simple-fail.rs:15:1 | 15 | #[patch(PATCH_PATH)] - | ^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the attribute macro `patch` (in Nightly builds, run with -Z macro-backtrace for more info) -error: Multiple paths specified! Should be only one! +error: expected identifier --> $DIR/simple-fail.rs:20:19 | 20 | #[delete("/four", "/five")]