GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — clippy `significant_drop_in_scrutinee` & `explicit_deref_methods` by ShaharNaveh · Pull Request #8104 · RustPython/RustPython

Conversation

ShaharNaveh commented Jun 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Closes #xxxx
This PR follows our AI policy

Summary

Summary by CodeRabbit

Release Notes

Chores
- Added new Clippy workspace lint rules to improve code quality.
- Refined internal implementation details across the VM and standard library (including sorting, descriptors, exception rendering, iterator handling, SSL/async, and ctypes) without changing public APIs.
- Made Scope::new a const fn for compile-time evaluation.
Documentation
- Updated a rustdoc comment to reference ALIGN using proper link formatting.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ShaharNaveh added 3 commits

June 14, 2026 17:02


          nits

87fea8f


          explicit_deref_methods

f03198f


          significant_drop_in_scrutinee

18753da

coderabbitai Bot commented Jun 14, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

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: 8039b369-40dc-43cc-b2d5-062291d78802

📥 Commits

Reviewing files that changed from the base of the PR and between 390038e and e139075.

📒 Files selected for processing (13)

crates/stdlib/src/_asyncio.rs
crates/stdlib/src/_sqlite3.rs
crates/stdlib/src/array.rs
crates/stdlib/src/ssl.rs
crates/vm/src/builtins/classmethod.rs
crates/vm/src/builtins/property.rs
crates/vm/src/builtins/staticmethod.rs
crates/vm/src/exceptions.rs
crates/vm/src/object/core.rs
crates/vm/src/stdlib/_ctypes/base.rs
crates/vm/src/stdlib/_ctypes/pointer.rs
crates/vm/src/stdlib/_thread.rs
crates/vm/src/types/slot.rs

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

crates/vm/src/stdlib/_ctypes/pointer.rs
crates/stdlib/src/_sqlite3.rs
crates/vm/src/stdlib/_thread.rs

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

crates/vm/src/stdlib/_ctypes/base.rs
crates/vm/src/builtins/staticmethod.rs
crates/vm/src/builtins/property.rs
crates/vm/src/exceptions.rs
crates/stdlib/src/ssl.rs
crates/vm/src/builtins/classmethod.rs
crates/stdlib/src/_asyncio.rs

📝 Walkthrough

Walkthrough

Two new Clippy workspace lint rules (significant_drop_in_scrutinee and explicit_deref_methods) are added to Cargo.toml, and the resulting violations are fixed across the entire codebase by replacing explicit .deref() calls with idiomatic double-deref syntax and by hoisting lock-read results into local variables before if let scrutinees. Scope::new is additionally made pub const fn.

Changes

Clippy Lint Enablement and Codebase-Wide Fixes

Layer / File(s)	Summary
Workspace Clippy lint configuration `Cargo.toml`	Adds `significant_drop_in_scrutinee = "warn"` and `explicit_deref_methods = "warn"` to workspace lint config.
`explicit_deref_methods` fixes — common, builtins, object core `crates/common/src/borrow.rs`, `crates/common/src/format.rs`, `crates/vm/src/builtins/set.rs`, `crates/vm/src/builtins/type.rs`, `crates/vm/src/builtins/list.rs`, `crates/vm/src/object/core.rs`, `crates/vm/src/intern.rs`, `crates/vm/src/datastack.rs`	Replaces explicit `.deref()` calls with `&**x` idiom in `BorrowedValue` Display, `format_sign_and_align`, `PySet::repr_wtf8`, `PyType` MRO helpers and `best_base`, `PyList::sort` guard access, and `PyObject::class()`/`PartialEq`. Reorganises `set.rs` imports to drop `Deref`. Switches `unwrap_or_else` to `expect` in `intern.rs`. Updates a rustdoc link format in `datastack.rs`.
`significant_drop_in_scrutinee` fixes — stdlib `crates/stdlib/src/_asyncio.rs`, `crates/stdlib/src/array.rs`, `crates/stdlib/src/ssl.rs`, `crates/stdlib/src/_sqlite3.rs`, `crates/vm/src/stdlib/_ctypes/base.rs`, `crates/vm/src/stdlib/_ctypes/pointer.rs`, `crates/vm/src/stdlib/_thread.rs`	Extracts lock-read results into local temporaries before `if let` scrutinees in `Future`/`Task` callbacks and destructors, array constructor and iterator, SSL `do_handshake`, SQLite autocommit getter, ctypes `keep_ref`/`keep_alive`/`set_contents`, and thread-local `l_dict`.
`significant_drop_in_scrutinee` fixes — VM builtins and types; `Scope::new` const fn `crates/vm/src/builtins/classmethod.rs`, `crates/vm/src/builtins/staticmethod.rs`, `crates/vm/src/builtins/property.rs`, `crates/vm/src/object/core.rs`, `crates/vm/src/exceptions.rs`, `crates/vm/src/types/slot.rs`, `crates/vm/src/scope.rs`	Introduces temporaries before `if let` branches in `PyClassMethod`/`PyStaticMethod.__isabstractmethod__`, all `PyProperty` descriptor methods, `gc_clear_raw` slot loop, `write_exception_inner` traceback and suggestion rendering, and `lookup_slot_in_mro`. Changes `Scope::new` to `pub const fn`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

