fix: Pavex always uses a public path to refer to public items, even i…

…f they are defined in a private module

libs/pavexc/src/compiler/resolvers.rs

@@ -12,8 +12,8 @@ use rustdoc_types::{GenericArg, GenericArgs, GenericParamDefKind, ItemEnum, Type
 
 use crate::language::{
     Callable, Generic, GenericArgument, GenericLifetimeParameter, InvocationStyle, PathType,
-    ResolvedPath, ResolvedPathGenericArgument, ResolvedPathLifetime, ResolvedPathType,
-    ResolvedType, Slice, Tuple, TypeReference, UnknownPath,
+    ResolvedPath, ResolvedPathGenericArgument, ResolvedPathLifetime, ResolvedPathSegment,
+    ResolvedPathType, ResolvedType, Slice, Tuple, TypeReference, UnknownPath,
 };
 use crate::rustdoc::{CannotGetCrateData, RustdocKindExt};
 use crate::rustdoc::{CrateCollection, ResolvedItem};
@@ -421,11 +421,52 @@ pub(crate) fn resolve_callable(
             }
         }
     };
+
+    // We need to make sure that we always refer to user-registered types using a public path, even though they
+    // might have been registered using a private path (e.g. `self::...` where `self` is a private module).
+    // For this reason, we fall back to the canonical path—i.e. the shortest public path for the given item.
+    // TODO: We should do the same for qualified self, if it's populated.
+    let canonical_path =
+        match krate_collection.get_canonical_path_by_global_type_id(&callable_type.item.item_id) {
+            Ok(p) => {
+                let segments = p.into_iter().skip(1).map(|s| ResolvedPathSegment {
+                    ident: s.into(),
+                    generic_arguments: vec![],
+                });
+                // We use the first segment from the original callable path, since that's been resolved to the defining crate
+                // and we want to keep that information.
+                let mut segments: Vec<_> = std::iter::once(callable_path.segments[0].clone())
+                    .chain(segments)
+                    .collect();
+                // The canonical path doesn't include the (populated or omitted) generic arguments from the user-provided callable path,
+                // so we need to add them back in.
+                segments.last_mut().unwrap().generic_arguments = callable_path
+                    .segments
+                    .last()
+                    .unwrap()
+                    .generic_arguments
+                    .clone();
+                ResolvedPath {
+                    segments,
+                    qualified_self: callable_path.qualified_self.clone(),
+                    package_id: callable_type.item.item_id.package_id.clone(),
+                }
+            }
+            Err(e) => {
+                tracing::warn!(
+                    package_id = ?callable_type.item.item_id.package_id,
+                    item_id = ?callable_type.item.item_id.rustdoc_item_id,
+                    "Failed to find canonical path: {e:?}"
+                );
+                callable_path.to_owned()
+            }
+        };
+
     let callable = Callable {
         is_async: header.is_async,
         takes_self_as_ref,
         output: output_type_path,
-        path: callable_path.to_owned(),
+        path: canonical_path,
         inputs: resolved_parameter_types,
         invocation_style,
         source_coordinates: Some(callable_type.item.item_id),

libs/ui_tests/Cargo.toml

@@ -239,6 +239,8 @@ members = [
   "path_parameters/path_parameters_unsupported_types/generated_app",
   "reflection/a_fix_is_suggested_for_function_invocations",
   "reflection/a_fix_is_suggested_for_function_invocations/generated_app",
+  "reflection/always_use_public_path",
+  "reflection/always_use_public_path/generated_app",
   "reflection/ambiguous_dependency_version_is_handled",
   "reflection/ambiguous_dependency_version_is_handled/generated_app",
   "reflection/arc_singletons_are_supported",

libs/ui_tests/reflection/always_use_public_path/Cargo.toml

@@ -0,0 +1,17 @@
+[package]
+name = "app_73b868ae"
+version = "0.1.0"
+edition = "2021"
+
+[lints.rust.unexpected_cfgs]
+level = "allow"
+check-cfg = ["cfg(pavex_ide_hint)"]
+
+[dependencies.pavex]
+workspace = true
+
+[dependencies.pavex_cli_client]
+workspace = true
+
+[dependencies.workspace_hack]
+workspace = true

libs/ui_tests/reflection/always_use_public_path/diagnostics.dot

@@ -0,0 +1,41 @@
+digraph "GET / - 0" {
+    0 [ label = "pavex::middleware::wrap_noop(pavex::middleware::Next<crate::route_0::Next0>) -> pavex::response::Response"]
+    1 [ label = "pavex::middleware::Next::new(crate::route_0::Next0) -> pavex::middleware::Next<crate::route_0::Next0>"]
+    2 [ label = "crate::route_0::Next0() -> crate::route_0::Next0"]
+    3 [ label = "<pavex::response::Response as pavex::response::IntoResponse>::into_response(pavex::response::Response) -> pavex::response::Response"]
+    1 -> 0 [ ]
+    2 -> 1 [ ]
+    0 -> 3 [ ]
+}
+
+digraph "GET / - 1" {
+    0 [ label = "app_73b868ae::handler(app_73b868ae::A) -> pavex::response::Response"]
+    1 [ label = "app_73b868ae::a() -> app_73b868ae::A"]
+    2 [ label = "<pavex::response::Response as pavex::response::IntoResponse>::into_response(pavex::response::Response) -> pavex::response::Response"]
+    1 -> 0 [ ]
+    0 -> 2 [ ]
+}
+
+digraph "* / - 0" {
+    0 [ label = "pavex::middleware::wrap_noop(pavex::middleware::Next<crate::route_1::Next0<'a>>) -> pavex::response::Response"]
+    1 [ label = "pavex::middleware::Next::new(crate::route_1::Next0<'a>) -> pavex::middleware::Next<crate::route_1::Next0<'a>>"]
+    2 [ label = "crate::route_1::Next0(&'a pavex::router::AllowedMethods) -> crate::route_1::Next0<'a>"]
+    4 [ label = "<pavex::response::Response as pavex::response::IntoResponse>::into_response(pavex::response::Response) -> pavex::response::Response"]
+    5 [ label = "&pavex::router::AllowedMethods"]
+    1 -> 0 [ ]
+    2 -> 1 [ ]
+    0 -> 4 [ ]
+    5 -> 2 [ ]
+}
+
+digraph "* / - 1" {
+    0 [ label = "pavex::router::default_fallback(&pavex::router::AllowedMethods) -> pavex::response::Response"]
+    2 [ label = "<pavex::response::Response as pavex::response::IntoResponse>::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"]
+}

libs/ui_tests/reflection/always_use_public_path/expectations/app.rs

@@ -0,0 +1,153 @@
+//! 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<u32>,
+    #[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<u32> {
+    let mut router = pavex_matchit::Router::new();
+    router.insert("/", 0u32).unwrap();
+    router
+}
+async fn route_request(
+    request: http::Request<hyper::body::Incoming>,
+    _connection_info: Option<pavex::connection::ConnectionInfo>,
+    server_state: std::sync::Arc<ServerState>,
+) -> 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;
+        <pavex::response::Response as pavex::response::IntoResponse>::into_response(v2)
+    }
+    async fn handler() -> pavex::response::Response {
+        let v0 = app::a();
+        let v1 = app::handler(v0);
+        <pavex::response::Response as pavex::response::IntoResponse>::into_response(v1)
+    }
+    struct Next0<T>
+    where
+        T: std::future::Future<Output = pavex::response::Response>,
+    {
+        next: fn() -> T,
+    }
+    impl<T> std::future::IntoFuture for Next0<T>
+    where
+        T: std::future::Future<Output = pavex::response::Response>,
+    {
+        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;
+        <pavex::response::Response as pavex::response::IntoResponse>::into_response(v3)
+    }
+    async fn handler(v0: &pavex::router::AllowedMethods) -> pavex::response::Response {
+        let v1 = pavex::router::default_fallback(v0).await;
+        <pavex::response::Response as pavex::response::IntoResponse>::into_response(v1)
+    }
+    struct Next0<'a, T>
+    where
+        T: std::future::Future<Output = pavex::response::Response>,
+    {
+        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<Output = pavex::response::Response>,
+    {
+        type Output = pavex::response::Response;
+        type IntoFuture = T;
+        fn into_future(self) -> Self::IntoFuture {
+            (self.next)(self.s_0)
+        }
+    }
+}

