Require explicit dependence on cairo_test (i.e. test_plugin)"

commit-id:83a53c59

examples/dependencies/Scarb.toml

@@ -6,3 +6,6 @@ version = "0.1.0"
 
 [dependencies]
 alexandria_math = { git = "https://github.com/keep-starknet-strange/alexandria.git" }
+
+[dev-dependencies]
+cairo_test = "2.6.0"

examples/hello_world/Scarb.toml

@@ -6,3 +6,6 @@ edition = "2023_10"
 # See more keys and their definitions at https://docs.swmansion.com/scarb/docs/reference/manifest.html
 
 [dependencies]
+
+[dev-dependencies]
+cairo_test = "2.6.0"

examples/starknet_hello_world/Scarb.toml

@@ -7,4 +7,7 @@ version = "0.1.0"
 [dependencies]
 starknet = "2.6.0"
 
+[dev-dependencies]
+cairo_test = "2.6.0"
+
 [[target.starknet-contract]]

examples/starknet_multiple_contracts/Scarb.toml

@@ -7,4 +7,7 @@ version = "0.1.0"
 [dependencies]
 starknet = "2.6.0"
 
+[dev-dependencies]
+cairo_test = "2.6.0"
+
 [[target.starknet-contract]]

examples/workspaces/Scarb.toml

@@ -10,6 +10,7 @@ test = "snforge"
 exit_first = true
 
 [workspace.dependencies]
+cairo_test = "2.6.0"
 starknet = "2.6.0"
 
 [workspace.package]
@@ -30,4 +31,7 @@ starknet.workspace = true
 fibonacci = { path = "crates/fibonacci" }
 addition = { path = "crates/addition" }
 
+[dev-dependencies]
+cairo_test.workspace = true
+
 [[target.starknet-contract]]

examples/workspaces/crates/addition/Scarb.toml

@@ -3,3 +3,6 @@ name = "addition"
 version.workspace = true
 
 [lib]
+
+[dev-dependencies]
+cairo_test.workspace = true

examples/workspaces/crates/fibonacci/Scarb.toml

@@ -10,3 +10,6 @@ snforge.workspace = true
 
 [dependencies]
 addition = { path = "../addition" }
+
+[dev-dependencies]
+cairo_test.workspace = true

extensions/scarb-cairo-test/tests/build.rs

@@ -33,6 +33,7 @@ fn can_test_without_gas() {
                 }
             }
         "#})
+        .dep_cairo_test()
         .manifest_extra(indoc! {r#"
             [cairo]
             enable-gas = false
@@ -86,6 +87,7 @@ fn can_print_test_resources() {
                 }
             }
         "#})
+        .dep_cairo_test()
         .build(&t);
     Scarb::quick_snapbox()
         .arg("cairo-test")
@@ -130,6 +132,7 @@ fn get_features_test_build(t: &TempDir) {
                 }
             }
         "#})
+        .dep_cairo_test()
         .build(t);
 }
 
@@ -219,6 +222,7 @@ fn integration_tests() {
 
             {test_case}
         "#})
+        .dep_cairo_test()
         .src("tests/a.cairo", test_case)
         .src("tests/b.cairo", test_case)
         .build(&t);

scarb/src/core/manifest/summary.rs

@@ -1,4 +1,3 @@
-use std::collections::HashSet;
 use std::ops::Deref;
 use std::sync::Arc;
 
@@ -8,8 +7,7 @@ use typed_builder::TypedBuilder;
 #[cfg(doc)]
 use crate::core::Manifest;
 use crate::core::{
-    Checksum, DepKind, DependencyVersionReq, ManifestDependency, PackageId, PackageName, SourceId,
-    TargetKind,
+    Checksum, DepKind, DependencyVersionReq, ManifestDependency, PackageId, PackageName,
 };
 
 /// Subset of a [`Manifest`] that contains only the most important information about a package.
@@ -27,7 +25,6 @@ pub struct SummaryInner {
     pub package_id: PackageId,
     #[builder(default)]
     pub dependencies: Vec<ManifestDependency>,
-    pub target_kinds: HashSet<TargetKind>,
     #[builder(default = false)]
     pub no_core: bool,
     #[builder(default)]
@@ -79,27 +76,10 @@ impl Summary {
                 .version_req(DependencyVersionReq::exact(&cairo_version))
                 .build()
         });
-
-        static TEST_PLUGIN_DEPENDENCY: Lazy<ManifestDependency> = Lazy::new(|| {
-            // NOTE: Pin test plugin to exact version, because we know that's the only one we have.
-            let cairo_version = crate::version::get().cairo.version.parse().unwrap();
-            ManifestDependency::builder()
-                .kind(DepKind::Target(TargetKind::TEST))
-                .name(PackageName::TEST_PLUGIN)
-                .source_id(SourceId::default())
-                .version_req(DependencyVersionReq::exact(&cairo_version))
-                .build()
-        });
-
         let mut deps: Vec<&ManifestDependency> = Vec::new();
-
         if !self.no_core {
             deps.push(&CORE_DEPENDENCY);
         }
-        if self.target_kinds.contains(&TargetKind::TEST) {
-            deps.push(&TEST_PLUGIN_DEPENDENCY);
-        }
-
         deps.into_iter()
     }
 

