Fix call site checking FPs

Author: sjwang05

clippy_lints/src/ambiguous_method_calls.rs

@@ -6,6 +6,7 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::intravisit::FnKind;
 use rustc_hir::{Body, FnDecl};
 use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::Ty;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::def_id::LocalDefId;
 use rustc_span::symbol::Ident;
@@ -69,7 +70,7 @@ declare_clippy_lint! {
     #[clippy::version = "1.74.0"]
     pub AMBIGUOUS_METHOD_CALLS,
     pedantic,
-    "same-named methods in struct impls and trait impls"
+    "declarations and calls for same-named methods in struct impls and trait impls"
 }
 declare_lint_pass!(AmbiguousMethodCalls => [AMBIGUOUS_METHOD_CALLS]);
 
@@ -86,25 +87,27 @@ impl<'tcx> LateLintPass<'tcx> for AmbiguousMethodCalls {
         let hir_id = cx.tcx.hir().local_def_id_to_hir_id(def_id);
         let is_trait_impl = is_trait_impl_item(cx, hir_id);
 
-        // Check methods in trait impls and struct impls
         if let FnKind::Method(ident, _) = kind {
-            // FIXME: also keep track of the Ty of the struct for call site checking
-            insert_method(is_trait_impl, ident);
+            let parent_item = cx.tcx.hir().get_parent_item(hir_id).to_def_id();
+            let parent_type = cx.tcx.type_of(parent_item).skip_binder();
+            let parent_ty_str = format!("{parent_type}");
 
-            if has_ambiguous_name(ident) {
+            insert_method(is_trait_impl, parent_type, ident);
+
+            if has_ambiguous_name(parent_type, ident) {
                 let trait_methods = trait_methods().lock().unwrap();
                 let struct_methods = struct_methods().lock().unwrap();
 
                 span_lint(
                     cx,
                     AMBIGUOUS_METHOD_CALLS,
-                    trait_methods.get(&ident.name).unwrap().span(),
+                    trait_methods.get(&(parent_ty_str.clone(), ident.name)).unwrap().span(),
                     "ambiguous trait method name",
                 );
                 span_lint_and_help(
                     cx,
                     AMBIGUOUS_METHOD_CALLS,
-                    struct_methods.get(&ident.name).unwrap().span(),
+                    struct_methods.get(&(parent_ty_str, ident.name)).unwrap().span(),
                     "ambiguous struct method name",
                     None,
                     "consider renaming the struct impl's method",
@@ -114,10 +117,15 @@ impl<'tcx> LateLintPass<'tcx> for AmbiguousMethodCalls {
     }
 
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'_>) {
-        if let rustc_hir::ExprKind::MethodCall(path, _receiver, _, call_span) = &expr.kind {
+        if let rustc_hir::ExprKind::MethodCall(path, receiver, _, call_span) = &expr.kind {
+            let recv_ty = cx.typeck_results().expr_ty(receiver).peel_refs();
+            let recv_ty_str = format!("{recv_ty}");
+
             let struct_methods = struct_methods().lock().unwrap();
-            if struct_methods.contains_key(&path.ident.name) {
-                // FIXME: only report after checking receiver's Ty
+            let trait_methods = trait_methods().lock().unwrap();
+            if struct_methods.contains_key(&(recv_ty_str.clone(), path.ident.name))
+                && trait_methods.contains_key(&(recv_ty_str, path.ident.name))
+            {
                 span_lint_and_help(
                     cx,
                     AMBIGUOUS_METHOD_CALLS,
@@ -131,30 +139,32 @@ impl<'tcx> LateLintPass<'tcx> for AmbiguousMethodCalls {
     }
 }
 
-fn has_ambiguous_name(ident: Ident) -> bool {
+fn has_ambiguous_name<'tcx>(ty: Ty<'tcx>, ident: Ident) -> bool {
+    let ty_str = format!("{ty}");
     let trait_methods = trait_methods().lock().unwrap();
     let struct_methods = struct_methods().lock().unwrap();
 
-    trait_methods.contains_key(&ident.name) && struct_methods.contains_key(&ident.name)
+    trait_methods.contains_key(&(ty_str.clone(), ident.name)) && struct_methods.contains_key(&(ty_str, ident.name))
 }
 
-fn trait_methods() -> &'static Mutex<FxHashMap<Symbol, SpanData>> {
-    static NAMES: OnceLock<Mutex<FxHashMap<Symbol, SpanData>>> = OnceLock::new();
+fn trait_methods() -> &'static Mutex<FxHashMap<(String, Symbol), SpanData>> {
+    static NAMES: OnceLock<Mutex<FxHashMap<(String, Symbol), SpanData>>> = OnceLock::new();
     NAMES.get_or_init(|| Mutex::new(FxHashMap::default()))
 }
 
-fn struct_methods() -> &'static Mutex<FxHashMap<Symbol, SpanData>> {
-    static NAMES: OnceLock<Mutex<FxHashMap<Symbol, SpanData>>> = OnceLock::new();
+fn struct_methods() -> &'static Mutex<FxHashMap<(String, Symbol), SpanData>> {
+    static NAMES: OnceLock<Mutex<FxHashMap<(String, Symbol), SpanData>>> = OnceLock::new();
     NAMES.get_or_init(|| Mutex::new(FxHashMap::default()))
 }
 
-fn insert_method(is_trait_impl: bool, ident: Ident) {
+fn insert_method<'tcx>(is_trait_impl: bool, ty: Ty<'tcx>, ident: Ident) {
+    let ty_str = format!("{ty}");
     let mut trait_methods = trait_methods().lock().unwrap();
     let mut struct_methods = struct_methods().lock().unwrap();
 
     if is_trait_impl {
-        trait_methods.insert(ident.name, ident.span.data());
+        trait_methods.insert((ty_str, ident.name), ident.span.data());
     } else {
-        struct_methods.insert(ident.name, ident.span.data());
+        struct_methods.insert((ty_str, ident.name), ident.span.data());
     }
 }

tests/ui/ambiguous_method_calls.rs

@@ -3,10 +3,9 @@
 
 trait MyTrait {
     fn ambiguous(&self);
-    fn not_ambiguous(&self);
 }
 
-struct Base {}
+struct Base;
 
 impl Base {
     fn ambiguous(&self) {
@@ -22,14 +21,17 @@ impl MyTrait for Base {
     fn ambiguous(&self) {
         println!("trait impl");
     }
+}
+
+struct Other;
 
-    fn not_ambiguous(&self) {
-        println!("not ambiguous trait impl");
+impl MyTrait for Other {
+    fn ambiguous(&self) {
+        println!("not actually ambiguous")
     }
 }
 
 fn main() {
-    let data = Base {};
-
-    data.ambiguous();
+    Base.ambiguous();
+    Other.ambiguous();
 }