cargo-semver-checks icon indicating copy to clipboard operation
cargo-semver-checks copied to clipboard

Refine lints about field and method removals to account for `Deref/DerefMut`

Open obi1kenobi opened this issue 10 months ago • 0 comments

This issue about trait associated consts and functions acting as a backstop for removed inherent items got me thinking, what about Deref? Is the following breaking if the // REMOVE lines are removed?

pub struct Foo {
    pub field: (), // REMOVE
    inner: Inner,
}

impl Foo {
    pub fn method(&self) {} // REMOVE

    pub fn new() -> Self {
        Self {
            field: (), // REMOVE
            inner: Inner { field: () },
        }
    }
}

pub struct Inner {
    pub field: (),
}

impl Inner {
    pub fn method(&self) {}
}

impl core::ops::Deref for Foo {
    type Target = Inner;
    fn deref(&self) -> &Self::Target {
        &self.inner
    }
}

impl core::ops::DerefMut for Foo {
    fn deref_mut(&mut self) -> &mut Self::Target {
        &mut self.inner
    }
}

fn test() {
    let mut foo = Foo::new();

    foo.field = ();
    foo.method();
}

Originally posted by @jw013 in https://github.com/obi1kenobi/cargo-semver-checks/issues/727#issuecomment-2065620946

The long-term solution we want here is similar to the case of clippy::correctness vs clippy::suspicious:

  • We want correctness-flavored lints (on by default, error level) that allow Deref/DerefMut to be used to replace the removed item.
  • We want suspicious-flavored lints (on by default, warning level) that do not allow the above, and never fire if their correctness sibling is already going to fire.

More discussion here: https://github.com/obi1kenobi/cargo-semver-checks/issues/727#issuecomment-2065658901

The suspicious-flavored lint is blocked on #58.

The correctness-flavored lints already exist and can be tweaked, though this is not a good onboarding task! It requires proficiency with the Trustfall query language, so highly recommend writing some other lints first!

obi1kenobi avatar Apr 19 '24 02:04 obi1kenobi