-
Notifications
You must be signed in to change notification settings - Fork 2
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
Allow to declare a reference as a constructor arg #387
Conversation
WalkthroughThis update refines the handling of argument types within a mapping operation in the codebase. It introduces a utility function dedicated to managing unreferenced types, streamlining the process and enhancing code efficiency and readability. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- odra-macros/src/ast/deployer_item.rs (1 hunks)
Additional comments (1)
odra-macros/src/ast/deployer_item.rs (1)
- 94-94: The modification to use
utils::ty::unreferenced_ty
for handling argument types in theInitArgsItem
struct is a crucial improvement. This change effectively addresses the issue of missing lifetime errors by ensuring that references are not included in theInitArgs
structs. By doing so, it aligns with Rust's ownership and borrowing rules, making the generated code more robust and compliant.However, it's important to ensure that this change does not inadvertently affect other parts of the codebase where references might be intentionally used or required. Additionally, considering the complexity of Rust's lifetime and borrowing rules, thorough testing is recommended to verify that the generated code behaves as expected in various scenarios.
Verification successful
The usage of
utils::ty::unreferenced_ty
across the codebase for handling argument types in AST manipulation is consistent and aligns with the intended improvement mentioned in the review. The function's application in various parts of the codebase supports the notion that this approach is beneficial for ensuring that references are not included in structs likeInitArgsItem
, without introducing unintended side effects. This consistency in usage across different AST manipulation contexts verifies the original review comment.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the unreferenced_ty function is used consistently across the codebase for similar purposes. rg --type rust 'utils::ty::unreferenced_ty' -C 3 # Ensure that there are no unintended side effects in other parts of the codebase due to this change. # This script is a placeholder for more specific tests that should be conducted. echo "Placeholder for specific tests to verify the behavior of the generated code."Length of output: 2200
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release/0.8.2 #387 +/- ##
================================================
Coverage ? 84.98%
================================================
Files ? 126
Lines ? 13740
Branches ? 0
================================================
Hits ? 11677
Misses ? 2063
Partials ? 0 ☔ View full report in Codecov by Sentry. |
Before if there was a reference in the constructor, the generated code was:
resulting a missing lifetime error.
InitArgs
should not have references as fields.Summary by CodeRabbit