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 support for generic functions to #[debug_handler] #1253

Closed
jplatte opened this issue Aug 12, 2022 · 9 comments
Closed

Add support for generic functions to #[debug_handler] #1253

jplatte opened this issue Aug 12, 2022 · 9 comments
Assignees
Labels
A-axum-macros C-enhancement Category: A PR with an enhancement E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Experience needed to fix: Medium / intermediate

Comments

@jplatte
Copy link
Member

jplatte commented Aug 12, 2022

I've had an idea for a while that would make generic handlers pretty much fully checkable, that is:

#[debug_handler(with(T = String, T = u64; U = i8, U = i16; N = 0))]
fn handler<T, U, const N: usize>(extract_t: Path<T>, extract_u: Json<U>, extract_n: Foo<N>)
where
    T: Deserialize,
    U: Deserialize,
{
    // ...
}

could make debug_handler add FromRequest assertions for Path<String>, Path<u64>, Json<i8>, Json<i16> and Foo<0> while also adding Send assertions for the futures returned by

  • handler::<String, i8, 0>
  • handler::<String, i16, 0>
  • handler::<u64, i8, 0>
  • handler::<u64, i16, 0>

If we were to do that, we could just require users to add this with() annotation before any parameters are checked on a generic function (even ones that don't mention the generic types) as it should be easy enough.

Originally posted in #1251 (comment)

@jplatte jplatte added C-enhancement Category: A PR with an enhancement E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Experience needed to fix: Medium / intermediate A-axum-macros labels Aug 12, 2022
@jplatte
Copy link
Member Author

jplatte commented Aug 12, 2022

For anybody who wants to work on this, here's some tips:

  • If you aren't familiar with the debug_handler macro yet, go read its documentation to get an understanding of what it does
  • We also have tests for it here, the most interesting ones are probably the ones in fail, where every .rs test case has a corresponding .stderr file showing the error generated by the macro
  • You can find the macro implementation here
  • If you have never worked with procedural macro code before and find the code hard to read, maybe my blog series on proc-macros over at blog.turbo.fish can help :)
  • The important parts for this this feature are the functions check_inputs_impl_from_request for the FromRequest implementation assertions and check_future_send for the Send assertions
  • I'm pretty sure there's nothing to be done for lifetime generics, those should just be an error
  • It's probably worthwhile to start with type generic support and add const generic support later, if at all
  • Use syn::visit_mut to substitute the concrete types for uses of a generic parameter, this ensures you find them all even in very complex cases like <<T as Trait>::Foo as some_crate::OtherTrait<U>>::Bar

@slessans
Copy link

@jplatte I am interested in taking a crack at this. thanks for the extensive write-up!

@davidpdrsn davidpdrsn changed the title Add support for generic functions to #[debug_handler)] Add support for generic functions to #[debug_handler] Aug 15, 2022
@slessans
Copy link

@jplatte here is a proof-of-concept that implements this for generic handler args for early feedback if you have some time.

#1265

will move onto return types next

@davidpdrsn davidpdrsn changed the title Add support for generic functions to #[debug_handler] Add support for generic functions to #[debug_handler] Aug 22, 2022
@RoKu1
Copy link

RoKu1 commented Nov 21, 2022

Is it still open for taking. I am new here, was looking for some simple issue to work on.

@jplatte
Copy link
Member Author

jplatte commented Nov 21, 2022

No, this is mostly done in #1265.

@RoKu1
Copy link

RoKu1 commented Nov 21, 2022

Hi, Is there any simple task I can take up.

@0xdeepmehta
Copy link

any updates here?

@davidpdrsn
Copy link
Member

@0xdeepmehta No. I still have to review the PR.

@jplatte
Copy link
Member Author

jplatte commented Apr 14, 2023

As per latest comments in the PR, I'll close this since it's probably not worth the maintenance burden to support.

@jplatte jplatte closed this as not planned Won't fix, can't repro, duplicate, stale Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-axum-macros C-enhancement Category: A PR with an enhancement E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Experience needed to fix: Medium / intermediate
Projects
None yet
Development

No branches or pull requests

5 participants