-
Notifications
You must be signed in to change notification settings - Fork 182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: bindgen nested types init generation #2580
fix: bindgen nested types init generation #2580
Conversation
WalkthroughOhayo, sensei! This pull request introduces a new Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (3)
crates/dojo-bindgen/src/plugins/mod.rs (2)
Line range hint
36-64
: **Ohayo sensei! The Buffer implementation needs some love ~**The implementation has several areas that could be improved:
- The
insert_after()
method usesunwrap()
which could panic if the position isn't found- Public API lacks documentation
- Complex index manipulation in
insert_after()
needs validationHere's a suggested improvement:
#[derive(Debug)] +/// Buffer wraps a Vec<String> to provide specialized string manipulation operations pub struct Buffer(Vec<String>); impl Buffer { + /// Creates a new empty Buffer pub fn new() -> Self { Self(Vec::new()) } + /// Checks if any string in the buffer contains the given substring pub fn has(&self, s: &str) -> bool { self.0.iter().any(|b| b.contains(s)) } + /// Adds a new string to the buffer pub fn push(&mut self, s: String) { self.0.push(s.clone()); } + /// Inserts a string after a specific position marked by a separator + /// Returns Result to handle cases where position or index is invalid - pub fn insert_after(&mut self, s: String, pos: &str, sep: &str, idx: usize) { - let pos = self.0.iter().position(|b| b.contains(pos)).unwrap(); + pub fn insert_after(&mut self, s: String, pos: &str, sep: &str, idx: usize) -> Result<(), String> { + let pos = self.0.iter().position(|b| b.contains(pos)) + .ok_or_else(|| format!("Position '{}' not found in buffer", pos))?; if let Some(st) = self.0.get_mut(pos) { let indices = st.match_indices(sep).map(|(i, _)| i).collect::<Vec<usize>>(); + if indices.len() <= idx { + return Err(format!("Separator index {} out of bounds", idx)); + } let append_after = indices[indices.len() - idx] + 1; st.insert_str(append_after, &s); + Ok(()) + } else { + Err("Failed to get mutable reference".to_string()) } } + /// Joins all strings in the buffer with the given separator pub fn join(&mut self, sep: &str) -> String { self.0.join(sep) } }
Dependency on cainome PR #67 needs attention
Ohayo sensei! The verification shows that cainome PR #67 is still open and not merged, while this implementation is using cainome v0.4.1. This creates a dependency risk since the required changes are not yet available in the current version.
- PR feat: deployment config #67 "feat: copy inner composite for struct array keys" is still in OPEN state
- Current implementation uses cainome v0.4.1 which doesn't include these changes
🔗 Analysis chain
Line range hint
1-4
: Verify dependency on cainome PR #67This implementation depends on cainome PR #67. Let's verify the current cainome version:
Let me check if PR #67 is already merged and included in the current version being used. This will help us verify if the dependency is properly handled.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check current cainome version in Cargo.toml rg -A 1 'cainome.*=' Cargo.tomlLength of output: 183
Script:
#!/bin/bash # Check if PR #67 exists and its merge status in cainome repository gh pr view 67 --repo cartridge-gg/cainome --json number,title,state,mergedAt # Get the commit history of cainome v0.4.1 tag gh api repos/cartridge-gg/cainome/git/refs/tags/v0.4.1Length of output: 575
crates/dojo-bindgen/src/plugins/typescript/generator/schema.rs (1)
247-299
: Consider adding documentation for the test data structure, sensei!The nested structure being tested represents a complex scenario with arrays and enums. Adding documentation would help other developers understand the test case's purpose and the expected TypeScript output.
Add a doc comment explaining the structure:
/// Creates a test token representing a nested structure with: /// - field1: Array of Direction enum (None, North, South, West, East) /// - field2: Nested Position struct /// /// Expected TypeScript output: /// ```typescript /// interface TestNestedStruct { /// field1: Direction[]; /// field2: Position; /// } /// ``` fn create_test_nested_struct_token(name: &str) -> Composite {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (7)
- crates/dojo-bindgen/src/plugins/mod.rs (1 hunks)
- crates/dojo-bindgen/src/plugins/typescript/generator/erc.rs (1 hunks)
- crates/dojo-bindgen/src/plugins/typescript/generator/function.rs (1 hunks)
- crates/dojo-bindgen/src/plugins/typescript/generator/mod.rs (1 hunks)
- crates/dojo-bindgen/src/plugins/typescript/generator/schema.rs (2 hunks)
- crates/dojo-world/src/contracts/abi/model.rs (1 hunks)
- crates/dojo-world/src/contracts/abi/world.rs (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/dojo-bindgen/src/plugins/typescript/generator/function.rs
🔇 Additional comments (7)
crates/dojo-bindgen/src/plugins/mod.rs (1)
Line range hint
66-75
: Deref implementations look solid, sensei!The implementation follows Rust's best practices for the newtype pattern, providing transparent access to the underlying Vec when needed.
crates/dojo-bindgen/src/plugins/typescript/generator/erc.rs (1)
Line range hint
41-47
: Verify field ordering with schema requirements.The fieldOrder array for tokenMetadata matches the defined fields in the correct order. However, since this PR depends on cainome#67, we should verify that this ordering aligns with any schema changes in that dependency.
✅ Verification successful
Let me search for more specific schema definitions related to ERC tokens.
These commands will help us find relevant token metadata schemas across the codebase to verify the field ordering.
Ohayo sensei! Based on the search results, I can now provide a definitive response about the field ordering.
Field ordering in ERC__Token matches the schema definition
Looking at the codebase, specifically in
crates/torii/graphql/src/mapping.rs
andcrates/dojo-bindgen/src/plugins/typescript/generator/erc.rs
, the field ordering['name', 'symbol', 'tokenId', 'decimals', 'contractAddress']
is consistently defined and matches across all token metadata implementations. This ordering is used in:
- ERC__Token interface definition
- Token metadata objects in ERC20 and ERC721 implementations
- GraphQL type mappings
The field order is correctly maintained and aligns with the schema requirements throughout the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for related schema definitions in the codebase rg --type rust "struct.*Token.*\{" -A 6Length of output: 990
Script:
#!/bin/bash # Search for ERC token related schema definitions rg -g '*.rs' -i "erc.*token|token.*metadata" -B 2 -A 6Length of output: 35266
Script:
#!/bin/bash # Look for any token interface or type definitions in typescript files fd -e ts -e tsx -x rg -l "interface.*Token|type.*Token|TokenMetadata"Length of output: 71
Script:
#!/bin/bash # Check for any schema files that might contain token definitions fd "schema" -e json -e yaml -e toml -x cat {}Length of output: 47
crates/dojo-world/src/contracts/abi/model.rs (2)
328-329
: The derive attributes look good, sensei! 🎉The addition of both
derives
andcontract_derives
with comprehensive trait implementations (Debug, Serialize, Deserialize, PartialEq, Eq, Clone) will enhance the generated types with:
- Debugging capabilities
- Serialization/deserialization support
- Equality comparison
- Cloning functionality
Let's verify the dependency on cainome:
#!/bin/bash # Check Cargo.toml for cainome dependency rg "cainome.*version" Cargo.toml # Verify if these derives are supported in cainome rg -A 5 "contract_derives" $(fd -t f -e rs . | grep -i "cainome")
Line range hint
1-2
: Ohayo sensei!⚠️ This is an auto-generated file.The comment at the top clearly states this is an auto-generated file. Any changes should be made in the generator (
cargo run --bin dojo-world-abigen
) rather than directly modifying this file.Let's verify the generator configuration:
✅ Verification successful
Let me search for the generator configuration and its source code to verify if the changes are properly made in the generator rather than directly in the generated file.
Let me search for more specific configuration details about the abigen tool and its location in the codebase.
Now let me examine the actual generator code to verify if the derive traits are properly configured in the source rather than the generated file.
Ohayo sensei! ✨ The auto-generated file is properly managed by the generator
I can see that the derive traits are correctly configured in the generator source code (
crates/dojo-world/abigen/src/main.rs
). Theget_bindings_file_content
function includes these derives in theabigen!
macro template:derives(Debug, serde::Serialize, serde::Deserialize, PartialEq, Eq, Clone)Any changes to the generated file should be made through the generator by running:
cargo run --bin dojo-world-abigen
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the generator configuration rg -l "dojo-world-abigen" # Check the generator's source code ast-grep --pattern 'fn main() { $$$ }'Length of output: 456
Script:
#!/bin/bash # Let's find the abigen binary source fd -t f "dojo-world-abigen" -x cat {} # Also check for any configuration files related to abigen rg -l "abigen" Cargo.tomlLength of output: 76
Script:
#!/bin/bash # Let's check the Cargo.toml for abigen configuration cat Cargo.toml | grep -A 5 "abigen" # Search for any build scripts or configuration files fd -t f "build.rs|\.toml$" -x grep -l "abigen" {} # Look for the binary in the crates directory fd -t f -e rs . crates/ | grep -i "abigen"Length of output: 589
Script:
#!/bin/bash # Let's check the abigen main source code cat crates/dojo-world/abigen/src/main.rs # Also check the abigen Cargo.toml for dependencies cat crates/dojo-world/abigen/Cargo.tomlLength of output: 5606
crates/dojo-bindgen/src/plugins/typescript/generator/mod.rs (1)
127-134
: Verify compatibility with cainome dependency.Since this PR depends on changes in the cainome repository (PR #67), we should ensure the new format is compatible with those changes.
Let's verify the cainome dependency version:
✅ Verification successful
Let me gather more information about the format compatibility.
This will help us verify if the format is compatible with the cainome dependency by:
- Checking the format implementation in cainome repository
- Looking for any related tests in our codebase that might validate this format
Let me check one more thing to verify the cainome version compatibility.
This will help us understand if there were any recent changes to the fieldOrder format and if we have test coverage for it.
Ohayo sensei! Based on the git history and codebase analysis, I can now provide a conclusive response:
The fieldOrder format is consistent with recent changes ✨
The format change from
{ field1: value1 }
to{ fieldOrder: ['field1'], field1: value1 }
was intentionally implemented across the codebase, as evidenced by:
- Recent git commits showing the standardization of
fieldOrder
casing- Comprehensive test coverage validating the format
- Consistent implementation in ERC types and other interfaces
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the cainome dependency version in Cargo.toml rg -A 1 'cainome.*=' Cargo.tomlLength of output: 183
Script:
#!/bin/bash # Check the format usage in cainome repository gh api repos/cartridge-gg/cainome/contents/src/abigen/typescript.rs -H "Accept: application/vnd.github.raw" | rg -A 5 'fieldOrder' # Check if there are any related tests in our repository rg -g '*.rs' 'fieldOrder' .Length of output: 2166
Script:
#!/bin/bash # Check the git history for any recent changes to fieldOrder format git log -p --since="3 months ago" | rg -B 3 -A 3 'fieldOrder' # Also check the tests directory for any format-related tests fd -t f -e rs . tests | xargs rg 'fieldOrder'Length of output: 3796
crates/dojo-world/src/contracts/abi/world.rs (2)
1240-1241
: LGTM! The derive attributes enhance type functionality.The additions look good:
- Type alias for
Config::Event
improves code readability- Contract derives ensure generated types implement essential traits for serialization, comparison, and cloning
Line range hint
1-1
: Ohayo sensei! Be cautious about modifying auto-generated files.This file is marked as auto-generated with a clear instruction to use
cargo run --bin dojo-world-abigen
for regenerating bindings.Let me verify if the abigen binary exists:
✅ Verification successful
Ohayo sensei! The auto-generated file warning is accurate and helpful!
I've confirmed that the
dojo-world-abigen
binary exists as a proper Rust project incrates/dojo-world/abigen/
with its ownCargo.toml
andmain.rs
. This validates that the warning about usingcargo run --bin dojo-world-abigen
to regenerate the bindings is correct and actionable.
- The binary is properly set up in
crates/dojo-world/abigen/
- It has the necessary dependencies for ABI generation (cairo-lang-starknet, scarb)
- The warning helps prevent manual modifications that would be lost on next generation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the abigen binary exists in the workspace fd -t f "dojo-world-abigen" -x echo "Found abigen binary at: {}"Length of output: 64
Script:
#!/bin/bash # Let's check for the binary in Cargo.toml and any build scripts rg -l "dojo-world-abigen" --type toml # Also check for any build scripts or source files related to abigen fd -t f "build.rs|abigen" -x echo "Found potential build-related file: {}" # Look for any main.rs files in bin directory that might contain the abigen binary fd -t f "main.rs" -p "bin" -x echo "Found binary entry point: {}"Length of output: 811
Script:
#!/bin/bash # Let's check the contents of the abigen directory since we found it in Cargo.toml fd . "crates/dojo-world/abigen" -x echo "Found in abigen dir: {}" # Check for any source files in the abigen directory rg -l "fn main" "crates/dojo-world/abigen" # Look at the Cargo.toml content to understand the binary configuration cat "crates/dojo-world/abigen/Cargo.toml"Length of output: 759
@@ -34,11 +34,11 @@ const ERC_TORII_TYPES: &str = "\n\t\tERC__Balance: ERC__Balance,\n\t\tERC__Token | |||
ERC__Token,\n\t\tERC__Transfer: ERC__Transfer,"; | |||
const ERC_TORII_INIT: &str = " | |||
\t\tERC__Balance: { | |||
\t\t\tfieldorder: ['balance', 'type', 'tokenmetadata'], | |||
\t\t\tfieldOrder: ['balance', 'type', 'tokenmetadata'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohayo sensei! Fix inconsistent casing in fieldOrder array.
The field 'tokenmetadata' in the fieldOrder array uses lowercase, while the actual field is defined as 'tokenMetadata'. This inconsistency could cause field matching issues.
Apply this fix:
- fieldOrder: ['balance', 'type', 'tokenmetadata'],
+ fieldOrder: ['balance', 'type', 'tokenMetadata'],
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
\t\t\tfieldOrder: ['balance', 'type', 'tokenmetadata'], | |
fieldOrder: ['balance', 'type', 'tokenMetadata'], |
"{{ fieldOrder: [{}], {} }}", | ||
value.inners.iter().map(|i| format!("'{}'", i.name)).collect::<Vec<_>>().join(", "), | ||
value | ||
.inners | ||
.iter() | ||
.map(|i| format!("{}: {},", i.name, JsDefaultValue::from(&i.token))) | ||
.collect::<Vec<_>>() | ||
.join("\n") | ||
.join(" ") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohayo sensei! Test case needs updating to match new format.
The implementation now uses space-separated fields, but the test case test_generate_type_init
still expects newline-separated fields. This inconsistency should be addressed.
Update the test case to match the new format:
let expected = "{
\t\t\tfieldOrder: ['field1', 'field2', 'field3'],
-\t\t\tfield1: 0,
-\t\t\tfield2: 0,
-\t\t\tfield3: 0,
+\t\t\tfield1: 0, field2: 0, field3: 0,
\t\t}";
Committable suggestion was skipped due to low confidence.
🛠️ Refactor suggestion
Consider maintaining newline separation for better readability.
The change to space-separated fields makes the output more compact but potentially harder to read, especially for structs with many fields or complex nested types.
Consider keeping the newline separation for better readability:
- .join(" ")
+ .join("\n\t\t\t")
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"{{ fieldOrder: [{}], {} }}", | |
value.inners.iter().map(|i| format!("'{}'", i.name)).collect::<Vec<_>>().join(", "), | |
value | |
.inners | |
.iter() | |
.map(|i| format!("{}: {},", i.name, JsDefaultValue::from(&i.token))) | |
.collect::<Vec<_>>() | |
.join("\n") | |
.join(" ") | |
"{{ fieldOrder: [{}], {} }}", | |
value.inners.iter().map(|i| format!("'{}'", i.name)).collect::<Vec<_>>().join(", "), | |
value | |
.inners | |
.iter() | |
.map(|i| format!("{}: {},", i.name, JsDefaultValue::from(&i.token))) | |
.collect::<Vec<_>>() | |
.join("\n\t\t\t") |
#[test] | ||
fn test_handle_nested_struct() { | ||
let generator = TsSchemaGenerator {}; | ||
let mut buffer = Buffer::new(); | ||
let nested_struct = create_test_nested_struct_token("TestNestedStruct"); | ||
let _res = generator.generate(&nested_struct, &mut buffer); | ||
assert_eq!(buffer.len(), 3); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohayo sensei! The test assertions need improvement.
The test only verifies the buffer length without checking the actual content. Consider adding assertions to verify:
- The correct TypeScript type definitions are generated
- The nested structure is properly handled
- The array type is correctly transformed
Here's a suggested improvement:
#[test]
fn test_handle_nested_struct() {
let generator = TsSchemaGenerator {};
let mut buffer = Buffer::new();
let nested_struct = create_test_nested_struct_token("TestNestedStruct");
let _res = generator.generate(&nested_struct, &mut buffer);
- assert_eq!(buffer.len(), 3);
+ // Verify import
+ assert!(buffer[0].contains("import type { SchemaType }"));
+
+ // Verify type definition
+ assert!(buffer[1].contains("TestNestedStruct: TestNestedStruct"));
+
+ // Verify schema initialization
+ assert!(buffer[2].contains("fieldOrder: ['field1', 'field2']"));
+ assert!(buffer[2].contains("field1: []")); // Array initialization
+ assert!(buffer[2].contains("field2: {")); // Nested struct initialization
}
Committable suggestion was skipped due to low confidence.
pub fn create_test_nested_struct_token(name: &str) -> Composite { | ||
Composite { | ||
type_path: format!("onchain_dash::{name}"), | ||
inners: vec![ | ||
CompositeInner { | ||
index: 0, | ||
name: "field1".to_owned(), | ||
kind: CompositeInnerKind::Key, | ||
token: Token::Array(cainome::parser::tokens::Array { | ||
type_path: "core::array::Array::<onchain_dah::Direction>".to_owned(), | ||
inner: Box::new(Token::Composite(Composite { | ||
type_path: "onchain_dah::Direction".to_owned(), | ||
inners: vec![ | ||
CompositeInner { | ||
index: 0, | ||
name: "None".to_owned(), | ||
kind: CompositeInnerKind::Key, | ||
token: Token::CoreBasic(CoreBasic { | ||
type_path: "core::fetl252".to_owned(), | ||
}), | ||
}, | ||
CompositeInner { | ||
index: 1, | ||
name: "North".to_owned(), | ||
kind: CompositeInnerKind::Key, | ||
token: Token::CoreBasic(CoreBasic { | ||
type_path: "core::fetl252".to_owned(), | ||
}), | ||
}, | ||
CompositeInner { | ||
index: 2, | ||
name: "South".to_owned(), | ||
kind: CompositeInnerKind::Key, | ||
token: Token::CoreBasic(CoreBasic { | ||
type_path: "core::fetl252".to_owned(), | ||
}), | ||
}, | ||
CompositeInner { | ||
index: 3, | ||
name: "West".to_owned(), | ||
kind: CompositeInnerKind::Key, | ||
token: Token::CoreBasic(CoreBasic { | ||
type_path: "core::fetl252".to_owned(), | ||
}), | ||
}, | ||
CompositeInner { | ||
index: 4, | ||
name: "East".to_owned(), | ||
kind: CompositeInnerKind::Key, | ||
token: Token::CoreBasic(CoreBasic { | ||
type_path: "core::fetl252".to_owned(), | ||
}), | ||
}, | ||
], | ||
generic_args: vec![], | ||
r#type: CompositeType::Enum, | ||
is_event: false, | ||
alias: None, | ||
})), | ||
is_legacy: false, | ||
}), | ||
}, | ||
CompositeInner { | ||
index: 1, | ||
name: "field2".to_owned(), | ||
kind: CompositeInnerKind::Key, | ||
token: Token::Composite(create_test_struct_token("Position")), | ||
}, | ||
], | ||
generic_args: vec![], | ||
r#type: CompositeType::Struct, | ||
is_event: false, | ||
alias: None, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple issues found in the test helper function, sensei!
- The function is marked as
pub
but is only used within the tests module - There are typos in the type paths:
- "fetl252" instead of "felt252"
- "onchain_dah" instead of "onchain_dash"
Apply these fixes:
- pub fn create_test_nested_struct_token(name: &str) -> Composite {
+ fn create_test_nested_struct_token(name: &str) -> Composite {
Composite {
type_path: format!("onchain_dash::{name}"),
inners: vec![
CompositeInner {
index: 0,
name: "field1".to_owned(),
kind: CompositeInnerKind::Key,
token: Token::Array(cainome::parser::tokens::Array {
- type_path: "core::array::Array::<onchain_dah::Direction>".to_owned(),
+ type_path: "core::array::Array::<onchain_dash::Direction>".to_owned(),
inner: Box::new(Token::Composite(Composite {
- type_path: "onchain_dah::Direction".to_owned(),
+ type_path: "onchain_dash::Direction".to_owned(),
// ... other fields ...
token: Token::CoreBasic(CoreBasic {
- type_path: "core::fetl252".to_owned(),
+ type_path: "core::felt252".to_owned(),
}),
Committable suggestion was skipped due to low confidence.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2580 +/- ##
==========================================
+ Coverage 70.24% 70.32% +0.08%
==========================================
Files 402 402
Lines 51131 51214 +83
==========================================
+ Hits 35916 36016 +100
+ Misses 15215 15198 -17 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
crates/dojo-world/abigen/src/main.rs (1)
88-88
: Consider documenting the contract_derives behavior.
Since this is a new feature affecting code generation, it would be helpful to add a comment explaining the purpose and impact of contract_derives(Debug)
for future maintainers.
Clone),contract_derives(Debug)\n);",
+ // Note: contract_derives applies Debug trait to the generated contract types
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (3)
Cargo.lock
is excluded by!**/*.lock
spawn-and-move-db.tar.gz
is excluded by!**/*.gz
types-test-db.tar.gz
is excluded by!**/*.gz
📒 Files selected for processing (6)
- Cargo.toml (2 hunks)
- crates/dojo-world/abigen/src/main.rs (1 hunks)
- crates/dojo-world/src/contracts/abi/model.rs (1 hunks)
- crates/dojo-world/src/contracts/abi/world.rs (1 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.json (1 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.toml (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- examples/spawn-and-move/manifests/dev/deployment/manifest.json
- examples/spawn-and-move/manifests/dev/deployment/manifest.toml
🚧 Files skipped from review as they are similar to previous changes (3)
- Cargo.toml
- crates/dojo-world/src/contracts/abi/model.rs
- crates/dojo-world/src/contracts/abi/world.rs
🔇 Additional comments (1)
crates/dojo-world/abigen/src/main.rs (1)
88-88
: Ohayo! Verify the contract_derives attribute compatibility.
The addition of contract_derives(Debug)
looks good, sensei! However, since this change depends on cartridge-gg/cainome#67, we should ensure compatibility.
✅ Verification successful
Ohayo! The contract_derives(Debug) addition is compatible, sensei!
The codebase is using cainome v0.4.6 which supports the contract_derives
attribute, and this feature is already being used in other parts of the codebase (world.rs and model.rs). The change aligns with existing patterns and doesn't require any external PR dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify cainome version compatibility
# Test: Check if Cargo.toml references the correct cainome version that supports contract_derives
rg -A 5 "cainome.*version" Cargo.toml
Length of output: 39
Script:
#!/bin/bash
# Let's check all Cargo.toml files in the repository for cainome dependency
fd Cargo.toml --exec rg -l "cainome"
# Then let's examine the content of those files
fd Cargo.toml --exec sh -c 'echo "=== {} ==="; cat {}'
Length of output: 57285
Script:
#!/bin/bash
# Let's check the crates/dojo-world/abigen/Cargo.toml for cainome dependency
echo "=== Checking abigen's Cargo.toml ==="
fd "Cargo.toml" crates/dojo-world/abigen/ --exec cat {}
# Let's also check the actual implementation to understand the contract_derives usage
echo -e "\n=== Checking contract_derives implementation ==="
rg -A 5 "contract_derives" crates/dojo-world/abigen/src/
Length of output: 1087
Script:
#!/bin/bash
# Let's check the workspace dependencies for cainome version
echo "=== Checking workspace cainome dependency ==="
rg "cainome.*=.*" -A 2 Cargo.toml
# Let's also check if there are any other contract_derives usages
echo -e "\n=== Checking other contract_derives usages ==="
rg -l "contract_derives" .
Length of output: 556
Description
This requires cartridge-gg/cainome#67 to be merged first and version updated in
Cargo.toml
Summary by CodeRabbit
Release Notes
New Features
Buffer
struct for improved string management, including methods for adding and manipulating strings.Improvements
Tests