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

axum-debug: Use proc-macro-crate for referencing axum in generated code #507

Closed
wants to merge 1 commit into from

Conversation

jplatte
Copy link
Member

@jplatte jplatte commented Nov 13, 2021

Allows users to rename axum (say to axum03) in Cargo.toml and still have axum-debug work.

Would also allow usage of axum-debug in axum itself (possibly relevant for in-crate tests) if bkchr/proc-macro-crate#11 were fixed (for this specific case there is a workaround though).

@jplatte
Copy link
Member Author

jplatte commented Nov 13, 2021

After reading through that issue again I'm no longer certain this is a great idea, since there's so many edge cases all over the place, but with the fallback being the old behavior it can't really do much harm either.

Feel free to simply close if you're not convinced that this makes sense.

Copy link
Member

@davidpdrsn davidpdrsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! I didn't know this was a thing.

@davidpdrsn
Copy link
Member

Ah I didn't read your comment before reviewing. I'm not really familiar with the solution so not sure 🤔

@jplatte
Copy link
Member Author

jplatte commented Nov 13, 2021

Okay, so quick overview: The proc-macro-crate crate does some ugly hacks to support renamed "sibling" crates, i.e. ones that you want to refer to from generated code. The most basic hack is finding the Cargo.toml of the crate currently being compiled, and searching its [dependencies] for the crate you want to refer to.

It also does some more checks to see whether maybe the crate currently being compiled is the one you want to refer to, in which case you would usually emit crate instead of ::axum. However, unfortunately it is currently impossible to distinguish between a crate and its doctests being compiled (which are separate compilation units where crate doesn't refer to anything meaningful) and so I didn't include the FoundCrate::Itself match arm.

If you feel like supporting axum03 = { package = "axum", version = "0.3" } is worthwhile even if it requires a few hacks, press Merge. If you'd rather not use these sorts of hacks, press Close. 😄

@davidpdrsn
Copy link
Member

Alright. If axum_debug::debug_handler was exported directly by axum, would we still have this issue? Then users wouldn't depend on axum-debug directly.

@jplatte
Copy link
Member Author

jplatte commented Nov 13, 2021

Alright. If axum_debug::debug_handler was exported directly by axum, would we still have this issue? Then users wouldn't depend on axum-debug directly.

That would actually make no difference at all. If somebody. was writing use axum03::debug_handler; where that's the macro that's currently on main (w/o these changes) and apply #[debug_handler], the macro would generate code referring to the non-existing ::axum.

@jplatte
Copy link
Member Author

jplatte commented Nov 13, 2021

Beta build broke because

  = note: this error originates in the attribute macro `debug_handler` (in Nightly builds, run with -Z macro-backtrace for more info)

wasn't expected in trybuild test output. Maybe a new beta was just released that changed sth. there?

@davidpdrsn
Copy link
Member

I think we should be a little conservative with this and not merge it. If someone files an issue asking it then we can reconsider.

@davidpdrsn davidpdrsn closed this Nov 13, 2021
@davidpdrsn davidpdrsn deleted the proc-macro-crate branch November 13, 2021 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants