From eca91697f202cb062007a1a2dab4bd3cbf9235ab Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 7 Jun 2024 21:02:43 +0100 Subject: [PATCH] test: add negative tests --- actix-web-codegen/src/lib.rs | 60 ++++++---- actix-web-codegen/src/route.rs | 110 +----------------- actix-web-codegen/src/scope.rs | 96 +++++++++++++++ actix-web-codegen/tests/scopes.rs | 8 +- actix-web-codegen/tests/trybuild.rs | 4 + .../tests/trybuild/scope-invalid-args.rs | 14 +++ .../tests/trybuild/scope-invalid-args.stderr | 17 +++ .../tests/trybuild/scope-missing-args.rs | 6 + .../tests/trybuild/scope-missing-args.stderr | 7 ++ .../tests/trybuild/scope-on-handler.rs | 8 ++ .../tests/trybuild/scope-on-handler.stderr | 5 + 11 files changed, 202 insertions(+), 133 deletions(-) create mode 100644 actix-web-codegen/src/scope.rs create mode 100644 actix-web-codegen/tests/trybuild/scope-invalid-args.rs create mode 100644 actix-web-codegen/tests/trybuild/scope-invalid-args.stderr create mode 100644 actix-web-codegen/tests/trybuild/scope-missing-args.rs create mode 100644 actix-web-codegen/tests/trybuild/scope-missing-args.stderr create mode 100644 actix-web-codegen/tests/trybuild/scope-on-handler.rs create mode 100644 actix-web-codegen/tests/trybuild/scope-on-handler.stderr diff --git a/actix-web-codegen/src/lib.rs b/actix-web-codegen/src/lib.rs index b45e75631..23a9ae89b 100644 --- a/actix-web-codegen/src/lib.rs +++ b/actix-web-codegen/src/lib.rs @@ -83,6 +83,7 @@ use proc_macro::TokenStream; use quote::quote; mod route; +mod scope; /// Creates resource handler, allowing multiple HTTP method guards. /// @@ -197,6 +198,33 @@ method_macro!(Options, options); method_macro!(Trace, trace); method_macro!(Patch, patch); +/// Creates scope. +/// +/// Syntax: `#[scope("/path")]` +/// +/// ## Attributes: +/// +/// - `"path"` - Raw literal string with path for which to register handler. Mandatory. +/// +/// # Example +/// +/// ``` +/// # use actix_web_codegen::scope; +/// # use actix_web::{get, HttpResponse, Responder}; +/// #[scope("/test")] +/// mod scope_module { +/// #[get("/test")] +/// pub async fn test() -> impl Responder { +/// // this has path /test/test +/// HttpResponse::Ok().finish() +/// } +/// } +/// ``` +#[proc_macro_attribute] +pub fn scope(args: TokenStream, input: TokenStream) -> TokenStream { + scope::with_scope(args, input) +} + /// Marks async main function as the Actix Web system entry-point. /// /// Note that Actix Web also works under `#[tokio::main]` since version 4.0. However, this macro is @@ -241,30 +269,14 @@ pub fn test(_: TokenStream, item: TokenStream) -> TokenStream { output } -/// Generates scope +/// Converts the error to a token stream and appends it to the original input. /// -/// Syntax: `#[scope("path")]` +/// Returning the original input in addition to the error is good for IDEs which can gracefully +/// recover and show more precise errors within the macro body. /// -/// ## Attributes: -/// -/// - `"path"` - Raw literal string with path for which to register handler. Mandatory. -/// -/// # Example -/// -/// ```rust -/// use actix_web_codegen::{scope}; -/// #[scope("/test")] -/// mod scope_module { -/// use actix_web::{get, HttpResponse, Responder}; -/// #[get("/test")] -/// pub async fn test() -> impl Responder { -/// // this has path /test/test -/// HttpResponse::Ok().finish() -/// } -/// } -/// ``` -/// -#[proc_macro_attribute] -pub fn scope(args: TokenStream, input: TokenStream) -> TokenStream { - route::with_scope(args, input) +/// See for more info. +fn input_and_compile_error(mut item: TokenStream, err: syn::Error) -> TokenStream { + let compile_err = TokenStream::from(err.to_compile_error()); + item.extend(compile_err); + item } diff --git a/actix-web-codegen/src/route.rs b/actix-web-codegen/src/route.rs index 1ff8efd48..d0605fc04 100644 --- a/actix-web-codegen/src/route.rs +++ b/actix-web-codegen/src/route.rs @@ -6,10 +6,12 @@ use proc_macro2::{Span, TokenStream as TokenStream2}; use quote::{quote, ToTokens, TokenStreamExt}; use syn::{punctuated::Punctuated, Ident, LitStr, Path, Token}; +use crate::input_and_compile_error; + #[derive(Debug)] pub struct RouteArgs { - path: syn::LitStr, - options: Punctuated, + pub(crate) path: syn::LitStr, + pub(crate) options: Punctuated, } impl syn::parse::Parse for RouteArgs { @@ -78,7 +80,7 @@ macro_rules! standard_method_type { } } - fn from_path(method: &Path) -> Result { + pub(crate) fn from_path(method: &Path) -> Result { match () { $(_ if method.is_ident(stringify!($lower)) => Ok(Self::$variant),)+ _ => Err(()), @@ -542,105 +544,3 @@ pub(crate) fn with_methods(input: TokenStream) -> TokenStream { Err(err) => input_and_compile_error(input, err), } } - -/// Converts the error to a token stream and appends it to the original input. -/// -/// Returning the original input in addition to the error is good for IDEs which can gracefully -/// recover and show more precise errors within the macro body. -/// -/// See for more info. -fn input_and_compile_error(mut item: TokenStream, err: syn::Error) -> TokenStream { - let compile_err = TokenStream::from(err.to_compile_error()); - item.extend(compile_err); - item -} - -pub fn with_scope(args: TokenStream, input: TokenStream) -> TokenStream { - // Attempt to parse the scope path, returning on error - if args.is_empty() { - return input_and_compile_error( - args.clone(), - syn::Error::new( - Span::call_site(), - "Missing arguments for scope macro, expected: #[scope(\"some path\")]", - ), - ); - } - let scope_path = syn::parse::(args.clone()); - if let Err(_err) = scope_path { - return input_and_compile_error( - args.clone(), - syn::Error::new( - Span::call_site(), - "Missing arguments for scope macro, expected: #[scope(\"some path\")]", - ), - ); - } - - // Expect macro to be for a module - match syn::parse::(input) { - Ok(mut ast) => { - // 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 syn::Item::Fn(fun) = item { - fun.attrs = fun - .attrs - .iter() - .map(|attr| { - modify_attribute_with_scope( - attr, - &scope_path.clone().unwrap().value(), - ) - }) - .collect(); - } - }) - } - TokenStream::from(quote! { #ast }) - } - Err(err) => { - input_and_compile_error(args, syn::Error::new(Span::call_site(), err.to_string())) - } - } -} - -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 (attr.parse_args::(), attr.clone().meta) { - (Ok(route_args), syn::Meta::List(meta_list)) if has_allowed_methods_in_scope(attr) => { - let modified_path = format!("{}{}", scope_path, route_args.path.value()); - - let options_tokens: Vec = 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 - }); - - syn::Attribute { - meta: syn::Meta::List(syn::MetaList { - tokens: quote! { #modified_path #combined_options_tokens }, - ..meta_list.clone() - }), - ..attr.clone() - } - } - _ => attr.clone(), - } -} diff --git a/actix-web-codegen/src/scope.rs b/actix-web-codegen/src/scope.rs new file mode 100644 index 000000000..12c1c7e2b --- /dev/null +++ b/actix-web-codegen/src/scope.rs @@ -0,0 +1,96 @@ +use proc_macro::TokenStream; +use proc_macro2::{Span, TokenStream as TokenStream2}; +use quote::quote; +use syn::LitStr; + +use crate::{ + input_and_compile_error, + route::{MethodType, RouteArgs}, +}; + +pub fn with_scope(args: TokenStream, input: TokenStream) -> TokenStream { + match with_scope_inner(args, input.clone()) { + Ok(stream) => stream, + Err(err) => input_and_compile_error(input, err), + } +} + +fn with_scope_inner(args: TokenStream, input: TokenStream) -> syn::Result { + if args.is_empty() { + // macro args are missing + return Err(syn::Error::new( + Span::call_site(), + "missing arguments for scope macro, \ + expected: #[scope(\"/prefix\")]", + )); + } + + let scope_prefix = syn::parse::(args.clone()).map_err(|err| { + // first macro arg is not a string literal + syn::Error::new( + err.span(), + "argument to scope macro is not a string literal, \ + expected: #[scope(\"/prefix\")]", + ) + })?; + + let mut module = syn::parse::(input).map_err(|err| { + syn::Error::new(err.span(), "#[scope] macro must be attached to a module") + })?; + + // modify any routing macros (method or route[s]) attached to + // functions by prefixing them with this scope macro's argument + if let Some((_, ref mut items)) = module.content { + for item in items { + if let syn::Item::Fn(fun) = item { + fun.attrs = fun + .attrs + .iter() + .map(|attr| modify_attribute_with_scope(attr, &scope_prefix.value())) + .collect(); + } + } + } + + Ok(TokenStream::from(quote! { #module })) +} + +// 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 (attr.parse_args::(), attr.clone().meta) { + (Ok(route_args), syn::Meta::List(meta_list)) if has_allowed_methods_in_scope(attr) => { + let modified_path = format!("{}{}", scope_path, route_args.path.value()); + + let options_tokens: Vec = 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 + }); + + syn::Attribute { + meta: syn::Meta::List(syn::MetaList { + tokens: quote! { #modified_path #combined_options_tokens }, + ..meta_list.clone() + }), + ..attr.clone() + } + } + _ => attr.clone(), + } +} + +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") +} diff --git a/actix-web-codegen/tests/scopes.rs b/actix-web-codegen/tests/scopes.rs index 9499af58b..198ba10e3 100644 --- a/actix-web-codegen/tests/scopes.rs +++ b/actix-web-codegen/tests/scopes.rs @@ -61,18 +61,18 @@ mod scope_module { /// Scope doc string to check in cargo expand. #[scope("/v1")] mod mod_scope_v1 { - use actix_web::{get, Responder}; + use super::*; /// Route doc string to check in cargo expand. #[get("/test")] pub async fn test() -> impl Responder { - super::scope_module::mod_common("version1 works".to_string()) + scope_module::mod_common("version1 works".to_string()) } } #[scope("/v2")] mod mod_scope_v2 { - use actix_web::{get, Responder}; + use super::*; // check to make sure non-function tokens in the scope block are preserved... enum TestEnum { @@ -85,7 +85,7 @@ mod mod_scope_v2 { let test_enum = TestEnum::Works; match test_enum { - TestEnum::Works => super::scope_module::mod_common("version2 works".to_string()), + TestEnum::Works => scope_module::mod_common("version2 works".to_string()), } } } diff --git a/actix-web-codegen/tests/trybuild.rs b/actix-web-codegen/tests/trybuild.rs index 88f77548b..1b56be72a 100644 --- a/actix-web-codegen/tests/trybuild.rs +++ b/actix-web-codegen/tests/trybuild.rs @@ -18,6 +18,10 @@ fn compile_macros() { t.compile_fail("tests/trybuild/routes-missing-method-fail.rs"); t.compile_fail("tests/trybuild/routes-missing-args-fail.rs"); + t.compile_fail("tests/trybuild/scope-on-handler.rs"); + t.compile_fail("tests/trybuild/scope-missing-args.rs"); + t.compile_fail("tests/trybuild/scope-invalid-args.rs"); + t.pass("tests/trybuild/docstring-ok.rs"); t.pass("tests/trybuild/test-runtime.rs"); diff --git a/actix-web-codegen/tests/trybuild/scope-invalid-args.rs b/actix-web-codegen/tests/trybuild/scope-invalid-args.rs new file mode 100644 index 000000000..ec021d5eb --- /dev/null +++ b/actix-web-codegen/tests/trybuild/scope-invalid-args.rs @@ -0,0 +1,14 @@ +use actix_web_codegen::scope; + +const PATH: &str = "/api"; + +#[scope(PATH)] +mod api_const {} + +#[scope(true)] +mod api_bool {} + +#[scope(123)] +mod api_num {} + +fn main() {} diff --git a/actix-web-codegen/tests/trybuild/scope-invalid-args.stderr b/actix-web-codegen/tests/trybuild/scope-invalid-args.stderr new file mode 100644 index 000000000..0ab335966 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/scope-invalid-args.stderr @@ -0,0 +1,17 @@ +error: argument to scope macro is not a string literal, expected: #[scope("/prefix")] + --> tests/trybuild/scope-invalid-args.rs:5:9 + | +5 | #[scope(PATH)] + | ^^^^ + +error: argument to scope macro is not a string literal, expected: #[scope("/prefix")] + --> tests/trybuild/scope-invalid-args.rs:8:9 + | +8 | #[scope(true)] + | ^^^^ + +error: argument to scope macro is not a string literal, expected: #[scope("/prefix")] + --> tests/trybuild/scope-invalid-args.rs:11:9 + | +11 | #[scope(123)] + | ^^^ diff --git a/actix-web-codegen/tests/trybuild/scope-missing-args.rs b/actix-web-codegen/tests/trybuild/scope-missing-args.rs new file mode 100644 index 000000000..39bcb9d1a --- /dev/null +++ b/actix-web-codegen/tests/trybuild/scope-missing-args.rs @@ -0,0 +1,6 @@ +use actix_web_codegen::scope; + +#[scope] +mod api {} + +fn main() {} diff --git a/actix-web-codegen/tests/trybuild/scope-missing-args.stderr b/actix-web-codegen/tests/trybuild/scope-missing-args.stderr new file mode 100644 index 000000000..d59842e39 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/scope-missing-args.stderr @@ -0,0 +1,7 @@ +error: missing arguments for scope macro, expected: #[scope("/prefix")] + --> tests/trybuild/scope-missing-args.rs:3:1 + | +3 | #[scope] + | ^^^^^^^^ + | + = note: this error originates in the attribute macro `scope` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/actix-web-codegen/tests/trybuild/scope-on-handler.rs b/actix-web-codegen/tests/trybuild/scope-on-handler.rs new file mode 100644 index 000000000..e5d478981 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/scope-on-handler.rs @@ -0,0 +1,8 @@ +use actix_web_codegen::scope; + +#[scope("/api")] +async fn index() -> &'static str { + "Hello World!" +} + +fn main() {} diff --git a/actix-web-codegen/tests/trybuild/scope-on-handler.stderr b/actix-web-codegen/tests/trybuild/scope-on-handler.stderr new file mode 100644 index 000000000..4491f42dd --- /dev/null +++ b/actix-web-codegen/tests/trybuild/scope-on-handler.stderr @@ -0,0 +1,5 @@ +error: #[scope] macro must be attached to a module + --> tests/trybuild/scope-on-handler.rs:4:1 + | +4 | async fn index() -> &'static str { + | ^^^^^