General code nits by ShaharNaveh · Pull Request #8041 · RustPython/RustPython

ShaharNaveh · 2026-06-04T08:27:03Z

Closes #xxxx
This PR follows our AI policy

Summary

Summary by CodeRabbit

Refactor
- Improved error handling construction patterns and reduced string allocations across standard library modules for better performance.
- Refactored control flow logic in decomposition, iterator, and threading implementations for enhanced code maintainability.
Optimization
- Performance improvements achieved through reduced memory allocations in standard library operations.

coderabbitai · 2026-06-04T08:27:11Z

Need the big picture first? Review this PR in Change Stack to see what changed before going file by file.

📝 Walkthrough

Walkthrough

This PR systematically refactors error message handling and code patterns across eleven stdlib modules to reduce string allocations and improve code style. The largest change updates the _io.rs error helper signature and all call sites; complementary changes in binascii.rs generalize error construction, and builtins.rs applies multiple control-flow simplifications. Smaller modules receive consistency updates and style cleanups.

Changes

Error handling and code pattern refactoring

Layer / File(s)	Summary
IO unsupported operation error signature refactoring `crates/vm/src/stdlib/_io.rs`	`new_unsupported_operation` signature changes from `(vm, msg: String)` to `(msg: T: Into<String>, vm)`, with 16+ call sites across `_IOBase`, `TextIOWrapper`, `FileIO`, and Windows console implementations updated to pass literals and format results directly.
Binascii error handler enhancement and const promotions `crates/stdlib/src/binascii.rs`	`new_binascii_error` accepts generic `Into<Wtf8Buf>` messages instead of requiring `String` allocation. Error paths in `unhexlify`, `uu_a2b_read`, `a2b_uu`, and `b2a_uu` pass literals directly. `hex_nibble` promoted to `const fn`.
Builtins compilation and encoding detection refactoring `crates/vm/src/stdlib/builtins.rs`	`detect_source_encoding` refactored with `matches!` patterns for PEP 263 parsing. `compile()` removes `.to_owned()` from predefined error constants. `feature_version_from_arg` uses early returns and `u8::try_from`. `ScopeArgs::make_scope` extracts globals validation into a dedicated helper.
Thread module consolidation and error message refactoring `crates/vm/src/stdlib/_thread.rs`	`current_thread_id()` consolidated with single `cfg_select!` macro for Unix/non-Unix selection. Thread argument validation uses `given`-based formatted messages. `start_joinable_thread` constructs optional error suggestions before interpolating into a single message.
Control flow simplification and code cleanup across remaining modules `crates/vm/src/stdlib/_weakref.rs`, `crates/vm/src/stdlib/itertools.rs`, `crates/stdlib/src/unicodedata.rs`, `crates/vm/src/builtins/list.rs`, `crates/vm/src/builtins/dict.rs`, `crates/vm/src/stdlib/_collections.rs`, `crates/vm/src/stdlib/typevar.rs`	`_weakref` adds blank lines between functions and refactors `getweakrefs` to `map_or_else`. `itertools` simplifies post-loop handling with `if matches!(...)`. `unicodedata` refactors `Ucd.decomposition` to early-return pattern and promotes `decomposition_type_tag` to `const fn`. `list`, `dict`, and `_collections` adjust whitespace. `typevar` removes `#[allow(dead_code)]` attributes and simplifies type inference.

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

youknowone

🐰 A rabbit's refactoring feast

