fix: Pavex will never use private modules in the paths used to refer …

…to methods in the generated server SDK code.

libs/pavexc/src/compiler/resolvers.rs

@@ -15,7 +15,7 @@ use crate::language::{
     ResolvedPath, ResolvedPathGenericArgument, ResolvedPathLifetime, ResolvedPathSegment,
     ResolvedPathType, ResolvedType, Slice, Tuple, TypeReference, UnknownPath,
 };
-use crate::rustdoc::{CannotGetCrateData, RustdocKindExt};
+use crate::rustdoc::{CannotGetCrateData, ResolvedItemWithParent, RustdocKindExt};
 use crate::rustdoc::{CrateCollection, ResolvedItem};
 
 use super::utils::process_framework_path;
@@ -293,10 +293,15 @@ pub(crate) fn resolve_callable(
     krate_collection: &CrateCollection,
     callable_path: &ResolvedPath,
 ) -> Result<Callable, CallableResolutionError> {
-    let (callable_type, qualified_self_type) =
-        callable_path.find_rustdoc_items(krate_collection)?;
+    let (
+        ResolvedItemWithParent {
+            item: callable,
+            parent,
+        },
+        qualified_self_type,
+    ) = callable_path.find_rustdoc_items(krate_collection)?;
     let used_by_package_id = &callable_path.package_id;
-    let (header, decl, fn_generics_defs, invocation_style) = match &callable_type.item.item.inner {
+    let (header, decl, fn_generics_defs, invocation_style) = match &callable.item.inner {
         ItemEnum::Function(f) => (
             &f.header,
             &f.sig,
@@ -317,13 +322,16 @@ pub(crate) fn resolve_callable(
     if let Some(qself) = qualified_self_type {
         generic_bindings.insert("Self".to_string(), qself);
     }
-    if let Some(parent) = &callable_type.parent {
+    let parent_info = parent.map(|p| {
         let parent_segments = callable_path.segments[..callable_path.segments.len() - 1].to_vec();
         let parent_path = ResolvedPath {
             segments: parent_segments,
             qualified_self: callable_path.qualified_self.clone(),
             package_id: callable_path.package_id.clone(),
         };
+        (p, parent_path)
+    });
+    if let Some((parent, parent_path)) = &parent_info {
         if matches!(parent.item.inner, ItemEnum::Trait(_)) {
             if let Err(e) = get_trait_generic_bindings(
                 parent,
@@ -334,7 +342,7 @@ pub(crate) fn resolve_callable(
                 tracing::trace!(error.msg = %e, error.details = ?e, "Error getting trait generic bindings");
             }
         } else {
-            match resolve_type_path_with_item(&parent_path, parent, krate_collection) {
+            match resolve_type_path_with_item(parent_path, parent, krate_collection) {
                 Ok(parent_type) => {
                     generic_bindings.insert("Self".to_string(), parent_type);
                 }
@@ -357,7 +365,7 @@ pub(crate) fn resolve_callable(
             GenericParameterResolutionError {
                 generic_type: generic_type.to_owned(),
                 callable_path: callable_path.to_owned(),
-                callable_item: callable_type.item.item.clone().into_owned(),
+                callable_item: callable.item.clone().into_owned(),
                 source: Arc::new(e),
             }
         })?;
@@ -390,7 +398,7 @@ pub(crate) fn resolve_callable(
                 return Err(InputParameterResolutionError {
                     parameter_type: parameter_type.to_owned(),
                     callable_path: callable_path.to_owned(),
-                    callable_item: callable_type.item.item.into_owned(),
+                    callable_item: callable.item.into_owned(),
                     source: Arc::new(e),
                     parameter_index,
                 }
@@ -413,7 +421,7 @@ pub(crate) fn resolve_callable(
                     return Err(OutputTypeResolutionError {
                         output_type: output_type.to_owned(),
                         callable_path: callable_path.to_owned(),
-                        callable_item: callable_type.item.item.into_owned(),
+                        callable_item: callable.item.into_owned(),
                         source: Arc::new(e),
                     }
                     .into());
@@ -426,41 +434,108 @@ pub(crate) fn resolve_callable(
     // 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();
+    let canonical_path = {
+        // If the item is a method, we start by finding the canonical path to its parent—i.e. the struct/enum/trait
+        // to which the method is attached.
+        let parent_canonical_path = match parent_info {
+            Some((parent, parent_path)) => {
+                match krate_collection.get_canonical_path_by_global_type_id(&parent.item_id) {
+                    Ok(canonical_segments) => {
+                        let segments =
+                            canonical_segments
+                                .into_iter()
+                                .skip(1)
+                                .map(|s| ResolvedPathSegment {
+                                    ident: s.into(),
+                                    generic_arguments: vec![],
+                                });
+                        // We use the first segment from the original path, since that's been resolved to the name of the defining crate
+                        // and we want to keep that information.
+                        let mut segments: Vec<_> = std::iter::once(parent_path.segments[0].clone())
+                            .chain(segments)
+                            .collect();
+                        // The canonical path doesn't include the (populated or omitted) generic arguments from the user-provided path,
+                        // so we need to add them back in.
+                        segments.last_mut().unwrap().generic_arguments = parent_path
+                            .segments
+                            .last()
+                            .unwrap()
+                            .generic_arguments
+                            .clone();
+                        Some(ResolvedPath {
+                            segments,
+                            qualified_self: parent_path.qualified_self.clone(),
+                            package_id: parent.item_id.package_id.clone(),
+                        })
+                    }
+                    Err(_) => {
+                        tracing::warn!(
+                            package_id = parent.item_id.package_id.repr(),
+                            item_id = ?parent.item_id.rustdoc_item_id,
+                            "Failed to find canonical path for {:?}",
+                            parent_path
+                        );
+                        Some(parent_path)
+                    }
+                }
+            }
+            _ => None,
+        };
+
+        let canonical_path = match parent_canonical_path {
+            Some(p) => {
+                // We have already canonicalized the parent path, so we just need to append the method name and we're done.
+                let mut segments = p.segments;
+                segments.push(callable_path.segments.last().unwrap().clone());
                 ResolvedPath {
                     segments,
                     qualified_self: callable_path.qualified_self.clone(),
-                    package_id: callable_type.item.item_id.package_id.clone(),
+                    package_id: callable_path.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()
+            None => {
+                // There was no parent, it's a free function or a straight struct/enum. We need to go through the same process
+                // we applied for the parent.
+                match krate_collection.get_canonical_path_by_global_type_id(&callable.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.item_id.package_id.clone(),
+                        }
+                    }
+                    Err(_) => {
+                        tracing::warn!(
+                            package_id = callable.item_id.package_id.repr(),
+                            item_id = ?callable.item_id.rustdoc_item_id,
+                            "Failed to find canonical path for {:?}",
+                            callable_path
+                        );
+                        callable_path.to_owned()
+                    }
+                }
             }
         };
+        canonical_path
+    };
 
     let callable = Callable {
         is_async: header.is_async,
@@ -469,7 +544,7 @@ pub(crate) fn resolve_callable(
         path: canonical_path,
         inputs: resolved_parameter_types,
         invocation_style,
-        source_coordinates: Some(callable_type.item.item_id),
+        source_coordinates: Some(callable.item_id),
     };
     Ok(callable)
 }

libs/ui_tests/reflection/always_use_public_path/diagnostics.dot

@@ -9,11 +9,13 @@ digraph "GET / - 0" {
 }
 
 digraph "GET / - 1" {
-    0 [ label = "app_73b868ae::handler(app_73b868ae::A) -> pavex::response::Response"]
+    0 [ label = "app_73b868ae::handler(app_73b868ae::A, app_73b868ae::B) -> 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"]
+    2 [ label = "app_73b868ae::B::new() -> app_73b868ae::B"]
+    3 [ label = "<pavex::response::Response as pavex::response::IntoResponse>::into_response(pavex::response::Response) -> pavex::response::Response"]
+    2 -> 0 [ ]
     1 -> 0 [ ]
-    0 -> 2 [ ]
+    0 -> 3 [ ]
 }
 
 digraph "* / - 0" {

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

@@ -84,9 +84,10 @@ pub mod route_0 {
         <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)
+        let v0 = app::B::new();
+        let v1 = app::a();
+        let v2 = app::handler(v1, v0);
+        <pavex::response::Response as pavex::response::IntoResponse>::into_response(v2)
     }
     struct Next0<T>
     where

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

@@ -9,11 +9,13 @@ digraph "GET / - 0" {
 }
 
 digraph "GET / - 1" {
-    0 [ label = "app::handler(app::A) -> pavex::response::Response"]
+    0 [ label = "app::handler(app::A, app::B) -> 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"]
+    2 [ label = "app::B::new() -> app::B"]
+    3 [ label = "<pavex::response::Response as pavex::response::IntoResponse>::into_response(pavex::response::Response) -> pavex::response::Response"]
+    2 -> 0 [ ]
     1 -> 0 [ ]
-    0 -> 2 [ ]
+    0 -> 3 [ ]
 }
 
 digraph "* / - 0" {

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

@@ -4,7 +4,7 @@ use pavex::response::Response;
 
 pub use private::*;
 
-pub fn handler(_a: A) -> Response {
+pub fn handler(_a: A, _b: B) -> Response {
     todo!()
 }
 
@@ -21,11 +21,20 @@ mod private {
 
     pub struct A;
 
-    pub fn register(bp: &mut Blueprint) {
-        bp.request_scoped(f!(self::a));
-    }
-
     pub fn a() -> A {
         todo!()
     }
+
+    pub struct B;
+
+    impl B {
+        pub fn new() -> B {
+            todo!()
+        }
+    }
+
+    pub fn register(bp: &mut Blueprint) {
+        bp.request_scoped(f!(self::a));
+        bp.request_scoped(f!(self::B::new));
+    }
 }