feat: Generate method from call

[mahdifrmz]

Needs a bit of refactoring. Tests also should be added.

[matklad]

Moves in the right direction!

It needs tests though :-) I'd even go as far as saying that, for assists, the tests are actually more important than the code.

Assists code is usually inherently messy, and that's exacerbated by fact that our APIs in this area are rather far from being frozen. So, I'd expect us to re-write every assist in the coming years.

On the other hand, the tests are here to stay -- they specify the final behavior that we want, and they are written in a way that's independent of the actual assist API.

I'd also say it's easier to iterate on the code when you run an isolated test -- re-running the test is much faster than re-running the whole of rust-analyzer.

[mahdifrmz]

Making them one function will complicate things a bit for now. Or maybe you want them to have one id?

[mahdifrmz]

It needs tests though :-)

Done :-)

[matklad]

Looks good to me overall. I suggest applying the following diff as well (you can copy it and use git apply):

diff --git a/crates/ide_assists/src/handlers/generate_function.rs b/crates/ide_assists/src/handlers/generate_function.rs
index 39756611d..cc1302e4a 100644
--- a/crates/ide_assists/src/handlers/generate_function.rs
+++ b/crates/ide_assists/src/handlers/generate_function.rs
@@ -6,7 +6,7 @@ use syntax::{
     ast::{
         self,
         edit::{AstNodeEdit, IndentLevel},
-        make, ArgList, ArgListOwner, AstNode, ModuleItemOwner,
+        make, ArgListOwner, AstNode, ModuleItemOwner,
     },
     SyntaxKind, SyntaxNode, TextSize,
 };
@@ -17,27 +17,6 @@ use crate::{
     AssistContext, AssistId, AssistKind, Assists,
 };
 
-enum FuncExpr {
-    Func(ast::CallExpr),
-    Method(ast::MethodCallExpr),
-}
-
-impl FuncExpr {
-    fn arg_list(&self) -> Option<ArgList> {
-        match self {
-            FuncExpr::Func(fn_call) => fn_call.arg_list(),
-            FuncExpr::Method(m_call) => m_call.arg_list(),
-        }
-    }
-
-    fn syntax(&self) -> &SyntaxNode {
-        match self {
-            FuncExpr::Func(fn_call) => fn_call.syntax(),
-            FuncExpr::Method(m_call) => m_call.syntax(),
-        }
-    }
-}
-
 // Assist: generate_function
 //
 // Adds a stub function with a signature matching the function under the cursor.
@@ -64,6 +43,31 @@ impl FuncExpr {
 //
 // ```
 pub(crate) fn generate_function(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
+    gen_fn(acc, ctx).or_else(|| gen_method(acc, ctx))
+}
+
+enum FuncExpr {
+    Func(ast::CallExpr),
+    Method(ast::MethodCallExpr),
+}
+
+impl FuncExpr {
+    fn arg_list(&self) -> Option<ast::ArgList> {
+        match self {
+            FuncExpr::Func(fn_call) => fn_call.arg_list(),
+            FuncExpr::Method(m_call) => m_call.arg_list(),
+        }
+    }
+
+    fn syntax(&self) -> &SyntaxNode {
+        match self {
+            FuncExpr::Func(fn_call) => fn_call.syntax(),
+            FuncExpr::Method(m_call) => m_call.syntax(),
+        }
+    }
+}
+
+fn gen_fn(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let path_expr: ast::PathExpr = ctx.find_node_at_offset()?;
     let call = path_expr.syntax().parent().and_then(ast::CallExpr::cast)?;
 
@@ -100,7 +104,7 @@ pub(crate) fn generate_function(acc: &mut Assists, ctx: &AssistContext) -> Optio
     )
 }
 
-pub(crate) fn generate_method(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
+fn gen_method(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let call: ast::MethodCallExpr = ctx.find_node_at_offset()?;
     let fn_name: ast::NameRef = ast::NameRef::cast(
         call.syntax().children().find(|child| child.kind() == SyntaxKind::NAME_REF)?,
@@ -1389,7 +1393,7 @@ async fn bar(arg: i32) ${0:-> ()} {
     #[test]
     fn create_method() {
         check_assist(
-            generate_method,
+            generate_function,
             r"
 struct S;
 
@@ -1419,7 +1423,7 @@ fn bar(&self) ${0:-> ()} {
     #[test]
     fn create_method_within_an_impl() {
         check_assist(
-            generate_method,
+            generate_function,
             r"
 struct S;
 
@@ -1448,7 +1452,7 @@ impl S {
     #[test]
     fn create_method_from_different_module() {
         check_assist(
-            generate_method,
+            generate_function,
             r"
 mod s {
     pub struct S;
@@ -1480,7 +1484,7 @@ fn foo() {
     #[test]
     fn create_method_from_descendant_module() {
         check_assist(
-            generate_method,
+            generate_function,
             r"
 struct S;
 mod s {
@@ -1512,7 +1516,7 @@ fn bar(&self) ${0:-> ()} {
     #[test]
     fn create_method_with_cursor_anywhere_on_call_expresion() {
         check_assist(
-            generate_method,
+            generate_function,
             r"
 struct S;
 
diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs
index 30b57ade1..bb9d5dd99 100644
--- a/crates/ide_assists/src/lib.rs
+++ b/crates/ide_assists/src/lib.rs
@@ -151,7 +151,6 @@ mod handlers {
             generate_enum_projection_method::generate_enum_try_into_method,
             generate_from_impl_for_enum::generate_from_impl_for_enum,
             generate_function::generate_function,
-            generate_function::generate_method,
             generate_getter::generate_getter,
             generate_getter::generate_getter_mut,
             generate_impl::generate_impl,

It does three things:

• change the code to export only one function. I'd say, in this specific case, just exporting two functions is maybe more convenient, but there's an extra consistency value in the fact that each assist file exports only one function.
• change use ArgList to ast::ArgList, as per style guide
• move pub(crate) function declaration before struct declaration, as per style-guide (structs generally go first, excpept for the case where structs are private, and there's a public function)

[mahdifrmz]

Applied the changes.

[matklad]

bors r+

Thanks!

(you might want to record & attach a gif showcasing the feature for next week's changelog)

[lnicola]

(you might want to record & attach a gif showcasing the feature for next week's changelog)

It's also fine if you don't, someone else will make one.

[bors]

Build failed:

• Rust (ubuntu-latest)

[mahdifrmz]

[lnicola]

bors retry

[bors]

Build succeeded:

• Rust (macos-latest)
• Rust (ubuntu-latest)
• Rust (windows-latest)
• TypeScript (ubuntu-latest)
• TypeScript (windows-latest)