Unnamed repository; edit this file 'description' to name the repository.
Merge #11195 #11202
11195: Correctly pass through reference modifiers when extracting a variable r=Veykril a=Vannevelj
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/10034
This will parse the field expression and look at whether it is marked `&` or `&mut` and include a modifier if appropriate. The original issue only mentions `&mut params` but I've found that this issue also occurs for `&mut locals` as well as `¶ms` and `&locals` so I've also added tests for them.
I'd definitely be interested in hearing where I can make my code more idiomatic for Rust.
11202: fix: Fix `apply_demorgan` assist hanging for certain binary expressions r=Veykril a=Veykril
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/10963
bors r+
Co-authored-by: Jeroen Vannevel <[email protected]>
Co-authored-by: Lukas Wirth <[email protected]>
| -rw-r--r-- | crates/ide_assists/src/handlers/apply_demorgan.rs | 17 | ||||
| -rw-r--r-- | crates/ide_assists/src/handlers/extract_variable.rs | 354 |
2 files changed, 366 insertions, 5 deletions
diff --git a/crates/ide_assists/src/handlers/apply_demorgan.rs b/crates/ide_assists/src/handlers/apply_demorgan.rs index b3fcf6578a..21907ab41f 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 }", + ) + } } diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index 7a57ab3b9b..aaed2b67fe 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 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)?; let indent = anchor.syntax().prev_sibling_or_token()?.as_token()?.clone(); let target = to_extract.syntax().text_range(); @@ -79,9 +85,11 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option 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 +154,22 @@ fn valid_target_expr(node: SyntaxNode) -> Option<ast::Expr> { } } +fn get_receiver_type(ctx: &AssistContext, expression: &ast::Expr) -> Option<hir::Type> { + let receiver = get_receiver(expression.clone())?; + Some(ctx.sema.type_of_expr(&receiver)?.original()) +} + +/// In the expression `a.b.c.x()`, find `a` +fn get_receiver(expression: ast::Expr) -> Option<ast::Expr> { + match expression { + ast::Expr::FieldExpr(field) if field.expr().is_some() => { + let nested_expression = &field.expr()?; + get_receiver(nested_expression.to_owned()) + } + _ => Some(expression), + } +} + #[derive(Debug)] enum Anchor { Before(SyntaxNode), @@ -900,4 +924,330 @@ const X: usize = $0100$0; ", ); } + + #[test] + fn test_extract_var_mutable_reference_parameter() { + check_assist( + extract_variable, + r#" +struct S { + vec: Vec<u8> +} + +fn foo(s: &mut S) { + $0s.vec$0.push(0); +}"#, + r#" +struct S { + vec: Vec<u8> +} + +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<u8> +} + +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<u8> +} + +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(); +}"#, + ); + } + + #[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(); +}"#, + ); + } + + #[test] + fn test_extract_var_mutable_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 = &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(); +}"#, + ); + } } |