Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add special handling of FromRequest extractors not being the last arg #1797

Merged
merged 2 commits into from
Mar 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion axum-macros/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

# Unreleased

- None.
- **fixed:** In `#[debug_handler]` provide specific errors about `FromRequest`
extractors not being the last argument ([#1797])

[#1797]: https://github.com/tokio-rs/axum/pull/1797

# 0.3.4 (12. February, 2022)

Expand Down
116 changes: 111 additions & 5 deletions axum-macros/src/debug_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,22 @@ pub(crate) fn expand(attr: Attrs, item_fn: ItemFn) -> TokenStream {
err.unwrap_or_else(|| {
let state_ty = state_ty.unwrap_or_else(|| syn::parse_quote!(()));

let check_inputs_impls_from_request =
check_inputs_impls_from_request(&item_fn, &body_ty, state_ty);
let check_input_order = check_input_order(&item_fn);
let check_future_send = check_future_send(&item_fn);

quote! {
#check_inputs_impls_from_request
#check_future_send
if let Some(check_input_order) = check_input_order {
quote! {
#check_input_order
#check_future_send
}
} else {
let check_inputs_impls_from_request =
check_inputs_impls_from_request(&item_fn, &body_ty, state_ty);

quote! {
#check_inputs_impls_from_request
#check_future_send
}
}
})
} else {
Expand Down Expand Up @@ -278,6 +287,103 @@ fn check_inputs_impls_from_request(
.collect::<TokenStream>()
}

fn check_input_order(item_fn: &ItemFn) -> Option<TokenStream> {
let types_that_consume_the_request = item_fn
.sig
.inputs
.iter()
.enumerate()
.filter_map(|(idx, arg)| {
let ty = match arg {
FnArg::Typed(pat_type) => &*pat_type.ty,
FnArg::Receiver(_) => return None,
};
let span = ty.span();

let path = match ty {
Type::Path(type_path) => &type_path.path,
_ => return None,
};

let ident = match path.segments.last() {
Some(path_segment) => &path_segment.ident,
None => return None,
};

let type_name = match &*ident.to_string() {
"Json" => "Json<_>",
"BodyStream" => "BodyStream",
"RawBody" => "RawBody<_>",
"RawForm" => "RawForm",
"Multipart" => "Multipart",
"Protobuf" => "Protobuf",
"JsonLines" => "JsonLines<_>",
"Form" => "Form<_>",
"Request" => "Request<_>",
"Bytes" => "Bytes",
"String" => "String",
"Parts" => "Parts",
_ => return None,
};

Some((idx, type_name, span))
})
.collect::<Vec<_>>();

if types_that_consume_the_request.is_empty() {
return None;
};

// exactly one type that consumes the request
if types_that_consume_the_request.len() == 1 {
// and that is not the last
if types_that_consume_the_request[0].0 != item_fn.sig.inputs.len() - 1 {
let (_idx, type_name, span) = &types_that_consume_the_request[0];
let error = format!(
"`{type_name}` consumes the request body and thus must be \
the last argument to the handler function"
);
return Some(quote_spanned! {*span=>
compile_error!(#error);
});
} else {
return None;
}
}

if types_that_consume_the_request.len() == 2 {
let (_, first, _) = &types_that_consume_the_request[0];
let (_, second, _) = &types_that_consume_the_request[1];
let error = format!(
"Can't have two extractors that consume the request body. \
`{first}` and `{second}` both do that.",
);
let span = item_fn.sig.inputs.span();
Some(quote_spanned! {span=>
compile_error!(#error);
})
} else {
let types = WithPosition::new(types_that_consume_the_request.into_iter())
.map(|pos| match pos {
Position::First((_, type_name, _)) | Position::Middle((_, type_name, _)) => {
format!("`{type_name}`, ")
}
Position::Last((_, type_name, _)) => format!("and `{type_name}`"),
Position::Only(_) => unreachable!(),
})
.collect::<String>();

let error = format!(
"Can't have more than one extractor that consume the request body. \
{types} all do that.",
);
let span = item_fn.sig.inputs.span();
Some(quote_spanned! {span=>
compile_error!(#error);
})
}
}

fn check_output_impls_into_response(item_fn: &ItemFn) -> TokenStream {
let ty = match &item_fn.sig.output {
syn::ReturnType::Default => return quote! {},
Expand Down

This file was deleted.

This file was deleted.

10 changes: 10 additions & 0 deletions axum-macros/tests/debug_handler/fail/multiple_request_consumers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
use axum_macros::debug_handler;
use axum::{Json, body::Bytes, http::{Method, Uri}};

#[debug_handler]
async fn one(_: Json<()>, _: String, _: Uri) {}

#[debug_handler]
async fn two(_: Json<()>, _: Method, _: Bytes, _: Uri, _: String) {}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: Can't have two extractors that consume the request body. `Json<_>` and `String` both do that.
--> tests/debug_handler/fail/multiple_request_consumers.rs:5:14
|
5 | async fn one(_: Json<()>, _: String, _: Uri) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: Can't have more than one extractor that consume the request body. `Json<_>`, `Bytes`, and `String` all do that.
--> tests/debug_handler/fail/multiple_request_consumers.rs:8:14
|
8 | async fn two(_: Json<()>, _: Method, _: Bytes, _: Uri, _: String) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
10 changes: 10 additions & 0 deletions axum-macros/tests/debug_handler/fail/wrong_order.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
use axum_macros::debug_handler;
use axum::{Json, http::Uri};

#[debug_handler]
async fn one(_: Json<()>, _: Uri) {}

#[debug_handler]
async fn two(_: String, _: Uri) {}

fn main() {}
11 changes: 11 additions & 0 deletions axum-macros/tests/debug_handler/fail/wrong_order.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: `Json<_>` consumes the request body and thus must be the last argument to the handler function
--> tests/debug_handler/fail/wrong_order.rs:5:17
|
5 | async fn one(_: Json<()>, _: Uri) {}
| ^^^^^^^^

error: `String` consumes the request body and thus must be the last argument to the handler function
--> tests/debug_handler/fail/wrong_order.rs:8:17
|
8 | async fn two(_: String, _: Uri) {}
| ^^^^^^