Merge #11481

[rust.git] / crates / ide_assists / src / handlers / remove_unused_param.rs

diff --git a/crates/ide_assists/src/handlers/remove_unused_param.rs b/crates/ide_assists/src/handlers/remove_unused_param.rs
index 2699d2861c60155df4f4517eb28678f3d7b3bf48..80e2ca918b94b388da1a3b1d15b6a45c2dd68b9b 100644 (file)
--- a/crates/ide_assists/src/handlers/remove_unused_param.rs
+++ b/crates/ide_assists/src/handlers/remove_unused_param.rs
@@ -1,7 +1,7 @@
 use ide_db::{base_db::FileId, defs::Definition, search::FileReference};
 use syntax::{
     algo::find_node_at_range,
-    ast::{self, ArgListOwner},
+    ast::{self, HasArgList},
     AstNode, SourceFile, SyntaxKind, SyntaxNode, TextRange, T,
 };
 
@@ -37,11 +37,32 @@ pub(crate) fn remove_unused_param(acc: &mut Assists, ctx: &AssistContext) -> Opt
         _ => return None,
     };
     let func = param.syntax().ancestors().find_map(ast::Fn::cast)?;
-    let param_position = func.param_list()?.params().position(|it| it == param)?;
+    let is_self_present =
+        param.syntax().parent()?.children().find_map(ast::SelfParam::cast).is_some();
 
+    // check if fn is in impl Trait for ..
+    if func
+        .syntax()
+        .parent() // AssocItemList
+        .and_then(|x| x.parent())
+        .and_then(ast::Impl::cast)
+        .map_or(false, |imp| imp.trait_().is_some())
+    {
+        cov_mark::hit!(trait_impl);
+        return None;
+    }
+
+    let mut param_position = func.param_list()?.params().position(|it| it == param)?;
+    // param_list() does not take the self param into consideration, hence this additional check
+    // is required. For associated functions, param_position is incremented here. For inherent
+    // calls we revet the increment below, in process_usage, as those calls will not have an
+    // explicit self parameter.
+    if is_self_present {
+        param_position += 1;
+    }
     let fn_def = {
         let func = ctx.sema.to_def(&func)?;
-        Definition::ModuleDef(func.into())
+        Definition::Function(func)
     };
 
     let param_def = {
@@ -59,7 +80,7 @@ pub(crate) fn remove_unused_param(acc: &mut Assists, ctx: &AssistContext) -> Opt
         |builder| {
             builder.delete(range_to_remove(param.syntax()));
             for (file_id, references) in fn_def.usages(&ctx.sema).all() {
-                process_usages(ctx, builder, file_id, references, param_position);
+                process_usages(ctx, builder, file_id, references, param_position, is_self_present);
             }
         },
     )
@@ -71,11 +92,13 @@ fn process_usages(
     file_id: FileId,
     references: Vec<FileReference>,
     arg_to_remove: usize,
+    is_self_present: bool,
 ) {
     let source_file = ctx.sema.parse(file_id);
     builder.edit_file(file_id);
     for usage in references {
-        if let Some(text_range) = process_usage(&source_file, usage, arg_to_remove) {
+        if let Some(text_range) = process_usage(&source_file, usage, arg_to_remove, is_self_present)
+        {
             builder.delete(text_range);
         }
     }
@@ -84,18 +107,40 @@ fn process_usages(
 fn process_usage(
     source_file: &SourceFile,
     FileReference { range, .. }: FileReference,
-    arg_to_remove: usize,
+    mut arg_to_remove: usize,
+    is_self_present: bool,
 ) -> Option<TextRange> {
-    let call_expr: ast::CallExpr = find_node_at_range(source_file.syntax(), range)?;
-    let call_expr_range = call_expr.expr()?.syntax().text_range();
-    if !call_expr_range.contains_range(range) {
-        return None;
+    let call_expr_opt: Option<ast::CallExpr> = find_node_at_range(source_file.syntax(), range);
+    if let Some(call_expr) = call_expr_opt {
+        let call_expr_range = call_expr.expr()?.syntax().text_range();
+        if !call_expr_range.contains_range(range) {
+            return None;
+        }
+
+        let arg = call_expr.arg_list()?.args().nth(arg_to_remove)?;
+        return Some(range_to_remove(arg.syntax()));
     }
-    let arg = call_expr.arg_list()?.args().nth(arg_to_remove)?;
-    Some(range_to_remove(arg.syntax()))
+
+    let method_call_expr_opt: Option<ast::MethodCallExpr> =
+        find_node_at_range(source_file.syntax(), range);
+    if let Some(method_call_expr) = method_call_expr_opt {
+        let method_call_expr_range = method_call_expr.name_ref()?.syntax().text_range();
+        if !method_call_expr_range.contains_range(range) {
+            return None;
+        }
+
+        if is_self_present {
+            arg_to_remove -= 1;
+        }
+
+        let arg = method_call_expr.arg_list()?.args().nth(arg_to_remove)?;
+        return Some(range_to_remove(arg.syntax()));
+    }
+
+    None
 }
 
-fn range_to_remove(node: &SyntaxNode) -> TextRange {
+pub(crate) fn range_to_remove(node: &SyntaxNode) -> TextRange {
     let up_to_comma = next_prev().find_map(|dir| {
         node.siblings_with_tokens(dir)
             .filter_map(|it| it.into_token())
@@ -253,6 +298,22 @@ fn main() { foo(9, 2) }
         );
     }
 
+    #[test]
+    fn trait_impl() {
+        cov_mark::check!(trait_impl);
+        check_assist_not_applicable(
+            remove_unused_param,
+            r#"
+trait Trait {
+    fn foo(x: i32);
+}
+impl Trait for () {
+    fn foo($0x: i32) {}
+}
+"#,
+        );
+    }
+
     #[test]
     fn remove_across_files() {
         check_assist(
@@ -282,6 +343,33 @@ fn bar() {
 fn bar() {
     let _ = foo(1);
 }
+"#,
+        )
+    }
+
+    #[test]
+    fn test_remove_method_param() {
+        check_assist(
+            remove_unused_param,
+            r#"
+struct S;
+impl S { fn f(&self, $0_unused: i32) {} }
+fn main() {
+    S.f(92);
+    S.f();
+    S.f(93, 92);
+    S::f(&S, 92);
+}
+"#,
+            r#"
+struct S;
+impl S { fn f(&self) {} }
+fn main() {
+    S.f();
+    S.f();
+    S.f(92);
+    S::f(&S);
+}
 "#,
         )
     }