Skip to content

Fixed local shadowing the caller's argument issue #13454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 5, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 74 additions & 12 deletions crates/ide-assists/src/handlers/inline_call.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::collections::BTreeSet;

use ast::make;
use either::Either;
use hir::{db::HirDatabase, PathResolution, Semantics, TypeInfo};
Expand Down Expand Up @@ -373,8 +375,44 @@ fn inline(
})
}
}

let mut func_let_vars: BTreeSet<String> = BTreeSet::new();

// grab all of the local variable declarations in the function
for stmt in fn_body.statements() {
if let Some(let_stmt) = ast::LetStmt::cast(stmt.syntax().to_owned()) {
for has_token in let_stmt.syntax().children_with_tokens() {
if let Some(node) = has_token.as_node() {
if let Some(ident_pat) = ast::IdentPat::cast(node.to_owned()) {
func_let_vars.insert(ident_pat.syntax().text().to_string());
}
}
}
}
}

// 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() {
// izip confuses RA due to our lack of hygiene info currently losing us type info causing incorrect errors
let usages: &[ast::PathExpr] = &*usages;
let expr: &ast::Expr = expr;

let insert_let_stmt = || {
let ty = sema.type_of_expr(expr).filter(TypeInfo::has_adjustment).and(param_ty.clone());
if let Some(stmt_list) = body.stmt_list() {
stmt_list.push_front(
make::let_stmt(pat.clone(), ty, Some(expr.clone())).clone_for_update().into(),
)
}
};

// check if there is a local var in the function that conflicts with parameter
// if it does then emit a let statement and continue
if func_let_vars.contains(&expr.syntax().text().to_string()) {
insert_let_stmt();
continue;
}

let inline_direct = |usage, replacement: &ast::Expr| {
if let Some(field) = path_expr_as_record_field(usage) {
cov_mark::hit!(inline_call_inline_direct_field);
Expand All @@ -383,9 +421,7 @@ fn inline(
ted::replace(usage.syntax(), &replacement.syntax().clone_for_update());
}
};
// izip confuses RA due to our lack of hygiene info currently losing us type info causing incorrect errors
let usages: &[ast::PathExpr] = &*usages;
let expr: &ast::Expr = expr;

match usages {
// inline single use closure arguments
[usage]
Expand All @@ -408,18 +444,11 @@ fn inline(
}
// can't inline, emit a let statement
_ => {
let ty =
sema.type_of_expr(expr).filter(TypeInfo::has_adjustment).and(param_ty.clone());
if let Some(stmt_list) = body.stmt_list() {
stmt_list.push_front(
make::let_stmt(pat.clone(), ty, Some(expr.clone()))
.clone_for_update()
.into(),
)
}
insert_let_stmt();
}
}
}

if let Some(generic_arg_list) = generic_arg_list.clone() {
if let Some((target, source)) = &sema.scope(node.syntax()).zip(sema.scope(fn_body.syntax()))
{
Expand Down Expand Up @@ -1256,4 +1285,37 @@ impl A {
"#,
)
}

#[test]
fn local_variable_shadowing_callers_argument() {
check_assist(
inline_call,
r#"
fn foo(bar: u32, baz: u32) -> u32 {
let a = 1;
bar * baz * a * 6
}
fn main() {
let a = 7;
let b = 1;
let res = foo$0(a, b);
}
"#,
r#"
fn foo(bar: u32, baz: u32) -> u32 {
let a = 1;
bar * baz * a * 6
}
fn main() {
let a = 7;
let b = 1;
let res = {
let bar = a;
let a = 1;
bar * b * a * 6
};
}
"#,
);
}
}