Merge pull request #20351 from ChayimFriedman2/rename-self1

feat: When renaming a parameter to `self`, change callers to use method call syntax

crates/base-db/src/lib.rs

@@ -206,6 +206,7 @@ impl EditionedFileId {
 
 #[salsa_macros::input(debug)]
 pub struct FileText {
+    #[returns(ref)]
     pub text: Arc<str>,
     pub file_id: vfs::FileId,
 }
@@ -357,7 +358,7 @@ fn parse(db: &dyn RootQueryDb, file_id: EditionedFileId) -> Parse<ast::SourceFil
     let _p = tracing::info_span!("parse", ?file_id).entered();
     let (file_id, edition) = file_id.unpack(db.as_dyn_database());
     let text = db.file_text(file_id).text(db);
-    ast::SourceFile::parse(&text, edition)
+    ast::SourceFile::parse(text, edition)
 }
 
 fn parse_errors(db: &dyn RootQueryDb, file_id: EditionedFileId) -> Option<&[SyntaxError]> {

crates/hir-expand/src/builtin/fn_macro.rs

@@ -890,7 +890,7 @@ fn include_str_expand(
     };
 
     let text = db.file_text(file_id.file_id(db));
-    let text = &*text.text(db);
+    let text = &**text.text(db);
 
     ExpandResult::ok(quote!(call_site =>#text))
 }

crates/hir-expand/src/files.rs

@@ -99,6 +99,16 @@ impl FileRange {
     pub fn into_file_id(self, db: &dyn ExpandDatabase) -> FileRangeWrapper<FileId> {
         FileRangeWrapper { file_id: self.file_id.file_id(db), range: self.range }
     }
+
+    #[inline]
+    pub fn file_text(self, db: &dyn ExpandDatabase) -> &triomphe::Arc<str> {
+        db.file_text(self.file_id.file_id(db)).text(db)
+    }
+
+    #[inline]
+    pub fn text(self, db: &dyn ExpandDatabase) -> &str {
+        &self.file_text(db)[self.range]
+    }
 }
 
 /// `AstId` points to an AST node in any file.

crates/hir-ty/src/test_db.rs

@@ -149,7 +149,7 @@ impl TestDB {
             .into_iter()
             .filter_map(|file_id| {
                 let text = self.file_text(file_id.file_id(self));
-                let annotations = extract_annotations(&text.text(self));
+                let annotations = extract_annotations(text.text(self));
                 if annotations.is_empty() {
                     return None;
                 }

crates/ide-db/src/lib.rs

@@ -244,7 +244,7 @@ pub trait LineIndexDatabase: base_db::RootQueryDb {
 
 fn line_index(db: &dyn LineIndexDatabase, file_id: FileId) -> Arc<LineIndex> {
     let text = db.file_text(file_id).text(db);
-    Arc::new(LineIndex::new(&text))
+    Arc::new(LineIndex::new(text))
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]

crates/ide-db/src/search.rs

@@ -487,9 +487,9 @@ impl<'a> FindUsages<'a> {
         scope.entries.iter().map(|(&file_id, &search_range)| {
             let text = db.file_text(file_id.file_id(db)).text(db);
             let search_range =
-                search_range.unwrap_or_else(|| TextRange::up_to(TextSize::of(&*text)));
+                search_range.unwrap_or_else(|| TextRange::up_to(TextSize::of(&**text)));
 
-            (text, file_id, search_range)
+            (text.clone(), file_id, search_range)
         })
     }
 
@@ -854,14 +854,7 @@ impl<'a> FindUsages<'a> {
                 &finder,
                 name,
                 is_possibly_self.into_iter().map(|position| {
-                    (
+                    (position.file_text(self.sema.db).clone(), position.file_id, position.range)
-                        self.sema
-                            .db
-                            .file_text(position.file_id.file_id(self.sema.db))
-                            .text(self.sema.db),
-                        position.file_id,
-                        position.range,
-                    )
                 }),
                 |path, name_position| {
                     let has_self = path
@@ -1067,12 +1060,12 @@ impl<'a> FindUsages<'a> {
                 let file_text = sema.db.file_text(file_id.file_id(self.sema.db));
                 let text = file_text.text(sema.db);
                 let search_range =
-                    search_range.unwrap_or_else(|| TextRange::up_to(TextSize::of(&*text)));
+                    search_range.unwrap_or_else(|| TextRange::up_to(TextSize::of(&**text)));
 
                 let tree = LazyCell::new(|| sema.parse(file_id).syntax().clone());
                 let finder = &Finder::new("self");
 
-                for offset in Self::match_indices(&text, finder, search_range) {
+                for offset in Self::match_indices(text, finder, search_range) {
                     for name_ref in Self::find_nodes(sema, "self", file_id, &tree, offset)
                         .filter_map(ast::NameRef::cast)
                     {

crates/ide-diagnostics/src/tests.rs

@@ -229,7 +229,7 @@ pub(crate) fn check_diagnostics_with_config(
         let line_index = db.line_index(file_id);
 
         let mut actual = annotations.remove(&file_id).unwrap_or_default();
-        let mut expected = extract_annotations(&db.file_text(file_id).text(&db));
+        let mut expected = extract_annotations(db.file_text(file_id).text(&db));
         expected.sort_by_key(|(range, s)| (range.start(), s.clone()));
         actual.sort_by_key(|(range, s)| (range.start(), s.clone()));
         // FIXME: We should panic on duplicates instead, but includes currently cause us to report

crates/ide-ssr/src/lib.rs

@@ -186,7 +186,7 @@ impl<'db> MatchFinder<'db> {
                     replacing::matches_to_edit(
                         self.sema.db,
                         &matches,
-                        &self.sema.db.file_text(file_id).text(self.sema.db),
+                        self.sema.db.file_text(file_id).text(self.sema.db),
                         &self.rules,
                     ),
                 )
@@ -228,7 +228,7 @@ impl<'db> MatchFinder<'db> {
         let file = self.sema.parse(file_id);
         let mut res = Vec::new();
         let file_text = self.sema.db.file_text(file_id.file_id(self.sema.db)).text(self.sema.db);
-        let mut remaining_text = &*file_text;
+        let mut remaining_text = &**file_text;
         let mut base = 0;
         let len = snippet.len() as u32;
         while let Some(offset) = remaining_text.find(snippet) {

crates/ide/src/lib.rs

@@ -299,7 +299,7 @@ impl Analysis {
 
     /// Gets the text of the source file.
     pub fn file_text(&self, file_id: FileId) -> Cancellable<Arc<str>> {
-        self.with_db(|db| SourceDatabase::file_text(db, file_id).text(db))
+        self.with_db(|db| SourceDatabase::file_text(db, file_id).text(db).clone())
     }
 
     /// Gets the syntax tree of the file.

crates/ide/src/rename.rs

@@ -13,8 +13,11 @@ use ide_db::{
 };
 use itertools::Itertools;
 use std::fmt::Write;
-use stdx::{always, never};
+use stdx::{always, format_to, never};
-use syntax::{AstNode, SyntaxKind, SyntaxNode, TextRange, TextSize, ast};
+use syntax::{
+    AstNode, SyntaxKind, SyntaxNode, TextRange, TextSize,
+    ast::{self, HasArgList, prec::ExprPrecedence},
+};
 
 use ide_db::text_edit::TextEdit;
 
@@ -331,6 +334,85 @@ fn find_definitions(
     }
 }
 
+fn transform_assoc_fn_into_method_call(
+    sema: &Semantics<'_, RootDatabase>,
+    source_change: &mut SourceChange,
+    f: hir::Function,
+) {
+    let calls = Definition::Function(f).usages(sema).all();
+    for (file_id, calls) in calls {
+        for call in calls {
+            let Some(fn_name) = call.name.as_name_ref() else { continue };
+            let Some(path) = fn_name.syntax().parent().and_then(ast::PathSegment::cast) else {
+                continue;
+            };
+            let path = path.parent_path();
+            // The `PathExpr` is the direct parent, above it is the `CallExpr`.
+            let Some(call) =
+                path.syntax().parent().and_then(|it| ast::CallExpr::cast(it.parent()?))
+            else {
+                continue;
+            };
+
+            let Some(arg_list) = call.arg_list() else { continue };
+            let mut args = arg_list.args();
+            let Some(mut self_arg) = args.next() else { continue };
+            let second_arg = args.next();
+
+            // Strip (de)references, as they will be taken automatically by auto(de)ref.
+            loop {
+                let self_ = match &self_arg {
+                    ast::Expr::RefExpr(self_) => self_.expr(),
+                    ast::Expr::ParenExpr(self_) => self_.expr(),
+                    ast::Expr::PrefixExpr(self_)
+                        if self_.op_kind() == Some(ast::UnaryOp::Deref) =>
+                    {
+                        self_.expr()
+                    }
+                    _ => break,
+                };
+                self_arg = match self_ {
+                    Some(it) => it,
+                    None => break,
+                };
+            }
+
+            let self_needs_parens =
+                self_arg.precedence().needs_parentheses_in(ExprPrecedence::Postfix);
+
+            let replace_start = path.syntax().text_range().start();
+            let replace_end = match second_arg {
+                Some(second_arg) => second_arg.syntax().text_range().start(),
+                None => arg_list
+                    .r_paren_token()
+                    .map(|it| it.text_range().start())
+                    .unwrap_or_else(|| arg_list.syntax().text_range().end()),
+            };
+            let replace_range = TextRange::new(replace_start, replace_end);
+
+            let Some(macro_mapped_self) = sema.original_range_opt(self_arg.syntax()) else {
+                continue;
+            };
+            let mut replacement = String::new();
+            if self_needs_parens {
+                replacement.push('(');
+            }
+            replacement.push_str(macro_mapped_self.text(sema.db));
+            if self_needs_parens {
+                replacement.push(')');
+            }
+            replacement.push('.');
+            format_to!(replacement, "{fn_name}");
+            replacement.push('(');
+
+            source_change.insert_source_edit(
+                file_id.file_id(sema.db),
+                TextEdit::replace(replace_range, replacement),
+            );
+        }
+    }
+}
+
 fn rename_to_self(
     sema: &Semantics<'_, RootDatabase>,
     local: hir::Local,
@@ -408,6 +490,7 @@ fn rename_to_self(
         file_id.original_file(sema.db).file_id(sema.db),
         TextEdit::replace(param_source.syntax().text_range(), String::from(self_param)),
     );
+    transform_assoc_fn_into_method_call(sema, &mut source_change, fn_def);
     Ok(source_change)
 }
 
@@ -3412,4 +3495,78 @@ fn other_place() { Quux::Bar$0; }
 "#,
         );
     }
+
+    #[test]
+    fn rename_to_self_callers() {
+        check(
+            "self",
+            r#"
+//- minicore: add
+struct Foo;
+impl core::ops::Add for Foo {
+    type Target = Foo;
+    fn add(self, _: Self) -> Foo { Foo }
+}
+
+impl Foo {
+    fn foo(th$0is: &Self) {}
+}
+
+fn bar(v: &Foo) {
+    Foo::foo(v);
+}
+
+fn baz() {
+    Foo::foo(&Foo);
+    Foo::foo(Foo + Foo);
+}
+        "#,
+            r#"
+struct Foo;
+impl core::ops::Add for Foo {
+    type Target = Foo;
+    fn add(self, _: Self) -> Foo { Foo }
+}
+
+impl Foo {
+    fn foo(&self) {}
+}
+
+fn bar(v: &Foo) {
+    v.foo();
+}
+
+fn baz() {
+    Foo.foo();
+    (Foo + Foo).foo();
+}
+        "#,
+        );
+        // Multiple arguments:
+        check(
+            "self",
+            r#"
+struct Foo;
+
+impl Foo {
+    fn foo(th$0is: &Self, v: i32) {}
+}
+
+fn bar(v: Foo) {
+    Foo::foo(&v, 123);
+}
+        "#,
+            r#"
+struct Foo;
+
+impl Foo {
+    fn foo(&self, v: i32) {}
+}
+
+fn bar(v: Foo) {
+    v.foo(123);
+}
+        "#,
+        );
+    }
 }

crates/rust-analyzer/src/global_state.rs

@@ -448,7 +448,7 @@ impl GlobalState {
                     tracing::info!(%vfs_path, ?change_kind, "Processing rust-analyzer.toml changes");
                     if vfs_path.as_path() == user_config_abs_path {
                         tracing::info!(%vfs_path, ?change_kind, "Use config rust-analyzer.toml changes");
-                        change.change_user_config(Some(db.file_text(file_id).text(db)));
+                        change.change_user_config(Some(db.file_text(file_id).text(db).clone()));
                     }
 
                     // If change has been made to a ratoml file that
@@ -462,14 +462,14 @@ impl GlobalState {
                             change.change_workspace_ratoml(
                                 source_root_id,
                                 vfs_path.clone(),
-                                Some(db.file_text(file_id).text(db)),
+                                Some(db.file_text(file_id).text(db).clone()),
                             )
                         } else {
                             tracing::info!(%vfs_path, ?source_root_id, "crate rust-analyzer.toml changes");
                             change.change_ratoml(
                                 source_root_id,
                                 vfs_path.clone(),
-                                Some(db.file_text(file_id).text(db)),
+                                Some(db.file_text(file_id).text(db).clone()),
                             )
                         };