fix(dojo-lang): raise an error on value before key in model (#2891)

* fix: Raise an error on value before key in model

* fix: Raise an error on value before key in model

* fix: Improve key member validation logic in parse_members function and clarity

* fix: reorder fields in Message struct

* fix: run linters

* chore: bump scarb to add key before member diagnostic

* tests: regenerate test db

---------

Co-authored-by: glihm <[email protected]>

Author: bengineer42

Cargo.lock

@@ -212,8 +212,8 @@ rpassword = "7.2.0"
 rstest = "0.18.2"
 rstest_reuse = "0.6.0"
 salsa = "0.16.1"
-scarb = { git = "https://github.com/dojoengine/scarb", rev = "7eac49b3e61236ce466e712225d9c989f9db1ef3" }
-scarb-ui = { git = "https://github.com/dojoengine/scarb", rev = "7eac49b3e61236ce466e712225d9c989f9db1ef3" }
+scarb = { git = "https://github.com/dojoengine/scarb", rev = "c811c5bcb073afa21cb9b1307adafe733c203eb9" }
+scarb-ui = { git = "https://github.com/dojoengine/scarb", rev = "c811c5bcb073afa21cb9b1307adafe733c203eb9" }
 semver = "1.0.5"
 serde = { version = "1.0", features = [ "derive" ] }
 serde_json = { version = "1.0", features = [ "arbitrary_precision" ] }

Cargo.toml

@@ -1,8 +1,10 @@
 use cairo_lang_defs::patcher::RewriteNode;
+use cairo_lang_defs::plugin::PluginDiagnostic;
+use cairo_lang_diagnostics::Severity;
 use cairo_lang_syntax::node::ast::Member as MemberAst;
 use cairo_lang_syntax::node::db::SyntaxGroup;
 use cairo_lang_syntax::node::helpers::QueryAttrs;
-use cairo_lang_syntax::node::{Terminal, TypedSyntaxNode};
+use cairo_lang_syntax::node::{Terminal, TypedStablePtr, TypedSyntaxNode};
 use dojo_types::naming::compute_bytearray_hash;
 use starknet_crypto::{poseidon_hash_many, Felt};
 
@@ -34,13 +36,45 @@ pub fn compute_unique_hash(
     poseidon_hash_many(&hashes)
 }
 
-pub fn parse_members(db: &dyn SyntaxGroup, members: &[MemberAst]) -> Vec<Member> {
+pub fn parse_members(
+    db: &dyn SyntaxGroup,
+    members: &[MemberAst],
+    diagnostics: &mut Vec<PluginDiagnostic>,
+) -> Vec<Member> {
+    let mut parsing_keys = true;
+
     members
         .iter()
-        .map(|member_ast| Member {
-            name: member_ast.name(db).text(db).to_string(),
-            ty: member_ast.type_clause(db).ty(db).as_syntax_node().get_text(db).trim().to_string(),
-            key: member_ast.has_attr(db, "key"),
+        .map(|member_ast| {
+            let is_key = member_ast.has_attr(db, "key");
+
+            let member = Member {
+                name: member_ast.name(db).text(db).to_string(),
+                ty: member_ast
+                    .type_clause(db)
+                    .ty(db)
+                    .as_syntax_node()
+                    .get_text(db)
+                    .trim()
+                    .to_string(),
+                key: is_key,
+            };
+
+            // Make sure all keys are before values in the model.
+            if is_key && !parsing_keys {
+                diagnostics.push(PluginDiagnostic {
+                    message: "Key members must be defined before non-key members.".into(),
+                    stable_ptr: member_ast.name(db).stable_ptr().untyped(),
+                    severity: Severity::Error,
+                });
+                // Don't return here, since we don't want to stop processing the members after the
+                // first error to avoid diagnostics just because the field is
+                // missing.
+            }
+
+            parsing_keys &= is_key;
+
+            member
         })
         .collect::<Vec<_>>()
 }

crates/dojo/lang/src/attribute_macros/element.rs

@@ -58,7 +58,7 @@ impl DojoEvent {
             }
         }
 
-        let members = parse_members(db, &struct_ast.members(db).elements(db));
+        let members = parse_members(db, &struct_ast.members(db).elements(db), &mut diagnostics);
 
         let mut serialized_keys: Vec<RewriteNode> = vec![];
         let mut serialized_values: Vec<RewriteNode> = vec![];

crates/dojo/lang/src/attribute_macros/event.rs

@@ -72,7 +72,7 @@ impl DojoModel {
         let mut model_member_store_impls_processed: HashSet<String> = HashSet::new();
         let mut model_member_store_impls: Vec<String> = vec![];
 
-        let members = parse_members(db, &struct_ast.members(db).elements(db));
+        let members = parse_members(db, &struct_ast.members(db).elements(db), &mut diagnostics);
 
         members.iter().for_each(|member| {
             if member.key {

crates/dojo/lang/src/attribute_macros/model.rs

@@ -28,9 +28,9 @@ pub struct Message {
     pub identity: ContractAddress,
     #[key]
     pub channel: felt252,
-    pub message: ByteArray,
     #[key]
-    pub salt: felt252
+    pub salt: felt252,
+    pub message: ByteArray,
 }
 
 #[derive(Copy, Drop, Serde, Debug)]