Merge #9218

9218: Item search now respects trait impl items r=Veykril a=Veykril

Fixes #2977

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

crates/hir/src/lib.rs

@@ -1451,6 +1451,20 @@ pub fn containing_trait(self, db: &dyn HirDatabase) -> Option<Trait> {
             _ => None,
         }
     }
+
+    pub fn containing_trait_impl(self, db: &dyn HirDatabase) -> Option<Trait> {
+        match self.container(db) {
+            AssocItemContainer::Impl(i) => i.trait_(db),
+            _ => None,
+        }
+    }
+
+    pub fn containing_trait_or_trait_impl(self, db: &dyn HirDatabase) -> Option<Trait> {
+        match self.container(db) {
+            AssocItemContainer::Trait(t) => Some(t),
+            AssocItemContainer::Impl(i) => i.trait_(db),
+        }
+    }
 }
 
 impl HasVisibility for AssocItem {

crates/ide/src/references/rename.rs

@@ -239,7 +239,7 @@ fn rename_mod(
 
 fn rename_reference(
     sema: &Semantics<RootDatabase>,
-    def: Definition,
+    mut def: Definition,
     new_name: &str,
 ) -> RenameResult<SourceChange> {
     let ident_kind = check_identifier(new_name)?;
@@ -285,7 +285,38 @@ fn rename_reference(
         }
     }
 
+    def = match def {
+        // HACK: resolve trait impl items to the item def of the trait definition
+        // so that we properly resolve all trait item references
+        Definition::ModuleDef(mod_def) => mod_def
+            .as_assoc_item(sema.db)
+            .and_then(|it| it.containing_trait_impl(sema.db))
+            .and_then(|it| {
+                it.items(sema.db).into_iter().find_map(|it| match (it, mod_def) {
+                    (hir::AssocItem::Function(trait_func), ModuleDef::Function(func))
+                        if trait_func.name(sema.db) == func.name(sema.db) =>
+                    {
+                        Some(Definition::ModuleDef(ModuleDef::Function(trait_func)))
+                    }
+                    (hir::AssocItem::Const(trait_konst), ModuleDef::Const(konst))
+                        if trait_konst.name(sema.db) == konst.name(sema.db) =>
+                    {
+                        Some(Definition::ModuleDef(ModuleDef::Const(trait_konst)))
+                    }
+                    (
+                        hir::AssocItem::TypeAlias(trait_type_alias),
+                        ModuleDef::TypeAlias(type_alias),
+                    ) if trait_type_alias.name(sema.db) == type_alias.name(sema.db) => {
+                        Some(Definition::ModuleDef(ModuleDef::TypeAlias(trait_type_alias)))
+                    }
+                    _ => None,
+                })
+            })
+            .unwrap_or(def),
+        _ => def,
+    };
     let usages = def.usages(sema).all();
+
     if !usages.is_empty() && ident_kind == IdentifierKind::Underscore {
         cov_mark::hit!(rename_underscore_multiple);
         bail!("Cannot rename reference to `_` as it is being referenced multiple times");
@@ -1938,4 +1969,136 @@ fn test_rename_fails_on_aliases() {
             "error: Renaming aliases is currently unsupported",
         );
     }
+
+    #[test]
+    fn test_rename_trait_method() {
+        let res = r"
+trait Foo {
+    fn foo(&self) {
+        self.foo();
+    }
+}
+
+impl Foo for () {
+    fn foo(&self) {
+        self.foo();
+    }
+}";
+        check(
+            "foo",
+            r#"
+trait Foo {
+    fn bar$0(&self) {
+        self.bar();
+    }
+}
+
+impl Foo for () {
+    fn bar(&self) {
+        self.bar();
+    }
+}"#,
+            res,
+        );
+        check(
+            "foo",
+            r#"
+trait Foo {
+    fn bar(&self) {
+        self.bar$0();
+    }
+}
+
+impl Foo for () {
+    fn bar(&self) {
+        self.bar();
+    }
+}"#,
+            res,
+        );
+        check(
+            "foo",
+            r#"
+trait Foo {
+    fn bar(&self) {
+        self.bar();
+    }
+}
+
+impl Foo for () {
+    fn bar$0(&self) {
+        self.bar();
+    }
+}"#,
+            res,
+        );
+        check(
+            "foo",
+            r#"
+trait Foo {
+    fn bar(&self) {
+        self.bar();
+    }
+}
+
+impl Foo for () {
+    fn bar(&self) {
+        self.bar$0();
+    }
+}"#,
+            res,
+        );
+    }
+
+    #[test]
+    fn test_rename_trait_const() {
+        let res = r"
+trait Foo {
+    const FOO: ();
+}
+
+impl Foo for () {
+    const FOO: ();
+}
+fn f() { <()>::FOO; }";
+        check(
+            "FOO",
+            r#"
+trait Foo {
+    const BAR$0: ();
+}
+
+impl Foo for () {
+    const BAR: ();
+}
+fn f() { <()>::BAR; }"#,
+            res,
+        );
+        check(
+            "FOO",
+            r#"
+trait Foo {
+    const BAR: ();
+}
+
+impl Foo for () {
+    const BAR$0: ();
+}
+fn f() { <()>::BAR; }"#,
+            res,
+        );
+        check(
+            "FOO",
+            r#"
+trait Foo {
+    const BAR: ();
+}
+
+impl Foo for () {
+    const BAR: ();
+}
+fn f() { <()>::BAR$0; }"#,
+            res,
+        );
+    }
 }

crates/ide_db/src/search.rs

@@ -8,7 +8,8 @@
 
 use base_db::{FileId, FileRange, SourceDatabase, SourceDatabaseExt};
 use hir::{
-    DefWithBody, HasAttrs, HasSource, InFile, ModuleDef, ModuleSource, Semantics, Visibility,
+    AsAssocItem, DefWithBody, HasAttrs, HasSource, InFile, ModuleDef, ModuleSource, Semantics,
+    Visibility,
 };
 use once_cell::unsync::Lazy;
 use rustc_hash::FxHashMap;
@@ -303,13 +304,13 @@ fn search_scope(&self, db: &RootDatabase) -> SearchScope {
         }
     }
 
