Skip to content

Commit cc0bb71

Browse files
committed
Auto merge of rust-lang#12662 - DorianListens:dscheidt/extract-function-duplicate-name, r=DorianListens
fix: Extract Function produces duplicate fn names This change fixes rust-lang#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.
2 parents d1ac462 + 0039d6f commit cc0bb71

File tree

1 file changed

+70
-2
lines changed

1 file changed

+70
-2
lines changed

crates/ide-assists/src/handlers/extract_function.rs

+70-2
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option
8181

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

8687
let ret_ty = body.return_ty(ctx)?;
8788
let control_flow = body.external_control_flow(ctx, &container_info)?;
@@ -105,8 +106,10 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option
105106
let params =
106107
body.extracted_function_params(ctx, &container_info, locals_used.iter().copied());
107108

109+
let name = make_function_name(&semantics_scope);
110+
108111
let fun = Function {
109-
name: make::name_ref("fun_name"),
112+
name,
110113
self_param,
111114
params,
112115
control_flow,
@@ -155,6 +158,21 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option
155158
)
156159
}
157160

161+
fn make_function_name(semantics_scope: &hir::SemanticsScope) -> ast::NameRef {
162+
let mut names_in_scope = vec![];
163+
semantics_scope.process_all_names(&mut |name, _| names_in_scope.push(name.to_string()));
164+
165+
let default_name = "fun_name";
166+
167+
let mut name = default_name.to_string();
168+
let mut counter = 0;
169+
while names_in_scope.contains(&name) {
170+
counter += 1;
171+
name = format!("{}{}", &default_name, counter)
172+
}
173+
make::name_ref(&name)
174+
}
175+
158176
/// Try to guess what user wants to extract
159177
///
160178
/// We have basically have two cases:
@@ -4709,6 +4727,56 @@ fn $0fun_name() {
47094727
/* a comment */
47104728
let x = 0;
47114729
}
4730+
"#,
4731+
);
4732+
}
4733+
4734+
#[test]
4735+
fn it_should_not_generate_duplicate_function_names() {
4736+
check_assist(
4737+
extract_function,
4738+
r#"
4739+
fn fun_name() {
4740+
$0let x = 0;$0
4741+
}
4742+
"#,
4743+
r#"
4744+
fn fun_name() {
4745+
fun_name1();
4746+
}
4747+
4748+
fn $0fun_name1() {
4749+
let x = 0;
4750+
}
4751+
"#,
4752+
);
4753+
}
4754+
4755+
#[test]
4756+
fn should_increment_suffix_until_it_finds_space() {
4757+
check_assist(
4758+
extract_function,
4759+
r#"
4760+
fn fun_name1() {
4761+
let y = 0;
4762+
}
4763+
4764+
fn fun_name() {
4765+
$0let x = 0;$0
4766+
}
4767+
"#,
4768+
r#"
4769+
fn fun_name1() {
4770+
let y = 0;
4771+
}
4772+
4773+
fn fun_name() {
4774+
fun_name2();
4775+
}
4776+
4777+
fn $0fun_name2() {
4778+
let x = 0;
4779+
}
47124780
"#,
47134781
);
47144782
}

0 commit comments

Comments
 (0)