From e457759cbb6905c0078059a0bb8a23fdf9aac1eb Mon Sep 17 00:00:00 2001 From: vsrs Date: Mon, 28 Aug 2023 15:23:20 +0700 Subject: [PATCH 1/5] Add bind_unused_param assistant. --- .../src/handlers/bind_unused_param.rs | 130 ++++++++++++++++++ .../src/handlers/remove_unused_param.rs | 16 ++- crates/ide-assists/src/lib.rs | 2 + crates/ide-assists/src/tests/generated.rs | 15 ++ 4 files changed, 156 insertions(+), 7 deletions(-) create mode 100644 crates/ide-assists/src/handlers/bind_unused_param.rs diff --git a/crates/ide-assists/src/handlers/bind_unused_param.rs b/crates/ide-assists/src/handlers/bind_unused_param.rs new file mode 100644 index 0000000000..0a22d26193 --- /dev/null +++ b/crates/ide-assists/src/handlers/bind_unused_param.rs @@ -0,0 +1,130 @@ +use crate::assist_context::{AssistContext, Assists}; +use ide_db::{ + assists::{AssistId, AssistKind}, + defs::Definition, + LineIndexDatabase, +}; +use syntax::{ + ast::{self, edit_in_place::Indent}, + AstNode, +}; + +use super::remove_unused_param::is_trait_impl; + +// Assist: bind_unused_param +// +// Binds unused function parameter to an underscore. +// +// ``` +// fn some_function(x: i32$0) {} +// ``` +// -> +// ``` +// fn some_function(x: i32) { +// let _ = x; +// } +// ``` +pub(crate) fn bind_unused_param(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { + let param: ast::Param = ctx.find_node_at_offset()?; + + let ident_pat = match param.pat()? { + ast::Pat::IdentPat(it) => it, + _ => return None, + }; + + let func = param.syntax().ancestors().find_map(ast::Fn::cast)?; + if is_trait_impl(&func) { + cov_mark::hit!(trait_impl); + return None; + } + + let param_def = { + let local = ctx.sema.to_def(&ident_pat)?; + Definition::Local(local) + }; + if param_def.usages(&ctx.sema).at_least_one() { + cov_mark::hit!(keep_used); + return None; + } + + let line_index = ctx.db().line_index(ctx.file_id()); + + let mut indent = func.indent_level(); + indent.0 += 1; + let mut text = format!("\n{indent}let _ = {ident_pat};"); + + let stmt_list = func.body()?.stmt_list()?; + let l_curly_range = stmt_list.l_curly_token()?.text_range(); + let r_curly_range = stmt_list.r_curly_token()?.text_range(); + let left_line = line_index.line_col(l_curly_range.end()).line; + let right_line = line_index.line_col(r_curly_range.start()).line; + + if left_line == right_line { + text.push('\n'); + } + + acc.add( + AssistId("bind_unused_param", AssistKind::Refactor), + &format!("Bind as `let _ = {};`", &ident_pat), + param.syntax().text_range(), + |builder| { + builder.insert(l_curly_range.end(), text); + }, + ) +} + +#[cfg(test)] +mod tests { + use crate::tests::check_assist; + + use super::*; + + #[test] + fn bind_unused_empty_block() { + check_assist( + bind_unused_param, + r#" +fn foo($0y: i32) {} +"#, + r#" +fn foo(y: i32) { + let _ = y; +} +"#, + ); + } + + #[test] + fn bind_unused_empty_block_with_newline() { + check_assist( + bind_unused_param, + r#" +fn foo($0y: i32) { +} +"#, + r#" +fn foo(y: i32) { + let _ = y; +} +"#, + ); + } + + #[test] + fn bind_unused_generic() { + check_assist( + bind_unused_param, + r#" +fn foo($0y: T) +where T : Default { +} +"#, + r#" +fn foo(y: T) +where T : Default { + let _ = y; +} +"#, + ); + } +} diff --git a/crates/ide-assists/src/handlers/remove_unused_param.rs b/crates/ide-assists/src/handlers/remove_unused_param.rs index 0772b168d4..aa1002ee39 100644 --- a/crates/ide-assists/src/handlers/remove_unused_param.rs +++ b/crates/ide-assists/src/handlers/remove_unused_param.rs @@ -42,13 +42,7 @@ pub(crate) fn remove_unused_param(acc: &mut Assists, ctx: &AssistContext<'_>) -> 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()) - { + if is_trait_impl(&func) { cov_mark::hit!(trait_impl); return None; } @@ -87,6 +81,14 @@ pub(crate) fn remove_unused_param(acc: &mut Assists, ctx: &AssistContext<'_>) -> ) } +pub(crate) fn is_trait_impl(func: &ast::Fn) -> bool { + func.syntax() + .parent() // AssocItemList + .and_then(|x| x.parent()) + .and_then(ast::Impl::cast) + .map_or(false, |imp| imp.trait_().is_some()) +} + fn process_usages( ctx: &AssistContext<'_>, builder: &mut SourceChangeBuilder, diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs index 2ebb5ef9b1..30a0ce40e3 100644 --- a/crates/ide-assists/src/lib.rs +++ b/crates/ide-assists/src/lib.rs @@ -114,6 +114,7 @@ mod handlers { mod add_turbo_fish; mod apply_demorgan; mod auto_import; + mod bind_unused_param; mod change_visibility; mod convert_bool_then; mod convert_comment_block; @@ -224,6 +225,7 @@ mod handlers { add_turbo_fish::add_turbo_fish, apply_demorgan::apply_demorgan, auto_import::auto_import, + bind_unused_param::bind_unused_param, change_visibility::change_visibility, convert_bool_then::convert_bool_then_to_if, convert_bool_then::convert_if_to_bool_then, diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index 6eadc3dbcb..0a30d6537d 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -265,6 +265,21 @@ pub mod std { pub mod collections { pub struct HashMap { } } } ) } +#[test] +fn doctest_bind_unused_param() { + check_doc_test( + "bind_unused_param", + r#####" +fn some_function(x: i32$0) {} +"#####, + r#####" +fn some_function(x: i32) { + let _ = x; +} +"#####, + ) +} + #[test] fn doctest_change_visibility() { check_doc_test( From 19e99941b640db1f4ea40952df925d00902ab8cf Mon Sep 17 00:00:00 2001 From: vsrs Date: Mon, 28 Aug 2023 15:30:44 +0700 Subject: [PATCH 2/5] Add cov_mark tests --- .../src/handlers/bind_unused_param.rs | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/ide-assists/src/handlers/bind_unused_param.rs b/crates/ide-assists/src/handlers/bind_unused_param.rs index 0a22d26193..7d8672dbbd 100644 --- a/crates/ide-assists/src/handlers/bind_unused_param.rs +++ b/crates/ide-assists/src/handlers/bind_unused_param.rs @@ -60,6 +60,7 @@ pub(crate) fn bind_unused_param(acc: &mut Assists, ctx: &AssistContext<'_>) -> O let right_line = line_index.line_col(r_curly_range.start()).line; if left_line == right_line { + cov_mark::hit!(single_line); text.push('\n'); } @@ -75,12 +76,13 @@ pub(crate) fn bind_unused_param(acc: &mut Assists, ctx: &AssistContext<'_>) -> O #[cfg(test)] mod tests { - use crate::tests::check_assist; + use crate::tests::{check_assist, check_assist_not_applicable}; use super::*; #[test] fn bind_unused_empty_block() { + cov_mark::check!(single_line); check_assist( bind_unused_param, r#" @@ -124,6 +126,33 @@ fn foo(y: T) where T : Default { let _ = y; } +"#, + ); + } + + #[test] + fn trait_impl() { + cov_mark::check!(trait_impl); + check_assist_not_applicable( + bind_unused_param, + r#" +trait Trait { + fn foo(x: i32); +} +impl Trait for () { + fn foo($0x: i32) {} +} +"#, + ); + } + + #[test] + fn keep_used() { + cov_mark::check!(keep_used); + check_assist_not_applicable( + bind_unused_param, + r#" +fn foo(x: i32, $0y: i32) { y; } "#, ); } From 6b20c1b09193603e9f76b2e73a5f04a982d38b4b Mon Sep 17 00:00:00 2001 From: vsrs Date: Tue, 29 Aug 2023 13:39:56 +0700 Subject: [PATCH 3/5] Apply suggestions. --- .../src/handlers/bind_unused_param.rs | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/crates/ide-assists/src/handlers/bind_unused_param.rs b/crates/ide-assists/src/handlers/bind_unused_param.rs index 7d8672dbbd..ef88592ba3 100644 --- a/crates/ide-assists/src/handlers/bind_unused_param.rs +++ b/crates/ide-assists/src/handlers/bind_unused_param.rs @@ -27,10 +27,7 @@ use super::remove_unused_param::is_trait_impl; pub(crate) fn bind_unused_param(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let param: ast::Param = ctx.find_node_at_offset()?; - let ident_pat = match param.pat()? { - ast::Pat::IdentPat(it) => it, - _ => return None, - }; + let Some(ast::Pat::IdentPat(ident_pat)) = param.pat() else { return None }; let func = param.syntax().ancestors().find_map(ast::Fn::cast)?; if is_trait_impl(&func) { @@ -47,28 +44,29 @@ pub(crate) fn bind_unused_param(acc: &mut Assists, ctx: &AssistContext<'_>) -> O return None; } - let line_index = ctx.db().line_index(ctx.file_id()); - - let mut indent = func.indent_level(); - indent.0 += 1; - let mut text = format!("\n{indent}let _ = {ident_pat};"); - let stmt_list = func.body()?.stmt_list()?; let l_curly_range = stmt_list.l_curly_token()?.text_range(); let r_curly_range = stmt_list.r_curly_token()?.text_range(); - let left_line = line_index.line_col(l_curly_range.end()).line; - let right_line = line_index.line_col(r_curly_range.start()).line; - - if left_line == right_line { - cov_mark::hit!(single_line); - text.push('\n'); - } acc.add( - AssistId("bind_unused_param", AssistKind::Refactor), + AssistId("bind_unused_param", AssistKind::QuickFix), &format!("Bind as `let _ = {};`", &ident_pat), param.syntax().text_range(), |builder| { + let line_index = ctx.db().line_index(ctx.file_id()); + + let mut indent = func.indent_level(); + indent.0 += 1; + let mut text = format!("\n{indent}let _ = {ident_pat};"); + + let left_line = line_index.line_col(l_curly_range.end()).line; + let right_line = line_index.line_col(r_curly_range.start()).line; + + if left_line == right_line { + cov_mark::hit!(single_line); + text.push('\n'); + } + builder.insert(l_curly_range.end(), text); }, ) From 6b559c4a9aabeb151678479265e4d82018fedaeb Mon Sep 17 00:00:00 2001 From: vsrs Date: Tue, 29 Aug 2023 22:56:31 +0700 Subject: [PATCH 4/5] Better trait implementation support --- .../src/handlers/bind_unused_param.rs | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/crates/ide-assists/src/handlers/bind_unused_param.rs b/crates/ide-assists/src/handlers/bind_unused_param.rs index ef88592ba3..45c1f0ccae 100644 --- a/crates/ide-assists/src/handlers/bind_unused_param.rs +++ b/crates/ide-assists/src/handlers/bind_unused_param.rs @@ -9,8 +9,6 @@ use syntax::{ AstNode, }; -use super::remove_unused_param::is_trait_impl; - // Assist: bind_unused_param // // Binds unused function parameter to an underscore. @@ -29,12 +27,6 @@ pub(crate) fn bind_unused_param(acc: &mut Assists, ctx: &AssistContext<'_>) -> O let Some(ast::Pat::IdentPat(ident_pat)) = param.pat() else { return None }; - let func = param.syntax().ancestors().find_map(ast::Fn::cast)?; - if is_trait_impl(&func) { - cov_mark::hit!(trait_impl); - return None; - } - let param_def = { let local = ctx.sema.to_def(&ident_pat)?; Definition::Local(local) @@ -44,6 +36,7 @@ pub(crate) fn bind_unused_param(acc: &mut Assists, ctx: &AssistContext<'_>) -> O return None; } + let func = param.syntax().ancestors().find_map(ast::Fn::cast)?; let stmt_list = func.body()?.stmt_list()?; let l_curly_range = stmt_list.l_curly_token()?.text_range(); let r_curly_range = stmt_list.r_curly_token()?.text_range(); @@ -55,16 +48,16 @@ pub(crate) fn bind_unused_param(acc: &mut Assists, ctx: &AssistContext<'_>) -> O |builder| { let line_index = ctx.db().line_index(ctx.file_id()); - let mut indent = func.indent_level(); - indent.0 += 1; - let mut text = format!("\n{indent}let _ = {ident_pat};"); + let indent = func.indent_level(); + let text_indent = indent + 1; + let mut text = format!("\n{text_indent}let _ = {ident_pat};"); let left_line = line_index.line_col(l_curly_range.end()).line; let right_line = line_index.line_col(r_curly_range.start()).line; if left_line == right_line { cov_mark::hit!(single_line); - text.push('\n'); + text.push_str(&format!("\n{indent}")); } builder.insert(l_curly_range.end(), text); @@ -130,8 +123,7 @@ where T : Default { #[test] fn trait_impl() { - cov_mark::check!(trait_impl); - check_assist_not_applicable( + check_assist( bind_unused_param, r#" trait Trait { @@ -140,6 +132,16 @@ trait Trait { impl Trait for () { fn foo($0x: i32) {} } +"#, + r#" +trait Trait { + fn foo(x: i32); +} +impl Trait for () { + fn foo(x: i32) { + let _ = x; + } +} "#, ); } From 1eb6d2e9a90da9a5738192b60b073b6b71c7deef Mon Sep 17 00:00:00 2001 From: vsrs Date: Tue, 29 Aug 2023 23:06:12 +0700 Subject: [PATCH 5/5] Rollback changes in remove_unused_param.rs --- .../src/handlers/remove_unused_param.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/ide-assists/src/handlers/remove_unused_param.rs b/crates/ide-assists/src/handlers/remove_unused_param.rs index aa1002ee39..0772b168d4 100644 --- a/crates/ide-assists/src/handlers/remove_unused_param.rs +++ b/crates/ide-assists/src/handlers/remove_unused_param.rs @@ -42,7 +42,13 @@ pub(crate) fn remove_unused_param(acc: &mut Assists, ctx: &AssistContext<'_>) -> param.syntax().parent()?.children().find_map(ast::SelfParam::cast).is_some(); // check if fn is in impl Trait for .. - if is_trait_impl(&func) { + 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; } @@ -81,14 +87,6 @@ pub(crate) fn remove_unused_param(acc: &mut Assists, ctx: &AssistContext<'_>) -> ) } -pub(crate) fn is_trait_impl(func: &ast::Fn) -> bool { - func.syntax() - .parent() // AssocItemList - .and_then(|x| x.parent()) - .and_then(ast::Impl::cast) - .map_or(false, |imp| imp.trait_().is_some()) -} - fn process_usages( ctx: &AssistContext<'_>, builder: &mut SourceChangeBuilder,