diff --git a/crates/ide_assists/src/handlers/merge_imports.rs b/crates/ide_assists/src/handlers/merge_imports.rs index fc117bd9a8..94400be7a5 100644 --- a/crates/ide_assists/src/handlers/merge_imports.rs +++ b/crates/ide_assists/src/handlers/merge_imports.rs @@ -80,7 +80,7 @@ use std::fmt$0::{Display, Debug}; use std::fmt::{Display, Debug}; ", r" -use std::fmt::{Debug, Display}; +use std::fmt::{Display, Debug}; ", ) } @@ -108,7 +108,7 @@ use std::fmt::Debug; use std::fmt$0::Display; ", r" -use std::fmt::{Debug, Display}; +use std::fmt::{Display, Debug}; ", ); } @@ -135,7 +135,7 @@ use std::fmt::{self, Display}; use std::{fmt, $0fmt::Display}; ", r" -use std::{fmt::{self, Display}}; +use std::{fmt::{Display, self}}; ", ); } @@ -247,7 +247,7 @@ use std::{fmt::{Debug, Display}}; use std::{fmt::Debug, fmt$0::Display}; ", r" -use std::{fmt::{Debug, Display}}; +use std::{fmt::{Display, Debug}}; ", ); } @@ -341,7 +341,9 @@ use foo::$0{ }; ", r" -use foo::{FooBar, bar::baz}; +use foo::{ + FooBar, bar::baz, +}; ", ) } 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 8df8c4b726..6b196681ca 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 @@ -299,7 +299,7 @@ fn main() { ", r" mod std { pub mod fmt { pub trait Display {} } } -use std::fmt::{self, Display}; +use std::fmt::{Display, self}; fn main() { fmt; diff --git a/crates/ide_assists/src/handlers/split_import.rs b/crates/ide_assists/src/handlers/split_import.rs index 090df25cbd..ac67874b31 100644 --- a/crates/ide_assists/src/handlers/split_import.rs +++ b/crates/ide_assists/src/handlers/split_import.rs @@ -19,14 +19,20 @@ pub(crate) fn split_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let use_tree = path.top_path().syntax().ancestors().find_map(ast::UseTree::cast)?; - let new_tree = use_tree.split_prefix(&path); - if new_tree == use_tree { + let has_errors = use_tree + .syntax() + .descendants_with_tokens() + .any(|it| it.kind() == syntax::SyntaxKind::ERROR); + let last_segment = use_tree.path().and_then(|it| it.segment()); + if has_errors || last_segment.is_none() { return None; } let target = colon_colon.text_range(); acc.add(AssistId("split_import", AssistKind::RefactorRewrite), "Split import", target, |edit| { - edit.replace_ast(use_tree, new_tree); + let use_tree = edit.make_mut(use_tree.clone()); + let path = edit.make_mut(path); + use_tree.split_prefix(&path); }) } diff --git a/crates/ide_db/src/helpers/insert_use/tests.rs b/crates/ide_db/src/helpers/insert_use/tests.rs index 34a6900e26..a5763eb41c 100644 --- a/crates/ide_db/src/helpers/insert_use/tests.rs +++ b/crates/ide_db/src/helpers/insert_use/tests.rs @@ -463,7 +463,7 @@ fn merge_groups_full() { #[test] fn merge_groups_long_full() { - check_crate("std::foo::bar::Baz", r"use std::foo::bar::Qux;", r"use std::foo::bar::{Baz, Qux};") + check_crate("std::foo::bar::Baz", r"use std::foo::bar::Qux;", r"use std::foo::bar::{Qux, Baz};") } #[test] @@ -471,7 +471,7 @@ fn merge_groups_long_last() { check_module( "std::foo::bar::Baz", r"use std::foo::bar::Qux;", - r"use std::foo::bar::{Baz, Qux};", + r"use std::foo::bar::{Qux, Baz};", ) } @@ -480,7 +480,7 @@ fn merge_groups_long_full_list() { check_crate( "std::foo::bar::Baz", r"use std::foo::bar::{Qux, Quux};", - r"use std::foo::bar::{Baz, Quux, Qux};", + r"use std::foo::bar::{Qux, Quux, Baz};", ) } @@ -489,7 +489,7 @@ fn merge_groups_long_last_list() { check_module( "std::foo::bar::Baz", r"use std::foo::bar::{Qux, Quux};", - r"use std::foo::bar::{Baz, Quux, Qux};", + r"use std::foo::bar::{Qux, Quux, Baz};", ) } @@ -498,7 +498,7 @@ fn merge_groups_long_full_nested() { check_crate( "std::foo::bar::Baz", r"use std::foo::bar::{Qux, quux::{Fez, Fizz}};", - r"use std::foo::bar::{Baz, Qux, quux::{Fez, Fizz}};", + r"use std::foo::bar::{Qux, quux::{Fez, Fizz}, Baz};", ) } @@ -517,7 +517,7 @@ fn merge_groups_full_nested_deep() { check_crate( "std::foo::bar::quux::Baz", r"use std::foo::bar::{Qux, quux::{Fez, Fizz}};", - r"use std::foo::bar::{Qux, quux::{Baz, Fez, Fizz}};", + r"use std::foo::bar::{Qux, quux::{Fez, Fizz, Baz}};", ) } @@ -526,7 +526,7 @@ fn merge_groups_full_nested_long() { check_crate( "std::foo::bar::Baz", r"use std::{foo::bar::Qux};", - r"use std::{foo::bar::{Baz, Qux}};", + r"use std::{foo::bar::{Qux, Baz}};", ); } @@ -535,7 +535,7 @@ fn merge_groups_last_nested_long() { check_crate( "std::foo::bar::Baz", r"use std::{foo::bar::Qux};", - r"use std::{foo::bar::{Baz, Qux}};", + r"use std::{foo::bar::{Qux, Baz}};", ); } @@ -600,7 +600,7 @@ fn merge_mod_into_glob() { check_with_config( "token::TokenKind", r"use token::TokenKind::*;", - r"use token::TokenKind::{*, self};", + r"use token::TokenKind::{self, *};", &InsertUseConfig { granularity: ImportGranularity::Crate, enforce_granularity: true, @@ -618,7 +618,7 @@ fn merge_self_glob() { check_with_config( "self", r"use self::*;", - r"use self::{*, self};", + r"use self::{self, *};", &InsertUseConfig { granularity: ImportGranularity::Crate, enforce_granularity: true, @@ -637,7 +637,7 @@ fn merge_glob() { r" use syntax::{SyntaxKind::*};", r" -use syntax::{SyntaxKind::{*, self}};", +use syntax::{SyntaxKind::{self, *}};", ) } diff --git a/crates/ide_db/src/helpers/merge_imports.rs b/crates/ide_db/src/helpers/merge_imports.rs index 6a7a9b8f61..8581a83436 100644 --- a/crates/ide_db/src/helpers/merge_imports.rs +++ b/crates/ide_db/src/helpers/merge_imports.rs @@ -44,10 +44,10 @@ pub fn try_merge_imports( } let lhs = lhs.clone_subtree().clone_for_update(); + let rhs = rhs.clone_subtree().clone_for_update(); let lhs_tree = lhs.use_tree()?; let rhs_tree = rhs.use_tree()?; - let merged = try_merge_trees(&lhs_tree, &rhs_tree, merge_behavior)?; - ted::replace(lhs_tree.syntax(), merged.syntax()); + try_merge_trees_mut(&lhs_tree, &rhs_tree, merge_behavior)?; Some(lhs) } @@ -56,57 +56,49 @@ pub fn try_merge_trees( rhs: &ast::UseTree, merge: MergeBehavior, ) -> Option { + let lhs = lhs.clone_subtree().clone_for_update(); + let rhs = rhs.clone_subtree().clone_for_update(); + try_merge_trees_mut(&lhs, &rhs, merge)?; + Some(lhs) +} + +fn try_merge_trees_mut(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehavior) -> Option<()> { let lhs_path = lhs.path()?; let rhs_path = rhs.path()?; let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?; - let (lhs, rhs) = if lhs.is_simple_path() + if !(lhs.is_simple_path() && rhs.is_simple_path() && lhs_path == lhs_prefix - && rhs_path == rhs_prefix + && rhs_path == rhs_prefix) { - (lhs.clone(), rhs.clone()) - } else { - (lhs.split_prefix(&lhs_prefix), rhs.split_prefix(&rhs_prefix)) - }; - recursive_merge(&lhs, &rhs, merge).map(|it| it.clone_for_update()) + lhs.split_prefix(&lhs_prefix); + rhs.split_prefix(&rhs_prefix); + } + recursive_merge(&lhs, &rhs, merge) } -/// Recursively "zips" together lhs and rhs. -fn recursive_merge( - lhs: &ast::UseTree, - rhs: &ast::UseTree, - merge: MergeBehavior, -) -> Option { - let mut use_trees = lhs +/// Recursively merges rhs to lhs +#[must_use] +fn recursive_merge(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehavior) -> Option<()> { + let mut use_trees: Vec = lhs .use_tree_list() .into_iter() .flat_map(|list| list.use_trees()) // We use Option here to early return from this function(this is not the // same as a `filter` op). - .map(|tree| match merge.is_tree_allowed(&tree) { - true => Some(tree), - false => None, - }) - .collect::>>()?; + .map(|tree| merge.is_tree_allowed(&tree).then(|| tree)) + .collect::>()?; use_trees.sort_unstable_by(|a, b| path_cmp_for_sort(a.path(), b.path())); for rhs_t in rhs.use_tree_list().into_iter().flat_map(|list| list.use_trees()) { if !merge.is_tree_allowed(&rhs_t) { return None; } let rhs_path = rhs_t.path(); - match use_trees.binary_search_by(|lhs_t| { - let (lhs_t, rhs_t) = match lhs_t - .path() - .zip(rhs_path.clone()) - .and_then(|(lhs, rhs)| common_prefix(&lhs, &rhs)) - { - Some((lhs_p, rhs_p)) => (lhs_t.split_prefix(&lhs_p), rhs_t.split_prefix(&rhs_p)), - None => (lhs_t.clone(), rhs_t.clone()), - }; - path_cmp_bin_search(lhs_t.path(), rhs_t.path()) - }) { + match use_trees + .binary_search_by(|lhs_t| path_cmp_bin_search(lhs_t.path(), rhs_path.as_ref())) + { Ok(idx) => { let lhs_t = &mut use_trees[idx]; let lhs_path = lhs_t.path()?; @@ -128,6 +120,7 @@ fn recursive_merge( match (tree_contains_self(lhs_t), tree_contains_self(&rhs_t)) { (true, false) => continue, (false, true) => { + ted::replace(lhs_t.syntax(), rhs_t.syntax()); *lhs_t = rhs_t; continue; } @@ -142,25 +135,27 @@ fn recursive_merge( if lhs_t.star_token().is_some() || rhs_t.star_token().is_some() { if tree_is_self(lhs_t) || tree_is_self(&rhs_t) { cov_mark::hit!(merge_self_glob); - *lhs_t = make::use_tree( + let self_tree = make::use_tree( make::path_unqualified(make::path_segment_self()), None, None, false, - ); - use_trees.insert(idx, make::use_tree_glob()); + ) + .clone_for_update(); + ted::replace(lhs_t.syntax(), self_tree.syntax()); + *lhs_t = self_tree; + let glob = make::use_tree_glob().clone_for_update(); + use_trees.insert(idx, glob.clone()); + lhs.get_or_create_use_tree_list().add_use_tree(glob); continue; } } else if lhs_t.use_tree_list().is_none() && rhs_t.use_tree_list().is_none() { continue; } } - let lhs = lhs_t.split_prefix(&lhs_prefix); - let rhs = rhs_t.split_prefix(&rhs_prefix); - match recursive_merge(&lhs, &rhs, merge) { - Some(use_tree) => use_trees[idx] = use_tree, - None => return None, - } + lhs_t.split_prefix(&lhs_prefix); + rhs_t.split_prefix(&rhs_prefix); + recursive_merge(&lhs_t, &rhs_t, merge)?; } Err(_) if merge == MergeBehavior::Module @@ -170,16 +165,12 @@ fn recursive_merge( return None } Err(idx) => { - use_trees.insert(idx, rhs_t); + use_trees.insert(idx, rhs_t.clone()); + lhs.get_or_create_use_tree_list().add_use_tree(rhs_t); } } } - - let lhs = lhs.clone_subtree().clone_for_update(); - if let Some(old) = lhs.use_tree_list() { - ted::replace(old.syntax(), make::use_tree_list(use_trees).syntax().clone_for_update()); - } - ast::UseTree::cast(lhs.syntax().clone_subtree()) + Some(()) } /// Traverses both paths until they differ, returning the common prefix of both. @@ -224,11 +215,9 @@ fn path_cmp_for_sort(a: Option, b: Option) -> Ordering { } /// Path comparison func for binary searching for merging. -fn path_cmp_bin_search(lhs: Option, rhs: Option) -> Ordering { - match ( - lhs.as_ref().and_then(ast::Path::first_segment), - rhs.as_ref().and_then(ast::Path::first_segment), - ) { +fn path_cmp_bin_search(lhs: Option, rhs: Option<&ast::Path>) -> Ordering { + match (lhs.as_ref().and_then(ast::Path::first_segment), rhs.and_then(ast::Path::first_segment)) + { (None, None) => Ordering::Equal, (None, Some(_)) => Ordering::Less, (Some(_), None) => Ordering::Greater, diff --git a/crates/syntax/src/ast/edit.rs b/crates/syntax/src/ast/edit.rs index 43a9c6756d..15805dfc86 100644 --- a/crates/syntax/src/ast/edit.rs +++ b/crates/syntax/src/ast/edit.rs @@ -4,48 +4,10 @@ use std::{fmt, iter, ops}; use crate::{ - algo, ast::{self, make, AstNode}, ted, AstToken, NodeOrToken, SyntaxElement, SyntaxNode, SyntaxToken, }; -impl ast::UseTree { - /// Splits off the given prefix, making it the path component of the use tree, appending the rest of the path to all UseTreeList items. - #[must_use] - pub fn split_prefix(&self, prefix: &ast::Path) -> ast::UseTree { - let suffix = if self.path().as_ref() == Some(prefix) && self.use_tree_list().is_none() { - make::path_unqualified(make::path_segment_self()) - } else { - match split_path_prefix(prefix) { - Some(it) => it, - None => return self.clone(), - } - }; - - let use_tree = make::use_tree( - suffix, - self.use_tree_list(), - self.rename(), - self.star_token().is_some(), - ); - let nested = make::use_tree_list(iter::once(use_tree)); - return make::use_tree(prefix.clone(), Some(nested), None, false); - - fn split_path_prefix(prefix: &ast::Path) -> Option { - let parent = prefix.parent_path()?; - let segment = parent.segment()?; - if algo::has_errors(segment.syntax()) { - return None; - } - let mut res = make::path_unqualified(segment); - for p in iter::successors(parent.parent_path(), |it| it.parent_path()) { - res = make::path_qualified(res, p.segment()?); - } - Some(res) - } - } -} - #[derive(Debug, Clone, Copy)] pub struct IndentLevel(pub u8); diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs index 552d6fc1e3..bf4f9d6d18 100644 --- a/crates/syntax/src/ast/edit_in_place.rs +++ b/crates/syntax/src/ast/edit_in_place.rs @@ -1,12 +1,12 @@ //! Structural editing for ast. -use std::iter::empty; +use std::iter::{empty, successors}; use parser::{SyntaxKind, T}; use rowan::SyntaxElement; use crate::{ - algo::neighbor, + algo::{self, neighbor}, ast::{self, edit::IndentLevel, make, HasGenericParams}, ted::{self, Position}, AstNode, AstToken, Direction, @@ -282,6 +282,78 @@ impl ast::UseTree { } ted::remove(self.syntax()); } + + pub fn get_or_create_use_tree_list(&self) -> ast::UseTreeList { + match self.use_tree_list() { + Some(it) => it, + None => { + let position = Position::last_child_of(self.syntax()); + let use_tree_list = make::use_tree_list(empty()).clone_for_update(); + let mut elements = Vec::with_capacity(2); + if self.coloncolon_token().is_none() { + elements.push(make::token(T![::]).into()); + } + elements.push(use_tree_list.syntax().clone().into()); + ted::insert_all_raw(position, elements); + use_tree_list + } + } + } + + /// Splits off the given prefix, making it the path component of the use tree, + /// appending the rest of the path to all UseTreeList items. + pub fn split_prefix(&self, prefix: &ast::Path) { + debug_assert_eq!(self.path(), Some(prefix.top_path())); + let path = self.path().unwrap(); + if &path == prefix && self.use_tree_list().is_none() { + let self_suffix = make::path_unqualified(make::path_segment_self()).clone_for_update(); + ted::replace(path.syntax(), self_suffix.syntax()); + } else if split_path_prefix(prefix).is_none() { + return; + } + + let subtree = self.clone_subtree().clone_for_update(); + ted::remove_all_iter(self.syntax().children_with_tokens()); + ted::insert(Position::first_child_of(self.syntax()), prefix.syntax()); + self.get_or_create_use_tree_list().add_use_tree(subtree); + + fn split_path_prefix(prefix: &ast::Path) -> Option<()> { + let parent = prefix.parent_path()?; + let segment = parent.segment()?; + if algo::has_errors(segment.syntax()) { + return None; + } + for p in successors(parent.parent_path(), |it| it.parent_path()) { + p.segment()?; + } + prefix.parent_path().and_then(|p| p.coloncolon_token()).map(ted::remove); + ted::remove(prefix.syntax()); + Some(()) + } + } +} + +impl ast::UseTreeList { + pub fn add_use_tree(&self, use_tree: ast::UseTree) { + let (position, elements) = match self.use_trees().last() { + Some(last_tree) => ( + Position::after(last_tree.syntax()), + vec![ + make::token(T![,]).into(), + make::tokens::single_space().into(), + use_tree.syntax.into(), + ], + ), + None => { + let position = match self.l_curly_token() { + Some(l_curly) => Position::after(l_curly), + None => Position::last_child_of(self.syntax()), + }; + (position, vec![use_tree.syntax.into()]) + } + }; + ted::insert_all_raw(position, elements); + } } impl ast::Use {