From 388525fa0dc4e38dfd927413b0f694467a7ca492 Mon Sep 17 00:00:00 2001 From: k-nasa Date: Sat, 9 Oct 2021 11:13:27 +0900 Subject: [PATCH 1/7] Add test casee --- .../src/handlers/replace_if_let_with_match.rs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs index 8aad3e2f52b..6f383ae8bba 100644 --- a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs +++ b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs @@ -885,6 +885,32 @@ fn foo() { Bar(bar) => println!("bar {}", bar), } } +"#, + ); + } + + #[test] + fn nested_type() { + check_assist( + replace_if_let_with_match, + r#" +//- minicore: result +fn foo(x: Result) { + let bar: Result<_, ()> = Ok(Some(1)); + $0if let Ok(Some(_)) = bar { + () + } else { + () + } +} +"#, + r#" +fn foo(x: Result) { + let bar: Result<_, ()> = Ok(Some(1)); + match bar { + Ok(Some(_)) => (), + _ => (), + } "#, ); } From aeee70397e8230117f0097fa8624d59d910bd324 Mon Sep 17 00:00:00 2001 From: k-nasa Date: Sat, 9 Oct 2021 13:19:21 +0900 Subject: [PATCH 2/7] Apply make_else_arm to general case --- .../src/handlers/replace_if_let_with_match.rs | 88 +++++++------------ 1 file changed, 34 insertions(+), 54 deletions(-) diff --git a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs index 6f383ae8bba..5b766ecbeae 100644 --- a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs +++ b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs @@ -93,7 +93,7 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext) available_range, move |edit| { let match_expr = { - let else_arm = make_else_arm(ctx, else_block, &cond_bodies); + let else_arm = make_else_arm(else_block); let make_match_arm = |(pat, body): (_, ast::BlockExpr)| { let body = body.reset_indent().indent(IndentLevel(1)); match pat { @@ -125,30 +125,9 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext) ) } -fn make_else_arm( - ctx: &AssistContext, - else_block: Option, - conditionals: &[(Either, ast::BlockExpr)], -) -> ast::MatchArm { +fn make_else_arm(else_block: Option) -> ast::MatchArm { if let Some(else_block) = else_block { - let pattern = if let [(Either::Left(pat), _)] = conditionals { - ctx.sema - .type_of_pat(pat) - .and_then(|ty| TryEnum::from_ty(&ctx.sema, &ty.adjusted())) - .zip(Some(pat)) - } else { - None - }; - let pattern = match pattern { - Some((it, pat)) => { - if does_pat_match_variant(pat, &it.sad_pattern()) { - it.happy_pattern_wildcard() - } else { - it.sad_pattern() - } - } - None => make::wildcard_pat().into(), - }; + let pattern = make::wildcard_pat().into(); make::match_arm(iter::once(pattern), None, unwrap_trivial_block(else_block)) } else { make::match_arm(iter::once(make::wildcard_pat().into()), None, make::expr_unit()) @@ -460,7 +439,7 @@ fn foo(x: Option) { fn foo(x: Option) { match x { Some(x) => println!("{}", x), - None => println!("none"), + _ => println!("none"), } } "#, @@ -485,7 +464,7 @@ fn foo(x: Option) { fn foo(x: Option) { match x { None => println!("none"), - Some(_) => println!("some"), + _ => println!("some"), } } "#, @@ -510,7 +489,7 @@ fn foo(x: Result) { fn foo(x: Result) { match x { Ok(x) => println!("{}", x), - Err(_) => println!("none"), + _ => println!("none"), } } "#, @@ -535,7 +514,7 @@ fn foo(x: Result) { fn foo(x: Result) { match x { Err(x) => println!("{}", x), - Ok(_) => println!("ok"), + _ => println!("ok"), } } "#, @@ -574,6 +553,33 @@ fn main() { ) } + #[test] + fn replace_if_let_with_match_nested_type() { + check_assist( + replace_if_let_with_match, + r#" +//- minicore: result +fn foo(x: Result) { + let bar: Result<_, ()> = Ok(Some(1)); + $0if let Ok(Some(_)) = bar { + () + } else { + () + } +} +"#, + r#" +fn foo(x: Result) { + let bar: Result<_, ()> = Ok(Some(1)); + match bar { + Ok(Some(_)) => (), + _ => (), + } +} +"#, + ); + } + #[test] fn test_replace_match_with_if_let_unwraps_simple_expressions() { check_assist( @@ -885,32 +891,6 @@ fn foo() { Bar(bar) => println!("bar {}", bar), } } -"#, - ); - } - - #[test] - fn nested_type() { - check_assist( - replace_if_let_with_match, - r#" -//- minicore: result -fn foo(x: Result) { - let bar: Result<_, ()> = Ok(Some(1)); - $0if let Ok(Some(_)) = bar { - () - } else { - () - } -} -"#, - r#" -fn foo(x: Result) { - let bar: Result<_, ()> = Ok(Some(1)); - match bar { - Ok(Some(_)) => (), - _ => (), - } "#, ); } From a6a052f4078825ba307843df1770c92d96827075 Mon Sep 17 00:00:00 2001 From: k-nasa Date: Wed, 13 Oct 2021 20:36:04 +0900 Subject: [PATCH 3/7] Revert "Apply make_else_arm to general case" This reverts commit aeee70397e8230117f0097fa8624d59d910bd324. --- .../src/handlers/replace_if_let_with_match.rs | 88 ++++++++++++------- 1 file changed, 54 insertions(+), 34 deletions(-) diff --git a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs index 5b766ecbeae..6f383ae8bba 100644 --- a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs +++ b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs @@ -93,7 +93,7 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext) available_range, move |edit| { let match_expr = { - let else_arm = make_else_arm(else_block); + let else_arm = make_else_arm(ctx, else_block, &cond_bodies); let make_match_arm = |(pat, body): (_, ast::BlockExpr)| { let body = body.reset_indent().indent(IndentLevel(1)); match pat { @@ -125,9 +125,30 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext) ) } -fn make_else_arm(else_block: Option) -> ast::MatchArm { +fn make_else_arm( + ctx: &AssistContext, + else_block: Option, + conditionals: &[(Either, ast::BlockExpr)], +) -> ast::MatchArm { if let Some(else_block) = else_block { - let pattern = make::wildcard_pat().into(); + let pattern = if let [(Either::Left(pat), _)] = conditionals { + ctx.sema + .type_of_pat(pat) + .and_then(|ty| TryEnum::from_ty(&ctx.sema, &ty.adjusted())) + .zip(Some(pat)) + } else { + None + }; + let pattern = match pattern { + Some((it, pat)) => { + if does_pat_match_variant(pat, &it.sad_pattern()) { + it.happy_pattern_wildcard() + } else { + it.sad_pattern() + } + } + None => make::wildcard_pat().into(), + }; make::match_arm(iter::once(pattern), None, unwrap_trivial_block(else_block)) } else { make::match_arm(iter::once(make::wildcard_pat().into()), None, make::expr_unit()) @@ -439,7 +460,7 @@ fn foo(x: Option) { fn foo(x: Option) { match x { Some(x) => println!("{}", x), - _ => println!("none"), + None => println!("none"), } } "#, @@ -464,7 +485,7 @@ fn foo(x: Option) { fn foo(x: Option) { match x { None => println!("none"), - _ => println!("some"), + Some(_) => println!("some"), } } "#, @@ -489,7 +510,7 @@ fn foo(x: Result) { fn foo(x: Result) { match x { Ok(x) => println!("{}", x), - _ => println!("none"), + Err(_) => println!("none"), } } "#, @@ -514,7 +535,7 @@ fn foo(x: Result) { fn foo(x: Result) { match x { Err(x) => println!("{}", x), - _ => println!("ok"), + Ok(_) => println!("ok"), } } "#, @@ -553,33 +574,6 @@ fn main() { ) } - #[test] - fn replace_if_let_with_match_nested_type() { - check_assist( - replace_if_let_with_match, - r#" -//- minicore: result -fn foo(x: Result) { - let bar: Result<_, ()> = Ok(Some(1)); - $0if let Ok(Some(_)) = bar { - () - } else { - () - } -} -"#, - r#" -fn foo(x: Result) { - let bar: Result<_, ()> = Ok(Some(1)); - match bar { - Ok(Some(_)) => (), - _ => (), - } -} -"#, - ); - } - #[test] fn test_replace_match_with_if_let_unwraps_simple_expressions() { check_assist( @@ -891,6 +885,32 @@ fn foo() { Bar(bar) => println!("bar {}", bar), } } +"#, + ); + } + + #[test] + fn nested_type() { + check_assist( + replace_if_let_with_match, + r#" +//- minicore: result +fn foo(x: Result) { + let bar: Result<_, ()> = Ok(Some(1)); + $0if let Ok(Some(_)) = bar { + () + } else { + () + } +} +"#, + r#" +fn foo(x: Result) { + let bar: Result<_, ()> = Ok(Some(1)); + match bar { + Ok(Some(_)) => (), + _ => (), + } "#, ); } From ef9c4b666f76cfaeea243760702d43719264f145 Mon Sep 17 00:00:00 2001 From: k-nasa Date: Wed, 13 Oct 2021 21:03:01 +0900 Subject: [PATCH 4/7] move test case --- .../src/handlers/replace_if_let_with_match.rs | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs index 6f383ae8bba..f7b601d144a 100644 --- a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs +++ b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs @@ -574,6 +574,32 @@ fn main() { ) } + #[test] + fn nested_type() { + check_assist( + replace_if_let_with_match, + r#" +//- minicore: result +fn foo(x: Result) { + let bar: Result<_, ()> = Ok(Some(1)); + $0if let Ok(Some(_)) = bar { + () + } else { + () + } +} +"#, + r#" +fn foo(x: Result) { + let bar: Result<_, ()> = Ok(Some(1)); + match bar { + Ok(Some(_)) => (), + _ => (), + } +"#, + ); + } + #[test] fn test_replace_match_with_if_let_unwraps_simple_expressions() { check_assist( @@ -885,32 +911,6 @@ fn foo() { Bar(bar) => println!("bar {}", bar), } } -"#, - ); - } - - #[test] - fn nested_type() { - check_assist( - replace_if_let_with_match, - r#" -//- minicore: result -fn foo(x: Result) { - let bar: Result<_, ()> = Ok(Some(1)); - $0if let Ok(Some(_)) = bar { - () - } else { - () - } -} -"#, - r#" -fn foo(x: Result) { - let bar: Result<_, ()> = Ok(Some(1)); - match bar { - Ok(Some(_)) => (), - _ => (), - } "#, ); } From b3930599a791682e5ae8268f0c779d74e1e3dfda Mon Sep 17 00:00:00 2001 From: k-nasa Date: Wed, 13 Oct 2021 22:02:39 +0900 Subject: [PATCH 5/7] calc depth --- .../src/handlers/replace_if_let_with_match.rs | 5 ++- crates/ide_assists/src/utils.rs | 37 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs index f7b601d144a..dcbc277b26a 100644 --- a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs +++ b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs @@ -12,7 +12,7 @@ use syntax::{ }; use crate::{ - utils::{does_pat_match_variant, unwrap_trivial_block}, + utils::{does_nested_pattern, does_pat_match_variant, unwrap_trivial_block}, AssistContext, AssistId, AssistKind, Assists, }; @@ -143,6 +143,8 @@ fn make_else_arm( Some((it, pat)) => { if does_pat_match_variant(pat, &it.sad_pattern()) { it.happy_pattern_wildcard() + } else if does_nested_pattern(pat) { + make::wildcard_pat().into() } else { it.sad_pattern() } @@ -596,6 +598,7 @@ fn foo(x: Result) { Ok(Some(_)) => (), _ => (), } +} "#, ); } diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs index 3d4ab968fbb..64adddb1de6 100644 --- a/crates/ide_assists/src/utils.rs +++ b/crates/ide_assists/src/utils.rs @@ -285,6 +285,43 @@ pub(crate) fn does_pat_match_variant(pat: &ast::Pat, var: &ast::Pat) -> bool { pat_head == var_head } +pub(crate) fn does_nested_pattern(pat: &ast::Pat) -> bool { + let depth = calc_depth(pat, 0); + + if 1 < depth { + return true; + } + false +} + +fn calc_depth(pat: &ast::Pat, mut depth: usize) -> usize { + match pat { + ast::Pat::IdentPat(_) + | ast::Pat::BoxPat(_) + | ast::Pat::RestPat(_) + | ast::Pat::LiteralPat(_) + | ast::Pat::MacroPat(_) + | ast::Pat::OrPat(_) + | ast::Pat::ParenPat(_) + | ast::Pat::PathPat(_) + | ast::Pat::WildcardPat(_) + | ast::Pat::RangePat(_) + | ast::Pat::RecordPat(_) + | ast::Pat::RefPat(_) + | ast::Pat::SlicePat(_) + | ast::Pat::TuplePat(_) + | ast::Pat::ConstBlockPat(_) => 1, + + // TODO implement + ast::Pat::TupleStructPat(pat) => { + for p in pat.fields() { + depth += calc_depth(&p, depth + 1); + } + depth + } + } +} + // Uses a syntax-driven approach to find any impl blocks for the struct that // exist within the module/file // From bc29b75b927cf42ee91f0cb17c6697bb1aff2166 Mon Sep 17 00:00:00 2001 From: k-nasa Date: Wed, 13 Oct 2021 22:06:53 +0900 Subject: [PATCH 6/7] update calc_depth --- crates/ide_assists/src/utils.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs index 64adddb1de6..22221afe74f 100644 --- a/crates/ide_assists/src/utils.rs +++ b/crates/ide_assists/src/utils.rs @@ -294,7 +294,7 @@ pub(crate) fn does_nested_pattern(pat: &ast::Pat) -> bool { false } -fn calc_depth(pat: &ast::Pat, mut depth: usize) -> usize { +fn calc_depth(pat: &ast::Pat, depth: usize) -> usize { match pat { ast::Pat::IdentPat(_) | ast::Pat::BoxPat(_) @@ -312,12 +312,16 @@ fn calc_depth(pat: &ast::Pat, mut depth: usize) -> usize { | ast::Pat::TuplePat(_) | ast::Pat::ConstBlockPat(_) => 1, - // TODO implement + // TODO: Other patterns may also be nested. Currently it simply supports only `TupleStructPat` ast::Pat::TupleStructPat(pat) => { + let mut max_depth = depth; for p in pat.fields() { - depth += calc_depth(&p, depth + 1); + let d = calc_depth(&p, depth + 1); + if d > max_depth { + max_depth = d + } } - depth + max_depth } } } From bd9bab87ed72779c3bf407137928b75555f5d2a9 Mon Sep 17 00:00:00 2001 From: k-nasa Date: Wed, 13 Oct 2021 23:07:49 +0900 Subject: [PATCH 7/7] fix --- crates/ide_assists/src/utils.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs index 22221afe74f..ea7edadfe95 100644 --- a/crates/ide_assists/src/utils.rs +++ b/crates/ide_assists/src/utils.rs @@ -310,9 +310,9 @@ fn calc_depth(pat: &ast::Pat, depth: usize) -> usize { | ast::Pat::RefPat(_) | ast::Pat::SlicePat(_) | ast::Pat::TuplePat(_) - | ast::Pat::ConstBlockPat(_) => 1, + | ast::Pat::ConstBlockPat(_) => depth, - // TODO: Other patterns may also be nested. Currently it simply supports only `TupleStructPat` + // FIXME: Other patterns may also be nested. Currently it simply supports only `TupleStructPat` ast::Pat::TupleStructPat(pat) => { let mut max_depth = depth; for p in pat.fields() {