Skip to content

CI: fix clippy lints#7620

Open
ShaharNaveh wants to merge 1 commit intoRustPython:mainfrom
ShaharNaveh:ci-fix-clippy
Open

CI: fix clippy lints#7620
ShaharNaveh wants to merge 1 commit intoRustPython:mainfrom
ShaharNaveh:ci-fix-clippy

Conversation

@ShaharNaveh
Copy link
Copy Markdown
Contributor

@ShaharNaveh ShaharNaveh commented Apr 16, 2026 •
edited by coderabbitai bot
Loading

Summary by CodeRabbit

  • Refactor

    • Streamlined internal handling of set-like collections and abstract-method construction.
    • Simplified slot inheritance logic and member offset computation.
    • Improved SQL whitespace/comment scanning and handling.

  • Style

    • Clarified type-code matching formatting for more consistent logic.

  • Chores

    • Tightened debug-time boundary checks.
    • Minor iteration/ownership tweaks and a platform-specific lint allowance for Windows environment handling.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 16, 2026 •
edited
Loading

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

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: 30880455-9a05-408a-a98d-7c2b845b54da

📥 Commits

Reviewing files that changed from the base of the PR and between c8325a3 and 640cbd7.

📒 Files selected for processing (9)
  • crates/stdlib/src/_sqlite3.rs
  • crates/vm/src/builtins/type.rs
  • crates/vm/src/builtins/union.rs
  • crates/vm/src/codecs.rs
  • crates/vm/src/stdlib/_abc.rs
  • crates/vm/src/stdlib/_ctypes/simple.rs
  • crates/vm/src/stdlib/_winapi.rs
  • crates/vm/src/stdlib/time.rs
  • crates/vm/src/types/slot.rs
✅ Files skipped from review due to trivial changes (5)
  • crates/vm/src/codecs.rs
  • crates/vm/src/stdlib/_abc.rs
  • crates/vm/src/types/slot.rs
  • crates/vm/src/stdlib/time.rs
  • crates/vm/src/builtins/union.rs
🚧 Files skipped from review as they are similar to previous changes (3)
  • crates/vm/src/stdlib/_winapi.rs
  • crates/vm/src/stdlib/_ctypes/simple.rs
  • crates/vm/src/builtins/type.rs
📝 Walkthrough

Walkthrough

Small, localized refactors across VM crates: pass collections directly to PyFrozenSet::from_iter, adjust a debug-assert boundary, reformat match arms, flatten a slot match guard, compute __slots__ offsets via zipped indices, change a zip(values) usage, add a clippy allow, and refactor SQL comment scanning. (≤50 words)

Changes

Cohort / File(s) Summary
PyFrozenSet construction
crates/vm/src/builtins/union.rs, crates/vm/src/stdlib/_abc.rs		 Pass collection containers (Vec / elements) directly to PyFrozenSet::from_iter(vm, ...) instead of calling .into_iter(); behavior unchanged.
Debug assertion tweak
crates/vm/src/codecs.rs		 Replaced 0.max(s.len() - 1) with s.len().saturating_sub(1) inside a debug_assert!; only debug-time expression changed.
Match-arm formatting
crates/vm/src/stdlib/_ctypes/simple.rs		 Rewrote integer/float type_code match arms to more concise forms; predicates and return values unchanged.
Slot inheritance guard
crates/vm/src/types/slot.rs		 Moved if !ADD into the `SqConcat
slots offset iteration
crates/vm/src/builtins/type.rs		 Replaced manual offset counter with zipping base_member_count.. and slots.as_slice().iter() to compute per-member offsets.
Windows env pairing
crates/vm/src/stdlib/_winapi.rs		 Changed zip(values.into_iter()) to zip(values) when pairing keys and values; iteration result unchanged.
Windows time lint allow
crates/vm/src/stdlib/time.rs		 Added #[allow(clippy::unnecessary_cast, reason = "info.Bias is not always i32")] on a cast in localtime_from_timestamp; arithmetic unchanged.
SQLite SQL trimming refactor
crates/stdlib/src/_sqlite3.rs		 Refactored lstrip_sql comment-handling from nested matches to guard-based patterns and simplified control flow when detecting -- and /* ... */ comments; scanning behavior preserved.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • youknowone

🐰
I nibbled through loops and tiny guards,
swapped iterators and soothed linting shards.
Offsets counted by a gentle zip,
comments trimmed with a quieter clip.
Hop—small hops, tidy paths.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The PR title 'CI: fix clippy lints' is too vague and generic; it describes the action taken (fixing lints) rather than the actual code changes, which involve refactoring iterator handling and control flow in 8 files across multiple crates. Consider a more specific title that reflects the primary refactoring effort, such as 'Refactor iterator patterns and control flow across builtins and stdlib' or 'Simplify iterator and guard patterns to fix clippy warnings'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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[bot]
coderabbitai bot reviewed Apr 16, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents 
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/vm/src/codecs.rs`:
- Line 535: The debug assertion uses unchecked subtraction (byte_range.start <=
s.len() - 1) which underflows when s is empty; replace it with a safe bounds
check such as debug_assert!(byte_range.start < s.len()); (or
debug_assert!(!s.is_empty() && byte_range.start <= s.len().saturating_sub(1));)
so that the assertion on byte_range.start is evaluated without doing s.len() - 1
and avoids usize underflow; update the assertion in the debug_assert! call that
references s and byte_range.start accordingly.
🪄 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: 42bfc4e9-2a89-4aff-864d-451c7ac91450

📥 Commits

Reviewing files that changed from the base of the PR and between aa12acc and 99d228b.

📒 Files selected for processing (5)
  • crates/vm/src/builtins/union.rs
  • crates/vm/src/codecs.rs
  • crates/vm/src/stdlib/_abc.rs
  • crates/vm/src/stdlib/_ctypes/simple.rs
  • crates/vm/src/types/slot.rs

Comment thread crates/vm/src/codecs.rs Outdated
@ShaharNaveh ShaharNaveh force-pushed the ci-fix-clippy branch 3 times, most recently from 92139da to c8325a3 Compare April 16, 2026 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ShaharNaveh