From 6077a6f4aa4e9315f591d0d7bd99094f05847e95 Mon Sep 17 00:00:00 2001 From: Luca Palmieri <20745048+LukeMathWalker@users.noreply.github.com> Date: Sun, 28 Apr 2024 19:02:41 +0200 Subject: [PATCH] fix: anyhow::Result can be returned from constructors and other fallible components (#293) This PR fixes an issue in the bindings of generic parameters for type aliases. --- .../expectations/app.rs | 164 ++++++++++++++++++ .../expectations/diagnostics.dot | 51 ++++++ .../expectations/stderr.txt | 26 +++ .../lib.rs | 28 +++ .../test_config.toml | 8 + libs/pavexc/src/compiler/resolvers.rs | 7 +- 6 files changed, 280 insertions(+), 4 deletions(-) create mode 100644 libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/expectations/app.rs create mode 100644 libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/expectations/diagnostics.dot create mode 100644 libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/expectations/stderr.txt create mode 100644 libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/lib.rs create mode 100644 libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/test_config.toml diff --git a/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/expectations/app.rs b/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/expectations/app.rs new file mode 100644 index 000000000..f93354ecf --- /dev/null +++ b/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/expectations/app.rs @@ -0,0 +1,164 @@ +//! Do NOT edit this code. +//! It was automatically generated by Pavex. +//! All manual edits will be lost next time the code is generated. +extern crate alloc; +struct ServerState { + router: pavex_matchit::Router, + #[allow(dead_code)] + application_state: ApplicationState, +} +pub struct ApplicationState {} +pub async fn build_application_state() -> crate::ApplicationState { + crate::ApplicationState {} +} +pub fn run( + server_builder: pavex::server::Server, + application_state: ApplicationState, +) -> pavex::server::ServerHandle { + let server_state = std::sync::Arc::new(ServerState { + router: build_router(), + application_state, + }); + server_builder.serve(route_request, server_state) +} +fn build_router() -> pavex_matchit::Router { + let mut router = pavex_matchit::Router::new(); + router.insert("/home", 0u32).unwrap(); + router +} +async fn route_request( + request: http::Request, + _connection_info: Option, + server_state: std::sync::Arc, +) -> pavex::response::Response { + let (request_head, request_body) = request.into_parts(); + #[allow(unused)] + let request_body = pavex::request::body::RawIncomingBody::from(request_body); + let request_head: pavex::request::RequestHead = request_head.into(); + let matched_route = match server_state.router.at(&request_head.target.path()) { + Ok(m) => m, + Err(_) => { + let allowed_methods: pavex::router::AllowedMethods = pavex::router::MethodAllowList::from_iter( + vec![], + ) + .into(); + return route_1::entrypoint(&allowed_methods).await; + } + }; + let route_id = matched_route.value; + #[allow(unused)] + let url_params: pavex::request::path::RawPathParams<'_, '_> = matched_route + .params + .into(); + match route_id { + 0u32 => { + match &request_head.method { + &pavex::http::Method::GET => route_0::entrypoint().await, + _ => { + let allowed_methods: pavex::router::AllowedMethods = pavex::router::MethodAllowList::from_iter([ + pavex::http::Method::GET, + ]) + .into(); + route_1::entrypoint(&allowed_methods).await + } + } + } + i => unreachable!("Unknown route id: {}", i), + } +} +pub mod route_0 { + pub async fn entrypoint() -> pavex::response::Response { + let response = wrapping_0().await; + response + } + async fn stage_1() -> pavex::response::Response { + let response = handler().await; + response + } + async fn wrapping_0() -> pavex::response::Response { + let v0 = crate::route_0::Next0 { + next: stage_1, + }; + let v1 = pavex::middleware::Next::new(v0); + let v2 = pavex::middleware::wrap_noop(v1).await; + ::into_response(v2) + } + async fn handler() -> pavex::response::Response { + let v0 = app::A::new(); + let v1 = match v0 { + Ok(ok) => ok, + Err(v1) => { + return { + let v2 = app::error_handler(&v1); + ::into_response( + v2, + ) + }; + } + }; + let v2 = app::handler(v1); + ::into_response(v2) + } + struct Next0 + where + T: std::future::Future, + { + next: fn() -> T, + } + impl std::future::IntoFuture for Next0 + where + T: std::future::Future, + { + type Output = pavex::response::Response; + type IntoFuture = T; + fn into_future(self) -> Self::IntoFuture { + (self.next)() + } + } +} +pub mod route_1 { + pub async fn entrypoint<'a>( + s_0: &'a pavex::router::AllowedMethods, + ) -> pavex::response::Response { + let response = wrapping_0(s_0).await; + response + } + async fn stage_1<'a>( + s_0: &'a pavex::router::AllowedMethods, + ) -> pavex::response::Response { + let response = handler(s_0).await; + response + } + async fn wrapping_0( + v0: &pavex::router::AllowedMethods, + ) -> pavex::response::Response { + let v1 = crate::route_1::Next0 { + s_0: v0, + next: stage_1, + }; + let v2 = pavex::middleware::Next::new(v1); + let v3 = pavex::middleware::wrap_noop(v2).await; + ::into_response(v3) + } + async fn handler(v0: &pavex::router::AllowedMethods) -> pavex::response::Response { + let v1 = pavex::router::default_fallback(v0).await; + ::into_response(v1) + } + struct Next0<'a, T> + where + T: std::future::Future, + { + s_0: &'a pavex::router::AllowedMethods, + next: fn(&'a pavex::router::AllowedMethods) -> T, + } + impl<'a, T> std::future::IntoFuture for Next0<'a, T> + where + T: std::future::Future, + { + type Output = pavex::response::Response; + type IntoFuture = T; + fn into_future(self) -> Self::IntoFuture { + (self.next)(self.s_0) + } + } +} \ No newline at end of file diff --git a/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/expectations/diagnostics.dot b/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/expectations/diagnostics.dot new file mode 100644 index 000000000..6986bbf40 --- /dev/null +++ b/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/expectations/diagnostics.dot @@ -0,0 +1,51 @@ +digraph "GET /home - 0" { + 0 [ label = "pavex::middleware::wrap_noop(pavex::middleware::Next) -> pavex::response::Response"] + 1 [ label = "pavex::middleware::Next::new(crate::route_0::Next0) -> pavex::middleware::Next"] + 2 [ label = "crate::route_0::Next0() -> crate::route_0::Next0"] + 3 [ label = "::into_response(pavex::response::Response) -> pavex::response::Response"] + 1 -> 0 [ ] + 2 -> 1 [ ] + 0 -> 3 [ ] +} + +digraph "GET /home - 1" { + 0 [ label = "app::handler(app::A) -> pavex::response::Response"] + 1 [ label = "core::prelude::rust_2015::Result -> app::A"] + 2 [ label = "app::A::new() -> core::prelude::rust_2015::Result"] + 3 [ label = "::into_response(pavex::response::Response) -> pavex::response::Response"] + 4 [ label = "core::prelude::rust_2015::Result -> anyhow::Error"] + 5 [ label = "app::error_handler(&anyhow::Error) -> pavex::response::Response"] + 6 [ label = "::into_response(pavex::response::Response) -> pavex::response::Response"] + 7 [ label = "`match`"] + 1 -> 0 [ ] + 7 -> 4 [ ] + 7 -> 1 [ ] + 0 -> 3 [ ] + 4 -> 5 [ label = "&"] + 5 -> 6 [ ] + 2 -> 7 [ ] +} + +digraph "* /home - 0" { + 0 [ label = "pavex::middleware::wrap_noop(pavex::middleware::Next>) -> pavex::response::Response"] + 1 [ label = "pavex::middleware::Next::new(crate::route_1::Next0<'a>) -> pavex::middleware::Next>"] + 2 [ label = "crate::route_1::Next0(&'a pavex::router::AllowedMethods) -> crate::route_1::Next0<'a>"] + 4 [ label = "::into_response(pavex::response::Response) -> pavex::response::Response"] + 5 [ label = "&pavex::router::AllowedMethods"] + 1 -> 0 [ ] + 2 -> 1 [ ] + 0 -> 4 [ ] + 5 -> 2 [ ] +} + +digraph "* /home - 1" { + 0 [ label = "pavex::router::default_fallback(&pavex::router::AllowedMethods) -> pavex::response::Response"] + 2 [ label = "::into_response(pavex::response::Response) -> pavex::response::Response"] + 3 [ label = "&pavex::router::AllowedMethods"] + 0 -> 2 [ ] + 3 -> 0 [ ] +} + +digraph app_state { + 0 [ label = "crate::ApplicationState() -> crate::ApplicationState"] +} \ No newline at end of file diff --git a/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/expectations/stderr.txt b/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/expectations/stderr.txt new file mode 100644 index 000000000..e3f2756a1 --- /dev/null +++ b/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/expectations/stderr.txt @@ -0,0 +1,26 @@ +ERROR: + ร— I am not smart enough to figure out the concrete type for all the generic + โ”‚ parameters in `app::stream_file::`. + โ”‚ There should no unassigned generic parameters in request handlers, but `T` + โ”‚ does not seem to have been assigned a concrete type. + โ”‚ + โ”‚ โ•ญโ”€[src/lib.rs:8:1] + โ”‚  8 โ”‚ let mut bp = Blueprint::new(); + โ”‚  9 โ”‚ bp.route(GET, "/home", f!(crate::stream_file::)); + โ”‚ ยท  โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ฌโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + โ”‚ ยท โ•ฐโ”€โ”€ The request handler was registered here + โ”‚ 10 โ”‚ bp + โ”‚ โ•ฐโ”€โ”€โ”€โ”€ + โ”‚ ร— + โ”‚ โ•ญโ”€[src/lib.rs:1:1] + โ”‚ 1 โ”‚ pub fn stream_file(_inner: T) -> pavex_runtime::response::Response { + โ”‚ ยท  โ”ฌ + โ”‚ ยท โ•ฐโ”€โ”€ The generic parameter without a concrete type + โ”‚ 2 โ”‚ todo!() + โ”‚ โ•ฐโ”€โ”€โ”€โ”€ + โ”‚  help: Specify the concrete type for `T` when registering the request + โ”‚ handler against the blueprint: + โ”‚ | bp.route( + โ”‚ | .. + โ”‚ | f!(my_crate::my_handler::), + โ”‚ | ) \ No newline at end of file diff --git a/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/lib.rs b/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/lib.rs new file mode 100644 index 000000000..81864ff55 --- /dev/null +++ b/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/lib.rs @@ -0,0 +1,28 @@ +use std::path::PathBuf; + +use pavex::blueprint::{constructor::Lifecycle, router::GET, Blueprint}; +use pavex::f; + +pub struct A {} + +impl A { + pub fn new() -> anyhow::Result { + todo!() + } +} + +pub fn error_handler(_err: &anyhow::Error) -> pavex::response::Response { + todo!() +} + +pub fn handler(_inner: A) -> pavex::response::Response { + todo!() +} + +pub fn blueprint() -> Blueprint { + let mut bp = Blueprint::new(); + bp.request_scoped(f!(crate::A::new)) + .error_handler(f!(crate::error_handler)); + bp.route(GET, "/home", f!(crate::handler)); + bp +} diff --git a/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/test_config.toml b/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/test_config.toml new file mode 100644 index 000000000..405e391ae --- /dev/null +++ b/libs/pavex_cli/tests/ui_tests/reflection/self_as_generic_parameter_is_supported/test_config.toml @@ -0,0 +1,8 @@ +description = "pavex does not support generic functions as handlers (yet)" + +[expectations] +codegen = "pass" + +[dependencies] +anyhow = "1.0" + diff --git a/libs/pavexc/src/compiler/resolvers.rs b/libs/pavexc/src/compiler/resolvers.rs index 1648813e5..46adefcdf 100644 --- a/libs/pavexc/src/compiler/resolvers.rs +++ b/libs/pavexc/src/compiler/resolvers.rs @@ -48,7 +48,7 @@ pub(crate) fn resolve_type( let type_item = krate_collection.get_type_by_global_type_id(&global_type_id); // We want to remove any indirections (e.g. `type Foo = Bar;`) and get the actual type. if let ItemEnum::TypeAlias(type_alias) = &type_item.inner { - let mut generic_bindings = HashMap::new(); + let mut alias_generic_bindings = HashMap::new(); // The generic arguments that have been passed to the type alias. // E.g. `u32` in `Foo` for `type Foo = Bar;` let generic_args = if let Some(args) = args { @@ -91,7 +91,7 @@ pub(crate) fn resolve_type( name: generic_param_def.name.clone(), }) }; - generic_bindings + alias_generic_bindings .insert(generic_param_def.name.to_string(), generic_type); } GenericParamDefKind::Const { .. } @@ -104,7 +104,7 @@ pub(crate) fn resolve_type( &type_alias.type_, &global_type_id.package_id, krate_collection, - &generic_bindings, + &alias_generic_bindings, )?; Ok(type_) } else { @@ -307,7 +307,6 @@ pub(crate) fn resolve_callable( } } } - let fn_generic_args = &callable_path.segments.last().unwrap().generic_arguments; for (generic_arg, generic_def) in fn_generic_args.iter().zip(&fn_generics_defs.params) { let generic_name = &generic_def.name;