RustPython/RustPython#8050: Adds multiple new Clippy warn rules to Cargo.toml workspace config, a direct parallel to this PR's lint enablement.
RustPython/RustPython#7755: Adds new workspace lint rules to Cargo.toml, using the same configuration mechanism.
RustPython/RustPython#7875: Reorganises and expands workspace Clippy lint configuration in Cargo.toml by enabling warn-level rules.

Suggested reviewers

youknowone

🐇 Two lints I've added to the config so true,
No more .deref() — just &**x will do!
Temporaries hoisted before if let each day,
Significant drops are now whisked far away.
The rabbit hops clean through a linted codebase! 🌿

🚥 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 title directly refers to the main changes: addressing Clippy linting rules `significant_drop_in_scrutinee` and `explicit_deref_methods` across the codebase, which matches the PR's core objective.
Docstring Coverage	✅ Passed	Docstring coverage is 100.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.}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ShaharNaveh marked this pull request as ready for review

June 14, 2026 14:07

ShaharNaveh added 2 commits

June 14, 2026 17:18


          stdlib

fde2f66

vm

e139075

ShaharNaveh force-pushed the clippy-pedantic-rules-4 branch from 390038e to e139075 Compare

June 14, 2026 14:18

coderabbitai Bot reviewed

Jun 14, 2026

View reviewed changes

coderabbitai Bot left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this 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

🧹 Nitpick comments (1)

crates/common/src/format.rs (1)
900-903: ⚡ Quick win

The explicit_deref_methods cleanup is incomplete.

These remaining .deref() call sites will keep the new workspace lint firing.

crates/common/src/format.rs#L900-L903: replace s.deref() with autoderef (s.chars(), s.to_owned()).

crates/vm/src/builtins/type.rs#L338-L344: replace self.init.deref() / self.getitem.deref() with borrowed autoderef.

crates/vm/src/builtins/type.rs#L1431-L1438: replace self.name().deref() with &self.name().
🤖 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/common/src/format.rs` around lines 900 - 903, Remove the explicit
deref() calls to allow Rust's autoderef mechanism to handle the dereferencing
automatically. In crates/common/src/format.rs lines 900-903, replace
s.deref().chars() with s.chars() and s.deref().to_owned() with s.to_owned(). In
crates/vm/src/builtins/type.rs lines 338-344, replace self.init.deref() and
self.getitem.deref() with borrowed references using the autoderef operator
(e.g., &self.init and &self.getitem). In crates/vm/src/builtins/type.rs lines
1431-1438, replace self.name().deref() with &self.name() to let autoderef handle
the dereferencing. These changes will resolve the explicit_deref_methods lint
warnings across all three affected sites.
Source: Coding guidelines

🤖 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/object/core.rs`:
- Around line 2133-2135: Replace explicit `.deref()` and `.deref_mut()` calls
with direct deref operator syntax across multiple files to resolve clippy's
explicit_deref_methods warnings. In crates/vm/src/object/core.rs at lines
2123-2125 (first Hash impl), replace `self.deref().hash(state)` with
`(*self).hash(state)`. At lines 2133-2135 (first PartialEq impl), replace
`self.deref().eq(&**other)` with `(*self).eq(&*other)`. At lines 2399-2402
(second Hash impl), apply the same Hash pattern replacement. At lines 2410-2412
(second PartialEq impl), apply the same PartialEq pattern replacement. In
crates/vm/src/builtins/list.rs at lines 239-240 in the clear() method, replace
`self.borrow_vec_mut().deref_mut()` with `&mut *self.borrow_vec_mut()`. At lines
479-480 in the init() method, replace `zelf.borrow_vec_mut().deref_mut()` with
`&mut *zelf.borrow_vec_mut()`. After applying all changes, run cargo clippy to
verify no remaining lint warnings.

