clippy `significant_drop_in_scrutinee` & `explicit_deref_methods` by ShaharNaveh · Pull Request #8104 · RustPython/RustPython

ShaharNaveh

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