From 406d96c7d4c3a78d42b58a91ba633333ec37d487 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 28 Feb 2021 21:11:41 +0300 Subject: Use consistent naming for assist --- .../src/handlers/convert_for_to_iter_for_each.rs | 321 --------------------- .../src/handlers/replace_for_loop_with_for_each.rs | 321 +++++++++++++++++++++ crates/ide_assists/src/lib.rs | 12 +- crates/ide_assists/src/tests.rs | 2 +- crates/ide_assists/src/tests/generated.rs | 46 +-- 5 files changed, 351 insertions(+), 351 deletions(-) delete mode 100644 crates/ide_assists/src/handlers/convert_for_to_iter_for_each.rs create mode 100644 crates/ide_assists/src/handlers/replace_for_loop_with_for_each.rs (limited to 'crates/ide_assists') diff --git a/crates/ide_assists/src/handlers/convert_for_to_iter_for_each.rs b/crates/ide_assists/src/handlers/convert_for_to_iter_for_each.rs deleted file mode 100644 index 011df1c18..000000000 --- a/crates/ide_assists/src/handlers/convert_for_to_iter_for_each.rs +++ /dev/null @@ -1,321 +0,0 @@ -use ast::LoopBodyOwner; -use hir::known; -use ide_db::helpers::FamousDefs; -use stdx::format_to; -use syntax::{ast, AstNode}; -use test_utils::mark; - -use crate::{AssistContext, AssistId, AssistKind, Assists}; - -// Assist: convert_for_to_iter_for_each -// -// Converts a for loop into a for_each loop on the Iterator. -// -// ``` -// fn main() { -// let x = vec![1, 2, 3]; -// for$0 v in x { -// let y = v * 2; -// } -// } -// ``` -// -> -// ``` -// fn main() { -// let x = vec![1, 2, 3]; -// x.into_iter().for_each(|v| { -// let y = v * 2; -// }); -// } -// ``` -pub(crate) fn convert_for_to_iter_for_each(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - let for_loop = ctx.find_node_at_offset::()?; - let iterable = for_loop.iterable()?; - let pat = for_loop.pat()?; - let body = for_loop.loop_body()?; - if body.syntax().text_range().start() < ctx.offset() { - mark::hit!(not_available_in_body); - return None; - } - - acc.add( - AssistId("convert_for_to_iter_for_each", AssistKind::RefactorRewrite), - "Convert a for loop into an Iterator::for_each", - for_loop.syntax().text_range(), - |builder| { - let mut buf = String::new(); - - if let Some((expr_behind_ref, method)) = - is_ref_and_impls_iter_method(&ctx.sema, &iterable) - { - // We have either "for x in &col" and col implements a method called iter - // or "for x in &mut col" and col implements a method called iter_mut - format_to!(buf, "{}.{}()", expr_behind_ref, method); - } else if impls_core_iter(&ctx.sema, &iterable) { - format_to!(buf, "{}", iterable); - } else { - if let ast::Expr::RefExpr(_) = iterable { - format_to!(buf, "({}).into_iter()", iterable); - } else { - format_to!(buf, "{}.into_iter()", iterable); - } - } - - format_to!(buf, ".for_each(|{}| {});", pat, body); - - builder.replace(for_loop.syntax().text_range(), buf) - }, - ) -} - -/// If iterable is a reference where the expression behind the reference implements a method -/// returning an Iterator called iter or iter_mut (depending on the type of reference) then return -/// the expression behind the reference and the method name -fn is_ref_and_impls_iter_method( - sema: &hir::Semantics, - iterable: &ast::Expr, -) -> Option<(ast::Expr, hir::Name)> { - let ref_expr = match iterable { - ast::Expr::RefExpr(r) => r, - _ => return None, - }; - let wanted_method = if ref_expr.mut_token().is_some() { known::iter_mut } else { known::iter }; - let expr_behind_ref = ref_expr.expr()?; - let typ = sema.type_of_expr(&expr_behind_ref)?; - let scope = sema.scope(iterable.syntax()); - let krate = scope.module()?.krate(); - let traits_in_scope = scope.traits_in_scope(); - let iter_trait = FamousDefs(sema, Some(krate)).core_iter_Iterator()?; - let has_wanted_method = typ.iterate_method_candidates( - sema.db, - krate, - &traits_in_scope, - Some(&wanted_method), - |_, func| { - if func.ret_type(sema.db).impls_trait(sema.db, iter_trait, &[]) { - return Some(()); - } - None - }, - ); - has_wanted_method.and(Some((expr_behind_ref, wanted_method))) -} - -/// Whether iterable implements core::Iterator -fn impls_core_iter(sema: &hir::Semantics, iterable: &ast::Expr) -> bool { - let it_typ = if let Some(i) = sema.type_of_expr(iterable) { - i - } else { - return false; - }; - let module = if let Some(m) = sema.scope(iterable.syntax()).module() { - m - } else { - return false; - }; - let krate = module.krate(); - if let Some(iter_trait) = FamousDefs(sema, Some(krate)).core_iter_Iterator() { - return it_typ.impls_trait(sema.db, iter_trait, &[]); - } - false -} - -#[cfg(test)] -mod tests { - use crate::tests::{check_assist, check_assist_not_applicable}; - - use super::*; - - const EMPTY_ITER_FIXTURE: &'static str = r" -//- /lib.rs deps:core crate:empty_iter -pub struct EmptyIter; -impl Iterator for EmptyIter { - type Item = usize; - fn next(&mut self) -> Option { None } -} - -pub struct Empty; -impl Empty { - pub fn iter(&self) -> EmptyIter { EmptyIter } - pub fn iter_mut(&self) -> EmptyIter { EmptyIter } -} - -pub struct NoIterMethod; -"; - - fn check_assist_with_fixtures(before: &str, after: &str) { - let before = &format!( - "//- /main.rs crate:main deps:core,empty_iter{}{}{}", - before, - FamousDefs::FIXTURE, - EMPTY_ITER_FIXTURE - ); - check_assist(convert_for_to_iter_for_each, before, after); - } - - #[test] - fn test_not_for() { - check_assist_not_applicable( - convert_for_to_iter_for_each, - r" -let mut x = vec![1, 2, 3]; -x.iter_mut().$0for_each(|v| *v *= 2); - ", - ) - } - - #[test] - fn test_simple_for() { - check_assist( - convert_for_to_iter_for_each, - r" -fn main() { - let x = vec![1, 2, 3]; - for $0v in x { - v *= 2; - } -}", - r" -fn main() { - let x = vec![1, 2, 3]; - x.into_iter().for_each(|v| { - v *= 2; - }); -}", - ) - } - - #[test] - fn not_available_in_body() { - mark::check!(not_available_in_body); - check_assist_not_applicable( - convert_for_to_iter_for_each, - r" -fn main() { - let x = vec![1, 2, 3]; - for v in x { - $0v *= 2; - } -}", - ) - } - - #[test] - fn test_for_borrowed() { - check_assist_with_fixtures( - r" -use empty_iter::*; -fn main() { - let x = Empty; - for $0v in &x { - let a = v * 2; - } -} -", - r" -use empty_iter::*; -fn main() { - let x = Empty; - x.iter().for_each(|v| { - let a = v * 2; - }); -} -", - ) - } - - #[test] - fn test_for_borrowed_no_iter_method() { - check_assist_with_fixtures( - r" -use empty_iter::*; -fn main() { - let x = NoIterMethod; - for $0v in &x { - let a = v * 2; - } -} -", - r" -use empty_iter::*; -fn main() { - let x = NoIterMethod; - (&x).into_iter().for_each(|v| { - let a = v * 2; - }); -} -", - ) - } - - #[test] - fn test_for_borrowed_mut() { - check_assist_with_fixtures( - r" -use empty_iter::*; -fn main() { - let x = Empty; - for $0v in &mut x { - let a = v * 2; - } -} -", - r" -use empty_iter::*; -fn main() { - let x = Empty; - x.iter_mut().for_each(|v| { - let a = v * 2; - }); -} -", - ) - } - - #[test] - fn test_for_borrowed_mut_behind_var() { - check_assist( - convert_for_to_iter_for_each, - r" -fn main() { - let x = vec![1, 2, 3]; - let y = &mut x; - for $0v in y { - *v *= 2; - } -}", - r" -fn main() { - let x = vec![1, 2, 3]; - let y = &mut x; - y.into_iter().for_each(|v| { - *v *= 2; - }); -}", - ) - } - - #[test] - fn test_already_impls_iterator() { - check_assist_with_fixtures( - r#" -use empty_iter::*; -fn main() { - let x = Empty; - for$0 a in x.iter().take(1) { - println!("{}", a); - } -} -"#, - r#" -use empty_iter::*; -fn main() { - let x = Empty; - x.iter().take(1).for_each(|a| { - println!("{}", a); - }); -} -"#, - ); - } -} diff --git a/crates/ide_assists/src/handlers/replace_for_loop_with_for_each.rs b/crates/ide_assists/src/handlers/replace_for_loop_with_for_each.rs new file mode 100644 index 000000000..27da28bc0 --- /dev/null +++ b/crates/ide_assists/src/handlers/replace_for_loop_with_for_each.rs @@ -0,0 +1,321 @@ +use ast::LoopBodyOwner; +use hir::known; +use ide_db::helpers::FamousDefs; +use stdx::format_to; +use syntax::{ast, AstNode}; +use test_utils::mark; + +use crate::{AssistContext, AssistId, AssistKind, Assists}; + +// Assist: replace_for_loop_with_for_each +// +// Converts a for loop into a for_each loop on the Iterator. +// +// ``` +// fn main() { +// let x = vec![1, 2, 3]; +// for$0 v in x { +// let y = v * 2; +// } +// } +// ``` +// -> +// ``` +// fn main() { +// let x = vec![1, 2, 3]; +// x.into_iter().for_each(|v| { +// let y = v * 2; +// }); +// } +// ``` +pub(crate) fn replace_for_loop_with_for_each(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let for_loop = ctx.find_node_at_offset::()?; + let iterable = for_loop.iterable()?; + let pat = for_loop.pat()?; + let body = for_loop.loop_body()?; + if body.syntax().text_range().start() < ctx.offset() { + mark::hit!(not_available_in_body); + return None; + } + + acc.add( + AssistId("replace_for_loop_with_for_each", AssistKind::RefactorRewrite), + "Replace this for loop with `Iterator::for_each`", + for_loop.syntax().text_range(), + |builder| { + let mut buf = String::new(); + + if let Some((expr_behind_ref, method)) = + is_ref_and_impls_iter_method(&ctx.sema, &iterable) + { + // We have either "for x in &col" and col implements a method called iter + // or "for x in &mut col" and col implements a method called iter_mut + format_to!(buf, "{}.{}()", expr_behind_ref, method); + } else if impls_core_iter(&ctx.sema, &iterable) { + format_to!(buf, "{}", iterable); + } else { + if let ast::Expr::RefExpr(_) = iterable { + format_to!(buf, "({}).into_iter()", iterable); + } else { + format_to!(buf, "{}.into_iter()", iterable); + } + } + + format_to!(buf, ".for_each(|{}| {});", pat, body); + + builder.replace(for_loop.syntax().text_range(), buf) + }, + ) +} + +/// If iterable is a reference where the expression behind the reference implements a method +/// returning an Iterator called iter or iter_mut (depending on the type of reference) then return +/// the expression behind the reference and the method name +fn is_ref_and_impls_iter_method( + sema: &hir::Semantics, + iterable: &ast::Expr, +) -> Option<(ast::Expr, hir::Name)> { + let ref_expr = match iterable { + ast::Expr::RefExpr(r) => r, + _ => return None, + }; + let wanted_method = if ref_expr.mut_token().is_some() { known::iter_mut } else { known::iter }; + let expr_behind_ref = ref_expr.expr()?; + let typ = sema.type_of_expr(&expr_behind_ref)?; + let scope = sema.scope(iterable.syntax()); + let krate = scope.module()?.krate(); + let traits_in_scope = scope.traits_in_scope(); + let iter_trait = FamousDefs(sema, Some(krate)).core_iter_Iterator()?; + let has_wanted_method = typ.iterate_method_candidates( + sema.db, + krate, + &traits_in_scope, + Some(&wanted_method), + |_, func| { + if func.ret_type(sema.db).impls_trait(sema.db, iter_trait, &[]) { + return Some(()); + } + None + }, + ); + has_wanted_method.and(Some((expr_behind_ref, wanted_method))) +} + +/// Whether iterable implements core::Iterator +fn impls_core_iter(sema: &hir::Semantics, iterable: &ast::Expr) -> bool { + let it_typ = if let Some(i) = sema.type_of_expr(iterable) { + i + } else { + return false; + }; + let module = if let Some(m) = sema.scope(iterable.syntax()).module() { + m + } else { + return false; + }; + let krate = module.krate(); + if let Some(iter_trait) = FamousDefs(sema, Some(krate)).core_iter_Iterator() { + return it_typ.impls_trait(sema.db, iter_trait, &[]); + } + false +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::*; + + const EMPTY_ITER_FIXTURE: &'static str = r" +//- /lib.rs deps:core crate:empty_iter +pub struct EmptyIter; +impl Iterator for EmptyIter { + type Item = usize; + fn next(&mut self) -> Option { None } +} + +pub struct Empty; +impl Empty { + pub fn iter(&self) -> EmptyIter { EmptyIter } + pub fn iter_mut(&self) -> EmptyIter { EmptyIter } +} + +pub struct NoIterMethod; +"; + + fn check_assist_with_fixtures(before: &str, after: &str) { + let before = &format!( + "//- /main.rs crate:main deps:core,empty_iter{}{}{}", + before, + FamousDefs::FIXTURE, + EMPTY_ITER_FIXTURE + ); + check_assist(replace_for_loop_with_for_each, before, after); + } + + #[test] + fn test_not_for() { + check_assist_not_applicable( + replace_for_loop_with_for_each, + r" +let mut x = vec![1, 2, 3]; +x.iter_mut().$0for_each(|v| *v *= 2); + ", + ) + } + + #[test] + fn test_simple_for() { + check_assist( + replace_for_loop_with_for_each, + r" +fn main() { + let x = vec![1, 2, 3]; + for $0v in x { + v *= 2; + } +}", + r" +fn main() { + let x = vec![1, 2, 3]; + x.into_iter().for_each(|v| { + v *= 2; + }); +}", + ) + } + + #[test] + fn not_available_in_body() { + mark::check!(not_available_in_body); + check_assist_not_applicable( + replace_for_loop_with_for_each, + r" +fn main() { + let x = vec![1, 2, 3]; + for v in x { + $0v *= 2; + } +}", + ) + } + + #[test] + fn test_for_borrowed() { + check_assist_with_fixtures( + r" +use empty_iter::*; +fn main() { + let x = Empty; + for $0v in &x { + let a = v * 2; + } +} +", + r" +use empty_iter::*; +fn main() { + let x = Empty; + x.iter().for_each(|v| { + let a = v * 2; + }); +} +", + ) + } + + #[test] + fn test_for_borrowed_no_iter_method() { + check_assist_with_fixtures( + r" +use empty_iter::*; +fn main() { + let x = NoIterMethod; + for $0v in &x { + let a = v * 2; + } +} +", + r" +use empty_iter::*; +fn main() { + let x = NoIterMethod; + (&x).into_iter().for_each(|v| { + let a = v * 2; + }); +} +", + ) + } + + #[test] + fn test_for_borrowed_mut() { + check_assist_with_fixtures( + r" +use empty_iter::*; +fn main() { + let x = Empty; + for $0v in &mut x { + let a = v * 2; + } +} +", + r" +use empty_iter::*; +fn main() { + let x = Empty; + x.iter_mut().for_each(|v| { + let a = v * 2; + }); +} +", + ) + } + + #[test] + fn test_for_borrowed_mut_behind_var() { + check_assist( + replace_for_loop_with_for_each, + r" +fn main() { + let x = vec![1, 2, 3]; + let y = &mut x; + for $0v in y { + *v *= 2; + } +}", + r" +fn main() { + let x = vec![1, 2, 3]; + let y = &mut x; + y.into_iter().for_each(|v| { + *v *= 2; + }); +}", + ) + } + + #[test] + fn test_already_impls_iterator() { + check_assist_with_fixtures( + r#" +use empty_iter::*; +fn main() { + let x = Empty; + for$0 a in x.iter().take(1) { + println!("{}", a); + } +} +"#, + r#" +use empty_iter::*; +fn main() { + let x = Empty; + x.iter().take(1).for_each(|a| { + println!("{}", a); + }); +} +"#, + ); + } +} diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 4c067d451..53542d433 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -114,7 +114,6 @@ mod handlers { mod apply_demorgan; mod auto_import; mod change_visibility; - mod convert_for_to_iter_for_each; mod convert_integer_literal; mod early_return; mod expand_glob_import; @@ -132,8 +131,8 @@ mod handlers { mod generate_enum_projection_method; mod generate_from_impl_for_enum; mod generate_function; - mod generate_getter; mod generate_getter_mut; + mod generate_getter; mod generate_impl; mod generate_new; mod generate_setter; @@ -156,6 +155,7 @@ mod handlers { mod reorder_fields; mod reorder_impl; mod replace_derive_with_manual_impl; + mod replace_for_loop_with_for_each; mod replace_if_let_with_match; mod replace_impl_trait_with_generic; mod replace_let_with_if_let; @@ -177,11 +177,9 @@ mod handlers { apply_demorgan::apply_demorgan, auto_import::auto_import, change_visibility::change_visibility, - convert_for_to_iter_for_each::convert_for_to_iter_for_each, convert_integer_literal::convert_integer_literal, early_return::convert_to_guarded_return, expand_glob_import::expand_glob_import, - move_module_to_file::move_module_to_file, extract_struct_from_enum_variant::extract_struct_from_enum_variant, fill_match_arms::fill_match_arms, fix_visibility::fix_visibility, @@ -191,12 +189,12 @@ mod handlers { generate_default_from_enum_variant::generate_default_from_enum_variant, generate_derive::generate_derive, generate_enum_is_method::generate_enum_is_method, - generate_enum_projection_method::generate_enum_try_into_method, generate_enum_projection_method::generate_enum_as_method, + generate_enum_projection_method::generate_enum_try_into_method, generate_from_impl_for_enum::generate_from_impl_for_enum, generate_function::generate_function, - generate_getter::generate_getter, generate_getter_mut::generate_getter_mut, + generate_getter::generate_getter, generate_impl::generate_impl, generate_new::generate_new, generate_setter::generate_setter, @@ -210,6 +208,7 @@ mod handlers { move_bounds::move_bounds_to_where_clause, move_guard::move_arm_cond_to_match_guard, move_guard::move_guard_to_arm_body, + move_module_to_file::move_module_to_file, pull_assignment_up::pull_assignment_up, qualify_path::qualify_path, raw_string::add_hash, @@ -221,6 +220,7 @@ mod handlers { reorder_fields::reorder_fields, reorder_impl::reorder_impl, replace_derive_with_manual_impl::replace_derive_with_manual_impl, + replace_for_loop_with_for_each::replace_for_loop_with_for_each, replace_if_let_with_match::replace_if_let_with_match, replace_if_let_with_match::replace_match_with_if_let, replace_impl_trait_with_generic::replace_impl_trait_with_generic, diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs index 384eb7eee..b7f616760 100644 --- a/crates/ide_assists/src/tests.rs +++ b/crates/ide_assists/src/tests.rs @@ -190,8 +190,8 @@ fn assist_order_field_struct() { let mut assists = assists.iter(); assert_eq!(assists.next().expect("expected assist").label, "Change visibility to pub(crate)"); - assert_eq!(assists.next().expect("expected assist").label, "Generate a getter method"); assert_eq!(assists.next().expect("expected assist").label, "Generate a mut getter method"); + assert_eq!(assists.next().expect("expected assist").label, "Generate a getter method"); assert_eq!(assists.next().expect("expected assist").label, "Generate a setter method"); assert_eq!(assists.next().expect("expected assist").label, "Add `#[derive]`"); } diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index 7f6dbbccf..4f007aa48 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -192,29 +192,6 @@ pub(crate) fn frobnicate() {} ) } -#[test] -fn doctest_convert_for_to_iter_for_each() { - check_doc_test( - "convert_for_to_iter_for_each", - r#####" -fn main() { - let x = vec![1, 2, 3]; - for $0v in x { - let y = v * 2; - } -} -"#####, - r#####" -fn main() { - let x = vec![1, 2, 3]; - x.into_iter().for_each(|v| { - let y = v * 2; - }); -} -"#####, - ) -} - #[test] fn doctest_convert_integer_literal() { check_doc_test( @@ -1179,6 +1156,29 @@ impl Debug for S { ) } +#[test] +fn doctest_replace_for_loop_with_for_each() { + check_doc_test( + "replace_for_loop_with_for_each", + r#####" +fn main() { + let x = vec![1, 2, 3]; + for$0 v in x { + let y = v * 2; + } +} +"#####, + r#####" +fn main() { + let x = vec![1, 2, 3]; + x.into_iter().for_each(|v| { + let y = v * 2; + }); +} +"#####, + ) +} + #[test] fn doctest_replace_if_let_with_match() { check_doc_test( -- cgit v1.2.3