From d5de3bc7e3f7c3d956081dbff129e2fce390a8c2 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Sat, 19 Nov 2022 12:45:03 +0100 Subject: [PATCH] Improve compile errors for unimplemented traits (#1436) * Improve `debug_handler` to use the correct span for specific bounds This results in better localised error messages, as they now point directly to the corresponding argument instead of to the macro itself. * Improve some error messages behind a `nightly-error-messages` feature flag This uses the nightly only `rustc_on_unimplemented` attribute to improve some error messages when users try to use invalid handler functions. This should be seen as prove of concept, not as full solution for all potential error cases. The underlying feature is currently marked as permanently unstable, but I'm working on getting this specific attribute (or an attribute with different name, similar functionality) ready to work on a stable compiler. * Apply suggestions from code review Co-authored-by: Jonas Platte * Enable the `nightly-error-messages` feature unconditionally for nightly compilers * Use a nightly compiler to run the axum-marcos compile fail tests * update to newer nightly * Run axum-macros tests on nightly * tweak compile error hints a bit * more tweaks * update test Co-authored-by: Jonas Platte Co-authored-by: David Pedersen --- .github/workflows/CI.yml | 19 +++++++++++++++++-- axum-core/Cargo.toml | 3 +++ axum-core/build.rs | 7 +++++++ axum-core/src/extract/mod.rs | 12 ++++++++++++ axum-core/src/lib.rs | 1 + axum-macros/rust-toolchain | 1 + axum-macros/src/debug_handler.rs | 4 ++-- axum-macros/src/lib.rs | 4 ++-- .../fail/argument_not_extractor.stderr | 9 ++++----- ...doesnt_implement_from_request_parts.stderr | 10 ++++++---- .../tests/debug_handler/fail/not_send.stderr | 14 ++++++++++---- .../fail/generic_without_via.stderr | 1 + .../fail/generic_without_via_rejection.stderr | 1 + ...rride_rejection_on_enum_without_via.stderr | 2 ++ .../fail/parts_extracting_body.stderr | 2 ++ axum/Cargo.toml | 3 +++ axum/build.rs | 7 +++++++ axum/src/handler/mod.rs | 6 ++++++ axum/src/lib.rs | 1 + 19 files changed, 88 insertions(+), 19 deletions(-) create mode 100644 axum-core/build.rs create mode 100644 axum-macros/rust-toolchain create mode 100644 axum/build.rs diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 760b63bf..328fec27 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -75,8 +75,6 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - # nightly has an ICE, so ignore it for now - # rust: [stable, beta, nightly] rust: [stable, beta] steps: - uses: actions/checkout@master @@ -92,6 +90,23 @@ jobs: command: test args: --all --all-features --all-targets + test-nightly: + needs: check + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@master + - uses: actions-rs/toolchain@v1 + with: + # same as `axum-macros/rust-toolchain` + toolchain: nightly-2022-11-18 + override: true + profile: minimal + - uses: Swatinem/rust-cache@v1 + - name: Run nightly tests + working-directory: axum-macros + run: | + cargo test + # some examples doesn't support our MSRV (such as async-graphql) # so we only test axum itself on our MSRV test-msrv: diff --git a/axum-core/Cargo.toml b/axum-core/Cargo.toml index fb173cb6..dbe5346c 100644 --- a/axum-core/Cargo.toml +++ b/axum-core/Cargo.toml @@ -21,6 +21,9 @@ mime = "0.3.16" tower-layer = "0.3" tower-service = "0.3" +[build-dependencies] +rustversion = "1.0.9" + [dev-dependencies] axum = { path = "../axum", version = "0.6.0-rc.2" } futures-util = "0.3" diff --git a/axum-core/build.rs b/axum-core/build.rs new file mode 100644 index 00000000..b52885c6 --- /dev/null +++ b/axum-core/build.rs @@ -0,0 +1,7 @@ +#[rustversion::nightly] +fn main() { + println!("cargo:rustc-cfg=nightly_error_messages"); +} + +#[rustversion::not(nightly)] +fn main() {} diff --git a/axum-core/src/extract/mod.rs b/axum-core/src/extract/mod.rs index 83b7e6fe..2eb37ea4 100644 --- a/axum-core/src/extract/mod.rs +++ b/axum-core/src/extract/mod.rs @@ -39,6 +39,12 @@ mod private { /// /// [`axum::extract`]: https://docs.rs/axum/0.6.0-rc.2/axum/extract/index.html #[async_trait] +#[cfg_attr( + nightly_error_messages, + rustc_on_unimplemented( + note = "Function argument is not a valid axum extractor. \nSee `https://docs.rs/axum/latest/axum/extract/index.html` for details", + ) +)] pub trait FromRequestParts: Sized { /// If the extractor fails it'll use this "rejection" type. A rejection is /// a kind of error that can be converted into a response. @@ -102,6 +108,12 @@ pub trait FromRequestParts: Sized { /// [`http::Request`]: http::Request /// [`axum::extract`]: https://docs.rs/axum/0.6.0-rc.2/axum/extract/index.html #[async_trait] +#[cfg_attr( + nightly_error_messages, + rustc_on_unimplemented( + note = "Function argument is not a valid axum extractor. \nSee `https://docs.rs/axum/latest/axum/extract/index.html` for details", + ) +)] pub trait FromRequest: Sized { /// If the extractor fails it'll use this "rejection" type. A rejection is /// a kind of error that can be converted into a response. diff --git a/axum-core/src/lib.rs b/axum-core/src/lib.rs index 52a382dc..974e5e18 100644 --- a/axum-core/src/lib.rs +++ b/axum-core/src/lib.rs @@ -1,3 +1,4 @@ +#![cfg_attr(nightly_error_messages, feature(rustc_attrs))] //! Core types and traits for [`axum`]. //! //! Libraries authors that want to provide [`FromRequest`] or [`IntoResponse`] implementations diff --git a/axum-macros/rust-toolchain b/axum-macros/rust-toolchain new file mode 100644 index 00000000..8141c9d1 --- /dev/null +++ b/axum-macros/rust-toolchain @@ -0,0 +1 @@ +nightly-2022-11-18 diff --git a/axum-macros/src/debug_handler.rs b/axum-macros/src/debug_handler.rs index 74366aa4..ea685e09 100644 --- a/axum-macros/src/debug_handler.rs +++ b/axum-macros/src/debug_handler.rs @@ -251,11 +251,11 @@ fn check_inputs_impls_from_request( }; let from_request_bound = if must_impl_from_request_parts { - quote! { + quote_spanned! {span=> #ty: ::axum::extract::FromRequestParts<#state_ty> + Send } } else { - quote! { + quote_spanned! {span=> #ty: ::axum::extract::FromRequest<#state_ty, #body_ty, M> + Send } }; diff --git a/axum-macros/src/lib.rs b/axum-macros/src/lib.rs index 217350f6..43741ddf 100644 --- a/axum-macros/src/lib.rs +++ b/axum-macros/src/lib.rs @@ -699,7 +699,7 @@ where #[cfg(test)] fn run_ui_tests(directory: &str) { - #[rustversion::stable] + #[rustversion::nightly] fn go(directory: &str) { let t = trybuild::TestCases::new(); @@ -725,7 +725,7 @@ fn run_ui_tests(directory: &str) { } } - #[rustversion::not(stable)] + #[rustversion::not(nightly)] fn go(_directory: &str) {} go(directory); diff --git a/axum-macros/tests/debug_handler/fail/argument_not_extractor.stderr b/axum-macros/tests/debug_handler/fail/argument_not_extractor.stderr index 15281ee2..0afef5f2 100644 --- a/axum-macros/tests/debug_handler/fail/argument_not_extractor.stderr +++ b/axum-macros/tests/debug_handler/fail/argument_not_extractor.stderr @@ -4,6 +4,8 @@ error[E0277]: the trait bound `bool: FromRequestParts<()>` is not satisfied 4 | async fn handler(foo: bool) {} | ^^^^ the trait `FromRequestParts<()>` is not implemented for `bool` | + = note: Function argument is not a valid axum extractor. + See `https://docs.rs/axum/latest/axum/extract/index.html` for details = help: the following other types implement trait `FromRequestParts`: <() as FromRequestParts> <(T1, T2) as FromRequestParts> @@ -16,10 +18,7 @@ error[E0277]: the trait bound `bool: FromRequestParts<()>` is not satisfied and 25 others = note: required for `bool` to implement `FromRequest<(), Body, axum_core::extract::private::ViaParts>` note: required by a bound in `__axum_macros_check_handler_0_from_request_check` - --> tests/debug_handler/fail/argument_not_extractor.rs:3:1 + --> tests/debug_handler/fail/argument_not_extractor.rs:4:23 | -3 | #[debug_handler] - | ^^^^^^^^^^^^^^^^ required by this bound in `__axum_macros_check_handler_0_from_request_check` 4 | async fn handler(foo: bool) {} - | ---- required by a bound in this - = note: this error originates in the attribute macro `debug_handler` (in Nightly builds, run with -Z macro-backtrace for more info) + | ^^^^ required by this bound in `__axum_macros_check_handler_0_from_request_check` diff --git a/axum-macros/tests/debug_handler/fail/doesnt_implement_from_request_parts.stderr b/axum-macros/tests/debug_handler/fail/doesnt_implement_from_request_parts.stderr index 03803249..9614cf4e 100644 --- a/axum-macros/tests/debug_handler/fail/doesnt_implement_from_request_parts.stderr +++ b/axum-macros/tests/debug_handler/fail/doesnt_implement_from_request_parts.stderr @@ -1,9 +1,11 @@ error[E0277]: the trait bound `String: FromRequestParts<()>` is not satisfied - --> tests/debug_handler/fail/doesnt_implement_from_request_parts.rs:4:1 + --> tests/debug_handler/fail/doesnt_implement_from_request_parts.rs:5:21 | -4 | #[debug_handler] - | ^^^^^^^^^^^^^^^^ the trait `FromRequestParts<()>` is not implemented for `String` +5 | async fn handler(_: String, _: Method) {} + | ^^^^^^ the trait `FromRequestParts<()>` is not implemented for `String` | + = note: Function argument is not a valid axum extractor. + See `https://docs.rs/axum/latest/axum/extract/index.html` for details = help: the following other types implement trait `FromRequestParts`: <() as FromRequestParts> <(T1, T2) as FromRequestParts> @@ -15,4 +17,4 @@ error[E0277]: the trait bound `String: FromRequestParts<()>` is not satisfied <(T1, T2, T3, T4, T5, T6, T7, T8) as FromRequestParts> and 25 others = help: see issue #48214 - = note: this error originates in the attribute macro `debug_handler` (in Nightly builds, run with -Z macro-backtrace for more info) + = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable diff --git a/axum-macros/tests/debug_handler/fail/not_send.stderr b/axum-macros/tests/debug_handler/fail/not_send.stderr index 3e9019cf..fd5d91e8 100644 --- a/axum-macros/tests/debug_handler/fail/not_send.stderr +++ b/axum-macros/tests/debug_handler/fail/not_send.stderr @@ -1,8 +1,11 @@ error: future cannot be sent between threads safely --> tests/debug_handler/fail/not_send.rs:4:1 | -4 | async fn handler() { - | ^^^^^ future returned by `handler` is not `Send` +4 | / async fn handler() { +5 | | let rc = std::rc::Rc::new(()); +6 | | async {}.await; +7 | | } + | |_^ future returned by `handler` is not `Send` | = help: within `impl Future`, the trait `Send` is not implemented for `Rc<()>` note: future is not `Send` as this value is used across an await @@ -17,5 +20,8 @@ note: future is not `Send` as this value is used across an await note: required by a bound in `check` --> tests/debug_handler/fail/not_send.rs:4:1 | -4 | async fn handler() { - | ^^^^^ required by this bound in `check` +4 | / async fn handler() { +5 | | let rc = std::rc::Rc::new(()); +6 | | async {}.await; +7 | | } + | |_^ required by this bound in `check` diff --git a/axum-macros/tests/from_request/fail/generic_without_via.stderr b/axum-macros/tests/from_request/fail/generic_without_via.stderr index a943a54f..760d36e9 100644 --- a/axum-macros/tests/from_request/fail/generic_without_via.stderr +++ b/axum-macros/tests/from_request/fail/generic_without_via.stderr @@ -12,6 +12,7 @@ error[E0277]: the trait bound `fn(Extractor<()>) -> impl Future {fo | | | required by a bound introduced by this call | + = note: Consider using `#[axum::debug_handler]` to improve the error message = help: the trait `Handler` is implemented for `Layered` note: required by a bound in `axum::routing::get` --> $WORKSPACE/axum/src/routing/method_routing.rs diff --git a/axum-macros/tests/from_request/fail/generic_without_via_rejection.stderr b/axum-macros/tests/from_request/fail/generic_without_via_rejection.stderr index 8d53346e..dd34af70 100644 --- a/axum-macros/tests/from_request/fail/generic_without_via_rejection.stderr +++ b/axum-macros/tests/from_request/fail/generic_without_via_rejection.stderr @@ -12,6 +12,7 @@ error[E0277]: the trait bound `fn(Extractor<()>) -> impl Future {fo | | | required by a bound introduced by this call | + = note: Consider using `#[axum::debug_handler]` to improve the error message = help: the trait `Handler` is implemented for `Layered` note: required by a bound in `axum::routing::get` --> $WORKSPACE/axum/src/routing/method_routing.rs diff --git a/axum-macros/tests/from_request/fail/override_rejection_on_enum_without_via.stderr b/axum-macros/tests/from_request/fail/override_rejection_on_enum_without_via.stderr index 6b2b8bbe..7e7e9be4 100644 --- a/axum-macros/tests/from_request/fail/override_rejection_on_enum_without_via.stderr +++ b/axum-macros/tests/from_request/fail/override_rejection_on_enum_without_via.stderr @@ -12,6 +12,7 @@ error[E0277]: the trait bound `fn(MyExtractor) -> impl Future {hand | | | required by a bound introduced by this call | + = note: Consider using `#[axum::debug_handler]` to improve the error message = help: the trait `Handler` is implemented for `Layered` note: required by a bound in `axum::routing::get` --> $WORKSPACE/axum/src/routing/method_routing.rs @@ -28,6 +29,7 @@ error[E0277]: the trait bound `fn(Result) -> impl Futu | | | required by a bound introduced by this call | + = note: Consider using `#[axum::debug_handler]` to improve the error message = help: the trait `Handler` is implemented for `Layered` note: required by a bound in `MethodRouter::::post` --> $WORKSPACE/axum/src/routing/method_routing.rs diff --git a/axum-macros/tests/from_request/fail/parts_extracting_body.stderr b/axum-macros/tests/from_request/fail/parts_extracting_body.stderr index 32341828..63062d42 100644 --- a/axum-macros/tests/from_request/fail/parts_extracting_body.stderr +++ b/axum-macros/tests/from_request/fail/parts_extracting_body.stderr @@ -4,6 +4,8 @@ error[E0277]: the trait bound `String: FromRequestParts` is not satisfied 5 | body: String, | ^^^^^^ the trait `FromRequestParts` is not implemented for `String` | + = note: Function argument is not a valid axum extractor. + See `https://docs.rs/axum/latest/axum/extract/index.html` for details = help: the following other types implement trait `FromRequestParts`: <() as FromRequestParts> <(T1, T2) as FromRequestParts> diff --git a/axum/Cargo.toml b/axum/Cargo.toml index 7cb9de12..06d73f04 100644 --- a/axum/Cargo.toml +++ b/axum/Cargo.toml @@ -63,6 +63,9 @@ sha-1 = { version = "0.10", optional = true } tokio = { package = "tokio", version = "1.21", features = ["time"], optional = true } tokio-tungstenite = { version = "0.17.2", optional = true } +[build-dependencies] +rustversion = "1.0.9" + [dev-dependencies] anyhow = "1.0" futures = "0.3" diff --git a/axum/build.rs b/axum/build.rs new file mode 100644 index 00000000..b52885c6 --- /dev/null +++ b/axum/build.rs @@ -0,0 +1,7 @@ +#[rustversion::nightly] +fn main() { + println!("cargo:rustc-cfg=nightly_error_messages"); +} + +#[rustversion::not(nightly)] +fn main() {} diff --git a/axum/src/handler/mod.rs b/axum/src/handler/mod.rs index fe76f9bd..6a2df1e6 100644 --- a/axum/src/handler/mod.rs +++ b/axum/src/handler/mod.rs @@ -94,6 +94,12 @@ pub use self::service::HandlerService; /// {} /// ``` #[doc = include_str!("../docs/debugging_handler_type_errors.md")] +#[cfg_attr( + nightly_error_messages, + rustc_on_unimplemented( + note = "Consider using `#[axum::debug_handler]` to improve the error message" + ) +)] pub trait Handler: Clone + Send + Sized + 'static { /// The type of future calling this handler returns. type Future: Future + Send + 'static; diff --git a/axum/src/lib.rs b/axum/src/lib.rs index 60c33834..5d2a8de0 100644 --- a/axum/src/lib.rs +++ b/axum/src/lib.rs @@ -1,3 +1,4 @@ +#![cfg_attr(nightly_error_messages, feature(rustc_attrs))] //! axum is a web application framework that focuses on ergonomics and modularity. //! //! # Table of contents