libs/ui_tests/reflection/always_use_public_path/expectations/diagnostics.dot

@@ -0,0 +1,41 @@
+digraph "GET / - 0" {
+    0 [ label = "pavex::middleware::wrap_noop(pavex::middleware::Next<crate::route_0::Next0>) -> pavex::response::Response"]
+    1 [ label = "pavex::middleware::Next::new(crate::route_0::Next0) -> pavex::middleware::Next<crate::route_0::Next0>"]
+    2 [ label = "crate::route_0::Next0() -> crate::route_0::Next0"]
+    3 [ label = "<pavex::response::Response as pavex::response::IntoResponse>::into_response(pavex::response::Response) -> pavex::response::Response"]
+    1 -> 0 [ ]
+    2 -> 1 [ ]
+    0 -> 3 [ ]
+}
+
+digraph "GET / - 1" {
+    0 [ label = "app::handler(app::A) -> pavex::response::Response"]
+    1 [ label = "app::a() -> app::A"]
+    2 [ label = "<pavex::response::Response as pavex::response::IntoResponse>::into_response(pavex::response::Response) -> pavex::response::Response"]
+    1 -> 0 [ ]
+    0 -> 2 [ ]
+}
+
+digraph "* / - 0" {
+    0 [ label = "pavex::middleware::wrap_noop(pavex::middleware::Next<crate::route_1::Next0<'a>>) -> pavex::response::Response"]
+    1 [ label = "pavex::middleware::Next::new(crate::route_1::Next0<'a>) -> pavex::middleware::Next<crate::route_1::Next0<'a>>"]
+    2 [ label = "crate::route_1::Next0(&'a pavex::router::AllowedMethods) -> crate::route_1::Next0<'a>"]
+    4 [ label = "<pavex::response::Response as pavex::response::IntoResponse>::into_response(pavex::response::Response) -> pavex::response::Response"]
+    5 [ label = "&pavex::router::AllowedMethods"]
+    1 -> 0 [ ]
+    2 -> 1 [ ]
+    0 -> 4 [ ]
+    5 -> 2 [ ]
+}
+
+digraph "* / - 1" {
+    0 [ label = "pavex::router::default_fallback(&pavex::router::AllowedMethods) -> pavex::response::Response"]
+    2 [ label = "<pavex::response::Response as pavex::response::IntoResponse>::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"]
+}

