Fix/bindgen types improvements #2832
Conversation
WalkthroughOhayo, sensei! This pull request introduces significant modifications to the Dojo bindgen TypeScript generator, focusing on refactoring type generation, removing the ERC-related generator, and introducing new methods for handling composite types. The changes streamline the code generation process, update type handling, and replace the ERC generator with a new models generator, enhancing the overall flexibility of the TypeScript code generation mechanism. Changes
Possibly Related PRs
Suggested Reviewers
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs (1)
Line range hint
678-687: Ohayo sensei, deeply nested struct approach.Recursively building items inside items might cause performance overhead. Consider short-circuit or references if repeated.
🧹 Nitpick comments (34)
crates/dojo/bindgen/src/plugins/typescript/generator/function.rs (6)
19-20: Ohayo sensei, watch out for potential import bloat!Adding imports "CairoOption, CairoCustomEnum, ByteArray" might bloat the generated bundle if not needed in every context. Double-check usage to ensure minimal overhead.
97-101: Ohayo sensei, consider centralizing prefix logic.The logic scattered among conditionals could be extracted into a small helper function, improving readability and maintainability.
103-103: Ohayo sensei, watch out for naming collisions.Hardcoding "models." as a prefix might cause naming collisions if another module or folder shares this name. Possibly consider a namespace alias or a dynamic approach if collisions become likely.
237-240: Ohayo sensei, ensuring grouping of core imports.The lines 237-240 add references to cainome, but also reference to dojo_world. Double-check if the usage still aligns with standard grouping or if more re-organization is needed.
326-333: Ohayo sensei, test name clarity.“test_format_function_inputs_cairo_option” is descriptive, but ensure naming remains consistent with other test methods. This aligns the entire test suite for easy scanning.
515-592: Ohayo sensei, watch out for deeply nested structures.“create_function_with_option_param” includes extensive nesting. Deep structures can hurt performance and readability. Consider factoring common logic or ensuring partial flattening where possible.
crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs (25)
3-5: Ohayo sensei, keep constants grouped.Lines 3-5 add references to CAIRO_BOOL, etc. Confirm these additions remain coherent with the existing constant sets in “constants.rs.”
40-40: Ohayo sensei, consider more explicit naming.Line 40 references “build_composite_inner_primitive_default_value.” Possibly rename it to clarify it is about defaulting. E.g., “build_inner_default_value.”
71-71: Ohayo sensei, watch out for potential newtype confusion.Line 71 declares “JsPrimitiveType(String).” Consider a clear approach if adding more fields in the future.
73-73: Ohayo sensei, ephemeral string conversions.Line 73’s “From<&str>” might be error-prone if not carefully tested for complex string inputs.
127-127: Ohayo sensei, watch out for naming conflict with “Enum.”Appending “Enum” might clash if the type path also ends with “Enum.” Consider a more dynamic naming approach.
131-131: Ohayo sensei, consider consistent error handling.Line 131 returns “value.type_name()” when a token is not recognized. Possibly add logging or fallback logic if “type_name()” fails.
136-136: Ohayo sensei, keep new Display trait minimal.Using “self.0” directly is minimal, so ensure this usage remains future-proof if the struct fields expand.
144-149: Ohayo sensei, consistent debug & display.Declaring debug + display ensures better introspection. Confirm the print outputs remain consistent across debugging contexts.
150-150: Ohayo sensei, from<&str> duplication.As with JsPrimitiveType, we have a near-identical approach for “From<&str>.” Factor common logic or ensure consistent coverage.
170-185: Ohayo sensei, unify array & tuple expansions.The array expansions in lines 170-185 replicate logic from lines 96-104. Consider a shared function or macro to avoid duplication.
216-233: Ohayo sensei, consider fallback logging.When encountering an unknown type in “From<&str> for JsPrimitiveDefaultValue,” you store the raw string. Consider logging a warning or validating the input.
266-270: Ohayo sensei, watch out for undefined fields.For remaining variants, the code sets them to
undefined. This might cause confusion if your runtime expects explicit discriminants.
271-271: Ohayo sensei, confirm string concatenations.Line 271 merges string results. Excess whitespace or line breaks might produce unexpected formatting.
280-285: Ohayo sensei, detail the core fallback.We treat everything else—like “core::felt252” or “core::integer::u64”—the same. Validate that we maintain correct defaults for these.
290-295: Ohayo sensei, re-check doc comments.Says “Builds the default value for an enum variant,” but the function name is “build_custom_enum_default_value.” Keep docs consistent.
296-320: Ohayo sensei, rearrange your struct formatting logic.“build_struct_default_value” lumps “fieldOrder” with recursively built fields. Possibly separate them to clarify the ordering logic.
346-346: Ohayo sensei, minimal Display trait usage.Same approach as above. If we expand the struct, we might unify the “fmt” logic.
364-365: Ohayo sensei, partialeq usage might mask some differences.Line 364-365 implements partialeq for &str with JsPrimitiveType. If logic gets more complex, we might prefer a direct compare approach.
380-388: Ohayo sensei, u8 with felt252 test coverage.We see we’re testing core integer types. Ensure index-based or range-based integer coverage is similarly tested (u32, i128, etc.).
Line range hint
410-419: Ohayo sensei, confirm array-of-tuples approach.Similar to line 396’s tuples, this tests arrays of tuples. Possibly add a test for arrays-of-arrays for completeness.
504-513: Ohayo sensei, default value testing for numeric felt.The test “test_default_value_basics” lines 504-513 checks intangible differences (0 vs empty). Confirm other numeric or textual defaults are also tested.
Line range hint
520-529: Ohayo sensei, check that multiple tuples remain valid.We only see a 2-element tuple in “test_tuple_default_value.” If more elements exist, confirm they follow the same pattern.
Line range hint
552-561: Ohayo sensei, single-variant fallback issues.If an enum with only one variant or more than 4 variants is tested, ensure the logic in these lines gracefully handles extremes.
Line range hint
645-655: Ohayo sensei, numeric defaults in nested structs.We return “id: 0, xp: 0” by default. Double-check the domain-specific meaning of 0 for these fields.
814-820: Ohayo sensei, confirm that unknown custom enum remains empty.Line 818 expects “{ fieldOrder: [], }” for “Option.” Ensure a partial or placeholder initialization is appropriate.
crates/dojo/bindgen/src/plugins/typescript/generator/constants.rs (1)
26-26: Ohayo sensei, keep “CairoOptionVariant” usage consistent.“CairoOptionVariant” is appended. Confirm that all code references it properly if it’s mandatory for certain logic paths.
crates/dojo/bindgen/src/plugins/typescript/writer.rs (1)
Line range hint
25-46: Consider extracting composite collection logicThe composite collection logic is complex and could benefit from being extracted into a separate method for better readability and maintainability.
impl TsFileWriter { + fn collect_composites<'a>(models: &'a [&'a DojoData]) -> Vec<&'a Composite> { + models + .iter() + .flat_map(|m| { + let mut composites: Vec<&Composite> = Vec::new(); + let mut enum_composites = + m.tokens.enums.iter().map(|e| e.to_composite().unwrap()).collect::<Vec<_>>(); + let mut struct_composites = + m.tokens.structs.iter().map(|s| s.to_composite().unwrap()).collect::<Vec<_>>(); + let mut func_composites = m + .tokens + .functions + .iter() + .map(|f| f.to_composite().unwrap()) + .collect::<Vec<_>>(); + composites.append(&mut enum_composites); + composites.append(&mut struct_composites); + composites.append(&mut func_composites); + composites + }) + .filter(|c| !(c.type_path.starts_with("dojo::") || c.type_path.starts_with("core::"))) + .collect() + }crates/dojo/bindgen/src/plugins/typescript/generator/enum.rs (1)
46-48: Clean variant mapping implementationThe variant mapping using
JsPrimitiveTypeis concise and type-safe. However, consider adding error handling for potential mapping failures..inners .iter() - .map(|inner| { - format!("\t{}: {};", inner.name, JsPrimitiveType::from(&inner.token)) - }) + .map(|inner| -> Result<String, &'static str> { + let type_str = JsPrimitiveType::from(&inner.token); + Ok(format!("\t{}: {};", inner.name, type_str)) + }) + .collect::<Result<Vec<_>, _>>() + .map_err(|e| format!("Failed to map variant: {}", e))? .join("\n")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
crates/dojo/bindgen/src/plugins/mod.rs(0 hunks)crates/dojo/bindgen/src/plugins/typescript/generator/constants.rs(1 hunks)crates/dojo/bindgen/src/plugins/typescript/generator/enum.rs(2 hunks)crates/dojo/bindgen/src/plugins/typescript/generator/erc.rs(0 hunks)crates/dojo/bindgen/src/plugins/typescript/generator/function.rs(7 hunks)crates/dojo/bindgen/src/plugins/typescript/generator/interface.rs(4 hunks)crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs(19 hunks)crates/dojo/bindgen/src/plugins/typescript/generator/models.rs(1 hunks)crates/dojo/bindgen/src/plugins/typescript/generator/schema.rs(4 hunks)crates/dojo/bindgen/src/plugins/typescript/mod.rs(2 hunks)crates/dojo/bindgen/src/plugins/typescript/writer.rs(2 hunks)
💤 Files with no reviewable changes (2)
- crates/dojo/bindgen/src/plugins/mod.rs
- crates/dojo/bindgen/src/plugins/typescript/generator/erc.rs
🔇 Additional comments (51)
crates/dojo/bindgen/src/plugins/typescript/generator/schema.rs (8)
30-30: Ohayo sensei, ensure proper import or declaration for "WithFieldOrder".
This line introduces a reference to "WithFieldOrder", but it is not visible where it’s imported or defined in the snippet. Please confirm that "WithFieldOrder" is declared or imported correctly to avoid reference errors.
39-39: Ohayo sensei, this addition for the namespace looks great!
The logic to conditionally introduce the namespace with "WithFieldOrder" is straightforward and consistent with the rest of the code.
45-45: Ohayo sensei, the property addition logic is consistent.
This scented snippet ensures the composite type is assigned "WithFieldOrder". The existing checks for duplicates keep the code stable.
174-174: Ohayo sensei, the test assertion for "TestStruct" looks splendid.
Verifying "TestStruct: WithFieldOrder" ensures the generator logic is properly validated.
190-190: Ohayo sensei, the test coverage for "TestStruct" is solid.
Including another assertion for "WithFieldOrder" confirms consistent schema generation.
198-199: Ohayo sensei, the introduction of "AvailableTheme" in tests is well done.
It further validates that multiple structs in the same namespace use "WithFieldOrder". Good consistency check.
206-208: Ohayo sensei, moving to the "combat" namespace is consistent with the approach above.
Validates the test coverage across multiple namespaces with "Player: WithFieldOrder". This ensures comprehensive verification.
215-217: Ohayo sensei, the additional "Position" property test is thorough.
This confirms the generation logic for a second type within the "combat" namespace, further building confidence in the code’s correctness.
crates/dojo/bindgen/src/plugins/typescript/generator/interface.rs (4)
4-4: Ohayo sensei! The import changes look good!
The transition from JsType to JsPrimitiveType indicates a more precise type handling approach, which aligns well with the bindgen types improvements objective.
Also applies to: 6-6
129-129: Test assertion update looks good, sensei!
The updated test assertion correctly reflects the changes in type generation behavior while maintaining the same structural validation.
32-33: Verify the impact of field order type definition change, sensei!
The semantic change from removing to including field order type definitions could affect the generated TypeScript code structure. Let's verify that this change doesn't introduce any breaking changes in the generated interfaces.
✅ Verification successful
Field order type definition change is safe, sensei!
The change from removing to including field order type definitions is actually a beneficial addition. The WithFieldOrder type is consistently used across the codebase to enhance TypeScript interfaces with field ordering information:
- It's properly defined in
constants.rsastype WithFieldOrder<T> = T & { fieldOrder: string[] } - It's correctly utilized in
schema.rsfor all schema type definitions - The change in
interface.rsensures this type definition is available when needed
The modification aligns with the existing type system and improves type safety by ensuring field order information is consistently available.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any existing usages of field order types in the codebase
rg -l "FieldOrder" --type rust
Length of output: 159
Script:
#!/bin/bash
# Let's check the content of these files to understand the field order type definition
echo "=== Checking schema.rs ==="
rg "FieldOrder" -A 5 -B 5 crates/dojo/bindgen/src/plugins/typescript/generator/schema.rs
echo -e "\n=== Checking constants.rs ==="
rg "FieldOrder" -A 5 -B 5 crates/dojo/bindgen/src/plugins/typescript/generator/constants.rs
echo -e "\n=== Checking interface.rs for context ==="
cat crates/dojo/bindgen/src/plugins/typescript/generator/interface.rs
Length of output: 12186
70-70: Ohayo! Let's ensure comprehensive test coverage for primitive type mapping, sensei!
The switch to JsPrimitiveType::from introduces more precise type mapping. While the existing tests cover basic scenarios, we should verify coverage for all primitive types.
✅ Verification successful
Ohayo! The type mapping is well-tested, sensei!
After reviewing the codebase, I can confirm that the JsPrimitiveType conversion is thoroughly tested through:
- Direct struct field type mapping tests in
test_interface_with_inners - Option type handling in
test_check_import - Various primitive type mappings via the comprehensive implementation in
mod.rs - Test coverage for different struct scenarios including empty structs and non-struct types
The implementation properly handles all Cairo primitive types (u8-u256, felt252, bool, etc.) with appropriate JavaScript type mappings.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for test cases covering primitive type conversions
rg -A 5 "test.*primitive.*type" --type rust
Length of output: 45
Script:
#!/bin/bash
# Let's search for test files related to JsPrimitiveType
fd "test.*\.rs" | xargs rg -l "JsPrimitiveType"
# Also search for actual implementation and usage
ast-grep --pattern 'impl From<$_> for JsPrimitiveType'
# And check test modules in the interface.rs file
rg "^mod tests" crates/dojo/bindgen/src/plugins/typescript/generator/interface.rs -A 10
Length of output: 7041
Script:
#!/bin/bash
# Let's check the actual test content in interface.rs
cat crates/dojo/bindgen/src/plugins/typescript/generator/interface.rs
# And check for any test files that might test type conversions
rg -l "test.*interface" crates/dojo/bindgen/src/plugins/typescript/generator/
Length of output: 8469
crates/dojo/bindgen/src/plugins/typescript/generator/function.rs (5)
6-6: Ohayo sensei, keep imports consistent!
Line 6 introduces "JsPrimitiveInputType" from the same module. Ensure that existing or corresponding imports remain consistent with the newly added import to avoid confusion.
29-29: Ohayo sensei, re-check function asynchronous behavior.
Previously, it might have been an async wrapper. By removing "async", any downstream code expecting an asynchronous setup call might break. Confirm it's still functioning as intended.
110-115: Ohayo sensei, verify generic type replacements.
Replacing "<" with a prefixed type string could introduce unexpected syntax if the type contains nested generics. Confirm comprehensive test coverage for multi-level generics.
335-341: Ohayo sensei, confirm the custom enum approach.
The test ensures “models.GatedType” is recognized in function inputs. Confirm that other custom enumerations also pass the same code path if they differ structurally.
593-671: Ohayo sensei, thorough coverage for custom enums.
“create_function_with_custom_enum” details an intricate, possibly multi-layered enum. Confirm that all relevant code paths (e.g., flattening, prefix logic) remain correct for these deeply nested styles.
crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs (26)
1-1: Ohayo sensei, new imports for Composite, CompositeInner, etc.
Confirm that the newly introduced imports are indeed used throughout this file. If any remain unused, consider removing them.
15-15: Ohayo sensei, ensure new “models;” mod is purposeful.
Line 15 introduces “mod models;”. Verify that referencing “models” is necessary in this file.
52-61: Ohayo sensei, double-check recursion in token_has_inner_composite.
Excessive recursion or nested checks can degrade performance. Confirm that large or deep composites remain handleable.
69-69: Ohayo sensei, keep struct naming consistent.
Renaming JsType to JsPrimitiveType might break references in other modules. Confirm all references are updated.
76-88: Ohayo sensei, verify fallback to value.to_owned().
When no known CAIRO type is matched, the code defaults to the raw string. Confirm it’s correct for domain-specific unknowns.
93-93: Ohayo sensei, confirm coverage for composite tokens.
This second “From<&Token>” can overshadow or chain with the prior “From<&str>.” Thorough testing ensures correctness for unusual token combos.
113-113: Ohayo sensei, confirm your custom CairoOption logic.
Line 113 returns “CairoOption<…>.” Ensure that any usage aligns with the actual runtime pattern in the consumer code.
118-118: Ohayo sensei, ensure unrolled generics.
Line 118 maps over c.generic_args. If generics get nested, confirm that the existing code recurses correctly.
129-129: Ohayo sensei, fallback to type_name.
When not an option or a custom enum, the fallback is “type_name.” Confirm it’s correct for all possible conditions.
153-165: Ohayo sensei, confirm type coverage.
It repeats the same CAIRO_* constants. Ensure there’s no missing or extra coverage, e.g., i64 or short int types.
169-169: Ohayo sensei, distinct approach for arrays/tuples.
We see parallel logic for arrays & tuples that might differ from the approach in JsPrimitiveType. Confirm both remain aligned.
188-210: Ohayo sensei, custom enum type usage differs from above.
Here we map it to “CairoCustomEnum,” in function inputs, whereas for an interface type we append “Enum.” Confirm that approach remains consistent or intentional.
235-235: Ohayo sensei, extended matching for composites.
Line 235 uses “From<&Composite>” to handle new composite states. Thorough testing ensures coverage for all composite variants.
239-241: Ohayo sensei, ensure consistent custom enum fallback.
The code calls “build_custom_enum_default_value.” Double-check it matches the logic used in function.rs for the same custom enums.
243-248: Ohayo sensei, fallback variant usage.
This portion attempts to read “value.inners[0].token.” Confirm the 0th index is valid for all standard Cairo enums.
259-265: Ohayo sensei, building default enum variant.
Ensure that skipping the rest of the variants (taking only the first in take(1)) is correct. If the user expects a default different from the first variant, code might become misleading.
272-279: Ohayo sensei, composite check for default value.
This block tries to parse inner.token.to_composite(). If it fails, it prints a fallback. Confirm that the fallback aligns with your domain needs.
321-341: Ohayo sensei, ties default array & tuple expansions to composite usage.
We chain expansions again in lines 321-341. Confirm alignment with the prior expansions for arrays & tuples to stay DRY.
360-362: Ohayo sensei, re-check test modules.
We see references to “generate_type_init, JsPrimitiveDefaultValue, JsPrimitiveType.” Confirm that each test covers new scenarios.
370-371: Ohayo sensei, partialeq for default values.
Similarly, lines 370-371 do partialeq for JsPrimitiveDefaultValue. Ensure you handle any future expansions or subfields.
Line range hint 396-405: Ohayo sensei, watch out for multi-element tuples.
Testing only two-element tuples might skip corner cases with more elements.
428-428: Ohayo sensei, “CairoOption” test coverage.
Line 428 ensures we can parse an Option of a composite type. Confirm we also test nested options (e.g., Option<Option>) if feasible.
Line range hint 534-543: Ohayo sensei, arrays tested with 2-element tuples.
Line 534 tests array-of-tuples. Adding a test with single-element or multi-dimensional arrays might highlight edge cases.
591-595: Ohayo sensei, new CairoCustomEnum usage.
We see “new CairoCustomEnum.” Confirm that other custom enum representations remain consistent so the user code doesn’t get confused.
766-797: Ohayo sensei, test coverage for “CairoOptionVariant.None.”
This test confirms default usage of “None.” If your domain typically uses “Some” defaults, watch for mismatches.
850-899: Ohayo sensei, new nested custom enum logic.
We see multi-level nesting in line 873. Confirm we handle multi-level generics or arrays in the same manner.
crates/dojo/bindgen/src/plugins/typescript/generator/models.rs (1)
1-29: 🛠️ Refactor suggestion
Ohayo sensei, validate the new TsModelsGenerator.
This file appears brand new. The approach for inserting “ModelsMapping” is straightforward but watch for:
• Collisions with the same model name in different namespaces
• Repeated insertion in the buffer if run multiple times
• Potential concurrency issues in memory-limited environments
Ensure test coverage for repeated or differently cased type names.
crates/dojo/bindgen/src/plugins/typescript/generator/constants.rs (3)
13-14: Ohayo sensei, check for usage of “Option” constant.
Ensure “CAIRO_OPTION” is consistently applied in function or interface generation. If it remains unused, remove to streamline.
21-23: Ohayo sensei, shift from “import type {...}” to “import {...}.”
This can affect tree-shaking in bundlers. Confirm that the new approach aligns with your bundling strategy.
29-30: Ohayo sensei, rename leftover references to “WITH_FIELD_ORDER_TYPE_DEF.”
We replaced “REMOVE_FIELD_ORDER_TYPE_DEF” with “WITH_FIELD_ORDER_TYPE_DEF.” Confirm we thoroughly updated references in code & docs.
crates/dojo/bindgen/src/plugins/typescript/mod.rs (1)
8-8: Ohayo! Clean architectural change replacing ERC generator with models generator
The replacement of TsErcGenerator with TsModelsGenerator improves code organization by separating model generation from ERC-specific functionality.
Let's verify no remaining ERC-specific code references:
Also applies to: 34-34
✅ Verification successful
Let me verify if there are any remaining ERC-specific code references by searching for the actual TsErcGenerator that was replaced:
Ohayo! Clean architectural change confirmed, no ERC references remain
The replacement of TsErcGenerator with TsModelsGenerator is clean - no remaining references to the old ERC generator exist in the codebase. The only token-related code found is for general Cairo type handling (like Token, CairoOption, etc.) which is expected and unrelated to ERC functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining ERC-related code
rg -i "erc|token" crates/dojo/bindgen/src/plugins/typescript/
Length of output: 41258
Script:
#!/bin/bash
# Search for TsErcGenerator references
rg "TsErcGenerator" crates/dojo/bindgen/src/plugins/typescript/
Length of output: 63
crates/dojo/bindgen/src/plugins/typescript/writer.rs (2)
23-23: Ohayo sensei! Good practice using immutable models
Making models immutable prevents accidental modifications and improves code safety.
47-50: Improved sorting stability using type_path
The change from sorting by tag to type_path provides a more stable and logical ordering, as type paths are unique identifiers.
crates/dojo/bindgen/src/plugins/typescript/generator/enum.rs (1)
6-6: Ohayo! More precise type system with JsPrimitiveType
The switch to JsPrimitiveType and simplified enum type definition with CairoCustomEnum improves type safety and reduces complexity.
Also applies to: 39-39
| /// Checks if Token::Composite is an custom enum (enum with nested Composite types) | ||
| /// * token - The token to check | ||
| pub(crate) fn token_is_custom_enum(token: &Composite) -> bool { | ||
| token.r#type == CompositeType::Enum | ||
| && token.inners.iter().any(|inner| inner.token.to_composite().is_ok()) | ||
| token.r#type == CompositeType::Enum && token_has_inner_composite(token) |
There was a problem hiding this comment.
Ohayo sensei, custom enum logic pinned.
Ensuring “token_is_custom_enum(token)” is robust is crucial. For example, a mistaken false negative might hamper enum handling or produce incorrectly typed code.
| Token::Array(a) => JsPrimitiveType::from( | ||
| format!("Array<{}>", JsPrimitiveType::from(&*a.inner)).as_str(), | ||
| ), | ||
| Token::Tuple(t) => JsPrimitiveType::from( | ||
| format!( | ||
| "[{}]", | ||
| t.inners | ||
| .iter() | ||
| .map(|i| JsType::from(i.type_name().as_str()).to_string()) | ||
| .map(|i| JsPrimitiveType::from(i.type_name().as_str()).to_string()) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ohayo sensei, array handling might become tricky.
Line 96 introduces array logic. Multi-dimensional arrays or arrays of tuples might require deeper checks.
| fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
| f.write_str(&self.0) | ||
| } | ||
| } | ||
|
|
||
| /// Type used to map cainome `Token` into javascript types in function definition |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ohayo sensei, new type for function definition.
“JsPrimitiveInputType” wraps a String. Confirm that it shares enough logic with “JsPrimitiveType” or if duplication can be avoided.
| #[derive(Debug)] | ||
| pub(crate) struct JsPrimitiveDefaultValue(String); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ohayo sensei, new struct for default values.
Introduce “JsPrimitiveDefaultValue.” This looks similar to “JsPrimitiveType.” Might unify them to reduce duplicative logic.
| cainome::parser::tokens::CompositeType::Struct => { | ||
| JsPrimitiveDefaultValue(build_struct_default_value(value)) | ||
| } | ||
| _ => JsPrimitiveDefaultValue::from(value.type_name().as_str()), | ||
| } |
There was a problem hiding this comment.
Ohayo sensei, new struct default approach for enumerations.
Mapping “CompositeType::Struct” to “build_struct_default_value.” Confirm no conflict arises if it’s actually an unrecognized enum.
| @@ -10,24 +10,21 @@ pub const CAIRO_U256: &str = "u256"; | |||
| pub const CAIRO_U256_STRUCT: &str = "U256"; | |||
| pub const CAIRO_I128: &str = "i128"; | |||
| pub const CAIRO_BOOL: &str = "bool"; | |||
| pub const CAIRO_OPTION: &str = "Option"; | |||
|
|
|||
| pub(crate) const CAIRO_OPTION_DEFAULT_VALUE: &str = "new CairoOption(CairoOptionVariant.None)"; | |||
There was a problem hiding this comment.
Ohayo sensei, new default “new CairoOption(CairoOptionVariant.None).”
If your code typically expects “Some” as a default, confirm that “None” doesn’t break the user’s flow.
Summary by CodeRabbit
insert_at_indexmethod from the Buffer struct, simplifying string management.