---

Nitpick comments:
In `@crates/common/src/format.rs`:
- Around line 900-903: Remove the explicit deref() calls to allow Rust's
autoderef mechanism to handle the dereferencing automatically. In
crates/common/src/format.rs lines 900-903, replace s.deref().chars() with
s.chars() and s.deref().to_owned() with s.to_owned(). In
crates/vm/src/builtins/type.rs lines 338-344, replace self.init.deref() and
self.getitem.deref() with borrowed references using the autoderef operator
(e.g., &self.init and &self.getitem). In crates/vm/src/builtins/type.rs lines
1431-1438, replace self.name().deref() with &self.name() to let autoderef handle
the dereferencing. These changes will resolve the explicit_deref_methods lint
warnings across all three affected sites.

🪄 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: 6e31d954-9ca8-402c-b36c-34450729bb75

📥 Commits

Reviewing files that changed from the base of the PR and between e423160 and 390038e.

📒 Files selected for processing (21)

Cargo.toml
crates/common/src/borrow.rs
crates/common/src/format.rs
crates/stdlib/src/_asyncio.rs
crates/stdlib/src/array.rs
crates/stdlib/src/ssl.rs
crates/vm/src/builtins/classmethod.rs
crates/vm/src/builtins/list.rs
crates/vm/src/builtins/property.rs
crates/vm/src/builtins/set.rs
crates/vm/src/builtins/staticmethod.rs
crates/vm/src/builtins/type.rs
crates/vm/src/datastack.rs
crates/vm/src/exceptions.rs
crates/vm/src/intern.rs
crates/vm/src/object/core.rs
crates/vm/src/scope.rs
crates/vm/src/stdlib/_ctypes/base.rs
crates/vm/src/stdlib/_ctypes/pointer.rs
crates/vm/src/stdlib/_thread.rs
crates/vm/src/types/slot.rs

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

crates/vm/src/object/core.rs

Comment on lines 2133 to 2135

		fn eq(&self, other: &Self) -> bool {
		self.deref().eq(other.deref())
		self.deref().eq(&**other)
		}

coderabbitai Bot Jun 14, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n '\.deref(_mut)?\(' crates/vm/src/builtins/list.rs crates/vm/src/object/core.rs

Repository: RustPython/RustPython

Length of output: 555

🏁 Script executed:

#!/bin/bash
# Check context for core.rs deref calls
echo "=== crates/vm/src/object/core.rs context ==="
sed -n '2120,2140p' crates/vm/src/object/core.rs | cat -n
echo
sed -n '2395,2415p' crates/vm/src/object/core.rs | cat -n
echo

# Check context for list.rs deref_mut calls
echo "=== crates/vm/src/builtins/list.rs context ==="
sed -n '235,245p' crates/vm/src/builtins/list.rs | cat -n
echo
sed -n '475,485p' crates/vm/src/builtins/list.rs | cat -n

Repository: RustPython/RustPython

Length of output: 1998

🏁 Script executed:

#!/bin/bash

# Verify exact line numbers for each site mentioned in review
echo "=== Verifying line numbers ==="

echo "core.rs Hash impl for Py:"
sed -n '2123,2126p' crates/vm/src/object/core.rs | cat -n

echo "core.rs Eq impl for Py:"
sed -n '2132,2136p' crates/vm/src/object/core.rs | cat -n

echo "core.rs Hash impl for PyRef:"
sed -n '2399,2403p' crates/vm/src/object/core.rs | cat -n

echo "core.rs Eq impl for PyRef:"
sed -n '2409,2413p' crates/vm/src/object/core.rs | cat -n

echo "list.rs clear method:"
sed -n '238,241p' crates/vm/src/builtins/list.rs | cat -n

echo "list.rs init method:"
sed -n '478,481p' crates/vm/src/builtins/list.rs | cat -n

Repository: RustPython/RustPython

Length of output: 1081

🏁 Script executed:

#!/bin/bash
# Final check: ensure only the 6 sites remain
echo "Total explicit deref calls in both files:"
rg -c '\.deref(_mut)?\(' crates/vm/src/builtins/list.rs crates/vm/src/object/core.rs | sort

echo
echo "All lines with explicit deref (should be exactly 6):"
rg -n '\.deref(_mut)?\(' crates/vm/src/builtins/list.rs crates/vm/src/object/core.rs | wc -l

Repository: RustPython/RustPython

Length of output: 227

Replace explicit .deref() and .deref_mut() calls with direct deref syntax to resolve clippy's explicit_deref_methods warnings.

crates/vm/src/object/core.rs#L2124 / L2401: Replace self.deref().hash(state) with (*self).hash(state) in Hash impls.
crates/vm/src/object/core.rs#L2134 / L2411: Replace self.deref().eq(&**other) with (*self).eq(&*other) in PartialEq impls.
crates/vm/src/builtins/list.rs#L240: Replace self.borrow_vec_mut().deref_mut() with &mut *self.borrow_vec_mut() in clear().
crates/vm/src/builtins/list.rs#L480: Replace zelf.borrow_vec_mut().deref_mut() with &mut *zelf.borrow_vec_mut() in init().

After changes, run cargo clippy to confirm no remaining lint warnings.

📍 Affects 2 files

crates/vm/src/object/core.rs#L2133-L2135 (this comment)
crates/vm/src/object/core.rs#L2410-L2412
crates/vm/src/object/core.rs#L2123-L2125
crates/vm/src/object/core.rs#L2399-L2402
crates/vm/src/builtins/list.rs#L239-L240
crates/vm/src/builtins/list.rs#L479-L480

🤖 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/object/core.rs` around lines 2133 - 2135, Replace explicit
`.deref()` and `.deref_mut()` calls with direct deref operator syntax across
multiple files to resolve clippy's explicit_deref_methods warnings. In
crates/vm/src/object/core.rs at lines 2123-2125 (first Hash impl), replace
`self.deref().hash(state)` with `(*self).hash(state)`. At lines 2133-2135 (first
PartialEq impl), replace `self.deref().eq(&**other)` with `(*self).eq(&*other)`.
At lines 2399-2402 (second Hash impl), apply the same Hash pattern replacement.
At lines 2410-2412 (second PartialEq impl), apply the same PartialEq pattern
replacement. In crates/vm/src/builtins/list.rs at lines 239-240 in the clear()
method, replace `self.borrow_vec_mut().deref_mut()` with `&mut
*self.borrow_vec_mut()`. At lines 479-480 in the init() method, replace
`zelf.borrow_vec_mut().deref_mut()` with `&mut *zelf.borrow_vec_mut()`. After
applying all changes, run cargo clippy to verify no remaining lint warnings.

Source: Coding guidelines

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

youknowone approved these changes

Jun 15, 2026

View reviewed changes

youknowone merged commit 880040d into RustPython:main

Jun 15, 2026

26 checks passed

Uh oh!

There was an error while loading. Please reload this page.

youknowone pushed a commit that referenced this pull request

Sep 16, 2026


          clippy significant_drop_in_scrutinee & explicit_deref_methods (#8104

Reviewers

coderabbitai[bot]

coderabbitai[bot] left review comments

youknowone

youknowone approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

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

Uh oh!

Summary

Summary by CodeRabbit

Release Notes

Uh oh!

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

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jun 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ShaharNaveh commented Jun 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

coderabbitai Bot commented Jun 14, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.