Remove some `#[allow(clippy::*)]` by ShaharNaveh · Pull Request #7878 · RustPython/RustPython

ShaharNaveh · 2026-05-16T09:56:31Z

Summary by CodeRabbit

Chores
- Updated internal lint annotations to be more explicit and informative.
- Removed several blanket lint suppressions across the codebase.
- Added Clone and Copy derives to an internal size type for safer value handling.
- Minor internal AST and I/O constructor adjustments for clarity and maintainability.

coderabbitai · 2026-05-16T09:56:43Z

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: e301313b-3c84-4806-acad-6b9f5e46ed2f

📥 Commits

Reviewing files that changed from the base of the PR and between d655415 and 18acc26.

📒 Files selected for processing (3)

crates/vm/src/stdlib/_io.rs
crates/vm/src/stdlib/_winapi.rs
crates/vm/src/stdlib/os.rs

✅ Files skipped from review due to trivial changes (1)

crates/vm/src/stdlib/os.rs

🚧 Files skipped from review as they are similar to previous changes (2)

crates/vm/src/stdlib/_winapi.rs
crates/vm/src/stdlib/_io.rs

📝 Walkthrough

Walkthrough

The PR standardizes Clippy annotations (converting many allow forms to expect or adding reasons), removes obsolete lint suppressions, and applies small functional fixes: mapping-pattern overflow validation, renaming/using AST conversion parameters, and adding Clone/Copy to OptionalSize.

Changes

Clippy Lint Handling Modernization

Layer / File(s)	Summary
Functional changes (mapping-pattern, AST, OptionalSize) `crates/codegen/src/compile.rs`, `crates/vm/src/stdlib/_ast/statement.rs`, `crates/vm/src/stdlib/_io.rs`	Mapping-pattern sub-pattern count validated via `u32::try_from(...)` with `SyntaxError` on overflow; `ast::Stmt::ast_from_object` uses `vm`/`object` names and `object.repr(vm)` in errors; `OptionalSize` now `#[derive(Clone, Copy, FromArgs)]`.
Allow → expect lint annotation standardization `crates/codegen/src/compile.rs`, `crates/vm/src/lib.rs`, `crates/vm/src/builtins/module.rs`, `crates/vm/src/sliceable.rs`, `crates/vm/src/stdlib/_winapi.rs`, `crates/vm/src/stdlib/_io.rs`	Replaces several `#[allow(...)]` forms with `#[expect(..., reason = "...")]` or adds `reason` strings on crate-level allows for lints like `too_many_arguments`, `module_inception`, `upper_case_acronyms`, `new_without_default`, `len_without_is_empty`, and `type_complexity`.
Removed unnecessary lint suppressions `crates/vm/src/builtins/dict.rs`, `crates/vm/src/builtins/list.rs`, `crates/stdlib/src/json/machinery.rs`, `crates/vm/src/builtins/module.rs`, `crates/vm/src/stdlib/os.rs`	Removes `redundant_closure_call`, `len_without_is_empty`, `unusual_byte_groupings`, `wrong_self_convention`, `unnecessary_cast`, and similar `#[allow(...)]` attributes where they are no longer present or needed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

youknowone
fanninpm
coolreader18

Poem

🐰 Clippy notes on my ear,
Expect, not silence, now appear,
Reasons tidy, warnings shown,
One overflow no longer grown,
I hop away — the code looks clear!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title directly and accurately describes the primary change: removing various `#[allow(clippy::*)]` attributes throughout multiple files in the codebase.
Docstring Coverage	✅ Passed	Docstring coverage is 80.95% 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

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

Generate code and open pull requests
Plan features and break down work
Investigate incidents and troubleshoot customer tickets together
Automate recurring tasks and respond to alerts with triggers
Summarize progress and report instantly

Built for teams:

Shared memory across your entire org—no repeating context
Per-thread sandboxes to safely plan and execute work
Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

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

🧹 Nitpick comments (1)

crates/codegen/src/compile.rs (1)
7085-7086: ⚡ Quick win

Update the overflow comment to match the new boundary check.

Line 7085 still describes an INT_MAX-based guard, but Line 7086 now validates via u32::try_from(size). Please rewrite the comment to reflect the actual check so the boundary contract is unambiguous.

As per coding guidelines, "Do not delete or rewrite existing comments unless they are factually wrong or directly contradict the new code."
🤖 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/codegen/src/compile.rs` around lines 7085 - 7086, The existing comment
"Check for overflow (INT_MAX < size - 1)" is now inaccurate because the code
performs a u32 conversion; update the comment immediately above the
u32::try_from(size) call to state that the function is validating that size fits
within a u32 (i.e., ensuring size is within 0..=u32::MAX) and will return an
error if it does not, referencing the u32::try_from(size) conversion and the
local variable size so the boundary contract matches the actual check.

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

Nitpick comments:
In `@crates/codegen/src/compile.rs`:
- Around line 7085-7086: The existing comment "Check for overflow (INT_MAX <
size - 1)" is now inaccurate because the code performs a u32 conversion; update
the comment immediately above the u32::try_from(size) call to state that the
function is validating that size fits within a u32 (i.e., ensuring size is
within 0..=u32::MAX) and will return an error if it does not, referencing the
u32::try_from(size) conversion and the local variable size so the boundary
contract matches the actual check.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 304f142c-e2dc-479c-8420-c8b2c1b9bf53

📥 Commits

Reviewing files that changed from the base of the PR and between 56269f7 and d655415.

📒 Files selected for processing (12)

crates/codegen/src/compile.rs
crates/stdlib/src/json/machinery.rs
crates/vm/src/builtins/dict.rs
crates/vm/src/builtins/list.rs
crates/vm/src/builtins/module.rs
crates/vm/src/lib.rs
crates/vm/src/sliceable.rs
crates/vm/src/stdlib/_ast/pyast.rs
crates/vm/src/stdlib/_ast/statement.rs
crates/vm/src/stdlib/_io.rs
crates/vm/src/stdlib/_winapi.rs
crates/vm/src/stdlib/os.rs

💤 Files with no reviewable changes (4)

crates/stdlib/src/json/machinery.rs
crates/vm/src/builtins/list.rs
crates/vm/src/stdlib/_ast/pyast.rs
crates/vm/src/builtins/dict.rs

…allow

Remove some #[allow(clippy::*)]

d655415

coderabbitai Bot reviewed May 16, 2026

View reviewed changes

ShaharNaveh added 2 commits May 16, 2026 13:30

Merge remote-tracking branch 'upstream/main' into remove-some-clippy-…

6ec49db

…allow

Fix after merge

18acc26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove some `#[allow(clippy::*)]`#7878

Remove some `#[allow(clippy::*)]`#7878
ShaharNaveh wants to merge 3 commits into
RustPython:mainfrom
ShaharNaveh:remove-some-clippy-allow

ShaharNaveh commented May 16, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 16, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

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

Uh oh!

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ShaharNaveh commented May 16, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 16, 2026 •

edited

Loading