libs/ui_tests/reflection/always_use_public_path/src/lib.rs

@@ -0,0 +1,31 @@
+use pavex::blueprint::{router::GET, Blueprint};
+use pavex::f;
+use pavex::response::Response;
+
+pub use private::*;
+
+pub fn handler(_a: A) -> Response {
+    todo!()
+}
+
+pub fn blueprint() -> Blueprint {
+    let mut bp = Blueprint::new();
+    register(&mut bp);
+    bp.route(GET, "/", f!(self::handler));
+    bp
+}
+
+mod private {
+    use pavex::blueprint::Blueprint;
+    use pavex::f;
+
+    pub struct A;
+
+    pub fn register(bp: &mut Blueprint) {
+        bp.request_scoped(f!(self::a));
+    }
+
+    pub fn a() -> A {
+        todo!()
+    }
+}

libs/ui_tests/reflection/always_use_public_path/src/main.rs

@@ -0,0 +1,24 @@
+//! This code is generated by `pavex_test_runner`,
+//! Do NOT modify it manually.
+use app_73b868ae::blueprint;
+use pavex_cli_client::{Client, config::Color};
+use pavex_cli_client::commands::generate::GenerateError;
+
+fn main() -> Result<(), Box<dyn std::error::Error>> {
+    let ui_test_dir: std::path::PathBuf = std::env::var("UI_TEST_DIR").unwrap().into();
+    let outcome = Client::new()
+        .color(Color::Always)
+        .pavex_cli_path(std::env::var("PAVEX_TEST_CLI_PATH").unwrap().into())
+        .generate(blueprint(), ui_test_dir.join("generated_app"))
+        .diagnostics_path("diagnostics.dot".into())
+        .execute();
+    match outcome {
+        Ok(_) => {},
+        Err(GenerateError::NonZeroExitCode(_)) => { std::process::exit(1); }
+        Err(e) => {
+            eprintln!("Failed to invoke `pavex generate`.\n{:?}", e);
+            std::process::exit(1);
+        }
+    }
+    Ok(())
+}

libs/ui_tests/reflection/always_use_public_path/test_config.toml

@@ -0,0 +1,4 @@
+description = "When an item is available via multiple paths, some private and some public, Pavex will always use one of the public ones"
+
+[expectations]
+codegen = "pass"