Skip to content

Commit

Permalink
fix(dojo-lang): verify constructor args to follow dojo rules (#2944)
Browse files Browse the repository at this point in the history
* fix: ensure constructor is checked to follow dojo rules

* chore: bump scarb

* fix: run linters + add tests
  • Loading branch information
glihm authored Jan 23, 2025
1 parent 8f54b66 commit f99ac3d
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 19 deletions.
32 changes: 16 additions & 16 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,9 @@ 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 = "f67331d99bca24c13ad56af6752b93f23e5ac373" }
scarb-metadata = { git = "https://github.com/dojoengine/scarb", rev = "f67331d99bca24c13ad56af6752b93f23e5ac373" }
scarb-ui = { git = "https://github.com/dojoengine/scarb", rev = "f67331d99bca24c13ad56af6752b93f23e5ac373" }
scarb = { git = "https://github.com/dojoengine/scarb", rev = "b5ab351aa9b52dcf27a397021d84a423afd016dc" }
scarb-metadata = { git = "https://github.com/dojoengine/scarb", rev = "b5ab351aa9b52dcf27a397021d84a423afd016dc" }
scarb-ui = { git = "https://github.com/dojoengine/scarb", rev = "b5ab351aa9b52dcf27a397021d84a423afd016dc" }
semver = "1.0.5"
serde = { version = "1.0", features = [ "derive" ] }
serde_json = { version = "1.0", features = [ "arbitrary_precision" ] }
Expand Down
44 changes: 44 additions & 0 deletions crates/dojo/lang/src/attribute_macros/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,16 @@ impl DojoContract {
let fn_decl = fn_ast.declaration(db);

let params_str = self.params_to_str(db, fn_decl.signature(db).parameters(db));
if !is_valid_constructor_params(&params_str) {
self.diagnostics.push(PluginDiagnostic {
stable_ptr: fn_ast.stable_ptr().untyped(),
message: "The constructor must have exactly one parameter, which is `ref self: \
ContractState`. Add a `dojo_init` function instead if you need to \
initialize the contract with parameters."
.to_string(),
severity: Severity::Error,
});
}

let declaration_node = RewriteNode::Mapped {
node: Box::new(RewriteNode::Text(format!(
Expand Down Expand Up @@ -372,3 +382,37 @@ impl DojoContract {
params.join(", ")
}
}

/// Checks if the constructor parameters are valid.
/// We only allow one parameter for the constructor, which is the contract state,
/// since `dojo_init` is called by the world after every resource has been deployed.
fn is_valid_constructor_params(params: &str) -> bool {
let frags = params.split(",").collect::<Vec<_>>();

if frags.len() != 1 {
return false;
}

frags.first().unwrap().contains("ref self: ContractState")
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_is_valid_constructor_params_ok() {
assert!(is_valid_constructor_params("ref self: ContractState"));
assert!(is_valid_constructor_params("ref self: ContractState "));
assert!(is_valid_constructor_params(" ref self: ContractState"));
}

#[test]
fn test_is_valid_constructor_params_not_ok() {
assert!(!is_valid_constructor_params(""));
assert!(!is_valid_constructor_params("self: ContractState"));
assert!(!is_valid_constructor_params("ref self: OtherState"));
assert!(!is_valid_constructor_params("ref self: ContractState, other: felt252"));
assert!(!is_valid_constructor_params("other: felt252"));
}
}

0 comments on commit f99ac3d

Please sign in to comment.