scarb/src/core/manifest/toml_manifest.rs

@@ -454,7 +454,6 @@ impl TomlManifest {
         let targets = self.collect_targets(package.name.to_smol_str(), root)?;
 
         let summary = Summary::builder()
-            .target_kinds(targets.iter().map(|t| t.kind.clone()).collect())
             .package_id(package_id)
             .dependencies(dependencies)
             .no_core(no_core)

scarb/src/core/registry/mod.rs

@@ -34,9 +34,7 @@ pub(crate) mod mock {
 
     use crate::core::package::PackageName;
     use crate::core::registry::Registry;
-    use crate::core::{
-        ManifestBuilder, ManifestDependency, Package, PackageId, SourceId, Summary, TargetKind,
-    };
+    use crate::core::{ManifestBuilder, ManifestDependency, Package, PackageId, SourceId, Summary};
 
     #[derive(Debug, Default)]
     pub struct MockRegistry {
@@ -111,7 +109,6 @@ pub(crate) mod mock {
 
         fn build_package(package_id: PackageId, dependencies: Vec<ManifestDependency>) -> Package {
             let summary = Summary::builder()
-                .target_kinds(HashSet::from_iter(vec![TargetKind::LIB]))
                 .package_id(package_id)
                 .dependencies(dependencies)
                 .no_core(package_id.is_core())

scarb/src/ops/resolve.rs

@@ -117,8 +117,7 @@ pub fn resolve_workspace_with_opts(
                 read_lockfile(ws)?
             };
 
-            let resolve =
-                resolver::resolve(&members_summaries, &patched, lockfile, ws.config().ui()).await?;
+            let resolve = resolver::resolve(&members_summaries, &patched, lockfile).await?;
 
             write_lockfile(Lockfile::from_resolve(&resolve), ws)?;
 
@@ -242,6 +241,7 @@ fn generate_cairo_compilation_units(
         .into_iter()
         .chain(grouped)
         .collect();
+    solution.show_warnings();
     Ok(result)
 }
 
@@ -433,6 +433,7 @@ pub struct PackageSolutionCollector<'a> {
     packages: Option<Vec<Package>>,
     cairo_plugins: Option<Vec<CompilationUnitCairoPlugin>>,
     target_kind: Option<TargetKind>,
+    warnings: HashSet<String>,
 }
 
 impl<'a> PackageSolutionCollector<'a> {
@@ -444,6 +445,7 @@ impl<'a> PackageSolutionCollector<'a> {
             packages: None,
             cairo_plugins: None,
             target_kind: None,
+            warnings: HashSet::new(),
         }
     }
 
@@ -464,7 +466,7 @@ impl<'a> PackageSolutionCollector<'a> {
     }
 
     fn pull_from_graph(
-        &self,
+        &mut self,
         target_kind: &TargetKind,
     ) -> Result<(Vec<Package>, Vec<CompilationUnitCairoPlugin>)> {
         let mut classes = self
@@ -504,6 +506,15 @@ impl<'a> PackageSolutionCollector<'a> {
 
         check_cairo_version_compatibility(&packages, self.ws)?;
 
+        // Print warnings for dependencies that are not usable.
+        let other = classes.remove(&PackageClass::Other).unwrap_or_default();
+        for pkg in other {
+            self.warnings.insert(format!(
+                "{} ignoring invalid dependency `{}` which is missing a lib or cairo-plugin target",
+                self.member.id, pkg.id.name
+            ));
+        }
+
         let cairo_plugins = cairo_plugins
             .into_iter()
             .map(|package| {
@@ -519,6 +530,12 @@ impl<'a> PackageSolutionCollector<'a> {
 
         Ok((packages, cairo_plugins))
     }
+
+    pub fn show_warnings(self) {
+        for warning in self.warnings {
+            self.ws.config().ui().warn(warning);
+        }
+    }
 }
 
 /// Build a set of `cfg` items to enable while building the compilation unit.

scarb/src/resolver/mod.rs

@@ -3,7 +3,6 @@ use std::collections::HashMap;
 use anyhow::{bail, Result};
 use indoc::{formatdoc, indoc};
 use petgraph::graphmap::DiGraphMap;
-use scarb_ui::Ui;
 use std::collections::HashSet;
 
 use crate::core::lockfile::Lockfile;
@@ -38,7 +37,6 @@ pub async fn resolve(
     summaries: &[Summary],
     registry: &dyn Registry,
     lockfile: Lockfile,
-    ui: Ui,
 ) -> Result<Resolve> {
     // TODO(#2): This is very bad, use PubGrub here.
     let mut graph = DiGraphMap::<PackageId, DependencyEdge>::new();
@@ -90,15 +88,6 @@ pub async fn resolve(
                 };
                 let dep = dep_summary.package_id;
 
-                if !(dep_summary.target_kinds.contains(&TargetKind::CAIRO_PLUGIN)
-                    || dep_summary.target_kinds.contains(&TargetKind::LIB))
-                {
-                    ui.warn(format!(
-                        "{} ignoring invalid dependency `{}` which is missing a lib or cairo-plugin target",
-                        package_id, dep.name
-                    ));
-                }
-
                 if let Some(existing) = packages.get(dep.name.as_ref()) {
                     if existing.source_id != dep.source_id {
                         bail!(
@@ -215,8 +204,6 @@ mod tests {
     use anyhow::Result;
     use indoc::indoc;
     use itertools::Itertools;
-    use scarb_ui::Verbosity::Verbose;
-    use scarb_ui::{OutputFormat, Ui};
     use semver::Version;
     use similar_asserts::assert_serde_eq;
     use tokio::runtime::Builder;
@@ -300,8 +287,7 @@ mod tests {
             .collect_vec();
 
         let lockfile = Lockfile::new(locks.iter().cloned());
-        let ui = Ui::new(Verbose, OutputFormat::Text);
-        runtime.block_on(super::resolve(&summaries, &registry, lockfile, ui))
+        runtime.block_on(super::resolve(&summaries, &registry, lockfile))
     }
 
     fn package_id<S: AsRef<str>>(name: S) -> PackageId {

scarb/src/sources/registry.rs

@@ -1,4 +1,3 @@
-use std::collections::HashSet;
 use std::fmt;
 
 use anyhow::{anyhow, bail, Context, Result};
@@ -16,7 +15,7 @@ use crate::core::registry::package_source_store::PackageSourceStore;
 use crate::core::source::Source;
 use crate::core::{
     Checksum, Config, DependencyVersionReq, ManifestDependency, Package, PackageId, SourceId,
-    Summary, TargetKind,
+    Summary,
 };
 use crate::flock::FileLockGuard;
 use crate::sources::PathSource;
@@ -105,7 +104,6 @@ impl<'c> Source for RegistrySource<'c> {
             Summary::builder()
                 .package_id(package_id)
                 .dependencies(dependencies)
-                .target_kinds(HashSet::from_iter([TargetKind::LIB]))
                 .no_core(record.no_core)
                 .checksum(Some(record.checksum.clone()))
                 .build()

scarb/tests/build_targets.rs

@@ -309,6 +309,7 @@ fn compile_test_target() {
     let t = TempDir::new().unwrap();
     ProjectBuilder::start()
         .name("hello")
+        .dep_cairo_test()
         .lib_cairo(r#"fn f() -> felt252 { 42 }"#)
         .build(&t);
     t.child("tests").create_dir_all().unwrap();
@@ -365,6 +366,7 @@ fn integration_tests_do_not_enable_cfg_in_main_package() {
     let t = TempDir::new().unwrap();
     ProjectBuilder::start()
         .name("hello")
+        .dep_cairo_test()
         .lib_cairo(indoc! {r#"
             #[cfg(test)]
             fn f() -> felt252 { 42 }

scarb/tests/expand.rs

@@ -56,6 +56,7 @@ fn expand_integration_test() {
     "#};
     ProjectBuilder::start()
         .name("hello")
+        .dep_cairo_test()
         .lib_cairo(formatdoc! {r#"
             fn fib(mut n: u32) -> u32 {{
                 let mut a: u32 = 0;
@@ -112,6 +113,7 @@ fn can_select_target_by_kind() {
     let t = TempDir::new().unwrap();
     ProjectBuilder::start()
         .name("hello")
+        .dep_cairo_test()
         .lib_cairo(indoc! {r#"
             fn hello() -> felt252 {
                 42
@@ -179,6 +181,7 @@ fn can_expand_multiple_targets() {
     "#};
     ProjectBuilder::start()
         .name("hello")
+        .dep_cairo_test()
         .lib_cairo(formatdoc! {r#"
             fn fib(mut n: u32) -> u32 {{
                 let mut a: u32 = 0;

scarb/tests/git_source.rs

@@ -423,10 +423,12 @@ fn transitive_path_dep() {
     let git_dep = gitx::new("dep1", |t| {
         ProjectBuilder::start()
             .name("dep0")
+            .dep_cairo_test()
             .lib_cairo("fn hello() -> felt252 { 42 }")
             .build(&t.child("zero"));
         ProjectBuilder::start()
             .name("dep1")
+            .dep_cairo_test()
             .lib_cairo("fn hello() -> felt252 { dep0::hello() }")
             .dep("dep0", Dep.path("../zero"))
             .build(&t.child("one"));
@@ -436,6 +438,7 @@ fn transitive_path_dep() {
     ProjectBuilder::start()
         .name("hello")
         .version("1.0.0")
+        .dep_cairo_test()
         .dep("dep0", &git_dep)
         .dep("dep1", &git_dep)
         .lib_cairo("fn world() -> felt252 { dep1::hello() }")