From 3db7891303093d5992cfd3483405b80b495b8f8e Mon Sep 17 00:00:00 2001 From: Jonathan Lim Date: Fri, 7 Jun 2024 15:10:48 -0700 Subject: [PATCH] Scope macro (#3136) * add scope proc macro * Update scope macro code to work with current HttpServiceFactory * started some test code * add some unit tests * code formatting cleanup * add another test for combining and calling 2 scopes * format code with formatter * Update actix-web-codegen/src/lib.rs with comment documentation fix Co-authored-by: oliver <151407407+kwfn@users.noreply.github.com> * work in progress. revised procedural macro to change othe macro call * add tests again. refactor nested code. * clean up code. fix bugs with route and method attributes with parameters * clean up for rust fmt * clean up for rust fmt * fix out of date comment for scope macro * sync to master branch by adding test_wrap * needed to format code * test: split out scope tests * test: add negative tests * chore: move imports back inside (?) * docs: tweak scope docs * fix: prevent trailing slashes in scope prefixes * chore: address clippy lints --------- Co-authored-by: oliver <151407407+kwfn@users.noreply.github.com> Co-authored-by: Rob Ede --- actix-web-codegen/CHANGES.md | 1 + actix-web-codegen/src/lib.rs | 50 +++++ actix-web-codegen/src/route.rs | 20 +- actix-web-codegen/src/scope.rs | 103 +++++++++ .../tests/{test_macro.rs => routes.rs} | 0 actix-web-codegen/tests/scopes.rs | 200 ++++++++++++++++++ actix-web-codegen/tests/trybuild.rs | 5 + .../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 + .../tests/trybuild/scope-trailing-slash.rs | 6 + .../trybuild/scope-trailing-slash.stderr | 5 + actix-web/src/lib.rs | 1 + awc/src/client/connector.rs | 2 +- justfile | 6 +- 18 files changed, 439 insertions(+), 17 deletions(-) create mode 100644 actix-web-codegen/src/scope.rs rename actix-web-codegen/tests/{test_macro.rs => routes.rs} (100%) create mode 100644 actix-web-codegen/tests/scopes.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 create mode 100644 actix-web-codegen/tests/trybuild/scope-trailing-slash.rs create mode 100644 actix-web-codegen/tests/trybuild/scope-trailing-slash.stderr diff --git a/actix-web-codegen/CHANGES.md b/actix-web-codegen/CHANGES.md index 875c4021e..792f6aa4f 100644 --- a/actix-web-codegen/CHANGES.md +++ b/actix-web-codegen/CHANGES.md @@ -2,6 +2,7 @@ ## Unreleased +- Add `#[scope]` macro. - Prevent inclusion of default `actix-router` features. - Minimum supported Rust version (MSRV) is now 1.72. diff --git a/actix-web-codegen/src/lib.rs b/actix-web-codegen/src/lib.rs index 6d6c9ab5c..c518007a0 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,43 @@ method_macro!(Options, options); method_macro!(Trace, trace); method_macro!(Patch, patch); +/// Prepends a path prefix to all handlers using routing macros inside the attached module. +/// +/// # Syntax +/// +/// ``` +/// # use actix_web_codegen::scope; +/// #[scope("/prefix")] +/// mod api { +/// // ... +/// } +/// ``` +/// +/// # Arguments +/// +/// - `"/prefix"` - Raw literal string to be prefixed onto contained handlers' paths. +/// +/// # Example +/// +/// ``` +/// # use actix_web_codegen::{scope, get}; +/// # use actix_web::Responder; +/// #[scope("/api")] +/// mod api { +/// # use super::*; +/// #[get("/hello")] +/// pub async fn hello() -> impl Responder { +/// // this has path /api/hello +/// "Hello, world!" +/// } +/// } +/// # fn main() {} +/// ``` +#[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 @@ -240,3 +278,15 @@ pub fn test(_: TokenStream, item: TokenStream) -> TokenStream { output.extend(item); output } + +/// 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 +} diff --git a/actix-web-codegen/src/route.rs b/actix-web-codegen/src/route.rs index 7a2dfc051..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,15 +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 -} diff --git a/actix-web-codegen/src/scope.rs b/actix-web-codegen/src/scope.rs new file mode 100644 index 000000000..067d95a60 --- /dev/null +++ b/actix-web-codegen/src/scope.rs @@ -0,0 +1,103 @@ +use proc_macro::TokenStream; +use proc_macro2::{Span, TokenStream as TokenStream2}; +use quote::{quote, ToTokens as _}; + +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() { + 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| { + syn::Error::new( + err.span(), + "argument to scope macro is not a string literal, expected: #[scope(\"/prefix\")]", + ) + })?; + + let scope_prefix_value = scope_prefix.value(); + + if scope_prefix_value.ends_with('/') { + // trailing slashes cause non-obvious problems + // it's better to point them out to developers rather than + + return Err(syn::Error::new( + scope_prefix.span(), + "scopes should not have trailing slashes; see https://docs.rs/actix-web/4/actix_web/struct.Scope.html#avoid-trailing-slashes", + )); + } + + 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((_, items)) = &mut 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(module.to_token_stream().into()) +} + +/// Checks if the attribute is a method type and has a route path, then modifies 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/test_macro.rs b/actix-web-codegen/tests/routes.rs similarity index 100% rename from actix-web-codegen/tests/test_macro.rs rename to actix-web-codegen/tests/routes.rs diff --git a/actix-web-codegen/tests/scopes.rs b/actix-web-codegen/tests/scopes.rs new file mode 100644 index 000000000..6a370a2e2 --- /dev/null +++ b/actix-web-codegen/tests/scopes.rs @@ -0,0 +1,200 @@ +use actix_web::{guard::GuardContext, http, http::header, web, App, HttpResponse, Responder}; +use actix_web_codegen::{delete, get, post, route, routes, scope}; + +pub fn image_guard(ctx: &GuardContext) -> bool { + ctx.header::() + .map(|h| h.preference() == "image/*") + .unwrap_or(false) +} + +#[scope("/test")] +mod scope_module { + // ensure that imports can be brought into the scope + use super::*; + + #[get("/test/guard", guard = "image_guard")] + pub async fn guard() -> impl Responder { + HttpResponse::Ok() + } + + #[get("/test")] + pub async fn test() -> impl Responder { + HttpResponse::Ok().finish() + } + + #[get("/twice-test/{value}")] + pub async fn twice(value: web::Path) -> impl actix_web::Responder { + let int_value: i32 = value.parse().unwrap_or(0); + let doubled = int_value * 2; + HttpResponse::Ok().body(format!("Twice value: {}", doubled)) + } + + #[post("/test")] + pub async fn post() -> impl Responder { + HttpResponse::Ok().body("post works") + } + + #[delete("/test")] + pub async fn delete() -> impl Responder { + "delete works" + } + + #[route("/test", method = "PUT", method = "PATCH", method = "CUSTOM")] + pub async fn multiple_shared_path() -> impl Responder { + HttpResponse::Ok().finish() + } + + #[routes] + #[head("/test1")] + #[connect("/test2")] + #[options("/test3")] + #[trace("/test4")] + async fn multiple_separate_paths() -> impl Responder { + HttpResponse::Ok().finish() + } + + // test calling this from other mod scope with scope attribute... + pub fn mod_common(message: String) -> impl actix_web::Responder { + HttpResponse::Ok().body(message) + } +} + +/// Scope doc string to check in cargo expand. +#[scope("/v1")] +mod mod_scope_v1 { + use super::*; + + /// Route doc string to check in cargo expand. + #[get("/test")] + pub async fn test() -> impl Responder { + scope_module::mod_common("version1 works".to_string()) + } +} + +#[scope("/v2")] +mod mod_scope_v2 { + use super::*; + + // check to make sure non-function tokens in the scope block are preserved... + enum TestEnum { + Works, + } + + #[get("/test")] + pub async fn test() -> impl Responder { + // make sure this type still exists... + let test_enum = TestEnum::Works; + + match test_enum { + TestEnum::Works => scope_module::mod_common("version2 works".to_string()), + } + } +} + +#[actix_rt::test] +async fn scope_get_async() { + let srv = actix_test::start(|| App::new().service(scope_module::test)); + + let request = srv.request(http::Method::GET, srv.url("/test/test")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); +} + +#[actix_rt::test] +async fn scope_get_param_async() { + let srv = actix_test::start(|| App::new().service(scope_module::twice)); + + let request = srv.request(http::Method::GET, srv.url("/test/twice-test/4")); + let mut response = request.send().await.unwrap(); + let body = response.body().await.unwrap(); + let body_str = String::from_utf8(body.to_vec()).unwrap(); + assert_eq!(body_str, "Twice value: 8"); +} + +#[actix_rt::test] +async fn scope_post_async() { + let srv = actix_test::start(|| App::new().service(scope_module::post)); + + let request = srv.request(http::Method::POST, srv.url("/test/test")); + let mut response = request.send().await.unwrap(); + let body = response.body().await.unwrap(); + let body_str = String::from_utf8(body.to_vec()).unwrap(); + assert_eq!(body_str, "post works"); +} + +#[actix_rt::test] +async fn multiple_shared_path_async() { + let srv = actix_test::start(|| App::new().service(scope_module::multiple_shared_path)); + + let request = srv.request(http::Method::PUT, srv.url("/test/test")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + + let request = srv.request(http::Method::PATCH, srv.url("/test/test")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); +} + +#[actix_rt::test] +async fn multiple_multi_path_async() { + let srv = actix_test::start(|| App::new().service(scope_module::multiple_separate_paths)); + + let request = srv.request(http::Method::HEAD, srv.url("/test/test1")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + + let request = srv.request(http::Method::CONNECT, srv.url("/test/test2")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + + let request = srv.request(http::Method::OPTIONS, srv.url("/test/test3")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + + let request = srv.request(http::Method::TRACE, srv.url("/test/test4")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); +} + +#[actix_rt::test] +async fn scope_delete_async() { + let srv = actix_test::start(|| App::new().service(scope_module::delete)); + + let request = srv.request(http::Method::DELETE, srv.url("/test/test")); + let mut response = request.send().await.unwrap(); + let body = response.body().await.unwrap(); + let body_str = String::from_utf8(body.to_vec()).unwrap(); + assert_eq!(body_str, "delete works"); +} + +#[actix_rt::test] +async fn scope_get_with_guard_async() { + let srv = actix_test::start(|| App::new().service(scope_module::guard)); + + let request = srv + .request(http::Method::GET, srv.url("/test/test/guard")) + .insert_header(("Accept", "image/*")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); +} + +#[actix_rt::test] +async fn scope_v1_v2_async() { + let srv = actix_test::start(|| { + App::new() + .service(mod_scope_v1::test) + .service(mod_scope_v2::test) + }); + + let request = srv.request(http::Method::GET, srv.url("/v1/test")); + let mut response = request.send().await.unwrap(); + let body = response.body().await.unwrap(); + let body_str = String::from_utf8(body.to_vec()).unwrap(); + assert_eq!(body_str, "version1 works"); + + let request = srv.request(http::Method::GET, srv.url("/v2/test")); + let mut response = request.send().await.unwrap(); + let body = response.body().await.unwrap(); + let body_str = String::from_utf8(body.to_vec()).unwrap(); + assert_eq!(body_str, "version2 works"); +} diff --git a/actix-web-codegen/tests/trybuild.rs b/actix-web-codegen/tests/trybuild.rs index 88f77548b..91073cf3b 100644 --- a/actix-web-codegen/tests/trybuild.rs +++ b/actix-web-codegen/tests/trybuild.rs @@ -18,6 +18,11 @@ 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.compile_fail("tests/trybuild/scope-trailing-slash.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 { + | ^^^^^ diff --git a/actix-web-codegen/tests/trybuild/scope-trailing-slash.rs b/actix-web-codegen/tests/trybuild/scope-trailing-slash.rs new file mode 100644 index 000000000..84632b59f --- /dev/null +++ b/actix-web-codegen/tests/trybuild/scope-trailing-slash.rs @@ -0,0 +1,6 @@ +use actix_web_codegen::scope; + +#[scope("/api/")] +mod api {} + +fn main() {} diff --git a/actix-web-codegen/tests/trybuild/scope-trailing-slash.stderr b/actix-web-codegen/tests/trybuild/scope-trailing-slash.stderr new file mode 100644 index 000000000..66933432e --- /dev/null +++ b/actix-web-codegen/tests/trybuild/scope-trailing-slash.stderr @@ -0,0 +1,5 @@ +error: scopes should not have trailing slashes; see https://docs.rs/actix-web/4/actix_web/struct.Scope.html#avoid-trailing-slashes + --> tests/trybuild/scope-trailing-slash.rs:3:9 + | +3 | #[scope("/api/")] + | ^^^^^^^ diff --git a/actix-web/src/lib.rs b/actix-web/src/lib.rs index f86a74406..205391388 100644 --- a/actix-web/src/lib.rs +++ b/actix-web/src/lib.rs @@ -145,5 +145,6 @@ codegen_reexport!(delete); codegen_reexport!(trace); codegen_reexport!(connect); codegen_reexport!(options); +codegen_reexport!(scope); pub(crate) type BoxError = Box; diff --git a/awc/src/client/connector.rs b/awc/src/client/connector.rs index 5d0b655a4..f3d443070 100644 --- a/awc/src/client/connector.rs +++ b/awc/src/client/connector.rs @@ -1080,7 +1080,7 @@ mod resolver { // resolver struct is cached in thread local so new clients can reuse the existing instance thread_local! { - static TRUST_DNS_RESOLVER: RefCell> = RefCell::new(None); + static TRUST_DNS_RESOLVER: RefCell> = const { RefCell::new(None) }; } // get from thread local or construct a new trust-dns resolver. diff --git a/justfile b/justfile index d92f4bd3d..4e106ef11 100644 --- a/justfile +++ b/justfile @@ -4,7 +4,7 @@ _list: # Format workspace. fmt: cargo +nightly fmt - npx -y prettier --write $(fd --type=file --hidden --extension=md --extension=yml) + fd --hidden --type=file --extension=md --extension=yml --exec-batch npx -y prettier --write # Downgrade dev-dependencies necessary to run MSRV checks/tests. [private] @@ -32,6 +32,10 @@ all_crate_features := if os() == "linux" { "--features='" + non_linux_all_features_list + "'" } +# Run Clippy over workspace. +clippy toolchain="": + cargo {{ toolchain }} clippy --workspace --all-targets {{ all_crate_features }} + # Test workspace using MSRV. test-msrv: downgrade-for-msrv (test msrv_rustup)