Don't insert Clone nodes if they are not necessary

libs/pavex_cli/tests/ui_tests/dependency_injection/clone_nodes_are_only_added_where_needed/expectations/app.rs

@@ -0,0 +1,148 @@
+//! 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: matchit::Router<u32>,
+    application_state: ApplicationState,
+}
+pub struct ApplicationState {
+    s0: app::Singleton,
+}
+pub async fn build_application_state() -> crate::ApplicationState {
+    let v0 = app::Singleton::new();
+    crate::ApplicationState { s0: v0 }
+}
+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() -> matchit::Router<u32> {
+    let mut router = matchit::Router::new();
+    router.insert("/", 0u32).unwrap();
+    router
+}
+async fn route_request(
+    request: http::Request<hyper::body::Incoming>,
+    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.uri.path()) {
+        Ok(m) => m,
+        Err(_) => {
+            let allowed_methods: pavex::router::AllowedMethods = pavex::router::MethodAllowList::from_iter(
+                    vec![],
+                )
+                .into();
+            return route_1::middleware_0(
+                    server_state.application_state.s0.clone(),
+                    &allowed_methods,
+                )
+                .await;
+        }
+    };
+    let route_id = matched_route.value;
+    #[allow(unused)]
+    let url_params: pavex::request::route::RawRouteParams<'_, '_> = matched_route
+        .params
+        .into();
+    match route_id {
+        0u32 => {
+            match &request_head.method {
+                &pavex::http::Method::GET => {
+                    route_0::middleware_0(server_state.application_state.s0.clone())
+                        .await
+                }
+                _ => {
+                    let allowed_methods: pavex::router::AllowedMethods = pavex::router::MethodAllowList::from_iter([
+                            pavex::http::Method::GET,
+                        ])
+                        .into();
+                    route_1::middleware_0(
+                            server_state.application_state.s0.clone(),
+                            &allowed_methods,
+                        )
+                        .await
+                }
+            }
+        }
+        i => unreachable!("Unknown route id: {}", i),
+    }
+}
+pub mod route_0 {
+    pub async fn middleware_0(v0: app::Singleton) -> pavex::response::Response {
+        let v1 = <app::Singleton as core::clone::Clone>::clone(&v0);
+        let v2 = crate::route_0::Next0 {
+            s_0: v0,
+            next: handler,
+        };
+        let v3 = pavex::middleware::Next::new(v2);
+        app::mw(v1, v3)
+    }
+    pub async fn handler(v0: app::Singleton) -> pavex::response::Response {
+        let v1 = app::handler(v0);
+        <pavex::response::Response as pavex::response::IntoResponse>::into_response(v1)
+    }
+    pub struct Next0<T>
+    where
+        T: std::future::Future<Output = pavex::response::Response>,
+    {
+        s_0: app::Singleton,
+        next: fn(app::Singleton) -> 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)(self.s_0)
+        }
+    }
+}
+pub mod route_1 {
+    pub async fn middleware_0(
+        v0: app::Singleton,
+        v1: &pavex::router::AllowedMethods,
+    ) -> pavex::response::Response {
+        let v2 = crate::route_1::Next0 {
+            s_0: v1,
+            next: handler,
+        };
+        let v3 = pavex::middleware::Next::new(v2);
+        app::mw(v0, v3)
+    }
+    pub 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)
+    }
+    pub 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/pavex_cli/tests/ui_tests/dependency_injection/clone_nodes_are_only_added_where_needed/expectations/diagnostics.dot

@@ -0,0 +1,46 @@
+digraph "GET / - 0" {
+    0 [ label = "app::mw(app::Singleton, 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(app::Singleton) -> crate::route_0::Next0"]
+    3 [ label = "app::Singleton"]
+    4 [ label = "<app::Singleton as core::clone::Clone>::clone(&app::Singleton) -> app::Singleton"]
+    1 -> 0 [ ]
+    2 -> 1 [ ]
+    3 -> 2 [ ]
+    3 -> 4 [ label = "&"]
+    4 -> 0 [ ]
+}
+
+digraph "GET / - 1" {
+    0 [ label = "app::handler(app::Singleton) -> pavex::response::Response"]
+    1 [ label = "app::Singleton"]
+    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 = "app::mw(app::Singleton, pavex::middleware::Next<crate::route_1::Next0>) -> pavex::response::Response"]
+    1 [ label = "pavex::middleware::Next::new(crate::route_1::Next0) -> pavex::middleware::Next<crate::route_1::Next0>"]
+    2 [ label = "crate::route_1::Next0(&pavex::router::AllowedMethods) -> crate::route_1::Next0"]
+    4 [ label = "app::Singleton"]
+    5 [ label = "&pavex::router::AllowedMethods"]
+    1 -> 0 [ ]
+    2 -> 1 [ ]
+    4 -> 0 [ ]
+    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(app::Singleton) -> crate::ApplicationState"]
+    1 [ label = "app::Singleton::new() -> app::Singleton"]
+    1 -> 0 [ ]
+}

libs/pavex_cli/tests/ui_tests/dependency_injection/clone_nodes_are_only_added_where_needed/lib.rs

@@ -0,0 +1,39 @@
+use std::future::IntoFuture;
+
+use pavex::blueprint::{
+    constructor::{CloningStrategy, Lifecycle},
+    router::GET,
+    Blueprint,
+};
+use pavex::f;
+use pavex::middleware::Next;
+use pavex::response::Response;
+
+#[derive(Clone)]
+pub struct Singleton;
+
+impl Singleton {
+    pub fn new() -> Singleton {
+        todo!()
+    }
+}
+
+pub fn mw<C>(s: Singleton, next: Next<C>) -> Response
+where
+    C: IntoFuture<Output = Response>,
+{
+    todo!()
+}
+
+pub fn handler(s: Singleton) -> pavex::response::Response {
+    todo!()
+}
+
+pub fn blueprint() -> Blueprint {
+    let mut bp = Blueprint::new();
+    bp.constructor(f!(crate::Singleton::new), Lifecycle::Singleton)
+        .cloning(CloningStrategy::CloneIfNecessary);
+    bp.wrap(f!(crate::mw));
+    bp.route(GET, "/", f!(crate::handler));
+    bp
+}

libs/pavex_cli/tests/ui_tests/dependency_injection/clone_nodes_are_only_added_where_needed/test_config.toml

@@ -0,0 +1,11 @@
+description = """
+Clone nodes are only added where they are needed.
+In particular, the fact that a node must be cloned in the call graph for a handler
+doesn't force the insertion of a clone node in the call graph for the application state.
+Regression test for https://github.com/LukeMathWalker/pavex/issues/134
+"""
+
+[expectations]
+codegen = "pass"
+
+

libs/pavexc/src/compiler/analyses/call_graph/application_state.rs

@@ -16,7 +16,7 @@ use crate::compiler::analyses::call_graph::{
     OrderedCallGraph,
 };
 use crate::compiler::analyses::components::{
-    ComponentDb, ComponentId, ConsumptionMode, HydratedComponent,
+    ComponentDb, ComponentId, ConsumptionMode, HydratedComponent, InsertTransformer,
 };
 use crate::compiler::analyses::computations::ComputationDb;
 use crate::compiler::analyses::constructibles::ConstructibleDb;
@@ -223,6 +223,7 @@ pub(crate) fn application_state_call_graph(
             computation_db.get_or_intern(ok_wrapper),
             application_state_id,
             application_state_scope_id,
+            InsertTransformer::Eagerly,
             ConsumptionMode::Move,
         );
 
@@ -290,13 +291,15 @@ pub(crate) fn application_state_call_graph(
                     computation_db.get_or_intern(error_variant_constructor.clone()),
                     *err_match_id,
                     application_state_scope_id,
+                    InsertTransformer::Eagerly,
                     ConsumptionMode::Move,
                 );
                 // We need to do an Err(..) wrap around the error variant returned by the transformer.
                 component_db.get_or_intern_transformer(
                     computation_db.get_or_intern(err_wrapper.clone()),
                     transformer_id,
                     application_state_scope_id,
+                    InsertTransformer::Eagerly,
                     ConsumptionMode::Move,
                 );
             }

libs/pavexc/src/compiler/analyses/call_graph/borrow_checker/clone.rs

@@ -6,7 +6,7 @@ use once_cell::sync::OnceCell;
 use pavex::blueprint::constructor::CloningStrategy;
 
 use crate::compiler::analyses::components::{
-    ComponentDb, ComponentId, ConsumptionMode, HydratedComponent,
+    ComponentDb, ComponentId, ConsumptionMode, HydratedComponent, InsertTransformer,
 };
 use crate::compiler::analyses::computations::ComputationDb;
 use crate::compiler::analyses::user_components::ScopeId;
@@ -91,6 +91,7 @@ pub(super) fn get_clone_component_id(
         clone_computation_id,
         *component_id,
         scope_id,
+        InsertTransformer::Lazily,
         ConsumptionMode::SharedBorrow,
     );
     Some(clone_component_id)

libs/pavexc/src/compiler/analyses/call_graph/borrow_checker/complex.rs

@@ -296,9 +296,9 @@ fn emit_borrow_checking_error(
 
             if let Some(user_component_id) = component_db.user_component_id(component_id) {
                 let help_msg = format!(
-                        "Allow me to clone `{type_:?}` in order to satisfy the borrow checker.\n\
+                    "Allow me to clone `{type_:?}` in order to satisfy the borrow checker.\n\
                         You can do so by invoking `.cloning(CloningStrategy::CloneIfNecessary)` on the type returned by `.constructor`.",
-                    );
+                );
                 let location = component_db
                     .user_component_db()
                     .get_location(user_component_id);
@@ -334,10 +334,10 @@ fn emit_borrow_checking_error(
                 .computation()
             {
                 let help_msg = format!(
-                        "Considering changing the signature of `{}`.\n\
+                    "Considering changing the signature of `{}`.\n\
                         It takes `{type_:?}` by value. Would a shared reference, `&{type_:?}`, be enough?",
-                        callable.path
-                    );
+                    callable.path
+                );
                 let help = HelpWithSnippet::new(
                     help_msg,
                     AnnotatedSnippet::new_with_labels(NamedSource::new("", ""), vec![]),

libs/pavexc/src/compiler/analyses/call_graph/core_graph.rs

@@ -1,20 +1,21 @@
-use crate::compiler::analyses::components::{
-    ComponentDb, ComponentId, ConsumptionMode, HydratedComponent,
-};
-use crate::compiler::analyses::computations::ComputationDb;
-use crate::compiler::analyses::constructibles::ConstructibleDb;
-use crate::compiler::analyses::user_components::ScopeId;
-use crate::compiler::computation::{Computation, MatchResultVariant};
 use ahash::{HashMap, HashMapExt, HashSet, HashSetExt};
 use bimap::BiHashMap;
 use guppy::PackageId;
 use indexmap::IndexSet;
-use pavex::blueprint::constructor::Lifecycle;
 use petgraph::prelude::{StableDiGraph, StableGraph};
 use petgraph::stable_graph::NodeIndex;
 use petgraph::visit::Dfs;
 use petgraph::Direction;
 
+use pavex::blueprint::constructor::Lifecycle;
+
+use crate::compiler::analyses::components::{
+    ComponentDb, ComponentId, ConsumptionMode, HydratedComponent, InsertTransformer,
+};
+use crate::compiler::analyses::computations::ComputationDb;
+use crate::compiler::analyses::constructibles::ConstructibleDb;
+use crate::compiler::analyses::user_components::ScopeId;
+use crate::compiler::computation::{Computation, MatchResultVariant};
 use crate::language::{Lifetime, ResolvedType, TypeReference};
 
 use super::dependency_graph::DependencyGraph;
@@ -288,6 +289,10 @@ where
                     break 'inner;
                 };
                 for transformer_id in transformer_ids {
+                    let when_to_insert = component_db.when_to_insert(*transformer_id);
+                    if when_to_insert == InsertTransformer::Lazily {
+                        continue;
+                    }
                     // Not all transformers might be relevant to this `CallGraph`, we need to take their scope into account.
                     let transformer_scope_id = component_db.scope_id(*transformer_id);
                     if root_scope_id