New lint: ambiguous_method_names

CHANGELOG.md

@@ -4776,6 +4776,7 @@ Released 2018-09-13
 [`almost_complete_letter_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_letter_range
 [`almost_complete_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_range
 [`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped
+[`ambiguous_method_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#ambiguous_method_names
 [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant
 [`arc_with_non_send_sync`]: https://rust-lang.github.io/rust-clippy/master/index.html#arc_with_non_send_sync
 [`arithmetic_side_effects`]: https://rust-lang.github.io/rust-clippy/master/index.html#arithmetic_side_effects

clippy_lints/src/ambiguous_method_names.rs

@@ -0,0 +1,142 @@
+use clippy_utils::diagnostics::span_lint_and_note;
+use clippy_utils::is_trait_impl_item;
+use clippy_utils::ty::implements_trait;
+use hir::{ImplItem, Item};
+use rustc_hir as hir;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::def_id::DefId;
+use rustc_span::symbol::Ident;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Looks for methods in trait impls and struct impls with the same name,
+    /// as well as method call sites.
+    ///
+    /// ### Why is this bad?
+    /// It is easy to accidentally override a trait method impl with a method
+    /// of the same name in a struct impl. Inherent methods are preferred if
+    /// the call site is unqualified, and naming conflicts will happen silently.
+    ///
+    /// ### Example
+    /// ```rust
+    /// trait MyTrait {
+    ///     fn ambiguous(&self);
+    /// }
+    ///
+    /// struct Base;
+    ///
+    /// impl Base {
+    ///     fn ambiguous(&self) {
+    ///         println!("struct impl");
+    ///     }
+    /// }
+    ///
+    /// impl MyTrait for Base {
+    ///     fn ambiguous(&self) {
+    ///         println!("trait impl");
+    ///     }
+    /// }
+    ///
+    /// Base.ambiguous(); // prints "struct impl"
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// trait MyTrait {
+    ///     fn ambiguous(&self);
+    /// }
+    ///
+    /// struct Base;
+    ///
+    /// impl Base {
+    ///     fn unambiguous(&self) {
+    ///         println!("struct impl");
+    ///     }
+    /// }
+    ///
+    /// impl MyTrait for Base {
+    ///     fn ambiguous(&self) {
+    ///         println!("trait impl");
+    ///     }
+    /// }
+    ///
+    /// Base.unambiguous(); // prints "struct impl"
+    /// Base.ambiguous(); // prints "trait impl"
+    /// ```
+    #[clippy::version = "1.74.0"]
+    pub AMBIGUOUS_METHOD_NAMES,
+    pedantic,
+    "declarations and calls for same-named methods in struct impls and trait impls"
+}
+
+#[derive(Clone)]
+pub struct AmbiguousMethodNames {
+    // Keeps track of trait methods
+    trait_methods: Vec<(DefId, Ident)>,
+    // Keeps track of inherent methods
+    inherent_methods: Vec<(DefId, Ident)>,
+}
+
+impl AmbiguousMethodNames {
+    pub fn new() -> Self {
+        Self {
+            trait_methods: Vec::default(),
+            inherent_methods: Vec::default(),
+        }
+    }
+}
+
+impl_lint_pass!(AmbiguousMethodNames => [AMBIGUOUS_METHOD_NAMES]);
+
+impl<'tcx> LateLintPass<'tcx> for AmbiguousMethodNames {
+    // Check trait impls
+    fn check_item(&mut self, _: &LateContext<'tcx>, item: &'tcx Item<'_>) {
+        if let hir::ItemKind::Trait(_, _, _, _, tr_items) = item.kind {
+            for tr_item in tr_items {
+                if let hir::AssocItemKind::Fn { .. } = tr_item.kind {
+                    self.trait_methods
+                        .push((item.owner_id.def_id.to_def_id(), tr_item.ident));
+                }
+            }
+        }
+    }
+
+    // Check inherent methods
+    fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) {
+        if let hir::ImplItemKind::Fn(..) = impl_item.kind {
+            let hir_id = cx.tcx.hir().local_def_id_to_hir_id(impl_item.owner_id.def_id);
+            if !is_trait_impl_item(cx, hir_id) {
+                let struct_id = cx.tcx.hir().get_parent_item(hir_id);
+                self.inherent_methods
+                    .push((struct_id.def_id.to_def_id(), impl_item.ident));
+            }
+        }
+    }
+
+    fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
+        for (r#trait, ident) in &self.trait_methods {
+            for (r#struct, inherent_ident) in &self.inherent_methods {
+                let struct_ty = cx.tcx.type_of(r#struct).skip_binder();
+                let trait_ref = ty::TraitRef::identity(cx.tcx, *r#trait);
+                let args = &trait_ref.args[1..];
+                if implements_trait(
+                    cx,
+                    struct_ty,
+                    *r#trait,
+                    &args[..cx.tcx.generics_of(r#trait).params.len() - 1],
+                ) && ident.name == inherent_ident.name
+                {
+                    span_lint_and_note(
+                        cx,
+                        AMBIGUOUS_METHOD_NAMES,
+                        inherent_ident.span,
+                        "ambiguous inherent method name",
+                        Some(ident.span),
+                        "trait method defined here",
+                    );
+                }
+            }
+        }
+    }
+}

clippy_lints/src/declared_lints.rs

@@ -40,6 +40,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::absolute_paths::ABSOLUTE_PATHS_INFO,
     crate::allow_attributes::ALLOW_ATTRIBUTES_INFO,
     crate::almost_complete_range::ALMOST_COMPLETE_RANGE_INFO,
+    crate::ambiguous_method_names::AMBIGUOUS_METHOD_NAMES_INFO,
     crate::approx_const::APPROX_CONSTANT_INFO,
     crate::arc_with_non_send_sync::ARC_WITH_NON_SEND_SYNC_INFO,
     crate::as_conversions::AS_CONVERSIONS_INFO,

clippy_lints/src/lib.rs

@@ -70,6 +70,7 @@ mod renamed_lints;
 mod absolute_paths;
 mod allow_attributes;
 mod almost_complete_range;
+mod ambiguous_method_names;
 mod approx_const;
 mod arc_with_non_send_sync;
 mod as_conversions;
@@ -1121,6 +1122,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         ))
     });
     store.register_late_pass(move |_| Box::new(manual_hash_one::ManualHashOne::new(msrv())));