-    pub fn usages<'a>(&'a self, sema: &'a Semantics<RootDatabase>) -> FindUsages<'a> {
+    pub fn usages<'a>(self, sema: &'a Semantics<RootDatabase>) -> FindUsages<'a> {
         FindUsages { def: self, sema, scope: None, include_self_kw_refs: None }
     }
 }
 
 pub struct FindUsages<'a> {
-    def: &'a Definition,
+    def: Definition,
     sema: &'a Semantics<'a, RootDatabase>,
     scope: Option<SearchScope>,
     include_self_kw_refs: Option<hir::Type>,
@@ -318,7 +319,7 @@ pub struct FindUsages<'a> {
 impl<'a> FindUsages<'a> {
     /// Enable searching for `Self` when the definition is a type.
     pub fn include_self_refs(mut self) -> FindUsages<'a> {
-        self.include_self_kw_refs = def_to_ty(self.sema, self.def);
+        self.include_self_kw_refs = def_to_ty(self.sema, &self.def);
         self
     }
 
@@ -445,7 +446,7 @@ fn found_lifetime(
         sink: &mut dyn FnMut(FileId, FileReference) -> bool,
     ) -> bool {
         match NameRefClass::classify_lifetime(self.sema, lifetime) {
-            Some(NameRefClass::Definition(def)) if &def == self.def => {
+            Some(NameRefClass::Definition(def)) if def == self.def => {
                 let FileRange { file_id, range } = self.sema.original_range(lifetime.syntax());
                 let reference = FileReference {
                     range,
@@ -464,7 +465,7 @@ fn found_name_ref(
         sink: &mut dyn FnMut(FileId, FileReference) -> bool,
     ) -> bool {
         match NameRefClass::classify(self.sema, &name_ref) {
-            Some(NameRefClass::Definition(def)) if &def == self.def => {
+            Some(NameRefClass::Definition(def)) if def == self.def => {
                 let FileRange { file_id, range } = self.sema.original_range(name_ref.syntax());
                 let reference = FileReference {
                     range,
@@ -489,10 +490,10 @@ fn found_name_ref(
             Some(NameRefClass::FieldShorthand { local_ref: local, field_ref: field }) => {
                 let FileRange { file_id, range } = self.sema.original_range(name_ref.syntax());
                 let access = match self.def {
-                    Definition::Field(_) if &field == self.def => {
+                    Definition::Field(_) if field == self.def => {
                         reference_access(&field, &name_ref)
                     }
-                    Definition::Local(l) if &local == l => {
+                    Definition::Local(l) if local == l => {
                         reference_access(&Definition::Local(local), &name_ref)
                     }
                     _ => return false,
@@ -513,7 +514,7 @@ fn found_name(
         match NameClass::classify(self.sema, name) {
             Some(NameClass::PatFieldShorthand { local_def: _, field_ref })
                 if matches!(
-                    self.def, Definition::Field(_) if &field_ref == self.def
+                    self.def, Definition::Field(_) if field_ref == self.def
                 ) =>
             {
                 let FileRange { file_id, range } = self.sema.original_range(name.syntax());
@@ -525,12 +526,38 @@ fn found_name(
                 };
                 sink(file_id, reference)
             }
-            Some(NameClass::ConstReference(def)) if *self.def == def => {
+            Some(NameClass::ConstReference(def)) if self.def == def => {
                 let FileRange { file_id, range } = self.sema.original_range(name.syntax());
                 let reference =
                     FileReference { range, name: ast::NameLike::Name(name.clone()), access: None };
                 sink(file_id, reference)
             }
+            // Resolve trait impl function definitions to the trait definition's version if self.def is the trait definition's
+            Some(NameClass::Definition(Definition::ModuleDef(mod_def))) => {
+                /* poor man's try block */
+                (|| {
+                    let this = match self.def {
+                        Definition::ModuleDef(this) if this != mod_def => this,
+                        _ => return None,
+                    };
+                    let this_trait = this
+                        .as_assoc_item(self.sema.db)?
+                        .containing_trait_or_trait_impl(self.sema.db)?;
+                    let trait_ = mod_def
+                        .as_assoc_item(self.sema.db)?
+                        .containing_trait_or_trait_impl(self.sema.db)?;
+                    (trait_ == this_trait).then(|| {
+                        let FileRange { file_id, range } = self.sema.original_range(name.syntax());
+                        let reference = FileReference {
+                            range,
+                            name: ast::NameLike::Name(name.clone()),
+                            access: None,
+                        };
+                        sink(file_id, reference)
+                    })
+                })()
+                .unwrap_or(false)
+            }
             _ => false,
         }
     }