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.39k stars 1.54k forks source link

New lint suggestion: Avoid inherent methods on generic smart pointers #11820

Open madsmtm opened 11 months ago

madsmtm commented 11 months ago

What it does

Generic smart pointers like Box<T>, Rc<T> and so on are recommended to use associated functions instead of inherent methods to avoid compatibility hazards with Deref. As an example, see Box::leak. This is also documented in the Rust API Guidelines.

This lint warns on every inherent method on types that implement Deref in a fully generic fashion.

(In particular, it should activate when type Target = T, but not when type Target = u8 or type Target = [T]).

Advantage

Drawbacks

Example

use std::ops::Deref;

pub struct MySmartPointer<T>(pub T);

impl<T> Deref for MySmartPointer<T> {
    type Target = T;

    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

impl<T> MySmartPointer<T> {
    pub fn foo(&self) {
        unimplemented!()
    }
}

Could be written as:

// ... snip

impl<T> MySmartPointer<T> {
    pub fn foo(this: &Self) {
        unimplemented!()
    }
}

Or, in the case where the user has read and understood the drawbacks, be explicitly allowed:

// ... snip

impl<T> MySmartPointer<T> {
    #[allow(clippy::bikeshed_lint_name)]
    pub fn foo(&self) {
        unimplemented!()
    }
}
Centri3 commented 11 months ago

Some (sometimes unjustifiably) use Deref for newtypes, even large projects like bevy. This would afaik lint if it's generic, as an example, struct Meters<T: num::Float>(pub T)

In the case fully generic means no trait bounds, yeah this would work. Even if it's a newtype/marker type that would still be prone to issues