Refactor: pull CU dependencies directly from dependency graph (#1820)

- **Add test case for transitive dev deps**
- **Refactor: pull CU deps directly from graph**

scarb/src/core/resolver.rs

@@ -4,7 +4,7 @@ use anyhow::{bail, Result};
 use indoc::formatdoc;
 use itertools::Itertools;
 use petgraph::graphmap::DiGraphMap;
-use petgraph::visit::{Dfs, EdgeFiltered, Walker};
+use petgraph::visit::{Dfs, EdgeFiltered, IntoNeighborsDirected, Walker};
 use smallvec::SmallVec;
 
 use crate::core::lockfile::Lockfile;
@@ -58,6 +58,20 @@ impl Resolve {
         self.graph
             .neighbors_directed(package_id, petgraph::Direction::Outgoing)
     }
+
+    /// Collect [`PackageId`]s of directed dependencies of the package, that accept the given target kind.
+    pub fn package_dependencies_for_target_kind(
+        &self,
+        package_id: PackageId,
+        target_kind: &TargetKind,
+    ) -> Vec<PackageId> {
+        let filtered_graph = EdgeFiltered::from_fn(&self.graph, move |(node_a, _node_b, edge)| {
+            edge.accepts_target(target_kind.clone(), node_a == package_id)
+        });
+        filtered_graph
+            .neighbors_directed(package_id, petgraph::Direction::Outgoing)
+            .collect_vec()
+    }
 }
 
 #[derive(Debug, Default, Clone, PartialEq, Eq)]

scarb/src/ops/resolve.rs

@@ -50,6 +50,19 @@ impl WorkspaceResolve {
             .map(|id| self.packages[id].clone())
             .collect_vec()
     }
+
+    pub fn package_dependencies(
+        &self,
+        package_id: PackageId,
+        target_kind: &TargetKind,
+    ) -> Vec<Package> {
+        assert!(self.packages.contains_key(&package_id));
+        self.resolve
+            .package_dependencies_for_target_kind(package_id, target_kind)
+            .iter()
+            .map(|id| self.packages[id].clone())
+            .collect_vec()
+    }
 }
 
 #[derive(Debug, Default)]
@@ -398,55 +411,23 @@ fn cairo_compilation_unit_for_target(
     };
 
     // Collect dependencies for the components.
+    let member_component = components
+        .iter()
+        .find(|component| component.package.id == member.id)
+        .unwrap();
+    let mut test_package_deps = solution.component_dependencies(member_component, &components);
+    test_package_deps.push(member_component.id.clone());
+
     let dependencies_for_components: Vec<_> = components
         .iter()
         .map(|component| {
-            // Those are direct dependencies of the component.
-            let dependencies_summary: Vec<&ManifestDependency> = component
-                .package
-                .manifest
-                .summary
-                .full_dependencies()
-                .collect();
-
-            // We iterate over all the compilation unit components to get dependency's version.
-            let mut dependencies: Vec<CompilationUnitComponentId> = components
-                .iter()
-                .filter(|component_as_dependency| {
-                    dependencies_summary.iter().any(|dependency_summary| {
-                        dependency_summary.name == component_as_dependency.package.id.name
-                    }) ||
-                        // This is a hacky way of accommodating integration test components,
-                        // which need to depend on the tested package.
-                        component_as_dependency
-                            .package
-                            .manifest
-                            .targets
-                            .iter()
-                            .filter(|target| target.kind.is_test())
-                            .any(|target| {
-                                target.group_id.clone().unwrap_or(target.name.clone())
-                                    == component.package.id.name.to_smol_str()
-                                    && component_as_dependency.cairo_package_name() != component.cairo_package_name()
-                            })
-                })
-                .map(|compilation_unit_component| compilation_unit_component.id.clone()
-                )
-                .collect();
-
-            // Adds itself to dependencies
-            let is_integration_test = if component.first_target().kind.is_test() {
-                let props: Option<TestTargetProps> = component.first_target().props().ok();
-                props
-                    .map(|props| props.test_type == TestTargetType::Integration)
-                    .unwrap_or_default()
-            } else { false };
-            if !is_integration_test {
-                dependencies.push(component.id.clone());
+            if component.package.id == test_package_id {
+                test_package_deps.clone()
+            } else {
+                solution.component_dependencies(component, &components)
             }
-
-            dependencies
-        }).collect();
+        })
+        .collect();
 
     for (component, dependencies) in zip(&mut components, dependencies_for_components) {
         component.dependencies = dependencies;
@@ -629,6 +610,45 @@ impl<'a> PackageSolutionCollector<'a> {
         Ok((packages, cairo_plugins))
     }
 
+    pub fn component_dependencies(
+        &self,
+        component: &CompilationUnitComponent,
+        components: &[CompilationUnitComponent],
+    ) -> Vec<CompilationUnitComponentId> {
+        let package_id = component.id.package_id;
+
+        // Those are direct dependencies of the component.
+        let dependencies_packages = self
+            .resolve
+            .package_dependencies(package_id, self.target_kind.as_ref().unwrap());
+
+        // We iterate over all the compilation unit components to get dependency's version.
+        let mut dependencies: Vec<CompilationUnitComponentId> = components
+            .iter()
+            .filter(|component_as_dependency| {
+                dependencies_packages.iter().any(|dependency_summary| {
+                    dependency_summary.id == component_as_dependency.package.id
+                })
+            })
+            .map(|compilation_unit_component| compilation_unit_component.id.clone())
+            .collect();
+
+        // Adds itself to dependencies
+        let is_integration_test = if component.first_target().kind.is_test() {
+            let props: Option<TestTargetProps> = component.first_target().props().ok();
+            props
+                .map(|props| props.test_type == TestTargetType::Integration)
+                .unwrap_or_default()
+        } else {
+            false
+        };
+        if !is_integration_test {
+            dependencies.push(component.id.clone());
+        }
+
+        dependencies
+    }
+
     pub fn show_warnings(self) {
         for warning in self.warnings {
             self.ws.config().ui().warn(warning);

scarb/tests/build_targets.rs

@@ -1047,3 +1047,46 @@ fn test_target_builds_external() {
     t.child("hello/target/dev/hello_unittest.test.starknet_artifacts.json")
         .assert_is_json::<serde_json::Value>();
 }
+
+#[test]
+fn transitive_dev_deps_not_available() {
+    let t = TempDir::new().unwrap();
+
+    let first = &t.child("first");
+    ProjectBuilder::start()
+        .lib_cairo(indoc! {r#"
+            pub fn forty_two() -> felt252 { 42 }
+        "#})
+        .name("first")
+        .build(first);
+    let second = &t.child("second");
+    ProjectBuilder::start()
+        .name("second")
+        .dep("first", first)
+        .build(second);
+    let hello = t.child("hello");
+    ProjectBuilder::start()
+        .name("hello")
+        .lib_cairo(indoc! {r#"
+            use first::forty_two;
+            pub fn main() -> felt252 { forty_two() }
+        "#})
+        .dep("second", second)
+        .dev_dep("first", first)
+        .build(&hello);
+
+    Scarb::quick_snapbox()
+        .arg("check")
+        .current_dir(hello)
+        .assert()
+        .failure()
+        .stdout_matches(indoc! {r#"
+            [..]Checking hello v1.0.0 ([..]Scarb.toml)
+            error: Identifier not found.
+             --> [..]lib.cairo:1:5
+            use first::forty_two;
+                ^***^
+
+            error: could not check `hello` due to previous error
+        "#});
+}