From eea21490e06807960269844ab0f50b1873e1c78b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luiz=20Carlos=20Mour=C3=A3o=20Paes=20de=20Carvalho?= Date: Tue, 9 Mar 2021 22:58:17 -0300 Subject: feat: add assist to conver for_each into for loops --- .../src/handlers/convert_iter_for_each_to_for.rs | 138 +++++++++++++++++++++ crates/ide_assists/src/lib.rs | 2 + 2 files changed, 140 insertions(+) create mode 100644 crates/ide_assists/src/handlers/convert_iter_for_each_to_for.rs 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 new file mode 100644 index 000000000..3220f2f46 --- /dev/null +++ b/crates/ide_assists/src/handlers/convert_iter_for_each_to_for.rs @@ -0,0 +1,138 @@ +use ide_db::helpers::FamousDefs; +use stdx::format_to; +use syntax::{AstNode, ast::{self, ArgListOwner}}; + +use crate::{AssistContext, AssistId, AssistKind, Assists}; + +/// Assist: convert_iter_for_each_to_for +// +/// Converts an Iterator::for_each function into a for loop. +/// +/// ```rust +/// fn main() { +/// let vec = vec![(1, 2), (2, 3), (3, 4)]; +/// x.iter().for_each(|(x, y)| { +/// println!("x: {}, y: {}", x, y); +/// }) +/// } +/// ``` +/// -> +/// ```rust +/// fn main() { +/// let vec = vec![(1, 2), (2, 3), (3, 4)]; +/// for (x, y) in x.iter() { +/// println!("x: {}, y: {}", x, y); +/// }); +/// } +/// ``` +pub(crate) fn convert_iter_for_each_to_for(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let closure; + + let total_expr = match ctx.find_node_at_offset::()? { + ast::Expr::MethodCallExpr(expr) => { + closure = match expr.arg_list()?.args().next()? { + ast::Expr::ClosureExpr(expr) => expr, + _ => { return None; } + }; + + expr + }, + ast::Expr::ClosureExpr(expr) => { + closure = expr; + ast::MethodCallExpr::cast(closure.syntax().ancestors().nth(2)?)? + }, + _ => { return None; } + }; + + let (total_expr, parent) = validate_method_call_expr(&ctx.sema, total_expr)?; + + let param_list = closure.param_list()?; + let param = param_list.params().next()?; + let body = closure.body()?; + + acc.add( + AssistId("convert_iter_for_each_to_for", AssistKind::RefactorRewrite), + "Replace this `Iterator::for_each` with a for loop", + total_expr.syntax().text_range(), + |builder| { + let mut buf = String::new(); + + format_to!(buf, "for {} in {} ", param, parent); + + match body { + ast::Expr::BlockExpr(body) => format_to!(buf, "{}", body), + _ => format_to!(buf, "{{\n{}\n}}", body) + } + + builder.replace(total_expr.syntax().text_range(), buf) + }, + ) +} + +fn validate_method_call_expr( + sema: &hir::Semantics, + expr: ast::MethodCallExpr, +) -> Option<(ast::Expr, ast::Expr)> { + if expr.name_ref()?.text() != "for_each" { + return None; + } + + let expr = ast::Expr::MethodCallExpr(expr); + let parent = ast::Expr::cast(expr.syntax().first_child()?)?; + + let it_type = sema.type_of_expr(&parent)?; + let module = sema.scope(parent.syntax()).module()?; + let krate = module.krate(); + + let iter_trait = FamousDefs(sema, Some(krate)).core_iter_Iterator()?; + it_type.impls_trait(sema.db, iter_trait, &[]).then(|| (expr, parent)) +} + +#[cfg(test)] +mod tests { + use crate::tests::check_assist; + + use super::*; + + #[test] + fn test_for_each_in_method() { + check_assist( + convert_iter_for_each_to_for, + r" +fn main() { + let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; + x.iter().$0for_each(|(x, y)| { + dbg!(x, y) + }); +}", + r" +fn main() { + let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; + for (x, y) in x.iter() { + dbg!(x, y) + }; +}", + ) + } + + #[test] + fn test_for_each_in_closure() { + check_assist( + convert_iter_for_each_to_for, + r" +fn main() { + let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; + x.iter().for_each($0|(x, y)| { + dbg!(x, y) + }); +}", + r" +fn main() { + let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; + for (x, y) in x.iter() { + dbg!(x, y) + }; +}", + ) + } +} \ No newline at end of file diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index ea62d5f5d..f1aab74d4 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -116,6 +116,7 @@ mod handlers { mod change_visibility; mod convert_integer_literal; mod convert_comment_block; + mod convert_iter_for_each_to_for; mod early_return; mod expand_glob_import; mod extract_function; @@ -181,6 +182,7 @@ mod handlers { change_visibility::change_visibility, convert_integer_literal::convert_integer_literal, convert_comment_block::convert_comment_block, + convert_iter_for_each_to_for::convert_iter_for_each_to_for, early_return::convert_to_guarded_return, expand_glob_import::expand_glob_import, extract_struct_from_enum_variant::extract_struct_from_enum_variant, -- cgit v1.2.3 From 61fb16577b3264a6ff64e33a89031db41218c840 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luiz=20Carlos=20Mour=C3=A3o=20Paes=20de=20Carvalho?= Date: Tue, 9 Mar 2021 23:54:35 -0300 Subject: feat: add expr_for_loop to make in syntax --- crates/syntax/src/ast/make.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 70ba8adb4..05a6b0b25 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -222,6 +222,9 @@ pub fn expr_if( }; expr_from_text(&format!("if {} {} {}", condition, then_branch, else_branch)) } +pub fn expr_for_loop(pat: ast::Pat, expr: ast::Expr, block: ast::BlockExpr) -> ast::Expr { + expr_from_text(&format!("for {} in {} {}", pat, expr, block)) +} pub fn expr_prefix(op: SyntaxKind, expr: ast::Expr) -> ast::Expr { let token = token(op); expr_from_text(&format!("{}{}", token, expr)) -- cgit v1.2.3 From 87dc9d1fcc91b95c35c31b6ecee10ef3954bc286 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luiz=20Carlos=20Mour=C3=A3o=20Paes=20de=20Carvalho?= Date: Tue, 9 Mar 2021 23:55:26 -0300 Subject: refactor: create block expressions and for loops using make --- .../src/handlers/convert_iter_for_each_to_for.rs | 79 ++++++++++++++-------- 1 file changed, 50 insertions(+), 29 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 3220f2f46..2dc4fbcd4 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,5 @@ use ide_db::helpers::FamousDefs; -use stdx::format_to; -use syntax::{AstNode, ast::{self, ArgListOwner}}; +use syntax::{AstNode, ast::{self, make, ArgListOwner, edit::AstNodeEdit}}; use crate::{AssistContext, AssistId, AssistKind, Assists}; @@ -47,7 +46,7 @@ pub(crate) fn convert_iter_for_each_to_for(acc: &mut Assists, ctx: &AssistContex let (total_expr, parent) = validate_method_call_expr(&ctx.sema, total_expr)?; let param_list = closure.param_list()?; - let param = param_list.params().next()?; + let param = param_list.params().next()?.pat()?; let body = closure.body()?; acc.add( @@ -55,16 +54,15 @@ pub(crate) fn convert_iter_for_each_to_for(acc: &mut Assists, ctx: &AssistContex "Replace this `Iterator::for_each` with a for loop", total_expr.syntax().text_range(), |builder| { - let mut buf = String::new(); + let original_indentation = total_expr.indent_level(); - format_to!(buf, "for {} in {} ", param, parent); + let block = match body { + ast::Expr::BlockExpr(block) => block, + _ => make::block_expr(Vec::new(), Some(body)) + }.reset_indent().indent(original_indentation); - match body { - ast::Expr::BlockExpr(body) => format_to!(buf, "{}", body), - _ => format_to!(buf, "{{\n{}\n}}", body) - } - - builder.replace(total_expr.syntax().text_range(), buf) + let expr_for_loop = make::expr_for_loop(param, parent, block); + builder.replace_ast(total_expr, expr_for_loop) }, ) } @@ -94,45 +92,68 @@ mod tests { use super::*; + fn check_assist_with_fixtures(before: &str, after: &str) { + let before = &format!( + "//- /main.rs crate:main deps:core,empty_iter{}{}", + before, + FamousDefs::FIXTURE, + ); + check_assist(convert_iter_for_each_to_for, before, after); + } + #[test] fn test_for_each_in_method() { - check_assist( - convert_iter_for_each_to_for, - r" + check_assist_with_fixtures( + r#" fn main() { let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; x.iter().$0for_each(|(x, y)| { - dbg!(x, y) + println!("x: {}, y: {}", x, y); }); -}", - r" +}"#, + r#" +fn main() { + let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; + for (x, y) in x.iter() { + println!("x: {}, y: {}", x, y); + }; +}"#, + ) + } + + #[test] + fn test_for_each_without_braces() { + check_assist_with_fixtures( + r#" +fn main() { + let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; + x.iter().$0for_each(|(x, y)| println!("x: {}, y: {}", x, y)); +}"#, + r#" fn main() { let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; for (x, y) in x.iter() { - dbg!(x, y) + println!("x: {}, y: {}", x, y) }; -}", +}"#, ) } #[test] fn test_for_each_in_closure() { - check_assist( - convert_iter_for_each_to_for, - r" + check_assist_with_fixtures( + r#" fn main() { let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; - x.iter().for_each($0|(x, y)| { - dbg!(x, y) - }); -}", - r" + x.iter().for_each($0|(x, y)| println!("x: {}, y: {}", x, y)); +}"#, + r#" fn main() { let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; for (x, y) in x.iter() { - dbg!(x, y) + println!("x: {}, y: {}", x, y) }; -}", +}"#, ) } } \ No newline at end of file -- cgit v1.2.3 From b7f97715a38c7486e1329340f6da5236ed5af095 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luiz=20Carlos=20Mour=C3=A3o=20Paes=20de=20Carvalho?= Date: Wed, 10 Mar 2021 00:23:20 -0300 Subject: fix: tests should work for convert_iter_for_each_to_for --- .../src/handlers/convert_iter_for_each_to_for.rs | 55 +++++++++++++++++----- 1 file changed, 44 insertions(+), 11 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 2dc4fbcd4..7903a18fa 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 @@ -88,14 +88,28 @@ fn validate_method_call_expr( #[cfg(test)] mod tests { - use crate::tests::check_assist; + 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 } +} +"; + fn check_assist_with_fixtures(before: &str, after: &str) { let before = &format!( - "//- /main.rs crate:main deps:core,empty_iter{}{}", + "//- /main.rs crate:main deps:core,empty_iter{}{}{}", before, + EMPTY_ITER_FIXTURE, FamousDefs::FIXTURE, ); check_assist(convert_iter_for_each_to_for, before, after); @@ -105,19 +119,22 @@ mod tests { fn test_for_each_in_method() { check_assist_with_fixtures( r#" +use empty_iter::*; fn main() { - let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; + let x = Empty; x.iter().$0for_each(|(x, y)| { println!("x: {}, y: {}", x, y); }); }"#, r#" +use empty_iter::*; fn main() { - let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; + let x = Empty; for (x, y) in x.iter() { println!("x: {}, y: {}", x, y); }; -}"#, +} +"#, ) } @@ -125,17 +142,20 @@ fn main() { fn test_for_each_without_braces() { check_assist_with_fixtures( r#" +use empty_iter::*; fn main() { - let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; + let x = Empty; x.iter().$0for_each(|(x, y)| println!("x: {}, y: {}", x, y)); }"#, r#" +use empty_iter::*; fn main() { - let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; + let x = Empty; for (x, y) in x.iter() { println!("x: {}, y: {}", x, y) }; -}"#, +} +"#, ) } @@ -143,17 +163,30 @@ fn main() { fn test_for_each_in_closure() { check_assist_with_fixtures( r#" +use empty_iter::*; fn main() { - let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; + let x = Empty; x.iter().for_each($0|(x, y)| println!("x: {}, y: {}", x, y)); }"#, r#" +use empty_iter::*; fn main() { - let x = vec![(1, 1), (2, 2), (3, 3), (4, 4)]; + let x = Empty; for (x, y) in x.iter() { println!("x: {}, y: {}", x, y) }; -}"#, +} +"#, ) } + + #[test] + fn test_for_each_not_applicable() { + check_assist_not_applicable( + convert_iter_for_each_to_for, + r#" +fn main() { + value.$0for_each(|x| println!("{}", x)); +}"#) + } } \ No newline at end of file -- cgit v1.2.3 From a224e0087d4faf8fbfae8074c384c4ba03217ba5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luiz=20Carlos=20Mour=C3=A3o=20Paes=20de=20Carvalho?= Date: Wed, 10 Mar 2021 00:32:25 -0300 Subject: fix: code formatting --- .../src/handlers/convert_iter_for_each_to_for.rs | 30 ++++++++++++++-------- 1 file changed, 20 insertions(+), 10 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 7903a18fa..73ef44685 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,5 +1,8 @@ use ide_db::helpers::FamousDefs; -use syntax::{AstNode, ast::{self, make, ArgListOwner, edit::AstNodeEdit}}; +use syntax::{ + ast::{self, edit::AstNodeEdit, make, ArgListOwner}, + AstNode, +}; use crate::{AssistContext, AssistId, AssistKind, Assists}; @@ -31,16 +34,20 @@ pub(crate) fn convert_iter_for_each_to_for(acc: &mut Assists, ctx: &AssistContex ast::Expr::MethodCallExpr(expr) => { closure = match expr.arg_list()?.args().next()? { ast::Expr::ClosureExpr(expr) => expr, - _ => { return None; } + _ => { + return None; + } }; - + expr - }, + } ast::Expr::ClosureExpr(expr) => { closure = expr; ast::MethodCallExpr::cast(closure.syntax().ancestors().nth(2)?)? - }, - _ => { return None; } + } + _ => { + return None; + } }; let (total_expr, parent) = validate_method_call_expr(&ctx.sema, total_expr)?; @@ -58,8 +65,10 @@ pub(crate) fn convert_iter_for_each_to_for(acc: &mut Assists, ctx: &AssistContex let block = match body { ast::Expr::BlockExpr(block) => block, - _ => make::block_expr(Vec::new(), Some(body)) - }.reset_indent().indent(original_indentation); + _ => make::block_expr(Vec::new(), Some(body)), + } + .reset_indent() + .indent(original_indentation); let expr_for_loop = make::expr_for_loop(param, parent, block); builder.replace_ast(total_expr, expr_for_loop) @@ -187,6 +196,7 @@ fn main() { r#" fn main() { value.$0for_each(|x| println!("{}", x)); -}"#) +}"#, + ) } -} \ No newline at end of file +} -- cgit v1.2.3 From 6236b1eaf85c3b93d03d4797aabf4ddc1360a2b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luiz=20Carlos=20Mour=C3=A3o=20Paes=20de=20Carvalho?= Date: Wed, 10 Mar 2021 15:43:57 -0300 Subject: fix: remove semicolon --- .../src/handlers/convert_iter_for_each_to_for.rs | 85 ++++++++++++++-------- 1 file changed, 56 insertions(+), 29 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 73ef44685..5700e6167 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 @@ -28,50 +28,54 @@ use crate::{AssistContext, AssistId, AssistKind, Assists}; /// } /// ``` pub(crate) fn convert_iter_for_each_to_for(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - let closure; - - let total_expr = match ctx.find_node_at_offset::()? { - ast::Expr::MethodCallExpr(expr) => { - closure = match expr.arg_list()?.args().next()? { - ast::Expr::ClosureExpr(expr) => expr, - _ => { - return None; - } - }; - - expr - } - ast::Expr::ClosureExpr(expr) => { - closure = expr; - ast::MethodCallExpr::cast(closure.syntax().ancestors().nth(2)?)? - } + let method; + + let stmt = if let Some(stmt) = ctx.find_node_at_offset::() { + method = ast::MethodCallExpr::cast(stmt.syntax().first_child()?)?; + Some(stmt) + } else { + method = match ctx.find_node_at_offset::()? { + ast::Expr::MethodCallExpr(expr) => expr, + ast::Expr::ClosureExpr(expr) => { + ast::MethodCallExpr::cast(expr.syntax().ancestors().nth(2)?)? + } + _ => { + return None; + } + }; + None + }; + + let closure = match method.arg_list()?.args().next()? { + ast::Expr::ClosureExpr(expr) => expr, _ => { return None; } }; - let (total_expr, parent) = validate_method_call_expr(&ctx.sema, total_expr)?; + let (method, parent) = validate_method_call_expr(&ctx.sema, method)?; let param_list = closure.param_list()?; let param = param_list.params().next()?.pat()?; let body = closure.body()?; + let indent = stmt.as_ref().map_or(method.indent_level(), |stmt| stmt.indent_level()); + let syntax = stmt.as_ref().map_or(method.syntax(), |stmt| stmt.syntax()); + acc.add( AssistId("convert_iter_for_each_to_for", AssistKind::RefactorRewrite), "Replace this `Iterator::for_each` with a for loop", - total_expr.syntax().text_range(), + syntax.text_range(), |builder| { - let original_indentation = total_expr.indent_level(); - let block = match body { ast::Expr::BlockExpr(block) => block, _ => make::block_expr(Vec::new(), Some(body)), } .reset_indent() - .indent(original_indentation); + .indent(indent); let expr_for_loop = make::expr_for_loop(param, parent, block); - builder.replace_ast(total_expr, expr_for_loop) + builder.replace(syntax.text_range(), expr_for_loop.syntax().text()) }, ) } @@ -125,7 +129,7 @@ impl Empty { } #[test] - fn test_for_each_in_method() { + fn test_for_each_in_method_stmt() { check_assist_with_fixtures( r#" use empty_iter::*; @@ -141,14 +145,37 @@ fn main() { let x = Empty; for (x, y) in x.iter() { println!("x: {}, y: {}", x, y); - }; + } } "#, ) } #[test] - fn test_for_each_without_braces() { + fn test_for_each_in_method() { + check_assist_with_fixtures( + r#" +use empty_iter::*; +fn main() { + let x = Empty; + x.iter().$0for_each(|(x, y)| { + println!("x: {}, y: {}", x, y); + }) +}"#, + r#" +use empty_iter::*; +fn main() { + let x = Empty; + for (x, y) in x.iter() { + println!("x: {}, y: {}", x, y); + } +} +"#, + ) + } + + #[test] + fn test_for_each_without_braces_stmt() { check_assist_with_fixtures( r#" use empty_iter::*; @@ -162,14 +189,14 @@ fn main() { let x = Empty; for (x, y) in x.iter() { println!("x: {}, y: {}", x, y) - }; + } } "#, ) } #[test] - fn test_for_each_in_closure() { + fn test_for_each_in_closure_stmt() { check_assist_with_fixtures( r#" use empty_iter::*; @@ -183,7 +210,7 @@ fn main() { let x = Empty; for (x, y) in x.iter() { println!("x: {}, y: {}", x, y) - }; + } } "#, ) -- cgit v1.2.3 From f67861310c1bdcb39301aa6c54d49aa719119a8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luiz=20Carlos=20Mour=C3=A3o=20Paes=20de=20Carvalho?= Date: Fri, 12 Mar 2021 08:06:50 -0300 Subject: refactor: refactored and reduced assist code --- .../src/handlers/convert_iter_for_each_to_for.rs | 57 ++++++++-------------- 1 file changed, 21 insertions(+), 36 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 5700e6167..7e6cae9e1 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 @@ -28,38 +28,20 @@ use crate::{AssistContext, AssistId, AssistKind, Assists}; /// } /// ``` pub(crate) fn convert_iter_for_each_to_for(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - let method; - - let stmt = if let Some(stmt) = ctx.find_node_at_offset::() { - method = ast::MethodCallExpr::cast(stmt.syntax().first_child()?)?; - Some(stmt) - } else { - method = match ctx.find_node_at_offset::()? { - ast::Expr::MethodCallExpr(expr) => expr, - ast::Expr::ClosureExpr(expr) => { - ast::MethodCallExpr::cast(expr.syntax().ancestors().nth(2)?)? - } - _ => { - return None; - } - }; - None - }; + let method = ctx.find_node_at_offset::()?; + let stmt = method.syntax().parent().and_then(ast::ExprStmt::cast); let closure = match method.arg_list()?.args().next()? { ast::Expr::ClosureExpr(expr) => expr, - _ => { - return None; - } + _ => return None, }; - let (method, parent) = validate_method_call_expr(&ctx.sema, method)?; + let (method, receiver) = validate_method_call_expr(&ctx.sema, method)?; let param_list = closure.param_list()?; let param = param_list.params().next()?.pat()?; let body = closure.body()?; - let indent = stmt.as_ref().map_or(method.indent_level(), |stmt| stmt.indent_level()); let syntax = stmt.as_ref().map_or(method.syntax(), |stmt| stmt.syntax()); acc.add( @@ -67,6 +49,8 @@ pub(crate) fn convert_iter_for_each_to_for(acc: &mut Assists, ctx: &AssistContex "Replace this `Iterator::for_each` with a for loop", syntax.text_range(), |builder| { + let indent = stmt.as_ref().map_or(method.indent_level(), |stmt| stmt.indent_level()); + let block = match body { ast::Expr::BlockExpr(block) => block, _ => make::block_expr(Vec::new(), Some(body)), @@ -74,7 +58,7 @@ pub(crate) fn convert_iter_for_each_to_for(acc: &mut Assists, ctx: &AssistContex .reset_indent() .indent(indent); - let expr_for_loop = make::expr_for_loop(param, parent, block); + let expr_for_loop = make::expr_for_loop(param, receiver, block); builder.replace(syntax.text_range(), expr_for_loop.syntax().text()) }, ) @@ -88,15 +72,15 @@ fn validate_method_call_expr( return None; } + let receiver = expr.receiver()?; let expr = ast::Expr::MethodCallExpr(expr); - let parent = ast::Expr::cast(expr.syntax().first_child()?)?; - let it_type = sema.type_of_expr(&parent)?; - let module = sema.scope(parent.syntax()).module()?; + let it_type = sema.type_of_expr(&receiver)?; + let module = sema.scope(receiver.syntax()).module()?; let krate = module.krate(); let iter_trait = FamousDefs(sema, Some(krate)).core_iter_Iterator()?; - it_type.impls_trait(sema.db, iter_trait, &[]).then(|| (expr, parent)) + it_type.impls_trait(sema.db, iter_trait, &[]).then(|| (expr, receiver)) } #[cfg(test)] @@ -175,20 +159,22 @@ fn main() { } #[test] - fn test_for_each_without_braces_stmt() { + fn test_for_each_in_iter_stmt() { check_assist_with_fixtures( r#" use empty_iter::*; fn main() { - let x = Empty; - x.iter().$0for_each(|(x, y)| println!("x: {}, y: {}", x, y)); + let x = Empty.iter(); + x.$0for_each(|(x, y)| { + println!("x: {}, y: {}", x, y); + }); }"#, r#" use empty_iter::*; fn main() { - let x = Empty; - for (x, y) in x.iter() { - println!("x: {}, y: {}", x, y) + let x = Empty.iter(); + for (x, y) in x { + println!("x: {}, y: {}", x, y); } } "#, @@ -196,13 +182,13 @@ fn main() { } #[test] - fn test_for_each_in_closure_stmt() { + fn test_for_each_without_braces_stmt() { check_assist_with_fixtures( r#" use empty_iter::*; fn main() { let x = Empty; - x.iter().for_each($0|(x, y)| println!("x: {}, y: {}", x, y)); + x.iter().$0for_each(|(x, y)| println!("x: {}, y: {}", x, y)); }"#, r#" use empty_iter::*; @@ -215,7 +201,6 @@ fn main() { "#, ) } - #[test] fn test_for_each_not_applicable() { check_assist_not_applicable( -- cgit v1.2.3 From 7a9230acdfe900d9e42f0b8cd37e4a7db6d8ff18 Mon Sep 17 00:00:00 2001 From: Luiz Carlos Date: Fri, 12 Mar 2021 08:44:03 -0300 Subject: fix: replace doc-comments with normal comments Co-authored-by: Lukas Wirth --- .../src/handlers/convert_iter_for_each_to_for.rs | 40 +++++++++++----------- 1 file changed, 20 insertions(+), 20 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 7e6cae9e1..661a3fbeb 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 @@ -6,27 +6,27 @@ use syntax::{ use crate::{AssistContext, AssistId, AssistKind, Assists}; -/// Assist: convert_iter_for_each_to_for +// Assist: convert_iter_for_each_to_for // -/// Converts an Iterator::for_each function into a for loop. -/// -/// ```rust -/// fn main() { -/// let vec = vec![(1, 2), (2, 3), (3, 4)]; -/// x.iter().for_each(|(x, y)| { -/// println!("x: {}, y: {}", x, y); -/// }) -/// } -/// ``` -/// -> -/// ```rust -/// fn main() { -/// let vec = vec![(1, 2), (2, 3), (3, 4)]; -/// for (x, y) in x.iter() { -/// println!("x: {}, y: {}", x, y); -/// }); -/// } -/// ``` +// Converts an Iterator::for_each function into a for loop. +// +// ```rust +// fn main() { +// let vec = vec![(1, 2), (2, 3), (3, 4)]; +// x.iter().for_each(|(x, y)| { +// println!("x: {}, y: {}", x, y); +// }); +// } +// ``` +// -> +// ```rust +// fn main() { +// let vec = vec![(1, 2), (2, 3), (3, 4)]; +// for (x, y) in x.iter() { +// println!("x: {}, y: {}", x, y); +// } +// } +// ``` pub(crate) fn convert_iter_for_each_to_for(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let method = ctx.find_node_at_offset::()?; let stmt = method.syntax().parent().and_then(ast::ExprStmt::cast); -- cgit v1.2.3 From e505752442f849c7d37465f0e0b6aa042644787b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luiz=20Carlos=20Mour=C3=A3o=20Paes=20de=20Carvalho?= Date: Fri, 12 Mar 2021 08:53:57 -0300 Subject: fix: generated test fixture --- .../src/handlers/convert_iter_for_each_to_for.rs | 4 ++-- crates/ide_assists/src/tests/generated.rs | 23 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 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 661a3fbeb..ed15f169e 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 @@ -10,7 +10,7 @@ use crate::{AssistContext, AssistId, AssistKind, Assists}; // // Converts an Iterator::for_each function into a for loop. // -// ```rust +// ``` // fn main() { // let vec = vec![(1, 2), (2, 3), (3, 4)]; // x.iter().for_each(|(x, y)| { @@ -19,7 +19,7 @@ use crate::{AssistContext, AssistId, AssistKind, Assists}; // } // ``` // -> -// ```rust +// ``` // fn main() { // let vec = vec![(1, 2), (2, 3), (3, 4)]; // for (x, y) in x.iter() { diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index 304b5798f..af513ca22 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -205,6 +205,29 @@ const _: i32 = 0b1010; ) } +#[test] +fn doctest_convert_iter_for_each_to_for() { + check_doc_test( + "convert_iter_for_each_to_for", + r#####" +fn main() { + let vec = vec![(1, 2), (2, 3), (3, 4)]; + x.iter().for_each(|(x, y)| { + println!("x: {}, y: {}", x, y); + }); +} +"#####, + r#####" +fn main() { + let vec = vec![(1, 2), (2, 3), (3, 4)]; + for (x, y) in x.iter() { + println!("x: {}, y: {}", x, y); + } +} +"#####, + ) +} + #[test] fn doctest_convert_to_guarded_return() { check_doc_test( -- cgit v1.2.3 From 6d35c67b6e39fae1efc48405b49d408b86666534 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 12 Mar 2021 15:41:08 +0100 Subject: Fix convert_iter_for_each_to_for doctest --- .../src/handlers/convert_iter_for_each_to_for.rs | 56 +++++++++++++++++----- crates/ide_assists/src/tests/generated.rs | 15 ++++-- 2 files changed, 56 insertions(+), 15 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 ed15f169e..4e75a7b14 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 @@ -11,37 +11,45 @@ use crate::{AssistContext, AssistId, AssistKind, Assists}; // Converts an Iterator::for_each function into a for loop. // // ``` +// # //- /lib.rs crate:core +// # pub mod iter { pub mod traits { pub mod iterator { pub trait Iterator {} } } } +// # pub struct SomeIter; +// # impl self::iter::traits::iterator::Iterator for SomeIter {} +// # //- /lib.rs crate:main deps:core +// # use core::SomeIter; // fn main() { -// let vec = vec![(1, 2), (2, 3), (3, 4)]; -// x.iter().for_each(|(x, y)| { +// let iter = SomeIter; +// iter.for_each$0(|(x, y)| { // println!("x: {}, y: {}", x, y); // }); // } // ``` // -> // ``` +// # use core::SomeIter; // fn main() { -// let vec = vec![(1, 2), (2, 3), (3, 4)]; -// for (x, y) in x.iter() { +// let iter = SomeIter; +// for (x, y) in iter { // println!("x: {}, y: {}", x, y); // } // } // ``` + pub(crate) fn convert_iter_for_each_to_for(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let method = ctx.find_node_at_offset::()?; - let stmt = method.syntax().parent().and_then(ast::ExprStmt::cast); let closure = match method.arg_list()?.args().next()? { ast::Expr::ClosureExpr(expr) => expr, _ => return None, }; - let (method, receiver) = validate_method_call_expr(&ctx.sema, method)?; + let (method, receiver) = validate_method_call_expr(ctx, method)?; let param_list = closure.param_list()?; let param = param_list.params().next()?.pat()?; let body = closure.body()?; + let stmt = method.syntax().parent().and_then(ast::ExprStmt::cast); let syntax = stmt.as_ref().map_or(method.syntax(), |stmt| stmt.syntax()); acc.add( @@ -65,13 +73,18 @@ pub(crate) fn convert_iter_for_each_to_for(acc: &mut Assists, ctx: &AssistContex } fn validate_method_call_expr( - sema: &hir::Semantics, + ctx: &AssistContext, expr: ast::MethodCallExpr, ) -> Option<(ast::Expr, ast::Expr)> { - if expr.name_ref()?.text() != "for_each" { + let name_ref = expr.name_ref()?; + if name_ref.syntax().text_range().intersect(ctx.frange.range).is_none() + || name_ref.text() != "for_each" + { return None; } + let sema = &ctx.sema; + let receiver = expr.receiver()?; let expr = ast::Expr::MethodCallExpr(expr); @@ -85,7 +98,7 @@ fn validate_method_call_expr( #[cfg(test)] mod tests { - use crate::tests::{check_assist, check_assist_not_applicable}; + use crate::tests::{self, check_assist}; use super::*; @@ -112,6 +125,16 @@ impl Empty { check_assist(convert_iter_for_each_to_for, before, after); } + fn check_assist_not_applicable(before: &str) { + let before = &format!( + "//- /main.rs crate:main deps:core,empty_iter{}{}{}", + before, + EMPTY_ITER_FIXTURE, + FamousDefs::FIXTURE, + ); + tests::check_assist_not_applicable(convert_iter_for_each_to_for, before); + } + #[test] fn test_for_each_in_method_stmt() { check_assist_with_fixtures( @@ -201,13 +224,24 @@ fn main() { "#, ) } + #[test] fn test_for_each_not_applicable() { check_assist_not_applicable( - convert_iter_for_each_to_for, r#" fn main() { - value.$0for_each(|x| println!("{}", x)); + ().$0for_each(|x| println!("{}", x)); +}"#, + ) + } + + #[test] + fn test_for_each_not_applicable_invalid_cursor_pos() { + check_assist_not_applicable( + r#" +use empty_iter::*; +fn main() { + Empty.iter().for_each(|(x, y)| $0println!("x: {}, y: {}", x, y)); }"#, ) } diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index af513ca22..3f77edd8d 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -210,17 +210,24 @@ fn doctest_convert_iter_for_each_to_for() { check_doc_test( "convert_iter_for_each_to_for", r#####" +//- /lib.rs crate:core +pub mod iter { pub mod traits { pub mod iterator { pub trait Iterator {} } } } +pub struct SomeIter; +impl self::iter::traits::iterator::Iterator for SomeIter {} +//- /lib.rs crate:main deps:core +use core::SomeIter; fn main() { - let vec = vec![(1, 2), (2, 3), (3, 4)]; - x.iter().for_each(|(x, y)| { + let iter = SomeIter; + iter.for_each$0(|(x, y)| { println!("x: {}, y: {}", x, y); }); } "#####, r#####" +use core::SomeIter; fn main() { - let vec = vec![(1, 2), (2, 3), (3, 4)]; - for (x, y) in x.iter() { + let iter = SomeIter; + for (x, y) in iter { println!("x: {}, y: {}", x, y); } } -- cgit v1.2.3