+    store.register_late_pass(|_| Box::new(ambiguous_method_names::AmbiguousMethodNames::new()));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

tests/ui/ambiguous_method_names.rs

@@ -0,0 +1,81 @@
+#![allow(dead_code)]
+#![warn(clippy::ambiguous_method_names)]
+
+fn main() {}
+
+fn ambiguous() {}
+
+trait MyTrait {
+    fn ambiguous(&self);
+    fn also_ambiguous(&self);
+    fn ambiguous_default(&self) {}
+}
+
+trait Another {
+    fn another(&self);
+}
+
+trait G<T, U> {
+    fn g(&self, a: T, b: U);
+}
+
+struct A;
+
+impl A {
+    fn ambiguous(&self) {}
+    fn also_ambiguous(&self) {}
+    fn ambiguous_default(&self) {}
+    fn unambiguous(&self) {}
+    fn another(&self) {}
+}
+
+impl MyTrait for A {
+    fn ambiguous(&self) {}
+    fn also_ambiguous(&self) {}
+}
+
+impl Another for A {
+    fn another(&self) {}
+}
+
+struct B;
+
+impl B {
+    fn ambiguous(&self) {}
+    fn also_ambiguous(&self) {}
+    fn ambiguous_default(&self) {}
+    fn another(&self) {}
+}
+
+impl MyTrait for B {
+    fn ambiguous(&self) {}
+    fn also_ambiguous(&self) {}
+}
+
+impl Another for B {
+    fn another(&self) {}
+}
+
+struct C;
+
+impl MyTrait for C {
+    fn ambiguous(&self) {}
+    fn also_ambiguous(&self) {}
+}
+
+struct D;
+
+impl D {
+    fn ambiguous(&self) {}
+    fn also_ambiguous(&self) {}
+}
+
+struct S<T>(T);
+
+impl S<i32> {
+    fn f(&self) {}
+}
+
+impl S<u64> {
+    fn f(&self) {}
+}

