From d22c9f9fb12db30ee1cb7183ce8cf395c707f30d Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Thu, 20 Jul 2023 10:49:01 +0100 Subject: [PATCH] update syn to 2 in web codegen (#3081) --- actix-multipart-derive/CHANGES.md | 1 + actix-multipart-derive/Cargo.toml | 4 +- actix-router/src/resource.rs | 3 +- actix-web-codegen/CHANGES.md | 1 + actix-web-codegen/Cargo.toml | 4 +- actix-web-codegen/src/route.rs | 223 ++++++++++-------- .../trybuild/routes-missing-args-fail.stderr | 12 +- .../tests/trybuild/simple-fail.stderr | 32 ++- 8 files changed, 164 insertions(+), 116 deletions(-) diff --git a/actix-multipart-derive/CHANGES.md b/actix-multipart-derive/CHANGES.md index caf75aeb3..8a78900ec 100644 --- a/actix-multipart-derive/CHANGES.md +++ b/actix-multipart-derive/CHANGES.md @@ -2,6 +2,7 @@ ## Unreleased +- Update `syn` dependency to `2`. - Minimum supported Rust version (MSRV) is now 1.65 due to transitive `time` dependency. ## 0.6.0 - 2023-02-26 diff --git a/actix-multipart-derive/Cargo.toml b/actix-multipart-derive/Cargo.toml index aca6de84a..41e687e50 100644 --- a/actix-multipart-derive/Cargo.toml +++ b/actix-multipart-derive/Cargo.toml @@ -17,11 +17,11 @@ all-features = true proc-macro = true [dependencies] -darling = "0.14" +darling = "0.20" parse-size = "1" proc-macro2 = "1" quote = "1" -syn = "1" +syn = "2" [dev-dependencies] actix-multipart = "0.6" diff --git a/actix-router/src/resource.rs b/actix-router/src/resource.rs index 860993a37..b8aa6c224 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 b9e4b0aad..2c5f17226 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 4f2fdc566..c202c5d6e 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 e87d37941..525a1c8ba 100644 --- a/actix-web-codegen/src/route.rs +++ b/actix-web-codegen/src/route.rs @@ -4,7 +4,54 @@ 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, Token}; + +#[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(|mut err| { + err.combine(syn::Error::new( + err.span(), + r#"invalid service definition, expected #[("")]"#, + )); + + err + })?; + + // verify that path pattern is valid + let _ = ResourceDef::new(path.value()); + + // if there's no comma, assume that no options are provided + if !input.peek(Token![,]) { + return Ok(Self { + path, + options: Punctuated::new(), + }); + } + + // advance past comma separator + input.parse::()?; + + // if next char is a literal, assume that it is a string and show multi-path error + if input.cursor().literal().is_some() { + return Err(syn::Error::new( + Span::call_site(), + r#"Multiple paths specified! There should be only one."#, + )); + } + + // zero or more options: name = "foo" + let options = input.parse_terminated(syn::MetaNameValue::parse, Token![,])?; + + Ok(Self { path, options }) + } +} macro_rules! standard_method_type { ( @@ -182,111 +229,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 +338,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 +346,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 +382,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 +477,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 +506,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 +518,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 e845241a4..2e84c296a 100644 --- a/actix-web-codegen/tests/trybuild/routes-missing-args-fail.stderr +++ b/actix-web-codegen/tests/trybuild/routes-missing-args-fail.stderr @@ -1,4 +1,4 @@ -error: invalid service definition, expected #[get("")] +error: unexpected end of input, expected string literal --> tests/trybuild/routes-missing-args-fail.rs:4:1 | 4 | #[get] @@ -6,11 +6,19 @@ 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 +error: invalid service definition, expected #[("")] --> tests/trybuild/routes-missing-args-fail.rs:4:1 | 4 | #[get] | ^^^^^^ + | + = note: this error originates in the attribute macro `get` (in Nightly builds, run with -Z macro-backtrace for more info) + +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 cffc81ff8..3b3f9d850 100644 --- a/actix-web-codegen/tests/trybuild/simple-fail.stderr +++ b/actix-web-codegen/tests/trybuild/simple-fail.stderr @@ -1,26 +1,42 @@ -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 +error: expected string literal --> $DIR/simple-fail.rs:8:8 | 8 | #[post(/two)] | ^ -error: Unknown attribute. +error: invalid service definition, expected #[("")] + --> $DIR/simple-fail.rs:8:8 + | +8 | #[post(/two)] + | ^ + +error: expected string literal --> $DIR/simple-fail.rs:15:9 | 15 | #[patch(PATCH_PATH)] | ^^^^^^^^^^ -error: Multiple paths specified! Should be only one! - --> $DIR/simple-fail.rs:20:19 +error: invalid service definition, expected #[("")] + --> $DIR/simple-fail.rs:15:9 + | +15 | #[patch(PATCH_PATH)] + | ^^^^^^^^^^ + +error: Multiple paths specified! There should be only one. + --> $DIR/simple-fail.rs:20:1 | 20 | #[delete("/four", "/five")] - | ^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the attribute macro `delete` (in Nightly builds, run with -Z macro-backtrace for more info) error: HTTP method forbidden here. To handle multiple methods, use `route` instead --> $DIR/simple-fail.rs:25:19