rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.43k stars 1.54k forks source link

New lint: deref coercions #7104

Open Aaron1011 opened 3 years ago

Aaron1011 commented 3 years ago

What it does

Lints on any use of deref coercions - for example, accessing a field or method through a Deref impl

Categories (optional)

What is the advantage of the recommended code over the original code

Normally, deref coercions are a very useful part of rust, making smart points like Box, Rc, Ref, and RefMut much easier to use. However, this can be undesirable when writing unsafe code, since a non-trivial function call could be completely hidden from view. For example:

Drawbacks

This would be a fairly niche lint - unless you're writing unsafe code, there's little need to use it.

Example

fn main() {
    let a = Box::new(true);
    let b: &bool = &a;
}

Could be written as:

use std::ops::Deref;
fn main() {
    let a = Box::new(true);
    let b: &bool = (&a).deref();
}
Heidar-An commented 1 year ago

This sounds interesting, and I'd want to do it, any help on how I would do this though (sorry for naive question, this would be my first issue).

Centri3 commented 1 year ago

You can ping rustbot like rustbot claim (plus an @ of course)

In regards to implementing this, I'm not sure. I don't think implicit deref coercion is explicitly in the HIR (as a node) so unless there's a query for it you'd need to figure it out yourself, maybe there's some method to do that in dereference.rs (likely where you'd implement the lint). Perhaps @xFrednet (or others) have any ideas :)?

PS: FnCtxt::deref_steps looks interesting! Same with can_coerce. Haven't tested them so perhaps they won't catch this but it could be a good starting point ^^

y21 commented 1 year ago

Perhaps this could use TypeckResults::expr_adjustments by checking if there are any Adjust::Derefs where the target type is different from the source type (from expr_ty without adjusts) with references removed

Centri3 commented 1 year ago

I hadn't thought of that; that sounds perfect for this

xFrednet commented 1 year ago

TypeckResults::expr_adjustments

You found the correct method :+1: