Skip to content

clippy significant_drop_in_scrutinee & explicit_deref_methods#8104

Merged
youknowone merged 5 commits into
RustPython:mainfrom
ShaharNaveh:clippy-pedantic-rules-4
Jun 15, 2026
Merged

clippy significant_drop_in_scrutinee & explicit_deref_methods#8104
youknowone merged 5 commits into
RustPython:mainfrom
ShaharNaveh:clippy-pedantic-rules-4

Conversation

@ShaharNaveh

@ShaharNaveh ShaharNaveh commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

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.

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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.

@ShaharNaveh ShaharNaveh marked this pull request as ready for review June 14, 2026 14:07
@ShaharNaveh ShaharNaveh force-pushed the clippy-pedantic-rules-4 branch from 390038e to e139075 Compare June 14, 2026 14:18

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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

Comment on lines 2133 to 2135
fn eq(&self, other: &Self) -> bool {
self.deref().eq(other.deref())
self.deref().eq(&**other)
}

Copy link
Copy Markdown
Contributor

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

@youknowone youknowone merged commit 880040d into RustPython:main Jun 15, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants