Skip to content

Cleanup some direct magic method definitions#7441

Merged
youknowone merged 5 commits intoRustPython:mainfrom
moreal:cleanup-legacy-magic-method-definitions
Mar 16, 2026
Merged

Cleanup some direct magic method definitions#7441
youknowone merged 5 commits intoRustPython:mainfrom
moreal:cleanup-legacy-magic-method-definitions

Conversation

@moreal
Copy link
Contributor

@moreal moreal commented Mar 16, 2026 •
edited by coderabbitai bot
Loading

This pull request enables some guards that restricts to implement slot that can be implemented through slot traits, directly.

Summary by CodeRabbit

  • Bug Fixes

    • Socket objects now emit resource warnings when not properly closed.

  • Changes

    • Asyncio futures no longer support iteration; await them instead.
    • String objects no longer expose len method.
    • SSL context and socket representations updated.

@moreal moreal self-assigned this Mar 16, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 16, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 3e61fd88-24ca-47b5-91a8-5230a7cd487a

📥 Commits

Reviewing files that changed from the base of the PR and between f27490c and 8791e15.

📒 Files selected for processing (5)
  • crates/derive-impl/src/pyclass.rs
  • crates/stdlib/src/_asyncio.rs
  • crates/stdlib/src/socket.rs
  • crates/stdlib/src/ssl.rs
  • crates/vm/src/builtins/str.rs
💤 Files with no reviewable changes (2)
  • crates/vm/src/builtins/str.rs
  • crates/stdlib/src/_asyncio.rs
📝 Walkthrough

Walkthrough

This PR refactors special method implementations across multiple modules to use trait-based mechanisms instead of inline implementations. Changes include expanding forbidden slot-method enforcement, migrating __repr__ and __del__ implementations to use Representable and Destructor traits, removing __iter__ from async types, and removing __len__ from PyStr. The modifications align special method handling with a unified trait-based pattern.

Changes

Cohort / File(s) Summary
Trait-based refactoring
crates/stdlib/src/ssl.rs, crates/stdlib/src/socket.rs		 Migrate from inline special method implementations to trait-based alternatives. PySSLContext and PySSLSocket add Representable trait, replacing manual __repr__ methods. PySocket adds Destructor trait, replacing inline __del__ with formal trait implementation.
Slot method enforcement
crates/derive-impl/src/pyclass.rs		 Expand forbidden slot-method list for special methods including __repr__, __len__, __contains__, item/descriptor/iterable methods. Reclassify methods under trait-specific constraints (Representable, AsSequence, GetDescriptor, etc.).
Special method removals
crates/stdlib/src/_asyncio.rs, crates/vm/src/builtins/str.rs		 Remove __iter__ methods from async types and remove __len__ pymethod from PyStr.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • youknowone

Poem

🐰 A rabbit's ode to trait-ful refactoring:

Methods once scattered, now consolidated,
__repr__ and __del__ delegated—
Traits take the stage with grace,
Forbidden slots enforce their place,
Code finds harmony at last! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 73.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly aligns with the PR's main objective: cleaning up legacy direct magic method definitions by consolidating them into trait implementations and enabling compile-time guards.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@moreal @claude
Migrate direct __repr__ definitions to Representable trait
8ef8c6f 
Move legacy #[pymethod] __repr__ to impl Representable for:
- PySSLContext, PySSLSocket (ssl.rs)
- BufferedReader, BufferedWriter, BufferedRandom (_io.rs)

Enable __repr__ guard in derive to prevent future direct definitions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@moreal moreal force-pushed the cleanup-legacy-magic-method-definitions branch from bc8dc5d to 5ea0b4d Compare March 16, 2026 09:15
moreal and others added 4 commits March 16, 2026 18:27
@moreal @claude
Migrate direct __del__ definition to Destructor trait for PySocket
4bbd54c 
Move #[pymethod] __del__ to impl Destructor for PySocket.
Preserves ResourceWarning emission and close behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@moreal @claude
Remove redundant __iter__ pymethods from PyFuture and PyTask
ecf9e21 
Both types already have impl Iterable, so the direct #[pymethod]
__iter__ definitions were duplicates. Enable __iter__/__next__
guards in derive to prevent future direct definitions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@moreal @claude
Enable derive guards for __get__/__set__/__delete__ descriptor methods
20ddf9c 
All concrete types already use GetDescriptor/SetDescriptor traits.
Activate the compile-time guard to prevent future direct definitions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@moreal @claude
Enable derive guards for AsMapping/AsSequence slot methods
8791e15 
Remove redundant #[pymethod(name = "__len__")] from PyStr
(already provided via AsMapping/AsSequence trait impls).

Enable compile-time guards for __len__, __contains__, __getitem__,
__setitem__, __delitem__ to prevent future direct definitions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@moreal moreal force-pushed the cleanup-legacy-magic-method-definitions branch from 5ea0b4d to 8791e15 Compare March 16, 2026 09:27
@moreal moreal changed the title Cleanup some legacy direct magic method definitions Cleanup some direct magic method definitions Mar 16, 2026
@moreal moreal marked this pull request as ready for review March 16, 2026 09:49
youknowone
youknowone approved these changes Mar 16, 2026
View reviewed changes
Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@youknowone youknowone merged commit 5f1d5d2 into RustPython:main Mar 16, 2026
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Assignees

@moreal moreal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@moreal @youknowone