feat: dispatcher_from_tag! macro #2416
Conversation
WalkthroughOhayo, sensei! The changes in this pull request enhance the handling of Dojo interfaces, introduce new traits management, and implement a dispatcher macro for improved contract interactions. Key modifications include updates to function signatures, the addition of new structs for traits and interfaces, and the introduction of a macro for generating dispatcher code based on tags. The overall structure of the codebase is refined to support clearer contract definitions and better manage auxiliary data related to contracts and interfaces. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dispatcher
participant Contract
participant Interface
User->>Dispatcher: Invoke dispatcher_from_tag(tag, address)
Dispatcher->>Contract: Retrieve contract using tag
Contract->>Interface: Get interface path
Interface-->>Dispatcher: Return interface path
Dispatcher-->>User: Return dispatcher instance
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: 0
Outside diff range and nitpick comments (1)
crates/dojo-lang/src/inline_macros/dispatcher_from_tag.rs (1)
62-74: Ohayo sensei! Consider enhancing the error message.The code segment correctly handles the case when the interface path is not found. However, to provide more clarity to the user, consider including the reason for the failure in the error message.
You can modify the error message as follows:
-message: format!("Failed to find the interface path of `{tag}`"), +message: format!("Failed to find the interface path of `{tag}`: {}", e),This way, the user will have more information about why the interface path couldn't be found.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (15)
- crates/dojo-lang/src/compiler.rs (7 hunks)
- crates/dojo-lang/src/contract.rs (4 hunks)
- crates/dojo-lang/src/inline_macros/dispatcher_from_tag.rs (1 hunks)
- crates/dojo-lang/src/inline_macros/mod.rs (1 hunks)
- crates/dojo-lang/src/inline_macros/utils.rs (1 hunks)
- crates/dojo-lang/src/interface.rs (2 hunks)
- crates/dojo-lang/src/plugin.rs (4 hunks)
- crates/dojo-lang/src/semantics/test_data/dispatcher_from_tag (1 hunks)
- crates/dojo-lang/src/semantics/tests.rs (1 hunks)
- crates/dojo-world/src/manifest/types.rs (1 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-dungeon-6620e0e6.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-others-61de2c18.toml (1 hunks)
- examples/spawn-and-move/src/actions.cairo (1 hunks)
Files skipped from review due to trivial changes (2)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-others-61de2c18.toml
Additional comments not posted (28)
examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-dungeon-6620e0e6.toml (1)
11-11: Ohayo sensei! The newinterface_pathparameter looks great! 👍Adding the
interface_pathto the manifest file is a fantastic way to establish a clear linkage between thedojo_examples-dungeonproject and its corresponding interface. This change will enhance the modularity and clarity of the codebase by explicitly defining the interface used for dungeon-related functionalities.By specifying the path to the
IDungeoninterface, you've made it easier for other parts of the system to interact with the dungeon contract using a well-defined interface. This will facilitate better integration and usage of the dungeon system throughout the project.Keep up the great work, sensei! 💪
examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml (1)
21-21: Ohayo sensei! The newinterface_pathentry looks great!The addition of the
interface_pathentry is a nice enhancement to the module's structure. It explicitly links the module to an interface that defines the available actions, which can lead to better organization, clarity, and potentially enforce a contract for the actions that can be performed.The change improves the module's interface definition without altering its existing logic or functionality, as evident from the unchanged
systemsarray. Great work, sensei!crates/dojo-lang/src/semantics/test_data/dispatcher_from_tag (3)
1-21: Ohayo sensei! The "no param" test case looks good.The test case correctly verifies the error scenario when the
dispatcher_from_tag!macro is invoked without any arguments. The expected output and semantic diagnostics match, indicating that the macro handles this case as intended.
24-44: Ohayo sensei! The "missing address" test case is on point.This test case effectively checks the error handling when the
dispatcher_from_tag!macro is called with only the tag argument. The macro correctly identifies the missing address argument and generates the appropriate error message in the semantic diagnostics, which aligns with the expected output.
47-66: Ohayo sensei! The "ok but expected to fail" test case is well-crafted.This test case demonstrates the scenario where the
dispatcher_from_tag!macro is provided with both the tag and address arguments. However, it is designed to fail intentionally due to the absence of thedojo_manifests_dir. The macro correctly detects the missing interface path and generates the appropriate error message in the semantic diagnostics, which matches the expected output.crates/dojo-lang/src/semantics/tests.rs (1)
22-23: Ohayo sensei! The new test case looks great! 👍Adding the
dispatcher_from_tagtest case aligns perfectly with the PR objective of introducing thedispatcher_from_tag!macro. This will help validate the dispatcher's behavior and improve the overall test coverage. Keep up the good work!crates/dojo-lang/src/inline_macros/mod.rs (1)
8-8: Ohayo, sensei! The new module looks good to me.The addition of the
dispatcher_from_tagmodule aligns perfectly with the PR objective. The public visibility is appropriate, and the code structure remains consistent. Keep up the great work!crates/dojo-lang/src/inline_macros/dispatcher_from_tag.rs (3)
1-11: Ohayo sensei! The imports are spot on.The code segment correctly imports the necessary dependencies for implementing the
dispatcher_from_tagmacro. Good job!
13-93: Ohayo sensei! Thedispatcher_from_tagmacro implementation is top-notch.The macro correctly handles the following:
- Validating the input arguments.
- Checking the tag format.
- Finding the corresponding interface path.
- Generating the dispatcher code using the interface path and contract address.
The error handling and diagnostics generation is also appropriate. Great work!
76-91: Ohayo sensei! The dispatcher code generation is flawless.The code segment correctly builds the dispatcher code using the
PatchBuilderwith the appropriate format. The generated code and the empty list of diagnostics are returned as expected. Well done!crates/dojo-lang/src/inline_macros/utils.rs (1)
36-50: Ohayo sensei! Thefind_interface_pathfunction looks great!The function correctly retrieves the base manifest, searches for the contract with the matching tag, and returns the corresponding interface path. The error handling for the case when no matching contract is found is also properly implemented.
crates/dojo-lang/src/interface.rs (2)
3-4: Ohayo, sensei! The imports look good to me.The imports of
DynGeneratedFileAuxDataandPluginGeneratedFilefromcairo_lang_defs::pluginare necessary for the changes made in the file. The imports are correctly specified.
89-94: Ohayo, sensei! The addition ofaux_datafield toPluginGeneratedFilestruct is a great improvement.The change enhances the functionality of the
PluginGeneratedFileby providing additional context and data related to the generated file. Theaux_datafield is correctly initialized withDynGeneratedFileAuxDatastruct andDojoAuxDatastruct. Theinterfacesfield is also correctly initialized withInterfaceAuxDatastruct containing thenamefield.This change suggests a more structured approach to managing the generated files and their associated metadata, potentially improving the overall robustness and extensibility of the plugin system. Great work, sensei!
crates/dojo-world/src/manifest/types.rs (1)
110-110: Ohayo sensei! The newinterface_pathfield looks good, but how does it help with thedispatcher_from_tag!macro?The addition of the
interface_pathfield to theDojoContractstruct is a nice enhancement to the contract's ability to manage interfaces. It aligns with the PR objective of improving the handling of Dojo interfaces.However, it's not clear how this field directly relates to the
dispatcher_from_tag!macro and addresses the challenges mentioned in the PR objectives, such as the macro's need to access the project base manifest during compilation.Could you please provide more context on the connection between the
interface_pathfield and thedispatcher_from_tag!macro? It would be helpful to understand how this change contributes to the macro's implementation.examples/spawn-and-move/src/actions.cairo (1)
169-170: Ohayo sensei! Thedispatcher_from_tagfunction is a good workaround for thedispatcher_from_tag!macro.I noticed that the intended
dispatcher_from_tag!macro could not be implemented due to challenges with accessing the project base manifest at compile time, as mentioned in the PR objectives. Using thedispatcher_from_tagfunction instead is a reasonable workaround that still achieves the goal of abstracting the dispatcher creation logic and avoiding the need for auseclause for the contract dispatcher. The core functionality of entering the dungeon remains intact. Nice work!crates/dojo-lang/src/plugin.rs (5)
58-62: Sugoi desu ne, sensei! The newTraitstruct looks perfect.The
Traitstruct withnameandpathfields is a great addition for better representation of traits within the system. This aligns perfectly with the PR objective of enhancing traits management.
70-70: Yatta! Thetraitsfield inContractAuxDatais a fantastic enhancement, sensei.Adding the
traitsfield of typeVec<Trait>to theContractAuxDatastruct is an excellent way to expand the auxiliary data associated with contracts to encompass traits. This modification aligns perfectly with the PR objective of enhancing the overall capability of the plugin to manage and represent traits alongside contracts.
73-75: Subarashii! The newInterfaceAuxDatastruct is a brilliant addition, sensei.Introducing the
InterfaceAuxDatastruct with anamefield is an excellent way to signify a new layer of abstraction for interfaces processed by the plugin. This addition aligns perfectly with the PR objective of enhancing the overall capability of the plugin to manage and represent interfaces alongside contracts.
87-88: Kanpeki desu! Theinterfacesfield inDojoAuxDatais a splendid enhancement, sensei.Adding the
interfacesfield of typeVec<InterfaceAuxData>to theDojoAuxDatastruct is an excellent way to allow the plugin to process and manage a list of interfaces. This modification aligns perfectly with the PR objective of enhancing the overall capability of the plugin to handle interfaces alongside contracts and events.
180-180: Ohayo gozaimasu, sensei! The addition ofDispatcherFromTagMacroto the plugin suite is a crucial step forward.Extending the
dojo_plugin_suitefunction to include theDispatcherFromTagMacroinline macro plugin is essential for supporting the newdispatcher_from_tag!macro, which is the primary objective of this PR. Great work on this addition!However, the PR description mentions a significant challenge with implementing this macro due to the lack of access to the project base manifest at compile time. Could you please verify if the current implementation of the
DispatcherFromTagMacroaddresses this limitation effectively? If not, it would be helpful to understand the current limitations and any potential workarounds or future enhancements planned to overcome this challenge.To assist with the verification, you can use the following script to search for the
DispatcherFromTagMacroimplementation and analyze how it handles the project base manifest:Verification successful
Ohayo, sensei! The
DispatcherFromTagMacroimplementation addresses the project base manifest challenge by using configuration metadata.The
generate_codefunction inDispatcherFromTagMacroattempts to find the interface path usingfind_interface_pathand handles errors gracefully by providing diagnostics. This approach suggests a workaround for the compile-time limitation mentioned in the PR description.
- The function validates arguments and tags, and uses
metadata.cfg_setto find the interface path.This implementation seems to effectively address the limitation, but further testing or documentation might be needed to ensure robustness in various scenarios.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `DispatcherFromTagMacro` and its handling of the project base manifest. # Test: Search for the `DispatcherFromTagMacro` implementation. rg --type rust -A 10 $'DispatcherFromTagMacro'Length of output: 3224
Script:
#!/bin/bash # Description: Extract the full implementation of the `generate_code` function in `DispatcherFromTagMacro`. # Search for the `generate_code` function implementation within `DispatcherFromTagMacro`. ast-grep --lang rust --pattern $'impl InlineMacroExprPlugin for DispatcherFromTagMacro { fn generate_code( &self, db: &dyn cairo_lang_syntax::node::db::SyntaxGroup, syntax: &ast::ExprInlineMacro, metadata: &MacroPluginMetadata, ) -> InlinePluginResult { $$$ } }'Length of output: 289
Script:
#!/bin/bash # Description: Use `rg` to locate the `generate_code` function within `DispatcherFromTagMacro`. # Search for the `generate_code` function in the `dispatcher_from_tag.rs` file. rg --type rust -A 20 'fn generate_code' crates/dojo-lang/src/inline_macros/dispatcher_from_tag.rsLength of output: 959
Script:
#!/bin/bash # Description: Extend the search to capture more lines of the `generate_code` function. # Search for the `generate_code` function in the `dispatcher_from_tag.rs` file with more context. rg --type rust -A 50 'fn generate_code' crates/dojo-lang/src/inline_macros/dispatcher_from_tag.rsLength of output: 2238
crates/dojo-lang/src/compiler.rs (6)
43-43: LGTM!The import statement is correct and necessary for the subsequent code changes.
Line range hint
353-398: Ohayo sensei! The code changes look good.The initialization of the
dojo_interfacesvector and the extension ofcrate_idswithexternal_crate_idsare correct and align with the AI-generated summary. Thedojo_interfacesvector will be used in subsequent code changes to validate contract implementations.
430-460: Ohayo sensei! The code changes are spot on.The validation and update logic for the contract's manifest based on the implemented Dojo interfaces is correct and aligns with the AI-generated summary. The code ensures that contracts can only implement a single Dojo interface and handles the case when the interface path starts with "super".
596-597: LGTM sensei!The modification to the function signature of
get_dojo_contract_artifactsto include the newtraitsparameter is correct and aligns with the list of alterations provided. Thetraitsparameter will be used in subsequent code changes to validate contract implementations.
623-623: Looks good sensei!The update to the return value of
get_dojo_contract_artifactsto include thetraitsparameter is correct and aligns with the modification made to the function signature.
382-382: Ohayo sensei! The code change is spot on.Passing the
&contract.traitsparameter to theget_dojo_contract_artifactsfunction call is correct and aligns with the modifications made to the function signature and return value.crates/dojo-lang/src/contract.rs (2)
739-795: Ohayo sensei! Theget_traitsfunction looks great! 🎉It correctly extracts the relevant traits from the module body and handles different cases for reconstructing the trait path. The unsupported relative path case is also clearly documented.
797-815: Ohayo sensei! Theget_trait_pathhelper function is implemented correctly. 👍It properly searches for the trait path in the module body's
usestatements and returns the full path if found. The logic is clear and it correctly handles the case where the path is not found.
@remybar does it mean this feature cannot be implemented and so been canceled? |
Hi ! The compilation process is being reworked so I will check if it becomes possible in the new version, but I think it won't 🫤 |
The idea of this PR was to provide a
dispatcher_from_tag!macro, allowing the developper to get a dispatcher from a contract tag and an address, without anyuseclause for the contract dispatcher.For example:
Unfortunately, this macro needs to access to the project base manifest which is built at compile time, meaning that it is not accessible by the macro during compilation.
Summary by CodeRabbit
Release Notes
New Features
get_traitsfunction for extracting traits from contract modules.DojoContractandPluginGeneratedFilestructures to include interface paths and traits.Bug Fixes
Tests
Chores