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

Improve debug_handler on tuple response types #2201

Merged
merged 27 commits into from
Dec 30, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
52c5634
Improve debug_handler macro on tuple output types.
SpeedReach Sep 1, 2023
c8acbe7
fix duplicate checking function names
SpeedReach Sep 2, 2023
b315200
added hints when named IntoResponse is placed in the middle of tuple.
SpeedReach Sep 2, 2023
4cd8498
formatted the file
SpeedReach Sep 2, 2023
1007d37
Added more tests, handled single tuple, and used filter and map for t…
SpeedReach Sep 11, 2023
befc809
Merge branch 'tokio-rs:main' into main
SpeedReach Sep 13, 2023
a5d9d0e
Use Position::Only to handle single tuple
SpeedReach Sep 13, 2023
1b259ba
cargo fix
SpeedReach Sep 13, 2023
c3709ea
Merge branch 'tokio-rs:main' into main
SpeedReach Oct 5, 2023
eab8716
fix clippy warnings
SpeedReach Oct 5, 2023
136b423
Merge branch 'tokio-rs:main' into main
SpeedReach Nov 9, 2023
e6a639d
Fixed tests, and simplify error messages.
SpeedReach Nov 9, 2023
4ebea36
Merge branch 'main' into SpeedReach/main
davidpdrsn Dec 30, 2023
863d21f
remove `.idea` dir
davidpdrsn Dec 30, 2023
e87f8f1
fix tests
davidpdrsn Dec 30, 2023
c9f40ce
clean up code a bit
davidpdrsn Dec 30, 2023
98bb828
add test for returning request parts
davidpdrsn Dec 30, 2023
b3c3363
changelog
davidpdrsn Dec 30, 2023
f04d3a4
fix indent
davidpdrsn Dec 30, 2023
1439d03
inline format args
davidpdrsn Dec 30, 2023
84638df
formatting
davidpdrsn Dec 30, 2023
40e06f2
use import
davidpdrsn Dec 30, 2023
80e51a1
more formatting
davidpdrsn Dec 30, 2023
0750e26
?
davidpdrsn Dec 30, 2023
44402d5
more formatting
davidpdrsn Dec 30, 2023
e9adb82
Update axum-macros/src/debug_handler.rs
davidpdrsn Dec 30, 2023
0ae6972
fix UI test output
davidpdrsn Dec 30, 2023
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
178 changes: 173 additions & 5 deletions axum-macros/src/debug_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
};
use proc_macro2::{Span, TokenStream};
use quote::{format_ident, quote, quote_spanned};
use syn::{parse::Parse, spanned::Spanned, FnArg, ItemFn, Token, Type};
use syn::{parse::Parse, spanned::Spanned, FnArg, ItemFn, ReturnType, Token, Type};

pub(crate) fn expand(attr: Attrs, item_fn: ItemFn) -> TokenStream {
let Attrs { state_ty } = attr;
Expand All @@ -15,6 +15,7 @@ pub(crate) fn expand(attr: Attrs, item_fn: ItemFn) -> TokenStream {

let check_extractor_count = check_extractor_count(&item_fn);
let check_path_extractor = check_path_extractor(&item_fn);
let check_output_tuples = check_output_tuples(&item_fn);
let check_output_impls_into_response = check_output_impls_into_response(&item_fn);

// If the function is generic, we can't reliably check its inputs or whether the future it
Expand Down Expand Up @@ -72,6 +73,7 @@ pub(crate) fn expand(attr: Attrs, item_fn: ItemFn) -> TokenStream {
#item_fn
#check_extractor_count
#check_path_extractor
#check_output_tuples
#check_output_impls_into_response
#check_inputs_and_future_send
}
Expand Down Expand Up @@ -284,6 +286,169 @@ fn check_inputs_impls_from_request(item_fn: &ItemFn, state_ty: Type) -> TokenStr
.collect::<TokenStream>()
}

///If the output is a tuple with 2 or more elements,
/// it checks with the following pattern,
/// first element => StatusCode || Parts || IntoResponseParts
///last element => IntoResponse
///other elements => IntoResponseParts
///the max numbers of IntoResponseParts(16)
fn check_output_tuples(item_fn: &ItemFn) -> Option<TokenStream> {
//Extract tuple types
let elements = match &item_fn.sig.output {
ReturnType::Type(_, ty) => match &**ty {
Type::Tuple(tuple) => &tuple.elems,
_ => return None,
},
_ => return None,
};

if elements.len() < 2 {
return None;
}
SpeedReach marked this conversation as resolved.
Show resolved Hide resolved
//Amount of IntoRequestParts
let mut parts_amount: i8 = 0;

let token_stream = WithPosition::new(elements.iter())
.enumerate()
.map(|(_idx, arg)| match &arg {
//First element type in the tuple
Position::First(ty) => {
let typename = extract_clean_typename(ty);
if typename.is_none() {
quote! {}
} else {
let typename = typename.unwrap();
match &*typename.to_string() {
"Parts" => quote! {},
"Response" => quote! {},
"StatusCode" => {
quote! {}
}
_ => {
parts_amount += 1;
check_into_response_parts(ty, parts_amount)
}
}
}
SpeedReach marked this conversation as resolved.
Show resolved Hide resolved
}
Position::Last(ty) => check_into_response(ty),
Position::Middle(ty) => {
parts_amount += 1;
if parts_amount > 16 {
let error_message = format!("Output Tuple cannot have more than 16 arguments.");
let error = syn::Error::new_spanned(&item_fn.sig.output, error_message)
.to_compile_error();
error
} else {
let name = _check_into_response_not_parts(ty);
match name {
Some(_) => {
quote_spanned!{ty.span()=>
compile_error!("This type only implements IntoResponse but not IntoResponseParts, try moving it to the last element");
}
},
None => check_into_response_parts(ty, parts_amount)
}
}
}
_ => quote! {},
})
.collect::<TokenStream>();
Some(token_stream)
}

fn _check_into_response_not_parts(ty: &Type) -> Option<&'static str> {
let name = extract_clean_typename(ty);
match name {
Some(name) => {
let s = match &*name {
"Json" => "Json<_>",
"String" => "String",
//todo add more types that implement IntoResponse but not IntoResponseParts
_ => return None,
};
Some(s)
}
None => None,
}
}
davidpdrsn marked this conversation as resolved.
Show resolved Hide resolved

