internal: migrate extract_module to SyntaxEditor#21868
internal: migrate extract_module to SyntaxEditor#21868itang06 wants to merge 5 commits intorust-lang:masterfrom
Conversation
rustbot
added
the
S-waiting-on-review
There was a problem hiding this comment.
Please migrate remaining make::* usages to SyntaxFactory APIs
| continue; | ||
| } | ||
|
|
||
| let mut editor = SyntaxEditor::new(body_item.syntax().clone()); |
There was a problem hiding this comment.
Is this new SyntaxEditor instance and .finish() call on it necessary? Couldn't it be done by passing the mutable reference of existing one from the assist entrance function as a parameter of this function instead?
There was a problem hiding this comment.
the items here are detached clone_subtree() nodes, so the editor from the entry point can't reach them. I can use SyntaxFactory::without_mappings() locally to get rid of the .clone_for_update() calls instead, does that work?
| }) | ||
| .collect(); | ||
| if !replacements.is_empty() { | ||
| let mut editor = SyntaxEditor::new(entry.syntax().clone()); |
| @@ -18,8 +18,10 @@ use syntax::{ | |||
| self, HasVisibility, | |||
| edit::{AstNodeEdit, IndentLevel}, | |||
| make, | |||
There was a problem hiding this comment.
Kindly replace use of make with SyntaxFactory
| let assoc_item_list = make::assoc_item_list(Some(assoc_items)).clone_for_update(); | ||
| let impl_ = impl_.reset_indent(); | ||
| ted::replace(impl_.get_or_create_assoc_item_list().syntax(), assoc_item_list.syntax()); | ||
| let impl_detached = ast::Impl::cast(impl_.syntax().clone_subtree()).unwrap(); |
|
|
||
| let (mut replacements, record_field_parents, impls) = | ||
| get_replacements_for_visibility_change(&mut self.body_items, false); | ||
| get_replacements_for_visibility_change(&mut self.body_items, true); |
There was a problem hiding this comment.
this was a workaround for creating local SyntaxEditor instances, but once the editor is passed through as a parameter the items won't need to be detached so i'll revert it to false
|
|
||
| let mut editor = SyntaxEditor::new(body_item.syntax().clone()); | ||
| for target in insert_targets { | ||
| let pub_crate_vis = make::visibility_pub_crate().clone_for_update(); |
| Some(( | ||
| seg.syntax().parent()?, | ||
| make::path_from_text(&format!("{mod_name}::{seg}")) | ||
| .clone_for_update(), |
There was a problem hiding this comment.
i missed that this should use SyntaxFactory, will replace it
4 participants
Part of #18285
Migrates all
ted::calls incrates/ide-assists/src/handlers/extract_module.rsto the newSyntaxEditor/SyntaxFactoryabstractionChanges:
generate_module_def: replacedted::withSyntaxEditor/SyntaxFactoryexpand_and_group_usages_file_wise: replacedted::replacewithSyntaxEditorchange_visibility: replacedted::insert(viaadd_change_vishelper) withSyntaxEditor::insert_all; removes theadd_change_visfunction entirelyAI assistance was used for this contribution