From 056e18fcbd6ac681b968d50534f2b7de21008316 Mon Sep 17 00:00:00 2001 From: Jeroen Vannevel Date: Wed, 5 Jan 2022 01:03:27 +0000 Subject: [PATCH 01/12] correctly handle mutable references --- .../src/handlers/extract_variable.rs | 90 ++++++++++++++++++- 1 file changed, 88 insertions(+), 2 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index 7a57ab3b9b7..d7a8e1dd4c2 100644 --- a/crates/ide_assists/src/handlers/extract_variable.rs +++ b/crates/ide_assists/src/handlers/extract_variable.rs @@ -52,6 +52,12 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option } } + let is_mutable_reference = if let Some(receiver_type) = get_receiver_type(&ctx, &to_extract) { + receiver_type.is_mutable_reference() + } else { + false + }; + let anchor = Anchor::from(&to_extract)?; let indent = anchor.syntax().prev_sibling_or_token()?.as_token()?.clone(); let target = to_extract.syntax().text_range(); @@ -77,11 +83,15 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option None => to_extract.syntax().text_range(), }; + let reference_modifier = if is_mutable_reference { "&mut " } else { "" }; + match anchor { Anchor::Before(_) | Anchor::Replace(_) => { - format_to!(buf, "let {} = ", var_name) + format_to!(buf, "let {} = {}", var_name, reference_modifier) + } + Anchor::WrapInBlock(_) => { + format_to!(buf, "{{ let {} = {}", var_name, reference_modifier) } - Anchor::WrapInBlock(_) => format_to!(buf, "{{ let {} = ", var_name), }; format_to!(buf, "{}", to_extract.syntax()); @@ -146,6 +156,22 @@ fn valid_target_expr(node: SyntaxNode) -> Option { } } +fn get_receiver_type(ctx: &AssistContext, expression: &ast::Expr) -> Option { + let receiver = get_receiver(expression.to_owned())?; + Some(ctx.sema.type_of_expr(&receiver)?.original()) +} + +fn get_receiver(expression: ast::Expr) -> Option { + match expression { + ast::Expr::FieldExpr(field) if field.expr().is_some() => { + let nested_expression = &field.expr()?; + get_receiver(nested_expression.to_owned()) + } + ast::Expr::PathExpr(_) => Some(expression), + _ => None, + } +} + #[derive(Debug)] enum Anchor { Before(SyntaxNode), @@ -900,4 +926,64 @@ const X: usize = $0100$0; ", ); } + + #[test] + fn test_extract_var_mutable_reference_parameter() { + check_assist( + extract_variable, + r#" +struct S { + vec: Vec +} + +fn foo(s: &mut S) { + $0s.vec$0.push(0); +}"#, + r#" +struct S { + vec: Vec +} + +fn foo(s: &mut S) { + let $0var_name = &mut s.vec; + var_name.push(0); +}"#, + ); + } + + #[test] + fn test_extract_var_mutable_reference_parameter_deep_nesting() { + check_assist( + extract_variable, + r#" +struct Y { + field: X +} +struct X { + field: S +} +struct S { + vec: Vec +} + +fn foo(f: &mut Y) { + $0f.field.field.vec$0.push(0); +}"#, + r#" +struct Y { + field: X +} +struct X { + field: S +} +struct S { + vec: Vec +} + +fn foo(f: &mut Y) { + let $0var_name = &mut f.field.field.vec; + var_name.push(0); +}"#, + ); + } } From 817f47828c86683e6950576eb213e9d9d2524bf8 Mon Sep 17 00:00:00 2001 From: Jeroen Vannevel Date: Wed, 5 Jan 2022 01:15:54 +0000 Subject: [PATCH 02/12] support ref params as well --- .../src/handlers/extract_variable.rs | 113 +++++++++++++++++- 1 file changed, 109 insertions(+), 4 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index d7a8e1dd4c2..fb48c0b4f12 100644 --- a/crates/ide_assists/src/handlers/extract_variable.rs +++ b/crates/ide_assists/src/handlers/extract_variable.rs @@ -52,10 +52,10 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option } } - let is_mutable_reference = if let Some(receiver_type) = get_receiver_type(&ctx, &to_extract) { - receiver_type.is_mutable_reference() + let ref_kind: RefKind = if let Some(receiver_type) = get_receiver_type(&ctx, &to_extract) { + if receiver_type.is_mutable_reference() { RefKind::MutRef } else { RefKind::Ref } } else { - false + RefKind::None }; let anchor = Anchor::from(&to_extract)?; @@ -83,7 +83,11 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option None => to_extract.syntax().text_range(), }; - let reference_modifier = if is_mutable_reference { "&mut " } else { "" }; + let reference_modifier = match ref_kind { + RefKind::MutRef => "&mut ", + RefKind::Ref => "&", + RefKind::None => "" + }; match anchor { Anchor::Before(_) | Anchor::Replace(_) => { @@ -172,6 +176,13 @@ fn get_receiver(expression: ast::Expr) -> Option { } } +#[derive(Debug)] +enum RefKind { + Ref, + MutRef, + None +} + #[derive(Debug)] enum Anchor { Before(SyntaxNode), @@ -983,6 +994,100 @@ struct S { fn foo(f: &mut Y) { let $0var_name = &mut f.field.field.vec; var_name.push(0); +}"#, + ); + } + + #[test] + fn test_extract_var_reference_parameter() { + check_assist( + extract_variable, + r#" +struct X; + +impl X { + fn do_thing(&self) { + + } +} + +struct S { + sub: X +} + +fn foo(s: &S) { + $0s.sub$0.do_thing(); +}"#, + r#" +struct X; + +impl X { + fn do_thing(&self) { + + } +} + +struct S { + sub: X +} + +fn foo(s: &S) { + let $0x = &s.sub; + x.do_thing(); +}"#, + ); + } + + #[test] + fn test_extract_var_reference_parameter_deep_nesting() { + check_assist( + extract_variable, + r#" +struct Z; +impl Z { + fn do_thing(&self) { + + } +} + +struct Y { + field: Z +} + +struct X { + field: Y +} + +struct S { + sub: X +} + +fn foo(s: &S) { + $0s.sub.field.field$0.do_thing(); +}"#, + r#" +struct Z; +impl Z { + fn do_thing(&self) { + + } +} + +struct Y { + field: Z +} + +struct X { + field: Y +} + +struct S { + sub: X +} + +fn foo(s: &S) { + let $0z = &s.sub.field.field; + z.do_thing(); }"#, ); } From cd5ad4e5007c0de29c487bb2763db7e3457b2a2c Mon Sep 17 00:00:00 2001 From: Jeroen Vannevel Date: Wed, 5 Jan 2022 01:18:55 +0000 Subject: [PATCH 03/12] Don't include a ref if none was declared --- .../src/handlers/extract_variable.rs | 52 +++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index fb48c0b4f12..39a4700915f 100644 --- a/crates/ide_assists/src/handlers/extract_variable.rs +++ b/crates/ide_assists/src/handlers/extract_variable.rs @@ -53,7 +53,13 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option } let ref_kind: RefKind = if let Some(receiver_type) = get_receiver_type(&ctx, &to_extract) { - if receiver_type.is_mutable_reference() { RefKind::MutRef } else { RefKind::Ref } + if receiver_type.is_mutable_reference() { + RefKind::MutRef + } else if receiver_type.is_reference() { + RefKind::Ref + } else { + RefKind::None + } } else { RefKind::None }; @@ -86,7 +92,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option let reference_modifier = match ref_kind { RefKind::MutRef => "&mut ", RefKind::Ref => "&", - RefKind::None => "" + RefKind::None => "", }; match anchor { @@ -180,7 +186,7 @@ fn get_receiver(expression: ast::Expr) -> Option { enum RefKind { Ref, MutRef, - None + None, } #[derive(Debug)] @@ -1088,6 +1094,46 @@ struct S { fn foo(s: &S) { let $0z = &s.sub.field.field; z.do_thing(); +}"#, + ); + } + + #[test] + fn test_extract_var_regular_parameter() { + check_assist( + extract_variable, + r#" +struct X; + +impl X { + fn do_thing(&self) { + + } +} + +struct S { + sub: X +} + +fn foo(s: S) { + $0s.sub$0.do_thing(); +}"#, + r#" +struct X; + +impl X { + fn do_thing(&self) { + + } +} + +struct S { + sub: X +} + +fn foo(s: S) { + let $0x = s.sub; + x.do_thing(); }"#, ); } From 4c1a1b2570e5d60b49259bea261f7db16ceb627a Mon Sep 17 00:00:00 2001 From: Jeroen Vannevel Date: Wed, 5 Jan 2022 01:27:15 +0000 Subject: [PATCH 04/12] failing test for a reference local --- .../src/handlers/extract_variable.rs | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index 39a4700915f..9e11e38d3d6 100644 --- a/crates/ide_assists/src/handlers/extract_variable.rs +++ b/crates/ide_assists/src/handlers/extract_variable.rs @@ -1134,6 +1134,72 @@ struct S { fn foo(s: S) { let $0x = s.sub; x.do_thing(); +}"#, + ); + } + + #[test] + fn test_extract_var_reference_local() { + check_assist( + extract_variable, + r#" +struct X; + +struct S { + sub: X +} + +impl S { + fn new() -> S { + S { + sub: X::new() + } + } +} + +impl X { + fn new() -> X { + X { } + } + fn do_thing(&self) { + + } +} + + +fn foo() { + let local = &mut S::new(); + $0local.sub$0.do_thing(); +}"#, + r#" +struct X; + +struct S { + sub: X +} + +impl S { + fn new() -> S { + S { + sub: X::new() + } + } +} + +impl X { + fn new() -> X { + X { } + } + fn do_thing(&self) { + + } +} + + +fn foo() { + let local = &mut S::new(); + let $0x = local.sub; + x.do_thing(); }"#, ); } From fa0afb957671b75bcbcc8fd8ebfdbcaf088ee2bf Mon Sep 17 00:00:00 2001 From: Jeroen Vannevel Date: Wed, 5 Jan 2022 01:36:04 +0000 Subject: [PATCH 05/12] additional test for a reference local (on top of mutable reference local) --- .../src/handlers/extract_variable.rs | 70 ++++++++++++++++++- 1 file changed, 68 insertions(+), 2 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index 9e11e38d3d6..b498d86fb46 100644 --- a/crates/ide_assists/src/handlers/extract_variable.rs +++ b/crates/ide_assists/src/handlers/extract_variable.rs @@ -1139,7 +1139,7 @@ fn foo(s: S) { } #[test] - fn test_extract_var_reference_local() { + fn test_extract_var_mutable_reference_local() { check_assist( extract_variable, r#" @@ -1198,7 +1198,73 @@ impl X { fn foo() { let local = &mut S::new(); - let $0x = local.sub; + let $0x = &mut local.sub; + x.do_thing(); +}"#, + ); + } + + #[test] + fn test_extract_var_reference_local() { + check_assist( + extract_variable, + r#" +struct X; + +struct S { + sub: X +} + +impl S { + fn new() -> S { + S { + sub: X::new() + } + } +} + +impl X { + fn new() -> X { + X { } + } + fn do_thing(&self) { + + } +} + + +fn foo() { + let local = &S::new(); + $0local.sub$0.do_thing(); +}"#, + r#" +struct X; + +struct S { + sub: X +} + +impl S { + fn new() -> S { + S { + sub: X::new() + } + } +} + +impl X { + fn new() -> X { + X { } + } + fn do_thing(&self) { + + } +} + + +fn foo() { + let local = &S::new(); + let $0x = &local.sub; x.do_thing(); }"#, ); From abab0154b69dcf565341117d7e83f3b76bcf06f6 Mon Sep 17 00:00:00 2001 From: Jeroen Vannevel Date: Wed, 5 Jan 2022 01:48:57 +0000 Subject: [PATCH 06/12] comment --- crates/ide_assists/src/handlers/extract_variable.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index b498d86fb46..cc5521201db 100644 --- a/crates/ide_assists/src/handlers/extract_variable.rs +++ b/crates/ide_assists/src/handlers/extract_variable.rs @@ -171,6 +171,7 @@ fn get_receiver_type(ctx: &AssistContext, expression: &ast::Expr) -> Option Option { match expression { ast::Expr::FieldExpr(field) if field.expr().is_some() => { From 053ae2452cbf08d4cd5e520a5aa56f1cfed336e6 Mon Sep 17 00:00:00 2001 From: Jeroen Vannevel Date: Wed, 5 Jan 2022 02:16:22 +0000 Subject: [PATCH 07/12] removed trailing whitespace --- crates/ide_assists/src/handlers/extract_variable.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index cc5521201db..c8c646d941d 100644 --- a/crates/ide_assists/src/handlers/extract_variable.rs +++ b/crates/ide_assists/src/handlers/extract_variable.rs @@ -1152,7 +1152,7 @@ struct S { impl S { fn new() -> S { - S { + S { sub: X::new() } } @@ -1181,7 +1181,7 @@ struct S { impl S { fn new() -> S { - S { + S { sub: X::new() } } @@ -1218,7 +1218,7 @@ struct S { impl S { fn new() -> S { - S { + S { sub: X::new() } } @@ -1247,7 +1247,7 @@ struct S { impl S { fn new() -> S { - S { + S { sub: X::new() } } From 2724b3549047dfdeda8e6746ba91efaabd232477 Mon Sep 17 00:00:00 2001 From: Jeroen Vannevel Date: Wed, 5 Jan 2022 21:08:46 +0000 Subject: [PATCH 08/12] less wordy ref_kind assignment --- .../ide_assists/src/handlers/extract_variable.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index c8c646d941d..19525fa254e 100644 --- a/crates/ide_assists/src/handlers/extract_variable.rs +++ b/crates/ide_assists/src/handlers/extract_variable.rs @@ -52,16 +52,10 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option } } - let ref_kind: RefKind = if let Some(receiver_type) = get_receiver_type(&ctx, &to_extract) { - if receiver_type.is_mutable_reference() { - RefKind::MutRef - } else if receiver_type.is_reference() { - RefKind::Ref - } else { - RefKind::None - } - } else { - RefKind::None + let ref_kind = match get_receiver_type(&ctx, &to_extract) { + Some(receiver_type) if receiver_type.is_mutable_reference() => RefKind::MutRef, + Some(receiver_type) if receiver_type.is_reference() => RefKind::Ref, + _ => RefKind::None, }; let anchor = Anchor::from(&to_extract)?; From 8c0b848694a5b69a8b33b79aa63a6c953f06ba21 Mon Sep 17 00:00:00 2001 From: Jeroen Vannevel Date: Wed, 5 Jan 2022 21:10:03 +0000 Subject: [PATCH 09/12] .clone() over .to_owned() --- crates/ide_assists/src/handlers/extract_variable.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index 19525fa254e..61a8750b0c4 100644 --- a/crates/ide_assists/src/handlers/extract_variable.rs +++ b/crates/ide_assists/src/handlers/extract_variable.rs @@ -161,7 +161,7 @@ fn valid_target_expr(node: SyntaxNode) -> Option { } fn get_receiver_type(ctx: &AssistContext, expression: &ast::Expr) -> Option { - let receiver = get_receiver(expression.to_owned())?; + let receiver = get_receiver(expression.clone())?; Some(ctx.sema.type_of_expr(&receiver)?.original()) } From 771c87f890998e92dd037c8711a90b71aae485d4 Mon Sep 17 00:00:00 2001 From: Jeroen Vannevel Date: Wed, 5 Jan 2022 21:12:09 +0000 Subject: [PATCH 10/12] no PathExpr arm --- crates/ide_assists/src/handlers/extract_variable.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index 61a8750b0c4..3cfd407b76a 100644 --- a/crates/ide_assists/src/handlers/extract_variable.rs +++ b/crates/ide_assists/src/handlers/extract_variable.rs @@ -172,8 +172,7 @@ fn get_receiver(expression: ast::Expr) -> Option { let nested_expression = &field.expr()?; get_receiver(nested_expression.to_owned()) } - ast::Expr::PathExpr(_) => Some(expression), - _ => None, + _ => Some(expression), } } From 035a373a6a025bd9b2f115adbe0884861ca2a9e4 Mon Sep 17 00:00:00 2001 From: Jeroen Vannevel Date: Wed, 5 Jan 2022 21:16:24 +0000 Subject: [PATCH 11/12] removed double matching --- .../src/handlers/extract_variable.rs | 21 ++++--------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index 3cfd407b76a..aaed2b67fe8 100644 --- a/crates/ide_assists/src/handlers/extract_variable.rs +++ b/crates/ide_assists/src/handlers/extract_variable.rs @@ -52,10 +52,10 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option } } - let ref_kind = match get_receiver_type(&ctx, &to_extract) { - Some(receiver_type) if receiver_type.is_mutable_reference() => RefKind::MutRef, - Some(receiver_type) if receiver_type.is_reference() => RefKind::Ref, - _ => RefKind::None, + let reference_modifier = match get_receiver_type(&ctx, &to_extract) { + Some(receiver_type) if receiver_type.is_mutable_reference() => "&mut ", + Some(receiver_type) if receiver_type.is_reference() => "&", + _ => "", }; let anchor = Anchor::from(&to_extract)?; @@ -83,12 +83,6 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option None => to_extract.syntax().text_range(), }; - let reference_modifier = match ref_kind { - RefKind::MutRef => "&mut ", - RefKind::Ref => "&", - RefKind::None => "", - }; - match anchor { Anchor::Before(_) | Anchor::Replace(_) => { format_to!(buf, "let {} = {}", var_name, reference_modifier) @@ -176,13 +170,6 @@ fn get_receiver(expression: ast::Expr) -> Option { } } -#[derive(Debug)] -enum RefKind { - Ref, - MutRef, - None, -} - #[derive(Debug)] enum Anchor { Before(SyntaxNode), From b92ed115c1b4760c33eaa0005fec1e85f9e18206 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 5 Jan 2022 22:30:19 +0100 Subject: [PATCH 12/12] fix: Fix `apply_demorgan` assist hanging for certain binary expressions --- .../ide_assists/src/handlers/apply_demorgan.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/crates/ide_assists/src/handlers/apply_demorgan.rs b/crates/ide_assists/src/handlers/apply_demorgan.rs index b3fcf6578a3..21907ab41fb 100644 --- a/crates/ide_assists/src/handlers/apply_demorgan.rs +++ b/crates/ide_assists/src/handlers/apply_demorgan.rs @@ -42,10 +42,11 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext) -> Option<( // Walk up the tree while we have the same binary operator while let Some(parent_expr) = expr.syntax().parent().and_then(ast::BinExpr::cast) { - if let Some(parent_op) = expr.op_kind() { - if parent_op == op { - expr = parent_expr + match expr.op_kind() { + Some(parent_op) if parent_op == op => { + expr = parent_expr; } + _ => break, } } @@ -220,4 +221,14 @@ fn f() { !(S <= S || S < S) } cov_mark::check!(demorgan_double_parens); check_assist(apply_demorgan, "fn f() { (x ||$0 x) }", "fn f() { !(!x && !x) }") } + + // https://github.com/rust-analyzer/rust-analyzer/issues/10963 + #[test] + fn demorgan_doesnt_hang() { + check_assist( + apply_demorgan, + "fn f() { 1 || 3 &&$0 4 || 5 }", + "fn f() { !(!1 || !3 || !4) || 5 }", + ) + } }