Skip to content

Commit

Permalink
fix: Extract Function produces duplicate fn names
Browse files Browse the repository at this point in the history
This change fixes issue #10037, in more or less the most naive fashion
possible.

We continue to start with the hardcoded default of "fun_name", and now append a
counter to the end of it if that name is already in scope.

In the future, we can probably apply more heuristics here to wind up with more
useful names by default, but for now this resolves the immediate problem.
  • Loading branch information
DorianListens committed Jul 2, 2022
1 parent d1ac462 commit 407d525
Showing 1 changed file with 71 additions and 2 deletions.
73 changes: 71 additions & 2 deletions crates/ide-assists/src/handlers/extract_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option

let anchor = if self_param.is_some() { Anchor::Method } else { Anchor::Freestanding };
let insert_after = node_to_insert_after(&body, anchor)?;
let module = ctx.sema.scope(&insert_after)?.module();
let semantics_scope = ctx.sema.scope(&insert_after)?;
let module = semantics_scope.module();

let ret_ty = body.return_ty(ctx)?;
let control_flow = body.external_control_flow(ctx, &container_info)?;
Expand All @@ -91,6 +92,7 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option

let scope = ImportScope::find_insert_use_container(&node, &ctx.sema)?;


acc.add(
AssistId("extract_function", crate::AssistKind::RefactorExtract),
"Extract into function",
Expand All @@ -105,8 +107,10 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option
let params =
body.extracted_function_params(ctx, &container_info, locals_used.iter().copied());

let name = make_function_name(&semantics_scope);

let fun = Function {
name: make::name_ref("fun_name"),
name,
self_param,
params,
control_flow,
Expand Down Expand Up @@ -155,6 +159,21 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option
)
}

fn make_function_name(semantics_scope: &hir::SemanticsScope) -> ast::NameRef {
let mut names_in_scope = vec![];
semantics_scope.process_all_names(&mut |name, _| names_in_scope.push(name.to_string()));

let default_name = "fun_name";

let mut name = default_name.to_string();
let mut counter = 0;
while names_in_scope.contains(&name) {
counter += 1;
name = format!("{}{}", &default_name, counter)
}
make::name_ref(&name)
}

/// Try to guess what user wants to extract
///
/// We have basically have two cases:
Expand Down Expand Up @@ -4709,6 +4728,56 @@ fn $0fun_name() {
/* a comment */
let x = 0;
}
"#,
);
}

#[test]
fn it_should_not_generate_duplicate_function_names() {
check_assist(
extract_function,
r#"
fn fun_name() {
$0let x = 0;$0
}
"#,
r#"
fn fun_name() {
fun_name1();
}
fn $0fun_name1() {
let x = 0;
}
"#,
);
}

#[test]
fn should_increment_suffix_until_it_finds_space() {
check_assist(
extract_function,
r#"
fn fun_name1() {
let y = 0;
}
fn fun_name() {
$0let x = 0;$0
}
"#,
r#"
fn fun_name1() {
let y = 0;
}
fn fun_name() {
fun_name2();
}
fn $0fun_name2() {
let x = 0;
}
"#,
);
}
Expand Down

0 comments on commit 407d525

Please sign in to comment.