feat(dojo-lang): add systems to contract's manifest and fix `DojoAuxD…

…ata` gathering (#2309)

* fix: ensure contract address for deployed contract is correct

* fix: fix compiler aux data to correctly fill dojo contract data

* fix: remove prints

bin/sozo/src/commands/build.rs

@@ -217,8 +217,10 @@ mod tests {
     use super::{create_stats_table, BuildArgs, *};
     use crate::commands::build::CONTRACT_NAME_LABEL;
 
-    // Uncomment once bindings support arrays.
+    // Ignored as scarb takes too much time to compile in debug mode.
+    // It's anyway run in the CI in the `test` job.
     #[test]
+    #[ignore]
     fn build_example_with_typescript_and_unity_bindings() {
         let setup = CompilerTestSetup::from_examples("../../crates/dojo-core", "../../examples/");
 

bin/sozo/src/commands/test.rs

@@ -233,7 +233,10 @@ mod tests {
 
     use super::*;
 
+    // Ignored as scarb takes too much time to compile in debug mode.
+    // It's anyway run in the CI in the `test` job.
     #[test]
+    #[ignore]
     fn test_spawn_and_move_test() {
         let setup = CompilerTestSetup::from_examples("../../crates/dojo-core", "../../examples/");
 

crates/dojo-lang/src/compiler.rs

@@ -1,4 +1,4 @@
-use std::collections::{BTreeMap, BTreeSet, HashMap};
+use std::collections::{BTreeMap, HashMap};
 use std::fs;
 use std::io::Write;
 use std::iter::zip;
@@ -14,7 +14,6 @@ use cairo_lang_formatter::format_string;
 use cairo_lang_semantic::db::SemanticGroup;
 use cairo_lang_starknet::compile::compile_prepared_db;
 use cairo_lang_starknet::contract::{find_contracts, ContractDeclaration};
-use cairo_lang_starknet::plugin::aux_data::StarkNetContractAuxData;
 use cairo_lang_starknet_classes::abi;
 use cairo_lang_starknet_classes::contract_class::ContractClass;
 use cairo_lang_utils::UpcastMut;
@@ -26,7 +25,6 @@ use dojo_world::manifest::{
     BASE_CONTRACT_TAG, BASE_DIR, BASE_QUALIFIED_PATH, CONTRACTS_DIR, MANIFESTS_DIR, MODELS_DIR,
     WORLD_CONTRACT_TAG, WORLD_QUALIFIED_PATH,
 };
-use dojo_world::metadata::get_namespace_config_from_ws;
 use itertools::Itertools;
 use scarb::compiler::helpers::{build_compiler_config, collect_main_crate_ids};
 use scarb::compiler::{CairoCompilationUnit, CompilationUnitAttributes, Compiler};
@@ -39,9 +37,7 @@ use starknet::core::types::contract::SierraClass;
 use starknet::core::types::Felt;
 use tracing::{debug, trace, trace_span};
 
-use crate::inline_macros::utils::{SYSTEM_READS, SYSTEM_WRITES};
 use crate::plugin::{DojoAuxData, Model};
-use crate::semantics::utils::find_module_rw;
 
 const CAIRO_PATH_SEPARATOR: &str = "::";
 
@@ -237,8 +233,6 @@ fn update_files(
     compiled_artifacts: HashMap<String, (Felt, ContractClass)>,
     external_contracts: Option<Vec<ContractSelector>>,
 ) -> anyhow::Result<()> {
-    let namespace_config = get_namespace_config_from_ws(ws)?;
-
     let profile_name =
         ws.current_profile().expect("Scarb profile expected to be defined.").to_string();
     let relative_manifest_dir = Utf8PathBuf::new().join(MANIFESTS_DIR).join(profile_name);
@@ -305,29 +299,37 @@ fn update_files(
                 .filter_map(|aux_data| aux_data.as_ref().map(|aux_data| aux_data.0.as_any()))
             {
                 if let Some(dojo_aux_data) = aux_data.downcast_ref::<DojoAuxData>() {
-                    for system in &dojo_aux_data.systems {
+                    // For the contracts, the `module_id` is the parent module of the actual
+                    // contract. Hence, the full path of the contract must be
+                    // reconstructed with the contract's name inside the
+                    // `get_dojo_contract_artifacts` function.
+                    for contract in &dojo_aux_data.contracts {
                         contracts.extend(get_dojo_contract_artifacts(
                             db,
                             module_id,
-                            &naming::get_tag(&system.namespace, &system.name),
+                            &naming::get_tag(&contract.namespace, &contract.name),
                             &compiled_artifacts,
+                            &contract.systems,
                         )?);
                     }
 
+                    // For the models, the `struct_id` is the full path including the struct's name
+                    // already. The `get_dojo_model_artifacts` function handles
+                    // the reconstruction of the full path by also using lower
+                    // case for the model's name to match the compiled artifacts of the generated
+                    // contract.
                     models.extend(get_dojo_model_artifacts(
                         db,
                         &dojo_aux_data.models,
                         *module_id,
                         &compiled_artifacts,
                     )?);
-                } else if let Some(aux_data) = aux_data.downcast_ref::<StarkNetContractAuxData>() {
-                    contracts.extend(get_dojo_contract_artifacts(
-                        db,
-                        module_id,
-                        &naming::get_tag(&namespace_config.default, &aux_data.contract_name),
-                        &compiled_artifacts,
-                    )?);
                 }
+
+                // StarknetAuxData shouldn't be required. Every dojo contract and model are starknet
+                // contracts under the hood. But the dojo aux data are attached to
+                // the parent module of the actual contract, so StarknetAuxData will
+                // only contain the contract's name.
             }
         }
     }
@@ -461,40 +463,35 @@ fn get_dojo_contract_artifacts(
     module_id: &ModuleId,
     tag: &str,
     compiled_classes: &HashMap<String, (Felt, ContractClass)>,
+    systems: &[String],
 ) -> anyhow::Result<HashMap<String, (Manifest<DojoContract>, ContractClass, ModuleId)>> {
     let mut result = HashMap::new();
 
     if !matches!(naming::get_name_from_tag(tag).as_str(), "world" | "resource_metadata" | "base") {
-        let qualified_path = module_id.full_path(db).to_string();
-
-        if let Some((class_hash, class)) = compiled_classes.get(&qualified_path) {
-            let reads = SYSTEM_READS
-                .lock()
-                .unwrap()
-                .get(&qualified_path as &str)   // should use tag instead of qualified_path
-                .map_or_else(Vec::new, |models| {
-                    models.clone().into_iter().collect::<BTreeSet<_>>().into_iter().collect()
-                });
-
-            let writes = SYSTEM_WRITES
-                .lock()
-                .unwrap()
-                .get(&qualified_path as &str)   // should use tag instead of qualified_path
-                .map_or_else(Vec::new, |write_ops| find_module_rw(db, module_id, write_ops));
-
+        // For the contracts, the `module_id` is the parent module of the actual contract.
+        // Hence, the full path of the contract must be reconstructed with the contract's name.
+        let (_, contract_name) = naming::split_tag(tag)?;
+        let contract_qualified_path =
+            format!("{}{}{}", module_id.full_path(db), CAIRO_PATH_SEPARATOR, contract_name);
+
+        if let Some((class_hash, class)) =
+            compiled_classes.get(&contract_qualified_path.to_string())
+        {
             let manifest = Manifest::new(
                 DojoContract {
                     tag: tag.to_string(),
-                    writes,
-                    reads,
+                    writes: vec![],
+                    reads: vec![],
                     class_hash: *class_hash,
                     original_class_hash: *class_hash,
+                    systems: systems.to_vec(),
                     ..Default::default()
                 },
                 naming::get_filename_from_tag(tag),
             );
 
-            result.insert(qualified_path.to_string(), (manifest, class.clone(), *module_id));
+            result
+                .insert(contract_qualified_path.to_string(), (manifest, class.clone(), *module_id));
         }
     }
 

crates/dojo-lang/src/compiler_test.rs

@@ -6,7 +6,10 @@ use scarb::ops::{CompileOpts, FeaturesOpts, FeaturesSelector};
 use crate::compiler::ContractSelector;
 use crate::scarb_internal;
 
+// Ignored as scarb takes too much time to compile in debug mode.
+// It's anyway run in the CI in the `test` job.
 #[test]
+#[ignore]
 fn test_compiler_cairo_features() {
     let config =
         build_test_config("./src/manifest_test_data/compiler_cairo/Scarb.toml", Profile::DEV)

crates/dojo-lang/src/contract.rs

@@ -18,7 +18,7 @@ use dojo_types::system::Dependency;
 use dojo_world::config::NamespaceConfig;
 use dojo_world::contracts::naming;
 
-use crate::plugin::{DojoAuxData, SystemAuxData, DOJO_CONTRACT_ATTR};
+use crate::plugin::{ContractAuxData, DojoAuxData, DOJO_CONTRACT_ATTR};
 use crate::syntax::world_param::{self, WorldParamInjectionKind};
 use crate::syntax::{self_param, utils as syntax_utils};
 
@@ -36,6 +36,7 @@ pub struct ContractParameters {
 pub struct DojoContract {
     diagnostics: Vec<PluginDiagnostic>,
     dependencies: HashMap<smol_str::SmolStr, Dependency>,
+    systems: Vec<String>,
 }
 
 impl DojoContract {
@@ -50,7 +51,8 @@ impl DojoContract {
         let mut diagnostics = vec![];
         let parameters = get_parameters(db, module_ast, &mut diagnostics);
 
-        let mut system = DojoContract { diagnostics, dependencies: HashMap::new() };
+        let mut contract =
+            DojoContract { diagnostics, dependencies: HashMap::new(), systems: vec![] };
 
         let mut has_event = false;
         let mut has_storage = false;
@@ -96,27 +98,27 @@ impl DojoContract {
                     if let ast::ModuleItem::Enum(ref enum_ast) = el {
                         if enum_ast.name(db).text(db).to_string() == "Event" {
                             has_event = true;
-                            return system.merge_event(db, enum_ast.clone());
+                            return contract.merge_event(db, enum_ast.clone());
                         }
                     } else if let ast::ModuleItem::Struct(ref struct_ast) = el {
                         if struct_ast.name(db).text(db).to_string() == "Storage" {
                             has_storage = true;
-                            return system.merge_storage(db, struct_ast.clone());
+                            return contract.merge_storage(db, struct_ast.clone());
                         }
                     } else if let ast::ModuleItem::Impl(ref impl_ast) = el {
                         // If an implementation is not targetting the ContractState,
                         // the auto injection of self and world is not applied.
                         let trait_path = impl_ast.trait_path(db).node.get_text(db);
                         if trait_path.contains("<ContractState>") {
-                            return system.rewrite_impl(db, impl_ast.clone(), metadata);
+                            return contract.rewrite_impl(db, impl_ast.clone(), metadata);
                         }
                     } else if let ast::ModuleItem::FreeFunction(ref fn_ast) = el {
                         let fn_decl = fn_ast.declaration(db);
                         let fn_name = fn_decl.name(db).text(db);
 
                         if fn_name == DOJO_INIT_FN {
                             has_init = true;
-                            return system.handle_init_fn(db, fn_ast);
+                            return contract.handle_init_fn(db, fn_ast);
                         }
                     }
 
@@ -151,11 +153,11 @@ impl DojoContract {
             }
 
             if !has_event {
-                body_nodes.append(&mut system.create_event())
+                body_nodes.append(&mut contract.create_event())
             }
 
             if !has_storage {
-                body_nodes.append(&mut system.create_storage())
+                body_nodes.append(&mut contract.create_storage())
             }
 
             let mut builder = PatchBuilder::new(db, module_ast);
@@ -251,16 +253,17 @@ impl DojoContract {
                     content: code,
                     aux_data: Some(DynGeneratedFileAuxData::new(DojoAuxData {
                         models: vec![],
-                        systems: vec![SystemAuxData {
+                        contracts: vec![ContractAuxData {
                             name,
                             namespace: contract_namespace.clone(),
-                            dependencies: system.dependencies.values().cloned().collect(),
+                            dependencies: contract.dependencies.values().cloned().collect(),
+                            systems: contract.systems.clone(),
                         }],
                         events: vec![],
                     })),
                     code_mappings,
                 }),
-                diagnostics: system.diagnostics,
+                diagnostics: contract.diagnostics,
                 remove_original_item: true,
             };
         }
@@ -518,6 +521,11 @@ impl DojoContract {
         let return_type =
             fn_ast.declaration(db).signature(db).ret_ty(db).as_syntax_node().get_text(db);
 
+        // Consider the function as a system if no return type is specified.
+        if return_type.is_empty() {
+            self.systems.push(fn_name.to_string());
+        }
+
         let (params_str, was_world_injected) = self.rewrite_parameters(
             db,
             fn_ast.declaration(db).signature(db).parameters(db),

crates/dojo-lang/src/inline_macros/delete.rs

@@ -1,18 +1,13 @@
-use std::collections::HashMap;
-
 use cairo_lang_defs::patcher::PatchBuilder;
 use cairo_lang_defs::plugin::{
     InlineMacroExprPlugin, InlinePluginResult, MacroPluginMetadata, NamedPlugin, PluginDiagnostic,
     PluginGeneratedFile,
 };
 use cairo_lang_defs::plugin_utils::unsupported_bracket_diagnostic;
 use cairo_lang_diagnostics::Severity;
-use cairo_lang_syntax::node::ast::{ExprPath, ExprStructCtorCall, FunctionWithBody, ItemModule};
-use cairo_lang_syntax::node::kind::SyntaxKind;
 use cairo_lang_syntax::node::{ast, TypedStablePtr, TypedSyntaxNode};
 
 use super::unsupported_arg_diagnostic;
-use super::utils::{parent_of_kind, SystemRWOpRecord, SYSTEM_WRITES};
 
 #[derive(Debug, Default)]
 pub struct DeleteMacro;
@@ -58,17 +53,17 @@ impl InlineMacroExprPlugin for DeleteMacro {
         match models.value(db) {
             ast::Expr::Parenthesized(parens) => {
                 let syntax_node = parens.expr(db).as_syntax_node();
-                bundle.push((syntax_node.get_text(db), syntax_node));
+                bundle.push(syntax_node.get_text(db));
             }
             ast::Expr::Tuple(list) => {
                 list.expressions(db).elements(db).into_iter().for_each(|expr| {
                     let syntax_node = expr.as_syntax_node();
-                    bundle.push((syntax_node.get_text(db), syntax_node));
+                    bundle.push(syntax_node.get_text(db));
                 })
             }
             ast::Expr::StructCtorCall(ctor) => {
                 let syntax_node = ctor.as_syntax_node();
-                bundle.push((syntax_node.get_text(db), syntax_node));
+                bundle.push(syntax_node.get_text(db));
             }
             _ => {
                 return InlinePluginResult {
@@ -93,58 +88,7 @@ impl InlineMacroExprPlugin for DeleteMacro {
             };
         }
 
-        let module_syntax_node =
-            parent_of_kind(db, &syntax.as_syntax_node(), SyntaxKind::ItemModule);
-        let module_name = if let Some(module_syntax_node) = &module_syntax_node {
-            let mod_ast = ItemModule::from_syntax_node(db, module_syntax_node.clone());
-            mod_ast.name(db).as_syntax_node().get_text_without_trivia(db)
-        } else {
-            "".into()
-        };
-
-        let fn_syntax_node =
-            parent_of_kind(db, &syntax.as_syntax_node(), SyntaxKind::FunctionWithBody);
-        let fn_name = if let Some(fn_syntax_node) = &fn_syntax_node {
-            let fn_ast = FunctionWithBody::from_syntax_node(db, fn_syntax_node.clone());
-            fn_ast.declaration(db).name(db).as_syntax_node().get_text_without_trivia(db)
-        } else {
-            "".into()
-        };
-
-        for (entity, syntax_node) in bundle {
-            // db.lookup_intern_file(key0);
-            if !module_name.is_empty() && !fn_name.is_empty() {
-                let mut system_writes = SYSTEM_WRITES.lock().unwrap();
-                // fn_syntax_node
-                if system_writes.get(&module_name).is_none() {
-                    system_writes.insert(module_name.clone(), HashMap::new());
-                }
-                let fns = system_writes.get_mut(&module_name).unwrap();
-                if fns.get(&fn_name).is_none() {
-                    fns.insert(fn_name.clone(), vec![]);
-                }
-
-                match syntax_node.kind(db) {
-                    SyntaxKind::ExprPath => {
-                        fns.get_mut(&fn_name).unwrap().push(SystemRWOpRecord::Path(
-                            ExprPath::from_syntax_node(db, syntax_node),
-                        ));
-                    }
-                    // SyntaxKind::StatementExpr => {
-                    //     todo!()
-                    // }
-                    SyntaxKind::ExprStructCtorCall => {
-                        fns.get_mut(&fn_name).unwrap().push(SystemRWOpRecord::StructCtor(
-                            ExprStructCtorCall::from_syntax_node(db, syntax_node.clone()),
-                        ));
-                    }
-                    _ => eprintln!(
-                        "Unsupport component value type {} for semantic writer analysis",
-                        syntax_node.kind(db)
-                    ),
-                }
-            }
-
+        for entity in bundle {
             builder.add_str(&format!(
                 "
                 let __delete_model_instance__ = {};