10382: fix: Fix inline_call breaking RecordExprField shorthands r=Veykril a=Veykril

Fixes #10349
bors r+

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
This commit is contained in:
bors[bot] 2021-09-28 17:23:11 +00:00 committed by GitHub
commit cd9f27d424
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 112 additions and 20 deletions

View file

@ -4,14 +4,14 @@ use hir::{db::HirDatabase, HasSource, PathResolution, Semantics, TypeInfo};
use ide_db::{ use ide_db::{
base_db::{FileId, FileRange}, base_db::{FileId, FileRange},
defs::Definition, defs::Definition,
helpers::insert_use::remove_path_if_in_use_stmt, helpers::{insert_use::remove_path_if_in_use_stmt, node_ext::expr_as_name_ref},
path_transform::PathTransform, path_transform::PathTransform,
search::{FileReference, SearchScope}, search::{FileReference, SearchScope},
RootDatabase, RootDatabase,
}; };
use itertools::{izip, Itertools}; use itertools::{izip, Itertools};
use syntax::{ use syntax::{
ast::{self, edit_in_place::Indent, HasArgList}, ast::{self, edit_in_place::Indent, HasArgList, PathExpr},
ted, AstNode, SyntaxNode, ted, AstNode, SyntaxNode,
}; };
@ -359,11 +359,17 @@ fn inline(
} }
// Inline parameter expressions or generate `let` statements depending on whether inlining works or not. // Inline parameter expressions or generate `let` statements depending on whether inlining works or not.
for ((pat, param_ty, _), usages, expr) in izip!(params, param_use_nodes, arguments).rev() { for ((pat, param_ty, _), usages, expr) in izip!(params, param_use_nodes, arguments).rev() {
let expr_is_name_ref = matches!(&expr, let inline_direct = |usage, replacement: &ast::Expr| {
ast::Expr::PathExpr(expr) if let Some(field) = path_expr_as_record_field(usage) {
if expr.path().and_then(|path| path.as_single_name_ref()).is_some() cov_mark::hit!(inline_call_inline_direct_field);
); field.replace_expr(replacement.clone_for_update());
match &*usages { } else {
ted::replace(usage.syntax(), &replacement.syntax().clone_for_update());
}
};
// izip confuses RA due to our lack of hygiene info currently losing us typeinfo
let usages: &[ast::PathExpr] = &*usages;
match usages {
// inline single use closure arguments // inline single use closure arguments
[usage] [usage]
if matches!(expr, ast::Expr::ClosureExpr(_)) if matches!(expr, ast::Expr::ClosureExpr(_))
@ -371,21 +377,19 @@ fn inline(
{ {
cov_mark::hit!(inline_call_inline_closure); cov_mark::hit!(inline_call_inline_closure);
let expr = make::expr_paren(expr.clone()); let expr = make::expr_paren(expr.clone());
ted::replace(usage.syntax(), expr.syntax().clone_for_update()); inline_direct(usage, &expr);
} }
// inline single use literals // inline single use literals
[usage] if matches!(expr, ast::Expr::Literal(_)) => { [usage] if matches!(expr, ast::Expr::Literal(_)) => {
cov_mark::hit!(inline_call_inline_literal); cov_mark::hit!(inline_call_inline_literal);
ted::replace(usage.syntax(), expr.syntax().clone_for_update()); inline_direct(usage, &expr);
} }
// inline direct local arguments // inline direct local arguments
[_, ..] if expr_is_name_ref => { [_, ..] if expr_as_name_ref(&expr).is_some() => {
cov_mark::hit!(inline_call_inline_locals); cov_mark::hit!(inline_call_inline_locals);
usages.into_iter().for_each(|usage| { usages.into_iter().for_each(|usage| inline_direct(usage, &expr));
ted::replace(usage.syntax(), &expr.syntax().clone_for_update());
});
} }
// cant inline, emit a let statement // can't inline, emit a let statement
_ => { _ => {
let ty = let ty =
sema.type_of_expr(expr).filter(TypeInfo::has_adjustment).and(param_ty.clone()); sema.type_of_expr(expr).filter(TypeInfo::has_adjustment).and(param_ty.clone());
@ -421,6 +425,12 @@ fn inline(
} }
} }
fn path_expr_as_record_field(usage: &PathExpr) -> Option<ast::RecordExprField> {
let path = usage.path()?;
let name_ref = path.as_single_name_ref()?;
ast::RecordExprField::for_name_ref(&name_ref)
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use crate::tests::{check_assist, check_assist_not_applicable}; use crate::tests::{check_assist, check_assist_not_applicable};
@ -1022,6 +1032,61 @@ fn foo$0() {
fn foo() { fn foo() {
foo$0(); foo$0();
} }
"#,
);
}
#[test]
fn inline_call_field_shorthand() {
cov_mark::check!(inline_call_inline_direct_field);
check_assist(
inline_call,
r#"
struct Foo {
field: u32,
field1: u32,
field2: u32,
field3: u32,
}
fn foo(field: u32, field1: u32, val2: u32, val3: u32) -> Foo {
Foo {
field,
field1,
field2: val2,
field3: val3,
}
}
fn main() {
let bar = 0;
let baz = 0;
foo$0(bar, 0, baz, 0);
}
"#,
r#"
struct Foo {
field: u32,
field1: u32,
field2: u32,
field3: u32,
}
fn foo(field: u32, field1: u32, val2: u32, val3: u32) -> Foo {
Foo {
field,
field1,
field2: val2,
field3: val3,
}
}
fn main() {
let bar = 0;
let baz = 0;
Foo {
field: bar,
field1: 0,
field2: baz,
field3: 0,
};
}
"#, "#,
); );
} }

View file

@ -7,18 +7,16 @@ use syntax::{
pub fn expr_as_name_ref(expr: &ast::Expr) -> Option<ast::NameRef> { pub fn expr_as_name_ref(expr: &ast::Expr) -> Option<ast::NameRef> {
if let ast::Expr::PathExpr(expr) = expr { if let ast::Expr::PathExpr(expr) = expr {
let path = expr.path()?; let path = expr.path()?;
let segment = path.segment()?; path.as_single_name_ref()
let name_ref = segment.name_ref()?; } else {
if path.qualifier().is_none() {
return Some(name_ref);
}
}
None None
} }
}
pub fn block_as_lone_tail(block: &ast::BlockExpr) -> Option<ast::Expr> { pub fn block_as_lone_tail(block: &ast::BlockExpr) -> Option<ast::Expr> {
block.statements().next().is_none().then(|| block.tail_expr()).flatten() block.statements().next().is_none().then(|| block.tail_expr()).flatten()
} }
/// Preorder walk all the expression's child expressions. /// Preorder walk all the expression's child expressions.
pub fn walk_expr(expr: &ast::Expr, cb: &mut dyn FnMut(ast::Expr)) { pub fn walk_expr(expr: &ast::Expr, cb: &mut dyn FnMut(ast::Expr)) {
preorder_expr(expr, &mut |ev| { preorder_expr(expr, &mut |ev| {

View file

@ -451,6 +451,35 @@ impl ast::RecordExprFieldList {
} }
} }
impl ast::RecordExprField {
/// This will either replace the initializer, or in the case that this is a shorthand convert
/// the initializer into the name ref and insert the expr as the new initializer.
pub fn replace_expr(&self, expr: ast::Expr) {
if let Some(_) = self.name_ref() {
match self.expr() {
Some(prev) => ted::replace(prev.syntax(), expr.syntax()),
None => ted::append_child(self.syntax(), expr.syntax()),
}
return;
}
// this is a shorthand
if let Some(ast::Expr::PathExpr(path_expr)) = self.expr() {
if let Some(path) = path_expr.path() {
if let Some(name_ref) = path.as_single_name_ref() {
path_expr.syntax().detach();
let children = vec![
name_ref.syntax().clone().into(),
ast::make::token(T![:]).into(),
ast::make::tokens::single_space().into(),
expr.syntax().clone().into(),
];
ted::insert_all_raw(Position::last_child_of(self.syntax()), children);
}
}
}
}
}
impl ast::StmtList { impl ast::StmtList {
pub fn push_front(&self, statement: ast::Stmt) { pub fn push_front(&self, statement: ast::Stmt) {
ted::insert(Position::after(self.l_curly_token().unwrap()), statement.syntax()); ted::insert(Position::after(self.l_curly_token().unwrap()), statement.syntax());