Allow multiple targets on single Compilation unit component (#1312)

commit-id:c5200c0d

---

**Stack**:
- #1313
- #1312 ⬅


⚠️ *Part of a stack created by [spr](https://github.com/ejoffe/spr). Do
not merge manually using the UI - doing so may have unexpected results.*

extensions/scarb-cairo-test/src/main.rs

@@ -1,3 +1,4 @@
+use std::collections::HashSet;
 use std::{env, fs};
 
 use anyhow::{Context, Result};
@@ -58,11 +59,22 @@ fn main() -> Result<()> {
         .unwrap_or(default_target_dir)
         .join(profile);
 
+    let mut deduplicator = TargetGroupDeduplicator::default();
     for package in matched {
         println!("testing {} ...", package.name);
 
         for target in find_testable_targets(&package) {
-            let file_path = target_dir.join(format!("{}.test.json", target.name.clone()));
+            let name = target
+                .params
+                .get("group-id")
+                .and_then(|v| v.as_str())
+                .map(ToString::to_string)
+                .unwrap_or(target.name.clone());
+            let already_seen = deduplicator.visit(package.name.clone(), name.clone());
+            if already_seen {
+                continue;
+            }
+            let file_path = target_dir.join(format!("{}.test.json", name));
             let test_compilation = serde_json::from_str::<TestCompilation>(
                 &fs::read_to_string(file_path.clone())
                     .with_context(|| format!("failed to read file: {file_path}"))?,
@@ -86,6 +98,18 @@ fn main() -> Result<()> {
     Ok(())
 }
 
+#[derive(Default)]
+struct TargetGroupDeduplicator {
+    seen: HashSet<(String, String)>,
+}
+
+impl TargetGroupDeduplicator {
+    /// Returns true if already visited.
+    pub fn visit(&mut self, package_name: String, group_name: String) -> bool {
+        !self.seen.insert((package_name, group_name))
+    }
+}
+
 fn is_gas_enabled(metadata: &Metadata, package_id: &PackageId, target: &TargetMetadata) -> bool {
     metadata
         .compilation_units

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

@@ -1,5 +1,5 @@
 use assert_fs::TempDir;
-use indoc::indoc;
+use indoc::{formatdoc, indoc};
 use scarb_test_support::command::Scarb;
 use scarb_test_support::project_builder::ProjectBuilder;
 
@@ -187,3 +187,59 @@ fn features_test_build_failed() {
         error: process did not exit successfully: exit code: 1
     "#});
 }
+
+#[test]
+fn integration_tests() {
+    let t = TempDir::new().unwrap();
+    let test_case = indoc! {r#"
+        #[cfg(test)]
+        mod tests {
+            use hello::fib;
+
+            #[test]
+            fn it_works() {
+                assert(fib(16) == 987, 'it works!');
+            }
+        }
+    "#};
+    ProjectBuilder::start()
+        .name("hello")
+        .lib_cairo(formatdoc! {r#"
+            fn fib(mut n: u32) -> u32 {{
+                let mut a: u32 = 0;
+                let mut b: u32 = 1;
+                while n != 0 {{
+                    n = n - 1;
+                    let temp = b;
+                    b = a + b;
+                    a = temp;
+                }};
+                a
+            }}
+
+            {test_case}
+        "#})
+        .src("tests/a.cairo", test_case)
+        .src("tests/b.cairo", test_case)
+        .build(&t);
+    Scarb::quick_snapbox()
+        .arg("cairo-test")
+        .current_dir(&t)
+        .assert()
+        .success()
+        .stdout_matches(indoc! {r#"
+            [..]Compiling test(hello_unittest) hello v1.0.0 ([..]Scarb.toml)
+            [..]Compiling test(hello_integrationtest) hello_a v1.0.0 ([..]Scarb.toml)
+            [..]Compiling test(hello_integrationtest) hello_b v1.0.0 ([..]Scarb.toml)
+            [..]Finished release target(s) in [..]
+            testing hello ...
+            running 1 test
+            test hello_b::b::tests::it_works ... ok (gas usage est.: 42440)
+            test result: ok. 1 passed; 0 failed; 0 ignored; 0 filtered out;
+
+            running 1 test
+            test hello::tests::it_works ... ok (gas usage est.: 42440)
+            test result: ok. 1 passed; 0 failed; 0 ignored; 0 filtered out;
+
+        "#});
+}

scarb/src/compiler/compilation_unit.rs

@@ -1,12 +1,15 @@
 use std::fmt::Write;
 use std::hash::{Hash, Hasher};
 
+use anyhow::{ensure, Result};
 use cairo_lang_filesystem::cfg::CfgSet;
+use itertools::Itertools;
+use serde::{Deserialize, Serialize};
 use smol_str::SmolStr;
 use typed_builder::TypedBuilder;
 
 use crate::compiler::Profile;
-use crate::core::{ManifestCompilerConfig, Package, PackageId, Target, Workspace};
+use crate::core::{ManifestCompilerConfig, Package, PackageId, Target, TargetKind, Workspace};
 use crate::flock::Filesystem;
 use scarb_stable_hash::StableHasher;
 
@@ -75,7 +78,7 @@ pub struct CompilationUnitComponent {
     /// The Scarb [`Package`] to be built.
     pub package: Package,
     /// Information about the specific target to build, out of the possible targets in `package`.
-    pub target: Target,
+    pub targets: Vec<Target>,
     /// Items for the Cairo's `#[cfg(...)]` attribute to be enabled in this component.
     pub cfg_set: Option<CfgSet>,
 }
@@ -101,10 +104,6 @@ pub trait CompilationUnitAttributes {
         component
     }
 
-    fn target(&self) -> &Target {
-        &self.main_component().target
-    }
-
     fn id(&self) -> String {
         format!("{}-{}", self.main_package_id().name, self.digest())
     }
@@ -121,18 +120,18 @@ pub trait CompilationUnitAttributes {
     }
 
     fn has_custom_name(&self) -> bool {
-        self.main_component().target.kind.as_str() != self.main_package_id().name.as_str()
+        self.main_component().target_kind().as_str() != self.main_package_id().name.as_str()
     }
 
     fn name(&self) -> String {
         let mut string = String::new();
 
         let main_component = self.main_component();
-        if self.is_sole_for_package() || self.target().is_test() {
-            write!(&mut string, "{}", main_component.target.kind).unwrap();
+        if self.is_sole_for_package() || self.main_component().target_kind().is_test() {
+            write!(&mut string, "{}", main_component.target_kind()).unwrap();
 
             if self.has_custom_name() {
-                write!(&mut string, "({})", main_component.target.name).unwrap();
+                write!(&mut string, "({})", main_component.target_name()).unwrap();
             }
 
             write!(&mut string, " ").unwrap();
@@ -218,15 +217,111 @@ impl CairoCompilationUnit {
     pub fn target_dir(&self, ws: &Workspace<'_>) -> Filesystem {
         ws.target_dir().child(self.profile.as_str())
     }
+
+    /// Rewrite single compilation unit with multiple targets, into multiple compilation units
+    /// with single targets.
+    pub fn rewrite_to_single_source_paths(&self) -> Vec<Self> {
+        let rewritten_main = self
+            .main_component()
+            .targets
+            .iter()
+            .map(|target| {
+                let mut main = self.main_component().clone();
+                main.targets = vec![target.clone()];
+                main
+            })
+            .collect_vec();
+
+        let mut components = self.components.clone();
+        components.remove(0);
+
+        rewritten_main
+            .into_iter()
+            .map(|component| {
+                let mut unit = self.clone();
+                unit.components = vec![component];
+                unit.components.extend(components.clone());
+                unit
+            })
+            .collect_vec()
+    }
 }
 
 impl CompilationUnitComponent {
+    /// Validate input and create new [CompilationUnitComponent] instance.
+    pub fn try_new(
+        package: Package,
+        targets: Vec<Target>,
+        cfg_set: Option<CfgSet>,
+    ) -> Result<Self> {
+        ensure!(
+            !targets.is_empty(),
+            "a compilation unit component must have at least one target"
+        );
+        ensure!(
+            targets
+                .iter()
+                .map(|t| &t.kind)
+                .collect::<std::collections::HashSet<_>>()
+                .len()
+                == 1,
+            "all targets in a compilation unit component must have the same kind"
+        );
+        ensure!(
+            targets
+                .iter()
+                .map(|t| &t.params)
+                .all(|p| *p == targets[0].params),
+            "all targets in a compilation unit component must have the same params"
+        );
+        ensure!(
+            targets
+                .iter()
+                .map(|t| t.source_root())
+                .all(|p| p == targets[0].source_root()),
+            "all targets in a compilation unit component must have the same source path parent"
+        );
+        if targets.len() > 1 {
+            ensure!(
+                targets.iter().all(|t| t.group_id.is_some()),
+                "all targets in a compilation unit component with multiple targets must have group_id defined"
+            );
+        }
+        Ok(Self {
+            package,
+            targets,
+            cfg_set,
+        })
+    }
+
+    pub fn first_target(&self) -> &Target {
+        &self.targets[0]
+    }
+
+    pub fn target_kind(&self) -> TargetKind {
+        self.first_target().kind.clone()
+    }
+
+    pub fn target_props<'de, P>(&self) -> Result<P>
+    where
+        P: Default + Serialize + Deserialize<'de>,
+    {
+        self.first_target().props::<P>()
+    }
+
+    pub fn target_name(&self) -> SmolStr {
+        self.first_target()
+            .group_id
+            .clone()
+            .unwrap_or(self.first_target().name.clone())
+    }
+
     pub fn cairo_package_name(&self) -> SmolStr {
         self.package.id.name.to_smol_str()
     }
 
     fn hash(&self, hasher: &mut impl Hasher) {
         self.package.id.hash(hasher);
-        self.target.hash(hasher);
+        self.targets.hash(hasher);
     }
 }

scarb/src/compiler/compilers/lib.rs

@@ -46,7 +46,7 @@ impl Compiler for LibCompiler {
         db: &mut RootDatabase,
         ws: &Workspace<'_>,
     ) -> Result<()> {
-        let props: Props = unit.target().props()?;
+        let props: Props = unit.main_component().target_props()?;
         if !props.sierra && !props.casm && !props.sierra_text {
             ws.config().ui().warn(
                 "Sierra, textual Sierra and CASM lib targets have been disabled, \
@@ -74,20 +74,23 @@ impl Compiler for LibCompiler {
 
         if props.sierra {
             write_json(
-                format!("{}.sierra.json", unit.target().name).as_str(),
+                format!("{}.sierra.json", unit.main_component().target_name()).as_str(),
                 "output file",
                 &target_dir,
                 ws,
                 &sierra_program,
             )
             .with_context(|| {
-                format!("failed to serialize Sierra program {}", unit.target().name)
+                format!(
+                    "failed to serialize Sierra program {}",
+                    unit.main_component().target_name()
+                )
             })?;
         }
 
         if props.sierra_text {
             write_string(
-                format!("{}.sierra", unit.target().name).as_str(),
+                format!("{}.sierra", unit.main_component().target_name()).as_str(),
                 "output file",
                 &target_dir,
                 ws,
@@ -121,7 +124,7 @@ impl Compiler for LibCompiler {
             };
 
             write_string(
-                format!("{}.casm", unit.target().name).as_str(),
+                format!("{}.casm", unit.main_component().target_name()).as_str(),
                 "output file",
                 &target_dir,
                 ws,

scarb/src/compiler/compilers/starknet_contract.rs

@@ -207,7 +207,7 @@ impl Compiler for StarknetContractCompiler {
         db: &mut RootDatabase,
         ws: &Workspace<'_>,
     ) -> Result<()> {
-        let props: Props = unit.target().props()?;
+        let props: Props = unit.main_component().target_props()?;
         if !props.sierra && !props.casm {
             ws.config().ui().warn(
                 "both Sierra and CASM Starknet contract targets have been disabled, \
@@ -276,7 +276,7 @@ impl Compiler for StarknetContractCompiler {
         let mut artifacts = StarknetArtifacts::default();
         let mut file_stem_calculator = ContractFileStemCalculator::new(contract_paths);
 
-        let target_name = &unit.target().name;
+        let target_name = &unit.main_component().target_name();
         for (decl, class, casm_class) in izip!(contracts, classes, casm_classes) {
             let contract_name = decl.submodule_id.name(db.upcast_mut());
             let contract_path = decl.module_id().full_path(db.upcast_mut());

scarb/src/compiler/compilers/test.rs

@@ -44,7 +44,7 @@ impl Compiler for TestCompiler {
 
         {
             let _ = trace_span!("serialize_test").enter();
-            let file_name = format!("{}.test.json", unit.target().name);
+            let file_name = format!("{}.test.json", unit.main_component().target_name());
             write_json(
                 &file_name,
                 "output file",