fn check_into_response(ty: &Type) -> TokenStream {
let (span, ty) = (ty.span(), ty.clone());

let check_fn = format_ident!("__axum_macros_check_into_response_check", span = span,);

let call_check_fn = format_ident!("__axum_macros_check_into_response_call_check", span = span,);

let call_check_fn_body = quote_spanned! {span=>
#check_fn();
};

let from_request_bound = quote_spanned! {span=>
#ty: ::axum::response::IntoResponse
};
quote::quote_spanned! {span=>
#[allow(warnings)]
#[allow(unreachable_code)]
#[doc(hidden)]
fn #check_fn()
where
#from_request_bound,
{}

// we have to call the function to actually trigger a compile error
// since the function is generic, just defining it is not enough
#[allow(warnings)]
#[allow(unreachable_code)]
#[doc(hidden)]
fn #call_check_fn()
{
davidpdrsn marked this conversation as resolved.
Show resolved Hide resolved
#call_check_fn_body
}
}
}

fn check_into_response_parts(ty: &Type, index: i8) -> TokenStream {
let (span, ty) = (ty.span(), ty.clone());

let check_fn = format_ident!(
"__axum_macros_check_into_response_parts_{index}_check",
span = span,
);

let call_check_fn = format_ident!(
"__axum_macros_check_into_response_parts_{index}_call_check",
span = span,
);

let call_check_fn_body = quote_spanned! {span=>
#check_fn();
};

let from_request_bound = quote_spanned! {span=>
#ty: ::axum::response::IntoResponseParts
};
quote::quote_spanned! {span=>
davidpdrsn marked this conversation as resolved.
Show resolved Hide resolved
#[allow(warnings)]
#[allow(unreachable_code)]
#[doc(hidden)]
fn #check_fn()
where
#from_request_bound,
{}

// we have to call the function to actually trigger a compile error
// since the function is generic, just defining it is not enough
#[allow(warnings)]
#[allow(unreachable_code)]
#[doc(hidden)]
fn #call_check_fn()
{
davidpdrsn marked this conversation as resolved.
Show resolved Hide resolved
#call_check_fn_body
}
}
}

fn check_input_order(item_fn: &ItemFn) -> Option<TokenStream> {
let types_that_consume_the_request = item_fn
.sig
Expand Down Expand Up @@ -355,18 +520,21 @@ fn check_input_order(item_fn: &ItemFn) -> Option<TokenStream> {
}
}

fn request_consuming_type_name(ty: &Type) -> Option<&'static str> {
fn extract_clean_typename(ty: &Type) -> Option<String> {
let path = match ty {
Type::Path(type_path) => &type_path.path,
_ => return None,
};
path.segments.last().map(|p| p.ident.to_string())
}

