Remove deleter attribute from pygetset
#5984
Conversation
WalkthroughThe changes remove all support for "deleter" functionality from the Changes
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
🧹 Nitpick comments (1)
derive-impl/src/pyclass.rs (1)
1074-1082: Review the unwrap() call safety in validationThe validation logic correctly iterates over getter and setter pairs. However, line 1077 uses
unwrap()on the setter without explicit safety documentation.While this should be safe based on the invariant that map entries are only created when either getter or setter is set, consider adding a comment to document this assumption:
if getter.is_none() { errors.push(err_span!( + // Safety: If getter is None, setter must be Some due to add_item invariant setter.as_ref().unwrap(), "GetSet '{}' is missing a getter", name )); };
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
derive-impl/src/pyclass.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .github/copilot-instructions.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Run rust tests (windows-latest)
🔇 Additional comments (4)
derive-impl/src/pyclass.rs (4)
1035-1035: LGTM - Correctly removes deleter from map structureThe change from a 3-tuple to a 2-tuple properly removes the deleter identifier storage, maintaining only getter and setter functionality.
1092-1109: LGTM - Token generation correctly updated for getter/setter onlyThe ToTokens implementation has been properly updated to generate PyGetSet objects with only getter and optional setter functionality, removing all deleter-related code generation.
1234-1234: LGTM - Correctly removes "deleter" from allowed attribute namesThe ALLOWED_NAMES constant has been properly updated to only include "name" and "setter", removing "deleter" support from the GetSetItemMeta parsing.
1248-1252: LGTM - Parsing logic correctly simplifiedThe getset_name method has been properly updated to only distinguish between getter and setter kinds, removing all deleter-related parsing logic. The boolean check for "setter" attribute is clean and straightforward.
Ref
Summary by CodeRabbit
New Features
Refactor
Documentation