Also handle deref expressions in "Extract variable"

And BTW, remove the parentheses of the extracted expression if there are.
This commit is contained in:
Chayim Refael Friedman 2024-08-29 00:35:45 +03:00
parent 1663891d3d
commit f297860715
2 changed files with 59 additions and 15 deletions

View file

@ -20,7 +20,7 @@ use crate::{utils::suggest_name, AssistContext, AssistId, AssistKind, Assists};
// -> // ->
// ``` // ```
// fn main() { // fn main() {
// let $0var_name = (1 + 2); // let $0var_name = 1 + 2;
// var_name * 4; // var_name * 4;
// } // }
// ``` // ```
@ -59,7 +59,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op
let parent = to_extract.syntax().parent().and_then(ast::Expr::cast); let parent = to_extract.syntax().parent().and_then(ast::Expr::cast);
// Any expression that autoderefs may need adjustment. // Any expression that autoderefs may need adjustment.
let needs_adjust = parent.as_ref().map_or(false, |it| match it { let mut needs_adjust = parent.as_ref().map_or(false, |it| match it {
ast::Expr::FieldExpr(_) ast::Expr::FieldExpr(_)
| ast::Expr::MethodCallExpr(_) | ast::Expr::MethodCallExpr(_)
| ast::Expr::CallExpr(_) | ast::Expr::CallExpr(_)
@ -67,15 +67,21 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op
ast::Expr::IndexExpr(index) if index.base().as_ref() == Some(&to_extract) => true, ast::Expr::IndexExpr(index) if index.base().as_ref() == Some(&to_extract) => true,
_ => false, _ => false,
}); });
let mut to_extract_no_ref = peel_parens(to_extract.clone());
let needs_ref = needs_adjust let needs_ref = needs_adjust
&& matches!( && match &to_extract_no_ref {
to_extract,
ast::Expr::FieldExpr(_) ast::Expr::FieldExpr(_)
| ast::Expr::IndexExpr(_) | ast::Expr::IndexExpr(_)
| ast::Expr::MacroExpr(_) | ast::Expr::MacroExpr(_)
| ast::Expr::ParenExpr(_) | ast::Expr::ParenExpr(_)
| ast::Expr::PathExpr(_) | ast::Expr::PathExpr(_) => true,
); ast::Expr::PrefixExpr(prefix) if prefix.op_kind() == Some(ast::UnaryOp::Deref) => {
to_extract_no_ref = prefix.expr()?;
needs_adjust = false;
false
}
_ => false,
};
let anchor = Anchor::from(&to_extract)?; let anchor = Anchor::from(&to_extract)?;
let target = to_extract.syntax().text_range(); let target = to_extract.syntax().text_range();
@ -111,19 +117,19 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op
_ => make::ident_pat(false, false, make::name(&var_name)), _ => make::ident_pat(false, false, make::name(&var_name)),
}; };
let to_extract = match ty.as_ref().filter(|_| needs_ref) { let to_extract_no_ref = match ty.as_ref().filter(|_| needs_ref) {
Some(receiver_type) if receiver_type.is_mutable_reference() => { Some(receiver_type) if receiver_type.is_mutable_reference() => {
make::expr_ref(to_extract, true) make::expr_ref(to_extract_no_ref, true)
} }
Some(receiver_type) if receiver_type.is_reference() => { Some(receiver_type) if receiver_type.is_reference() => {
make::expr_ref(to_extract, false) make::expr_ref(to_extract_no_ref, false)
} }
_ => to_extract, _ => to_extract_no_ref,
}; };
let expr_replace = edit.make_syntax_mut(expr_replace); let expr_replace = edit.make_syntax_mut(expr_replace);
let let_stmt = let let_stmt =
make::let_stmt(ident_pat.into(), None, Some(to_extract)).clone_for_update(); make::let_stmt(ident_pat.into(), None, Some(to_extract_no_ref)).clone_for_update();
let name_expr = make::expr_path(make::ext::ident_path(&var_name)).clone_for_update(); let name_expr = make::expr_path(make::ext::ident_path(&var_name)).clone_for_update();
match anchor { match anchor {
@ -223,6 +229,14 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op
) )
} }
fn peel_parens(mut expr: ast::Expr) -> ast::Expr {
while let ast::Expr::ParenExpr(parens) = &expr {
let Some(expr_inside) = parens.expr() else { break };
expr = expr_inside;
}
expr
}
/// Check whether the node is a valid expression which can be extracted to a variable. /// Check whether the node is a valid expression which can be extracted to a variable.
/// In general that's true for any expression, but in some cases that would produce invalid code. /// In general that's true for any expression, but in some cases that would produce invalid code.
fn valid_target_expr(node: SyntaxNode) -> Option<ast::Expr> { fn valid_target_expr(node: SyntaxNode) -> Option<ast::Expr> {
@ -1547,4 +1561,34 @@ fn foo() {
}"#, }"#,
); );
} }
#[test]
fn generates_no_ref_for_deref() {
check_assist(
extract_variable,
r#"
struct S;
impl S {
fn do_work(&mut self) {}
}
fn bar() -> S { S }
fn foo() {
let v = &mut &mut bar();
$0(**v)$0.do_work();
}
"#,
r#"
struct S;
impl S {
fn do_work(&mut self) {}
}
fn bar() -> S { S }
fn foo() {
let v = &mut &mut bar();
let $0s = *v;
s.do_work();
}
"#,
);
}
} }

View file

@ -994,7 +994,7 @@ fn main() {
"#####, "#####,
r#####" r#####"
fn main() { fn main() {
let $0var_name = (1 + 2); let $0var_name = 1 + 2;
var_name * 4; var_name * 4;
} }
"#####, "#####,