Lint for ambiguous method defined in trait impl for struct & on struct itself #11499
Description
What it does
This was inspired by rust-lang #37741. In short, it's very easy to accidentally override a trait method impl. by adding a method of the same name to a struct impl.. This causes unqualified call sites to land in the struct's method instead of the trait method impl for the struct. (Generally call sites will be unqualified because naming conflicts between traits are rare, so this will happen silently.) Since clippy generally warns for potentially incorrect code, this feels like a good candidate for a new lint.
The problematic situation is shown in this playground. For simplicity, here's the code directly:
trait DataExt {
fn ambiguous(&self);
}
struct BaseData {}
impl BaseData {
// This method shadows DataExt::ambiguous(self).
fn ambiguous(&self) {
println!("struct impl");
}
}
impl DataExt for BaseData {
fn ambiguous(&self) {
println!("trait impl");
}
}
fn main() {
let data = BaseData{};
// This calls BaseData::ambiguous instead of DataExt::ambiguous & generates no warnings,
// despite the trait being in scope.
data.ambiguous();
}The proposal would be to flag BaseData::ambiguous as creating ambiguity with DataExt::ambiguous. In other words, if a struct S implements a trait T with method T::foo, then a method S::foo should generate an ambiguity warning.
Advantage
- warns about unintentional "overriding" of a trait method already defined on a struct.
- encourages writing easier to understand code
Drawbacks
- complex object hierarchies may intentionally want this behavior (this is why we probably want the lint to be default allowed)
Example
The lint should trigger as follows:
trait DataExt {
fn ambiguous(&self);
}
struct BaseData {}
impl BaseData {
// This method shadows DataExt::ambiguous(self) and should fail the lint.
fn ambiguous(&self) {
println!("struct impl");
}
}
impl DataExt for BaseData {
fn ambiguous(&self) {
println!("trait impl");
}
}The developer could then rewrite as:
trait DataExt {
fn ambiguous(&self);
}
struct BaseData {}
impl BaseData {
fn no_longer_ambiguous(&self) {
println!("struct impl");
}
}
impl DataExt for BaseData {
fn ambiguous(&self) {
println!("trait impl");
}
}