cargo-semver-checks
cargo-semver-checks copied to clipboard
Refine lints about field and method removals to account for `Deref/DerefMut`
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 allowDeref/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 theircorrectness
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!