From e8744ed9bb3e139e5d427db1f4f219f1fdeee13e Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 19 Apr 2021 20:29:14 +0200 Subject: Replace SyntaxRewriter usage with ted in reorder_impl assist --- crates/ide_assists/src/handlers/reorder_impl.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) (limited to 'crates/ide_assists') diff --git a/crates/ide_assists/src/handlers/reorder_impl.rs b/crates/ide_assists/src/handlers/reorder_impl.rs index f976e73ad..fd2897c4c 100644 --- a/crates/ide_assists/src/handlers/reorder_impl.rs +++ b/crates/ide_assists/src/handlers/reorder_impl.rs @@ -4,9 +4,8 @@ use rustc_hash::FxHashMap; use hir::{PathResolution, Semantics}; use ide_db::RootDatabase; use syntax::{ - algo, ast::{self, NameOwner}, - AstNode, + ted, AstNode, }; use crate::{AssistContext, AssistId, AssistKind, Assists}; @@ -75,13 +74,18 @@ pub(crate) fn reorder_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()> } let target = items.syntax().text_range(); - acc.add(AssistId("reorder_impl", AssistKind::RefactorRewrite), "Sort methods", target, |edit| { - let mut rewriter = algo::SyntaxRewriter::default(); - for (old, new) in methods.iter().zip(&sorted) { - rewriter.replace(old.syntax(), new.syntax()); - } - edit.rewrite(rewriter); - }) + acc.add( + AssistId("reorder_impl", AssistKind::RefactorRewrite), + "Sort methods", + target, + |builder| { + for (old, new) in + methods.into_iter().zip(sorted).filter(|(field, sorted)| field != sorted) + { + ted::replace(builder.make_ast_mut(old).syntax(), new.clone_for_update().syntax()); + } + }, + ) } fn compute_method_ranks(path: &ast::Path, ctx: &AssistContext) -> Option> { -- cgit v1.2.3 From fa20a5064be85349d2d05abcd66f5662d3aecb0c Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 20 Apr 2021 02:05:22 +0200 Subject: Remove SyntaxRewriter usage in insert_use in favor of ted --- crates/ide_assists/src/handlers/auto_import.rs | 8 +- .../handlers/extract_struct_from_enum_variant.rs | 85 ++++++++++++---------- .../handlers/replace_qualified_name_with_use.rs | 28 +++---- 3 files changed, 62 insertions(+), 59 deletions(-) (limited to 'crates/ide_assists') diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs index 49aa70f74..6db2d2edd 100644 --- a/crates/ide_assists/src/handlers/auto_import.rs +++ b/crates/ide_assists/src/handlers/auto_import.rs @@ -101,9 +101,11 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> format!("Import `{}`", import.import_path), range, |builder| { - let rewriter = - insert_use(&scope, mod_path_to_ast(&import.import_path), ctx.config.insert_use); - builder.rewrite(rewriter); + let scope = match scope.clone() { + ImportScope::File(it) => ImportScope::File(builder.make_ast_mut(it)), + ImportScope::Module(it) => ImportScope::Module(builder.make_ast_mut(it)), + }; + insert_use(&scope, mod_path_to_ast(&import.import_path), ctx.config.insert_use); }, ); } diff --git a/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs index a8d6355bd..26e1c66ab 100644 --- a/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -13,9 +13,9 @@ use ide_db::{ }; use rustc_hash::FxHashSet; use syntax::{ - algo::{find_node_at_offset, SyntaxRewriter}, - ast::{self, edit::IndentLevel, make, AstNode, NameOwner, VisibilityOwner}, - SourceFile, SyntaxElement, SyntaxNode, T, + algo::find_node_at_offset, + ast::{self, make, AstNode, NameOwner, VisibilityOwner}, + ted, SourceFile, SyntaxElement, SyntaxNode, T, }; use crate::{AssistContext, AssistId, AssistKind, Assists}; @@ -62,14 +62,17 @@ pub(crate) fn extract_struct_from_enum_variant( let mut visited_modules_set = FxHashSet::default(); let current_module = enum_hir.module(ctx.db()); visited_modules_set.insert(current_module); - let mut def_rewriter = None; + let mut def_file_references = None; for (file_id, references) in usages { - let mut rewriter = SyntaxRewriter::default(); - let source_file = ctx.sema.parse(file_id); + if file_id == ctx.frange.file_id { + def_file_references = Some(references); + continue; + } + builder.edit_file(file_id); + let source_file = builder.make_ast_mut(ctx.sema.parse(file_id)); for reference in references { update_reference( ctx, - &mut rewriter, reference, &source_file, &enum_module_def, @@ -77,25 +80,27 @@ pub(crate) fn extract_struct_from_enum_variant( &mut visited_modules_set, ); } - if file_id == ctx.frange.file_id { - def_rewriter = Some(rewriter); - continue; - } - builder.edit_file(file_id); - builder.rewrite(rewriter); } - let mut rewriter = def_rewriter.unwrap_or_default(); - update_variant(&mut rewriter, &variant); + builder.edit_file(ctx.frange.file_id); + let variant = builder.make_ast_mut(variant.clone()); + let source_file = builder.make_ast_mut(ctx.sema.parse(ctx.frange.file_id)); + for reference in def_file_references.into_iter().flatten() { + update_reference( + ctx, + reference, + &source_file, + &enum_module_def, + &variant_hir_name, + &mut visited_modules_set, + ); + } extract_struct_def( - &mut rewriter, - &enum_ast, variant_name.clone(), &field_list, &variant.parent_enum().syntax().clone().into(), enum_ast.visibility(), ); - builder.edit_file(ctx.frange.file_id); - builder.rewrite(rewriter); + update_variant(&variant); }, ) } @@ -138,7 +143,6 @@ fn existing_definition(db: &RootDatabase, variant_name: &ast::Name, variant: &Va fn insert_import( ctx: &AssistContext, - rewriter: &mut SyntaxRewriter, scope_node: &SyntaxNode, module: &Module, enum_module_def: &ModuleDef, @@ -151,14 +155,12 @@ fn insert_import( mod_path.pop_segment(); mod_path.push_segment(variant_hir_name.clone()); let scope = ImportScope::find_insert_use_container(scope_node, &ctx.sema)?; - *rewriter += insert_use(&scope, mod_path_to_ast(&mod_path), ctx.config.insert_use); + insert_use(&scope, mod_path_to_ast(&mod_path), ctx.config.insert_use); } Some(()) } fn extract_struct_def( - rewriter: &mut SyntaxRewriter, - enum_: &ast::Enum, variant_name: ast::Name, field_list: &Either, start_offset: &SyntaxElement, @@ -180,33 +182,34 @@ fn extract_struct_def( .into(), }; - rewriter.insert_before( - start_offset, - make::struct_(visibility, variant_name, None, field_list).syntax(), + ted::insert_raw( + ted::Position::before(start_offset), + make::struct_(visibility, variant_name, None, field_list).clone_for_update().syntax(), ); - rewriter.insert_before(start_offset, &make::tokens::blank_line()); + ted::insert_raw(ted::Position::before(start_offset), &make::tokens::blank_line()); - if let indent_level @ 1..=usize::MAX = IndentLevel::from_node(enum_.syntax()).0 as usize { - rewriter - .insert_before(start_offset, &make::tokens::whitespace(&" ".repeat(4 * indent_level))); - } + // if let indent_level @ 1..=usize::MAX = IndentLevel::from_node(enum_.syntax()).0 as usize { + // ted::insert(ted::Position::before(start_offset), &make::tokens::blank_line()); + // rewriter + // .insert_before(start_offset, &make::tokens::whitespace(&" ".repeat(4 * indent_level))); + // } Some(()) } -fn update_variant(rewriter: &mut SyntaxRewriter, variant: &ast::Variant) -> Option<()> { +fn update_variant(variant: &ast::Variant) -> Option<()> { let name = variant.name()?; let tuple_field = make::tuple_field(None, make::ty(&name.text())); let replacement = make::variant( name, Some(ast::FieldList::TupleFieldList(make::tuple_field_list(iter::once(tuple_field)))), - ); - rewriter.replace(variant.syntax(), replacement.syntax()); + ) + .clone_for_update(); + ted::replace(variant.syntax(), replacement.syntax()); Some(()) } fn update_reference( ctx: &AssistContext, - rewriter: &mut SyntaxRewriter, reference: FileReference, source_file: &SourceFile, enum_module_def: &ModuleDef, @@ -230,14 +233,16 @@ fn update_reference( let module = ctx.sema.scope(&expr).module()?; if !visited_modules_set.contains(&module) { - if insert_import(ctx, rewriter, &expr, &module, enum_module_def, variant_hir_name).is_some() - { + if insert_import(ctx, &expr, &module, enum_module_def, variant_hir_name).is_some() { visited_modules_set.insert(module); } } - rewriter.insert_after(segment.syntax(), &make::token(T!['('])); - rewriter.insert_after(segment.syntax(), segment.syntax()); - rewriter.insert_after(&expr, &make::token(T![')'])); + ted::insert_raw( + ted::Position::before(segment.syntax()), + make::path_from_text(&format!("{}", segment)).clone_for_update().syntax(), + ); + ted::insert_raw(ted::Position::before(segment.syntax()), make::token(T!['('])); + ted::insert_raw(ted::Position::after(&expr), make::token(T![')'])); Some(()) } diff --git a/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs b/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs index 36d2e0331..2f2306fcc 100644 --- a/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs +++ b/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs @@ -1,5 +1,5 @@ use ide_db::helpers::insert_use::{insert_use, ImportScope}; -use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SyntaxNode}; +use syntax::{ast, match_ast, ted, AstNode, SyntaxNode}; use crate::{AssistContext, AssistId, AssistKind, Assists}; @@ -40,18 +40,17 @@ pub(crate) fn replace_qualified_name_with_use( |builder| { // Now that we've brought the name into scope, re-qualify all paths that could be // affected (that is, all paths inside the node we added the `use` to). - let mut rewriter = SyntaxRewriter::default(); - shorten_paths(&mut rewriter, syntax.clone(), &path); + let syntax = builder.make_mut(syntax.clone()); if let Some(ref import_scope) = ImportScope::from(syntax.clone()) { - rewriter += insert_use(import_scope, path, ctx.config.insert_use); - builder.rewrite(rewriter); + insert_use(import_scope, path.clone(), ctx.config.insert_use); } + shorten_paths(syntax.clone(), &path.clone_for_update()); }, ) } /// Adds replacements to `re` that shorten `path` in all descendants of `node`. -fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: &ast::Path) { +fn shorten_paths(node: SyntaxNode, path: &ast::Path) { for child in node.children() { match_ast! { match child { @@ -62,32 +61,28 @@ fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: ast::Module(_it) => continue, ast::Path(p) => { - match maybe_replace_path(rewriter, p.clone(), path.clone()) { + match maybe_replace_path(p.clone(), path.clone()) { Some(()) => {}, - None => shorten_paths(rewriter, p.syntax().clone(), path), + None => shorten_paths(p.syntax().clone(), path), } }, - _ => shorten_paths(rewriter, child, path), + _ => shorten_paths(child, path), } } } } -fn maybe_replace_path( - rewriter: &mut SyntaxRewriter<'static>, - path: ast::Path, - target: ast::Path, -) -> Option<()> { +fn maybe_replace_path(path: ast::Path, target: ast::Path) -> Option<()> { if !path_eq(path.clone(), target) { return None; } // Shorten `path`, leaving only its last segment. if let Some(parent) = path.qualifier() { - rewriter.delete(parent.syntax()); + ted::remove(parent.syntax()); } if let Some(double_colon) = path.coloncolon_token() { - rewriter.delete(&double_colon); + ted::remove(&double_colon); } Some(()) @@ -150,6 +145,7 @@ Debug ", ); } + #[test] fn test_replace_add_use_no_anchor_with_item_below() { check_assist( -- cgit v1.2.3 From 2c8f1b5c301a5d7a84d3c74a9fc48ef76cd11bd6 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 20 Apr 2021 17:36:36 +0200 Subject: Rewrite extract_struct_from_enum_variant assist --- .../handlers/extract_struct_from_enum_variant.rs | 176 +++++++++++---------- 1 file changed, 91 insertions(+), 85 deletions(-) (limited to 'crates/ide_assists') diff --git a/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs index 26e1c66ab..1f800f82b 100644 --- a/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -15,7 +15,7 @@ use rustc_hash::FxHashSet; use syntax::{ algo::find_node_at_offset, ast::{self, make, AstNode, NameOwner, VisibilityOwner}, - ted, SourceFile, SyntaxElement, SyntaxNode, T, + ted, SyntaxNode, T, }; use crate::{AssistContext, AssistId, AssistKind, Assists}; @@ -62,6 +62,7 @@ pub(crate) fn extract_struct_from_enum_variant( let mut visited_modules_set = FxHashSet::default(); let current_module = enum_hir.module(ctx.db()); visited_modules_set.insert(current_module); + // record file references of the file the def resides in, we only want to swap to the edited file in the builder once let mut def_file_references = None; for (file_id, references) in usages { if file_id == ctx.frange.file_id { @@ -70,36 +71,57 @@ pub(crate) fn extract_struct_from_enum_variant( } builder.edit_file(file_id); let source_file = builder.make_ast_mut(ctx.sema.parse(file_id)); - for reference in references { - update_reference( - ctx, - reference, - &source_file, - &enum_module_def, - &variant_hir_name, - &mut visited_modules_set, + let processed = process_references( + ctx, + &mut visited_modules_set, + source_file.syntax(), + &enum_module_def, + &variant_hir_name, + references, + ); + processed.into_iter().for_each(|(segment, node, import)| { + if let Some((scope, path)) = import { + insert_use(&scope, mod_path_to_ast(&path), ctx.config.insert_use); + } + ted::insert_raw( + ted::Position::before(segment.syntax()), + make::path_from_text(&format!("{}", segment)).clone_for_update().syntax(), ); - } + ted::insert_raw(ted::Position::before(segment.syntax()), make::token(T!['('])); + ted::insert_raw(ted::Position::after(&node), make::token(T![')'])); + }); } builder.edit_file(ctx.frange.file_id); - let variant = builder.make_ast_mut(variant.clone()); let source_file = builder.make_ast_mut(ctx.sema.parse(ctx.frange.file_id)); - for reference in def_file_references.into_iter().flatten() { - update_reference( + let variant = builder.make_ast_mut(variant.clone()); + if let Some(references) = def_file_references { + let processed = process_references( ctx, - reference, - &source_file, + &mut visited_modules_set, + source_file.syntax(), &enum_module_def, &variant_hir_name, - &mut visited_modules_set, + references, ); + processed.into_iter().for_each(|(segment, node, import)| { + if let Some((scope, path)) = import { + insert_use(&scope, mod_path_to_ast(&path), ctx.config.insert_use); + } + ted::insert_raw( + ted::Position::before(segment.syntax()), + make::path_from_text(&format!("{}", segment)).clone_for_update().syntax(), + ); + ted::insert_raw(ted::Position::before(segment.syntax()), make::token(T!['('])); + ted::insert_raw(ted::Position::after(&node), make::token(T![')'])); + }); } - extract_struct_def( - variant_name.clone(), - &field_list, - &variant.parent_enum().syntax().clone().into(), - enum_ast.visibility(), - ); + + let def = create_struct_def(variant_name.clone(), &field_list, enum_ast.visibility()) + .unwrap(); + let start_offset = &variant.parent_enum().syntax().clone(); + ted::insert_raw(ted::Position::before(start_offset), def.syntax()); + ted::insert_raw(ted::Position::before(start_offset), &make::tokens::blank_line()); + update_variant(&variant); }, ) @@ -141,31 +163,11 @@ fn existing_definition(db: &RootDatabase, variant_name: &ast::Name, variant: &Va .any(|(name, _)| name.to_string() == variant_name.to_string()) } -fn insert_import( - ctx: &AssistContext, - scope_node: &SyntaxNode, - module: &Module, - enum_module_def: &ModuleDef, - variant_hir_name: &Name, -) -> Option<()> { - let db = ctx.db(); - let mod_path = - module.find_use_path_prefixed(db, *enum_module_def, ctx.config.insert_use.prefix_kind); - if let Some(mut mod_path) = mod_path { - mod_path.pop_segment(); - mod_path.push_segment(variant_hir_name.clone()); - let scope = ImportScope::find_insert_use_container(scope_node, &ctx.sema)?; - insert_use(&scope, mod_path_to_ast(&mod_path), ctx.config.insert_use); - } - Some(()) -} - -fn extract_struct_def( +fn create_struct_def( variant_name: ast::Name, field_list: &Either, - start_offset: &SyntaxElement, visibility: Option, -) -> Option<()> { +) -> Option { let pub_vis = Some(make::visibility_pub()); let field_list = match field_list { Either::Left(field_list) => { @@ -182,18 +184,7 @@ fn extract_struct_def( .into(), }; - ted::insert_raw( - ted::Position::before(start_offset), - make::struct_(visibility, variant_name, None, field_list).clone_for_update().syntax(), - ); - ted::insert_raw(ted::Position::before(start_offset), &make::tokens::blank_line()); - - // if let indent_level @ 1..=usize::MAX = IndentLevel::from_node(enum_.syntax()).0 as usize { - // ted::insert(ted::Position::before(start_offset), &make::tokens::blank_line()); - // rewriter - // .insert_before(start_offset, &make::tokens::whitespace(&" ".repeat(4 * indent_level))); - // } - Some(()) + Some(make::struct_(visibility, variant_name, None, field_list).clone_for_update()) } fn update_variant(variant: &ast::Variant) -> Option<()> { @@ -208,42 +199,57 @@ fn update_variant(variant: &ast::Variant) -> Option<()> { Some(()) } -fn update_reference( +fn process_references( ctx: &AssistContext, - reference: FileReference, - source_file: &SourceFile, + visited_modules: &mut FxHashSet, + source_file: &SyntaxNode, enum_module_def: &ModuleDef, variant_hir_name: &Name, - visited_modules_set: &mut FxHashSet, -) -> Option<()> { + refs: Vec, +) -> Vec<(ast::PathSegment, SyntaxNode, Option<(ImportScope, hir::ModPath)>)> { + refs.into_iter() + .flat_map(|reference| { + let (segment, scope_node, module) = + reference_to_node(&ctx.sema, source_file, reference)?; + if !visited_modules.contains(&module) { + let mod_path = module.find_use_path_prefixed( + ctx.sema.db, + *enum_module_def, + ctx.config.insert_use.prefix_kind, + ); + if let Some(mut mod_path) = mod_path { + mod_path.pop_segment(); + mod_path.push_segment(variant_hir_name.clone()); + // uuuh this wont properly work, find_insert_use_container ascends macros so we might a get new syntax node??? + let scope = ImportScope::find_insert_use_container(&scope_node, &ctx.sema)?; + visited_modules.insert(module); + return Some((segment, scope_node, Some((scope, mod_path)))); + } + } + Some((segment, scope_node, None)) + }) + .collect() +} + +fn reference_to_node( + sema: &hir::Semantics, + source_file: &SyntaxNode, + reference: FileReference, +) -> Option<(ast::PathSegment, SyntaxNode, hir::Module)> { let offset = reference.range.start(); - let (segment, expr) = if let Some(path_expr) = - find_node_at_offset::(source_file.syntax(), offset) - { + if let Some(path_expr) = find_node_at_offset::(source_file, offset) { // tuple variant - (path_expr.path()?.segment()?, path_expr.syntax().parent()?) - } else if let Some(record_expr) = - find_node_at_offset::(source_file.syntax(), offset) - { + Some((path_expr.path()?.segment()?, path_expr.syntax().parent()?)) + } else if let Some(record_expr) = find_node_at_offset::(source_file, offset) { // record variant - (record_expr.path()?.segment()?, record_expr.syntax().clone()) + Some((record_expr.path()?.segment()?, record_expr.syntax().clone())) } else { - return None; - }; - - let module = ctx.sema.scope(&expr).module()?; - if !visited_modules_set.contains(&module) { - if insert_import(ctx, &expr, &module, enum_module_def, variant_hir_name).is_some() { - visited_modules_set.insert(module); - } + None } - ted::insert_raw( - ted::Position::before(segment.syntax()), - make::path_from_text(&format!("{}", segment)).clone_for_update().syntax(), - ); - ted::insert_raw(ted::Position::before(segment.syntax()), make::token(T!['('])); - ted::insert_raw(ted::Position::after(&expr), make::token(T![')'])); - Some(()) + .and_then(|(segment, expr)| { + let module = sema.scope(&expr).module()?; + Some((segment, expr, module)) + }) } #[cfg(test)] @@ -350,7 +356,7 @@ mod my_mod { pub struct MyField(pub u8, pub u8); - pub enum MyEnum { +pub enum MyEnum { MyField(MyField), } } -- cgit v1.2.3 From b290cd578260f307e872a95f971e5a7c656a80ed Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 20 Apr 2021 19:28:18 +0200 Subject: Add cov_marks to insert_use tests --- crates/ide_assists/src/handlers/auto_import.rs | 2 +- .../handlers/extract_struct_from_enum_variant.rs | 55 +++++++++++----------- .../handlers/replace_qualified_name_with_use.rs | 18 +++---- 3 files changed, 36 insertions(+), 39 deletions(-) (limited to 'crates/ide_assists') diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs index 6db2d2edd..a454a2af3 100644 --- a/crates/ide_assists/src/handlers/auto_import.rs +++ b/crates/ide_assists/src/handlers/auto_import.rs @@ -93,7 +93,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let range = ctx.sema.original_range(&syntax_under_caret).range; let group_label = group_label(import_assets.import_candidate()); - let scope = ImportScope::find_insert_use_container(&syntax_under_caret, &ctx.sema)?; + let scope = ImportScope::find_insert_use_container_with_macros(&syntax_under_caret, &ctx.sema)?; for import in proposed_imports { acc.add_group( &group_label, diff --git a/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs index 1f800f82b..66f274fa7 100644 --- a/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -5,7 +5,7 @@ use hir::{Module, ModuleDef, Name, Variant}; use ide_db::{ defs::Definition, helpers::{ - insert_use::{insert_use, ImportScope}, + insert_use::{insert_use, ImportScope, InsertUseConfig}, mod_path_to_ast, }, search::FileReference, @@ -79,16 +79,8 @@ pub(crate) fn extract_struct_from_enum_variant( &variant_hir_name, references, ); - processed.into_iter().for_each(|(segment, node, import)| { - if let Some((scope, path)) = import { - insert_use(&scope, mod_path_to_ast(&path), ctx.config.insert_use); - } - ted::insert_raw( - ted::Position::before(segment.syntax()), - make::path_from_text(&format!("{}", segment)).clone_for_update().syntax(), - ); - ted::insert_raw(ted::Position::before(segment.syntax()), make::token(T!['('])); - ted::insert_raw(ted::Position::after(&node), make::token(T![')'])); + processed.into_iter().for_each(|(path, node, import)| { + apply_references(ctx.config.insert_use, path, node, import) }); } builder.edit_file(ctx.frange.file_id); @@ -103,21 +95,12 @@ pub(crate) fn extract_struct_from_enum_variant( &variant_hir_name, references, ); - processed.into_iter().for_each(|(segment, node, import)| { - if let Some((scope, path)) = import { - insert_use(&scope, mod_path_to_ast(&path), ctx.config.insert_use); - } - ted::insert_raw( - ted::Position::before(segment.syntax()), - make::path_from_text(&format!("{}", segment)).clone_for_update().syntax(), - ); - ted::insert_raw(ted::Position::before(segment.syntax()), make::token(T!['('])); - ted::insert_raw(ted::Position::after(&node), make::token(T![')'])); + processed.into_iter().for_each(|(path, node, import)| { + apply_references(ctx.config.insert_use, path, node, import) }); } - let def = create_struct_def(variant_name.clone(), &field_list, enum_ast.visibility()) - .unwrap(); + let def = create_struct_def(variant_name.clone(), &field_list, enum_ast.visibility()); let start_offset = &variant.parent_enum().syntax().clone(); ted::insert_raw(ted::Position::before(start_offset), def.syntax()); ted::insert_raw(ted::Position::before(start_offset), &make::tokens::blank_line()); @@ -167,7 +150,7 @@ fn create_struct_def( variant_name: ast::Name, field_list: &Either, visibility: Option, -) -> Option { +) -> ast::Struct { let pub_vis = Some(make::visibility_pub()); let field_list = match field_list { Either::Left(field_list) => { @@ -184,7 +167,7 @@ fn create_struct_def( .into(), }; - Some(make::struct_(visibility, variant_name, None, field_list).clone_for_update()) + make::struct_(visibility, variant_name, None, field_list).clone_for_update() } fn update_variant(variant: &ast::Variant) -> Option<()> { @@ -199,6 +182,23 @@ fn update_variant(variant: &ast::Variant) -> Option<()> { Some(()) } +fn apply_references( + insert_use_cfg: InsertUseConfig, + segment: ast::PathSegment, + node: SyntaxNode, + import: Option<(ImportScope, hir::ModPath)>, +) { + if let Some((scope, path)) = import { + insert_use(&scope, mod_path_to_ast(&path), insert_use_cfg); + } + ted::insert_raw( + ted::Position::before(segment.syntax()), + make::path_from_text(&format!("{}", segment)).clone_for_update().syntax(), + ); + ted::insert_raw(ted::Position::before(segment.syntax()), make::token(T!['('])); + ted::insert_raw(ted::Position::after(&node), make::token(T![')'])); +} + fn process_references( ctx: &AssistContext, visited_modules: &mut FxHashSet, @@ -207,6 +207,8 @@ fn process_references( variant_hir_name: &Name, refs: Vec, ) -> Vec<(ast::PathSegment, SyntaxNode, Option<(ImportScope, hir::ModPath)>)> { + // we have to recollect here eagerly as we are about to edit the tree we need to calculate the changes + // and corresponding nodes up front refs.into_iter() .flat_map(|reference| { let (segment, scope_node, module) = @@ -220,8 +222,7 @@ fn process_references( if let Some(mut mod_path) = mod_path { mod_path.pop_segment(); mod_path.push_segment(variant_hir_name.clone()); - // uuuh this wont properly work, find_insert_use_container ascends macros so we might a get new syntax node??? - let scope = ImportScope::find_insert_use_container(&scope_node, &ctx.sema)?; + let scope = ImportScope::find_insert_use_container(&scope_node)?; visited_modules.insert(module); return Some((segment, scope_node, Some((scope, mod_path)))); } diff --git a/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs b/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs index 2f2306fcc..99ba79860 100644 --- a/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs +++ b/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs @@ -31,7 +31,7 @@ pub(crate) fn replace_qualified_name_with_use( } let target = path.syntax().text_range(); - let scope = ImportScope::find_insert_use_container(path.syntax(), &ctx.sema)?; + let scope = ImportScope::find_insert_use_container_with_macros(path.syntax(), &ctx.sema)?; let syntax = scope.as_syntax_node(); acc.add( AssistId("replace_qualified_name_with_use", AssistKind::RefactorRewrite), @@ -42,15 +42,15 @@ pub(crate) fn replace_qualified_name_with_use( // affected (that is, all paths inside the node we added the `use` to). let syntax = builder.make_mut(syntax.clone()); if let Some(ref import_scope) = ImportScope::from(syntax.clone()) { - insert_use(import_scope, path.clone(), ctx.config.insert_use); + shorten_paths(&syntax, &path.clone_for_update()); + insert_use(import_scope, path, ctx.config.insert_use); } - shorten_paths(syntax.clone(), &path.clone_for_update()); }, ) } /// Adds replacements to `re` that shorten `path` in all descendants of `node`. -fn shorten_paths(node: SyntaxNode, path: &ast::Path) { +fn shorten_paths(node: &SyntaxNode, path: &ast::Path) { for child in node.children() { match_ast! { match child { @@ -59,14 +59,10 @@ fn shorten_paths(node: SyntaxNode, path: &ast::Path) { ast::Use(_it) => continue, // Don't descend into submodules, they don't have the same `use` items in scope. ast::Module(_it) => continue, - - ast::Path(p) => { - match maybe_replace_path(p.clone(), path.clone()) { - Some(()) => {}, - None => shorten_paths(p.syntax().clone(), path), - } + ast::Path(p) => if maybe_replace_path(p.clone(), path.clone()).is_none() { + shorten_paths(p.syntax(), path); }, - _ => shorten_paths(child, path), + _ => shorten_paths(&child, path), } } } -- cgit v1.2.3 From d5c9de65c555235d7a27dc4617858b7a24be4935 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 22 Apr 2021 00:54:31 +0200 Subject: Don't filter equal nodes in reorder assists --- crates/ide_assists/src/handlers/reorder_fields.rs | 8 +++----- crates/ide_assists/src/handlers/reorder_impl.rs | 8 +++----- 2 files changed, 6 insertions(+), 10 deletions(-) (limited to 'crates/ide_assists') diff --git a/crates/ide_assists/src/handlers/reorder_fields.rs b/crates/ide_assists/src/handlers/reorder_fields.rs index 1a95135ca..e90bbdbcf 100644 --- a/crates/ide_assists/src/handlers/reorder_fields.rs +++ b/crates/ide_assists/src/handlers/reorder_fields.rs @@ -83,11 +83,9 @@ fn replace( fields: impl Iterator, sorted_fields: impl IntoIterator, ) { - fields.zip(sorted_fields).filter(|(field, sorted)| field != sorted).for_each( - |(field, sorted_field)| { - ted::replace(field.syntax(), sorted_field.syntax().clone_for_update()); - }, - ); + fields.zip(sorted_fields).for_each(|(field, sorted_field)| { + ted::replace(field.syntax(), sorted_field.syntax().clone_for_update()) + }); } fn compute_fields_ranks(path: &ast::Path, ctx: &AssistContext) -> Option> { diff --git a/crates/ide_assists/src/handlers/reorder_impl.rs b/crates/ide_assists/src/handlers/reorder_impl.rs index fd2897c4c..72d889248 100644 --- a/crates/ide_assists/src/handlers/reorder_impl.rs +++ b/crates/ide_assists/src/handlers/reorder_impl.rs @@ -79,11 +79,9 @@ pub(crate) fn reorder_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()> "Sort methods", target, |builder| { - for (old, new) in - methods.into_iter().zip(sorted).filter(|(field, sorted)| field != sorted) - { - ted::replace(builder.make_ast_mut(old).syntax(), new.clone_for_update().syntax()); - } + methods.into_iter().zip(sorted).for_each(|(old, new)| { + ted::replace(builder.make_ast_mut(old).syntax(), new.clone_for_update().syntax()) + }); }, ) } -- cgit v1.2.3 From e6e4417bbbcc7e135d1b372e4e278cd3efa2d4b8 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 23 Apr 2021 18:36:43 +0200 Subject: Remove SyntaxRewriter::from_fn --- crates/ide_assists/src/ast_transform.rs | 33 +++++++++++++++++++-------------- crates/ide_assists/src/utils.rs | 3 ++- 2 files changed, 21 insertions(+), 15 deletions(-) (limited to 'crates/ide_assists') diff --git a/crates/ide_assists/src/ast_transform.rs b/crates/ide_assists/src/ast_transform.rs index 4a3ed7783..e5ae718c9 100644 --- a/crates/ide_assists/src/ast_transform.rs +++ b/crates/ide_assists/src/ast_transform.rs @@ -3,20 +3,27 @@ use hir::{HirDisplay, PathResolution, SemanticsScope}; use ide_db::helpers::mod_path_to_ast; use rustc_hash::FxHashMap; use syntax::{ - algo::SyntaxRewriter, ast::{self, AstNode}, - SyntaxNode, + ted, SyntaxNode, }; -pub fn apply<'a, N: AstNode>(transformer: &dyn AstTransform<'a>, node: N) -> N { - SyntaxRewriter::from_fn(|element| match element { - syntax::SyntaxElement::Node(n) => { - let replacement = transformer.get_substitution(&n, transformer)?; - Some(replacement.into()) +pub fn apply<'a, N: AstNode>(transformer: &dyn AstTransform<'a>, node: &N) { + let mut skip_to = None; + for event in node.syntax().preorder() { + match event { + syntax::WalkEvent::Enter(node) if skip_to.is_none() => { + skip_to = transformer.get_substitution(&node, transformer).zip(Some(node)); + } + syntax::WalkEvent::Enter(_) => (), + syntax::WalkEvent::Leave(node) => match &skip_to { + Some((replacement, skip_target)) if *skip_target == node => { + ted::replace(node, replacement.clone_for_update()); + skip_to.take(); + } + _ => (), + }, } - _ => None, - }) - .rewrite_ast(&node) + } } /// `AstTransform` helps with applying bulk transformations to syntax nodes. @@ -191,11 +198,9 @@ impl<'a> AstTransform<'a> for QualifyPaths<'a> { let found_path = from.find_use_path(self.source_scope.db.upcast(), def)?; let mut path = mod_path_to_ast(&found_path); - let type_args = p - .segment() - .and_then(|s| s.generic_arg_list()) - .map(|arg_list| apply(recur, arg_list)); + let type_args = p.segment().and_then(|s| s.generic_arg_list()); if let Some(type_args) = type_args { + apply(recur, &type_args); let last_segment = path.segment().unwrap(); path = path.with_segment(last_segment.with_generic_args(type_args)) } diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs index d67524937..5a90ad715 100644 --- a/crates/ide_assists/src/utils.rs +++ b/crates/ide_assists/src/utils.rs @@ -140,7 +140,8 @@ pub fn add_trait_assoc_items_to_impl( let items = items .into_iter() - .map(|it| ast_transform::apply(&*ast_transform, it)) + .map(|it| it.clone_for_update()) + .inspect(|it| ast_transform::apply(&*ast_transform, it)) .map(|it| match it { ast::AssocItem::Fn(def) => ast::AssocItem::Fn(add_body(def)), ast::AssocItem::TypeAlias(def) => ast::AssocItem::TypeAlias(def.remove_bounds()), -- cgit v1.2.3