From .to_owned() chains now freed,
Error helpers generified with speed,
Early returns and matches! bright,
Control flow flows just right—
Cleanup hops through every crate tonight! 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'General code nits' is vague and generic, using non-descriptive terminology that fails to convey meaningful information about the substantial refactoring across 11 files.	Use a more specific title that highlights the primary refactoring pattern (e.g., 'Refactor error construction helpers to reduce allocations' or 'Simplify control flow with Rust idioms').

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/vm/src/stdlib/_io.rs`:
- Around line 366-367: Update the UnsupportedOperation error messages to match
CPython exactly by adding the trailing period to the strings "File or stream is
not readable" and "File or stream is not writable" wherever they are raised in
crates/vm/src/stdlib/_io.rs (the UnsupportedOperation paths around the locations
using those exact texts). Search for the two literal strings and modify them to
"File or stream is not readable." and "File or stream is not writable." so
functions/branches that construct or raise UnsupportedOperation use the exact
CPython wording.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: ab7e2efc-03e4-4d74-ac52-98c55b448dac

📥 Commits

Reviewing files that changed from the base of the PR and between 51c97b9 and 9d02178.

📒 Files selected for processing (11)

crates/stdlib/src/binascii.rs
crates/stdlib/src/unicodedata.rs
crates/vm/src/builtins/dict.rs
crates/vm/src/builtins/list.rs
crates/vm/src/stdlib/_collections.rs
crates/vm/src/stdlib/_io.rs
crates/vm/src/stdlib/_thread.rs
crates/vm/src/stdlib/_weakref.rs
crates/vm/src/stdlib/builtins.rs
crates/vm/src/stdlib/itertools.rs
crates/vm/src/stdlib/typevar.rs

coderabbitai · 2026-06-06T07:26:12Z

+                "File or stream is not readable",
                vm,


⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preserve exact CPython-readable/writable error text (trailing period).

Line 366 and Lines 5588/5646/5681 use messages without the trailing . in some readable/writable UnsupportedOperation paths. That diverges from CPython’s _pyio text ("File or stream is not readable." / "File or stream is not writable.") and can break strict behavior parity tests.

Suggested patch

- Err(new_unsupported_operation( - "File or stream is not readable", - vm, - )) + Err(new_unsupported_operation( + "File or stream is not readable.", + vm, + ))

- return Err(new_unsupported_operation( - "File or stream is not readable", - vm, - )); + return Err(new_unsupported_operation( + "File or stream is not readable.", + vm, + ));

- return Err(new_unsupported_operation( - "File or stream is not readable", - vm, - )); + return Err(new_unsupported_operation( + "File or stream is not readable.", + vm, + ));

- return Err(new_unsupported_operation( - "File or stream is not writable", - vm, - )); + return Err(new_unsupported_operation( + "File or stream is not writable.", + vm, + ));

Also applies to: 5587-5590, 5645-5648, 5680-5683

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/vm/src/stdlib/_io.rs` around lines 366 - 367, Update the UnsupportedOperation error messages to match CPython exactly by adding the trailing period to the strings "File or stream is not readable" and "File or stream is not writable" wherever they are raised in crates/vm/src/stdlib/_io.rs (the UnsupportedOperation paths around the locations using those exact texts). Search for the two literal strings and modify them to "File or stream is not readable." and "File or stream is not writable." so functions/branches that construct or raise UnsupportedOperation use the exact CPython wording.

ShaharNaveh added 2 commits June 4, 2026 11:22

Some nits

b125b8f

more

2e26336

ShaharNaveh added 4 commits June 5, 2026 09:53

Fix

acd6699

fix

5bcda0b

simplify

9c2974e

Fix logic error

9d02178

ShaharNaveh marked this pull request as ready for review June 6, 2026 07:20

coderabbitai Bot reviewed Jun 6, 2026

View reviewed changes

youknowone approved these changes Jun 10, 2026

View reviewed changes

youknowone merged commit c13db75 into RustPython:main Jun 10, 2026
26 checks passed

coderabbitai Bot mentioned this pull request Jun 11, 2026

Remove redundant to_owned() calls #8077

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

General code nits#8041

General code nits#8041
youknowone merged 6 commits into
RustPython:mainfrom
ShaharNaveh:nits-3

ShaharNaveh commented Jun 4, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 4, 2026 •

edited

Loading

Walkthrough

Changes

Suggested reviewers

❌ Failed checks (1 inconclusive)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Jun 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ShaharNaveh commented Jun 4, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Suggested reviewers

❌ Failed checks (1 inconclusive)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jun 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ShaharNaveh commented Jun 4, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 4, 2026 •

edited

Loading