Unnamed repository; edit this file 'description' to name the repository.
Merge iter_for_each_to_for and for_to_iter_for_each assists modules
| -rw-r--r-- | crates/ide_assists/src/handlers/convert_iter_for_each_to_for.rs | 362 | ||||
| -rw-r--r-- | crates/ide_assists/src/handlers/move_guard.rs | 19 | ||||
| -rw-r--r-- | crates/ide_assists/src/handlers/replace_for_loop_with_for_each.rs | 372 | ||||
| -rw-r--r-- | crates/ide_assists/src/lib.rs | 3 | ||||
| -rw-r--r-- | crates/ide_assists/src/tests/generated.rs | 46 |
5 files changed, 404 insertions, 398 deletions
diff --git a/crates/ide_assists/src/handlers/convert_iter_for_each_to_for.rs b/crates/ide_assists/src/handlers/convert_iter_for_each_to_for.rs index de79023f01..07d4acb152 100644 --- a/crates/ide_assists/src/handlers/convert_iter_for_each_to_for.rs +++ b/crates/ide_assists/src/handlers/convert_iter_for_each_to_for.rs @@ -1,6 +1,8 @@ +use hir::known; use ide_db::helpers::FamousDefs; +use stdx::format_to; use syntax::{ - ast::{self, edit_in_place::Indent, make, ArgListOwner}, + ast::{self, edit_in_place::Indent, make, ArgListOwner, LoopBodyOwner}, AstNode, }; @@ -69,6 +71,130 @@ pub(crate) fn convert_iter_for_each_to_for(acc: &mut Assists, ctx: &AssistContex ) } +// Assist: convert_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 convert_for_loop_with_for_each(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let for_loop = ctx.find_node_at_offset::<ast::ForExpr>()?; + let iterable = for_loop.iterable()?; + let pat = for_loop.pat()?; + let body = for_loop.loop_body()?; + if body.syntax().text_range().start() < ctx.offset() { + cov_mark::hit!(not_available_in_body); + return None; + } + + acc.add( + AssistId("convert_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 let ast::Expr::RangeExpr(..) = iterable { + // range expressions need to be parenthesized for the syntax to be correct + format_to!(buf, "({})", iterable); + } 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<ide_db::RootDatabase>, + 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 ty = sema.type_of_expr(&expr_behind_ref)?.adjusted(); + 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 = ty + .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 + }, + ) + .is_some(); + if !has_wanted_method { + return None; + } + + Some((expr_behind_ref, wanted_method)) +} + +/// Whether iterable implements core::Iterator +fn impls_core_iter(sema: &hir::Semantics<ide_db::RootDatabase>, iterable: &ast::Expr) -> bool { + let it_typ = match sema.type_of_expr(iterable) { + Some(it) => it.adjusted(), + None => return false, + }; + + let module = match sema.scope(iterable.syntax()).module() { + Some(it) => it, + None => return false, + }; + + let krate = module.krate(); + match FamousDefs(sema, Some(krate)).core_iter_Iterator() { + Some(iter_trait) => { + cov_mark::hit!(test_already_impls_iterator); + it_typ.impls_trait(sema.db, iter_trait, &[]) + } + None => false, + } +} + fn validate_method_call_expr( ctx: &AssistContext, expr: ast::MethodCallExpr, @@ -195,4 +321,238 @@ fn main() { }"#, ) } + + #[test] + fn each_to_for_not_for() { + check_assist_not_applicable( + convert_for_loop_with_for_each, + r" +let mut x = vec![1, 2, 3]; +x.iter_mut().$0for_each(|v| *v *= 2); + ", + ) + } + + #[test] + fn each_to_for_simple_for() { + check_assist( + convert_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 each_to_for_for_in_range() { + check_assist( + convert_for_loop_with_for_each, + r#" +//- minicore: range, iterators +impl<T> core::iter::Iterator for core::ops::Range<T> { + type Item = T; + + fn next(&mut self) -> Option<Self::Item> { + None + } +} + +fn main() { + for $0x in 0..92 { + print!("{}", x); + } +}"#, + r#" +impl<T> core::iter::Iterator for core::ops::Range<T> { + type Item = T; + + fn next(&mut self) -> Option<Self::Item> { + None + } +} + +fn main() { + (0..92).for_each(|x| { + print!("{}", x); + }); +}"#, + ) + } + + #[test] + fn each_to_for_not_available_in_body() { + cov_mark::check!(not_available_in_body); + check_assist_not_applicable( + convert_for_loop_with_for_each, + r" +fn main() { + let x = vec![1, 2, 3]; + for v in x { + $0v *= 2; + } +}", + ) + } + + #[test] + fn each_to_for_for_borrowed() { + check_assist( + convert_for_loop_with_for_each, + r#" +//- minicore: iterators +use core::iter::{Repeat, repeat}; + +struct S; +impl S { + fn iter(&self) -> Repeat<i32> { repeat(92) } + fn iter_mut(&mut self) -> Repeat<i32> { repeat(92) } +} + +fn main() { + let x = S; + for $0v in &x { + let a = v * 2; + } +} +"#, + r#" +use core::iter::{Repeat, repeat}; + +struct S; +impl S { + fn iter(&self) -> Repeat<i32> { repeat(92) } + fn iter_mut(&mut self) -> Repeat<i32> { repeat(92) } +} + +fn main() { + let x = S; + x.iter().for_each(|v| { + let a = v * 2; + }); +} +"#, + ) + } + + #[test] + fn each_to_for_for_borrowed_no_iter_method() { + check_assist( + convert_for_loop_with_for_each, + r" +struct NoIterMethod; +fn main() { + let x = NoIterMethod; + for $0v in &x { + let a = v * 2; + } +} +", + r" +struct NoIterMethod; +fn main() { + let x = NoIterMethod; + (&x).into_iter().for_each(|v| { + let a = v * 2; + }); +} +", + ) + } + + #[test] + fn each_to_for_for_borrowed_mut() { + check_assist( + convert_for_loop_with_for_each, + r#" +//- minicore: iterators +use core::iter::{Repeat, repeat}; + +struct S; +impl S { + fn iter(&self) -> Repeat<i32> { repeat(92) } + fn iter_mut(&mut self) -> Repeat<i32> { repeat(92) } +} + +fn main() { + let x = S; + for $0v in &mut x { + let a = v * 2; + } +} +"#, + r#" +use core::iter::{Repeat, repeat}; + +struct S; +impl S { + fn iter(&self) -> Repeat<i32> { repeat(92) } + fn iter_mut(&mut self) -> Repeat<i32> { repeat(92) } +} + +fn main() { + let x = S; + x.iter_mut().for_each(|v| { + let a = v * 2; + }); +} +"#, + ) + } + + #[test] + fn each_to_for_for_borrowed_mut_behind_var() { + check_assist( + convert_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 each_to_for_already_impls_iterator() { + cov_mark::check!(test_already_impls_iterator); + check_assist( + convert_for_loop_with_for_each, + r#" +//- minicore: iterators +fn main() { + for$0 a in core::iter::repeat(92).take(1) { + println!("{}", a); + } +} +"#, + r#" +fn main() { + core::iter::repeat(92).take(1).for_each(|a| { + println!("{}", a); + }); +} +"#, + ); + } } diff --git a/crates/ide_assists/src/handlers/move_guard.rs b/crates/ide_assists/src/handlers/move_guard.rs index 6841f506aa..d7e7c363af 100644 --- a/crates/ide_assists/src/handlers/move_guard.rs +++ b/crates/ide_assists/src/handlers/move_guard.rs @@ -35,6 +35,10 @@ use crate::{AssistContext, AssistId, AssistKind, Assists}; pub(crate) fn move_guard_to_arm_body(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let match_arm = ctx.find_node_at_offset::<MatchArm>()?; let guard = match_arm.guard()?; + if ctx.offset() > guard.syntax().text_range().end() { + cov_mark::hit!(move_guard_unapplicable_in_arm_body); + return None; + } let space_before_guard = guard.syntax().prev_sibling_or_token(); // FIXME: support `if let` guards too @@ -156,6 +160,21 @@ mod tests { use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target}; #[test] + fn move_guard_to_arm_body_range() { + cov_mark::check!(move_guard_unapplicable_in_arm_body); + check_assist_not_applicable( + move_guard_to_arm_body, + r#" +fn main() { + match 92 { + x if x > 10 => $0false, + _ => true + } +} +"#, + ); + } + #[test] fn move_guard_to_arm_body_target() { check_assist_target( move_guard_to_arm_body, 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 deleted file mode 100644 index fcf7973bd0..0000000000 --- a/crates/ide_assists/src/handlers/replace_for_loop_with_for_each.rs +++ /dev/null @@ -1,372 +0,0 @@ -use ast::LoopBodyOwner; -use hir::known; -use ide_db::helpers::FamousDefs; -use stdx::format_to; -use syntax::{ast, AstNode}; - -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::<ast::ForExpr>()?; - let iterable = for_loop.iterable()?; - let pat = for_loop.pat()?; - let body = for_loop.loop_body()?; - if body.syntax().text_range().start() < ctx.offset() { - cov_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 let ast::Expr::RangeExpr(..) = iterable { - // range expressions need to be parenthesized for the syntax to be correct - format_to!(buf, "({})", iterable); - } 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<ide_db::RootDatabase>, - 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 ty = sema.type_of_expr(&expr_behind_ref)?.adjusted(); - 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 = ty - .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 - }, - ) - .is_some(); - if !has_wanted_method { - return None; - } - - Some((expr_behind_ref, wanted_method)) -} - -/// Whether iterable implements core::Iterator -fn impls_core_iter(sema: &hir::Semantics<ide_db::RootDatabase>, iterable: &ast::Expr) -> bool { - let it_typ = match sema.type_of_expr(iterable) { - Some(it) => it.adjusted(), - None => return false, - }; - - let module = match sema.scope(iterable.syntax()).module() { - Some(it) => it, - None => return false, - }; - - let krate = module.krate(); - match FamousDefs(sema, Some(krate)).core_iter_Iterator() { - Some(iter_trait) => { - cov_mark::hit!(test_already_impls_iterator); - it_typ.impls_trait(sema.db, iter_trait, &[]) - } - None => false, - } -} - -#[cfg(test)] -mod tests { - use crate::tests::{check_assist, check_assist_not_applicable}; - - use super::*; - - #[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 test_for_in_range() { - check_assist( - replace_for_loop_with_for_each, - r#" -//- minicore: range, iterators -impl<T> core::iter::Iterator for core::ops::Range<T> { - type Item = T; - - fn next(&mut self) -> Option<Self::Item> { - None - } -} - -fn main() { - for $0x in 0..92 { - print!("{}", x); - } -}"#, - r#" -impl<T> core::iter::Iterator for core::ops::Range<T> { - type Item = T; - - fn next(&mut self) -> Option<Self::Item> { - None - } -} - -fn main() { - (0..92).for_each(|x| { - print!("{}", x); - }); -}"#, - ) - } - - #[test] - fn not_available_in_body() { - cov_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( - replace_for_loop_with_for_each, - r#" -//- minicore: iterators -use core::iter::{Repeat, repeat}; - -struct S; -impl S { - fn iter(&self) -> Repeat<i32> { repeat(92) } - fn iter_mut(&mut self) -> Repeat<i32> { repeat(92) } -} - -fn main() { - let x = S; - for $0v in &x { - let a = v * 2; - } -} -"#, - r#" -use core::iter::{Repeat, repeat}; - -struct S; -impl S { - fn iter(&self) -> Repeat<i32> { repeat(92) } - fn iter_mut(&mut self) -> Repeat<i32> { repeat(92) } -} - -fn main() { - let x = S; - x.iter().for_each(|v| { - let a = v * 2; - }); -} -"#, - ) - } - - #[test] - fn test_for_borrowed_no_iter_method() { - check_assist( - replace_for_loop_with_for_each, - r" -struct NoIterMethod; -fn main() { - let x = NoIterMethod; - for $0v in &x { - let a = v * 2; - } -} -", - r" -struct NoIterMethod; -fn main() { - let x = NoIterMethod; - (&x).into_iter().for_each(|v| { - let a = v * 2; - }); -} -", - ) - } - - #[test] - fn test_for_borrowed_mut() { - check_assist( - replace_for_loop_with_for_each, - r#" -//- minicore: iterators -use core::iter::{Repeat, repeat}; - -struct S; -impl S { - fn iter(&self) -> Repeat<i32> { repeat(92) } - fn iter_mut(&mut self) -> Repeat<i32> { repeat(92) } -} - -fn main() { - let x = S; - for $0v in &mut x { - let a = v * 2; - } -} -"#, - r#" -use core::iter::{Repeat, repeat}; - -struct S; -impl S { - fn iter(&self) -> Repeat<i32> { repeat(92) } - fn iter_mut(&mut self) -> Repeat<i32> { repeat(92) } -} - -fn main() { - let x = S; - 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() { - cov_mark::check!(test_already_impls_iterator); - check_assist( - replace_for_loop_with_for_each, - r#" -//- minicore: iterators -fn main() { - for$0 a in core::iter::repeat(92).take(1) { - println!("{}", a); - } -} -"#, - r#" -fn main() { - core::iter::repeat(92).take(1).for_each(|a| { - println!("{}", a); - }); -} -"#, - ); - } -} diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 6632b7e78d..b2c8cd7694 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -162,7 +162,6 @@ 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 introduce_named_generic; mod replace_let_with_if_let; @@ -192,6 +191,7 @@ mod handlers { convert_integer_literal::convert_integer_literal, convert_into_to_from::convert_into_to_from, convert_iter_for_each_to_for::convert_iter_for_each_to_for, + convert_iter_for_each_to_for::convert_for_loop_with_for_each, convert_to_guarded_return::convert_to_guarded_return, convert_tuple_struct_to_named_struct::convert_tuple_struct_to_named_struct, convert_while_to_loop::convert_while_to_loop, @@ -237,7 +237,6 @@ 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_let_with_if_let::replace_let_with_if_let, diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index 252f375d85..95a68ca989 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -253,6 +253,29 @@ fn main() { } #[test] +fn doctest_convert_for_loop_with_for_each() { + check_doc_test( + "convert_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_convert_if_to_bool_then() { check_doc_test( "convert_if_to_bool_then", @@ -1491,29 +1514,6 @@ 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( "replace_if_let_with_match", |