Skip to content

Commit 227490c

Browse files
feniljainlnicola
authored andcommitted
fix: arbitary noop of assist and same file double writes
1 parent 32b95ea commit 227490c

File tree

1 file changed

+50
-44
lines changed

1 file changed

+50
-44
lines changed

crates/ide_assists/src/handlers/extract_module.rs

Lines changed: 50 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -73,63 +73,69 @@ pub(crate) fn extract_module(acc: &mut Assists, ctx: &AssistContext) -> Option<(
7373

7474
let old_item_indent = module.body_items[0].indent_level();
7575

76+
//This takes place in three steps:
77+
//
78+
//- Firstly, we will update the references(usages) e.g. converting a
79+
// function call bar() to modname::bar(), and similarly for other items
80+
//
81+
//- Secondly, changing the visibility of each item inside the newly selected module
82+
// i.e. making a fn a() {} to pub(crate) fn a() {}
83+
//
84+
//- Thirdly, resolving all the imports this includes removing paths from imports
85+
// outside the module, shifting/cloning them inside new module, or shifting the imports, or making
86+
// new import statemnts
87+
88+
//We are getting item usages and record_fields together, record_fields
89+
//for change_visibility and usages for first point mentioned above in the process
90+
let (usages_to_be_processed, record_fields) = module.get_usages_and_record_fields(ctx);
91+
92+
let import_paths_to_be_removed = module.resolve_imports(curr_parent_module, &ctx);
93+
module.body_items = module.change_visibility(record_fields)?;
94+
if module.body_items.len() == 0 {
95+
return None;
96+
}
97+
7698
acc.add(
7799
AssistId("extract_module", AssistKind::RefactorExtract),
78100
"Extract Module",
79101
module.text_range,
80102
|builder| {
81-
//This takes place in three steps:
82-
//
83-
//- Firstly, we will update the references(usages) e.g. converting a
84-
// function call bar() to modname::bar(), and similarly for other items
85-
//
86-
//- Secondly, changing the visibility of each item inside the newly selected module
87-
// i.e. making a fn a() {} to pub(crate) fn a() {}
88-
//
89-
//- Thirdly, resolving all the imports this includes removing paths from imports
90-
// outside the module, shifting/cloning them inside new module, or shifting the imports, or making
91-
// new import statemnts
103+
let mut body_items = Vec::new();
104+
let new_item_indent = old_item_indent + 1;
105+
for item in module.body_items {
106+
let item = item.indent(IndentLevel(1));
107+
let mut indented_item = String::new();
108+
format_to!(indented_item, "{}{}", new_item_indent, item.to_string());
109+
body_items.push(indented_item);
110+
}
92111

93-
//We are getting item usages and record_fields together, record_fields
94-
//for change_visibility and usages for first point mentioned above in the process
95-
let (usages_to_be_processed, record_fields) = module.get_usages_and_record_fields(ctx);
112+
let body = body_items.join("\n\n");
96113

97-
let import_paths_to_be_removed = module.resolve_imports(curr_parent_module, &ctx);
114+
let mut module_def = String::new();
98115

99-
if let Some(block_items) = module.change_visibility(record_fields) {
100-
module.body_items = block_items;
101-
if module.body_items.len() == 0 {
102-
return;
103-
}
116+
format_to!(module_def, "mod {} {{\n{}\n{}}}", module.name, body, old_item_indent);
104117

105-
let mut body_items = Vec::new();
106-
let new_item_indent = old_item_indent + 1;
107-
for item in module.body_items {
108-
let item = item.indent(IndentLevel(1));
109-
let mut indented_item = String::new();
110-
format_to!(indented_item, "{}{}", new_item_indent, item.to_string());
111-
body_items.push(indented_item);
118+
let mut usages_to_be_updated_for_curr_file = vec![];
119+
for usages_to_be_updated_for_file in usages_to_be_processed {
120+
if usages_to_be_updated_for_file.0 == ctx.frange.file_id {
121+
usages_to_be_updated_for_curr_file = usages_to_be_updated_for_file.1;
122+
continue;
112123
}
113-
114-
let body = body_items.join("\n\n");
115-
116-
let mut module_def = String::new();
117-
118-
format_to!(module_def, "mod {} {{\n{}\n{}}}", module.name, body, old_item_indent);
119-
120-
for usages_to_be_updated_for_file in usages_to_be_processed {
121-
builder.edit_file(usages_to_be_updated_for_file.0);
122-
for usage_to_be_processed in usages_to_be_updated_for_file.1 {
123-
builder.replace(usage_to_be_processed.0, usage_to_be_processed.1)
124-
}
124+
builder.edit_file(usages_to_be_updated_for_file.0);
125+
for usage_to_be_processed in usages_to_be_updated_for_file.1 {
126+
builder.replace(usage_to_be_processed.0, usage_to_be_processed.1)
125127
}
128+
}
126129

127-
builder.edit_file(ctx.frange.file_id);
128-
for import_path_text_range in import_paths_to_be_removed {
129-
builder.delete(import_path_text_range);
130-
}
131-
builder.replace(module.text_range, module_def)
130+
builder.edit_file(ctx.frange.file_id);
131+
for usage_to_be_processed in usages_to_be_updated_for_curr_file {
132+
builder.replace(usage_to_be_processed.0, usage_to_be_processed.1)
133+
}
134+
135+
for import_path_text_range in import_paths_to_be_removed {
136+
builder.delete(import_path_text_range);
132137
}
138+
builder.replace(module.text_range, module_def)
133139
},
134140
)
135141
}

0 commit comments

Comments
 (0)