let ident = match path.segments.last() {
Some(path_segment) => &path_segment.ident,
fn request_consuming_type_name(ty: &Type) -> Option<&'static str> {
let typename = match extract_clean_typename(ty) {
Some(tn) => tn,
None => return None,
};

let type_name = match &*ident.to_string() {
let type_name = match &*typename {
"Json" => "Json<_>",
"RawBody" => "RawBody<_>",
"RawForm" => "RawForm",
Expand Down
29 changes: 29 additions & 0 deletions axum-macros/tests/debug_handler/fail/output_tuple_too_many.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
use axum::response::AppendHeaders;

#[axum::debug_handler]
async fn handler(
) -> (
axum::http::StatusCode,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
axum::http::StatusCode,
) {
panic!()
}

fn main(){}
davidpdrsn marked this conversation as resolved.
Show resolved Hide resolved
48 changes: 48 additions & 0 deletions axum-macros/tests/debug_handler/fail/output_tuple_too_many.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
error: Output Tuple cannot have more than 16 arguments.
--> tests/debug_handler/fail/output_tuple_too_many.rs:5:3
|
5 | ) -> (
| ___^
6 | | axum::http::StatusCode,
7 | | AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
8 | | AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
... |
24 | | axum::http::StatusCode,
25 | | ) {
| |_^

error[E0277]: the trait bound `(StatusCode, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, StatusCode): IntoResponse` is not satisfied
--> tests/debug_handler/fail/output_tuple_too_many.rs:5:6
|
5 | ) -> (
| ______^
6 | | axum::http::StatusCode,
7 | | AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
8 | | AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
... |
24 | | axum::http::StatusCode,
25 | | ) {
| |_^ the trait `IntoResponse` is not implemented for `(StatusCode, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, AppendHeaders<[(HeaderName, &str); 1]>, StatusCode)`
|
= help: the following other types implement trait `IntoResponse`:
()
(R,)
(Response<()>, R)
(Response<()>, T1, R)
(Response<()>, T1, T2, R)
(Response<()>, T1, T2, T3, R)
(Response<()>, T1, T2, T3, T4, R)
(Response<()>, T1, T2, T3, T4, T5, R)
and $N others
note: required by a bound in `__axum_macros_check_handler_into_response::{closure#0}::check`
--> tests/debug_handler/fail/output_tuple_too_many.rs:5:6
|
5 | ) -> (
| ______^
6 | | axum::http::StatusCode,
7 | | AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
8 | | AppendHeaders<[(axum::http::HeaderName, &'static str); 1]>,
... |
24 | | axum::http::StatusCode,
25 | | ) {
| |_^ required by this bound in `check`
11 changes: 11 additions & 0 deletions axum-macros/tests/debug_handler/fail/wrong_return_tuple.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@

#[axum::debug_handler]
async fn handler() -> (
axum::http::StatusCode,
axum::Json<&'static str>,
axum::response::AppendHeaders<[( axum::http::HeaderName,&'static str); 1]>,
) {
SpeedReach marked this conversation as resolved.
Show resolved Hide resolved
panic!()
}

fn main(){}
davidpdrsn marked this conversation as resolved.
Show resolved Hide resolved
37 changes: 37 additions & 0 deletions axum-macros/tests/debug_handler/fail/wrong_return_tuple.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
error: This type only implements IntoResponse but not IntoResponseParts, try moving it to the last element
--> tests/debug_handler/fail/wrong_return_tuple.rs:5:5
|
5 | axum::Json<&'static str>,
| ^^^^^^^^^^^^^^^^^^^^^^^^

error[E0277]: the trait bound `(StatusCode, Json<&str>, AppendHeaders<[(HeaderName, &str); 1]>): IntoResponse` is not satisfied
--> tests/debug_handler/fail/wrong_return_tuple.rs:3:23
|
3 | async fn handler() -> (
| _______________________^
4 | | axum::http::StatusCode,
5 | | axum::Json<&'static str>,
6 | | axum::response::AppendHeaders<[( axum::http::HeaderName,&'static str); 1]>,
7 | | ) {
| |_^ the trait `IntoResponse` is not implemented for `(StatusCode, Json<&str>, AppendHeaders<[(HeaderName, &str); 1]>)`
|
= help: the following other types implement trait `IntoResponse`:
()
(R,)
(Response<()>, R)
(Response<()>, T1, R)
(Response<()>, T1, T2, R)
(Response<()>, T1, T2, T3, R)
(Response<()>, T1, T2, T3, T4, R)
(Response<()>, T1, T2, T3, T4, T5, R)
and $N others
note: required by a bound in `__axum_macros_check_handler_into_response::{closure#0}::check`
--> tests/debug_handler/fail/wrong_return_tuple.rs:3:23
|
3 | async fn handler() -> (
| _______________________^
4 | | axum::http::StatusCode,
5 | | axum::Json<&'static str>,
6 | | axum::response::AppendHeaders<[( axum::http::HeaderName,&'static str); 1]>,
7 | | ) {
| |_^ required by this bound in `check`