tests/ui/ambiguous_method_names.stderr

@@ -0,0 +1,100 @@
+error: ambiguous inherent method name
+  --> $DIR/ambiguous_method_names.rs:25:8
+   |
+LL |     fn ambiguous(&self) {}
+   |        ^^^^^^^^^
+   |
+note: trait method defined here
+  --> $DIR/ambiguous_method_names.rs:9:8
+   |
+LL |     fn ambiguous(&self);
+   |        ^^^^^^^^^
+   = note: `-D clippy::ambiguous-method-names` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::ambiguous_method_names)]`
+
+error: ambiguous inherent method name
+  --> $DIR/ambiguous_method_names.rs:44:8
+   |
+LL |     fn ambiguous(&self) {}
+   |        ^^^^^^^^^
+   |
+note: trait method defined here
+  --> $DIR/ambiguous_method_names.rs:9:8
+   |
+LL |     fn ambiguous(&self);
+   |        ^^^^^^^^^
+
+error: ambiguous inherent method name
+  --> $DIR/ambiguous_method_names.rs:26:8
+   |
+LL |     fn also_ambiguous(&self) {}
+   |        ^^^^^^^^^^^^^^
+   |
+note: trait method defined here
+  --> $DIR/ambiguous_method_names.rs:10:8
+   |
+LL |     fn also_ambiguous(&self);
+   |        ^^^^^^^^^^^^^^
+
+error: ambiguous inherent method name
+  --> $DIR/ambiguous_method_names.rs:45:8
+   |
+LL |     fn also_ambiguous(&self) {}
+   |        ^^^^^^^^^^^^^^
+   |
+note: trait method defined here
+  --> $DIR/ambiguous_method_names.rs:10:8
+   |
+LL |     fn also_ambiguous(&self);
+   |        ^^^^^^^^^^^^^^
+
+error: ambiguous inherent method name
+  --> $DIR/ambiguous_method_names.rs:27:8
+   |
+LL |     fn ambiguous_default(&self) {}
+   |        ^^^^^^^^^^^^^^^^^
+   |
+note: trait method defined here
+  --> $DIR/ambiguous_method_names.rs:11:8
+   |
+LL |     fn ambiguous_default(&self) {}
+   |        ^^^^^^^^^^^^^^^^^
+
+error: ambiguous inherent method name
+  --> $DIR/ambiguous_method_names.rs:46:8
+   |
+LL |     fn ambiguous_default(&self) {}
+   |        ^^^^^^^^^^^^^^^^^
+   |
+note: trait method defined here
+  --> $DIR/ambiguous_method_names.rs:11:8
+   |
+LL |     fn ambiguous_default(&self) {}
+   |        ^^^^^^^^^^^^^^^^^
+
+error: ambiguous inherent method name
+  --> $DIR/ambiguous_method_names.rs:29:8
+   |
+LL |     fn another(&self) {}
+   |        ^^^^^^^
+   |
+note: trait method defined here
+  --> $DIR/ambiguous_method_names.rs:15:8
+   |
+LL |     fn another(&self);
+   |        ^^^^^^^
+
+error: ambiguous inherent method name
+  --> $DIR/ambiguous_method_names.rs:47:8
+   |
+LL |     fn another(&self) {}
+   |        ^^^^^^^
+   |
+note: trait method defined here
+  --> $DIR/ambiguous_method_names.rs:15:8
+   |
+LL |     fn another(&self);
+   |        ^^^^^^^
+
+error: aborting due to 8 previous errors
+