aboutsummaryrefslogtreecommitdiff
path: root/crates
diff options
context:
space:
mode:
authorLukas Wirth <[email protected]>2020-09-30 17:11:46 +0100
committerLukas Wirth <[email protected]>2020-09-30 18:09:17 +0100
commit0671bf2d734c6d579896a8f976a2c65e5a7f405e (patch)
tree5355f982aa2c2386672e21824d38227d74d4dfb4 /crates
parent7c9ae771bca39d511a0ea7395da2b4b91b44ee12 (diff)
Fix MergingBehaviour::Last not working properly
Diffstat (limited to 'crates')
-rw-r--r--crates/assists/src/utils/insert_use.rs73
1 files changed, 52 insertions, 21 deletions
diff --git a/crates/assists/src/utils/insert_use.rs b/crates/assists/src/utils/insert_use.rs
index 5719b06af..8dd4fe607 100644
--- a/crates/assists/src/utils/insert_use.rs
+++ b/crates/assists/src/utils/insert_use.rs
@@ -174,7 +174,7 @@ pub(crate) fn try_merge_trees(
174 let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?; 174 let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?;
175 let lhs = lhs.split_prefix(&lhs_prefix); 175 let lhs = lhs.split_prefix(&lhs_prefix);
176 let rhs = rhs.split_prefix(&rhs_prefix); 176 let rhs = rhs.split_prefix(&rhs_prefix);
177 recursive_merge(&lhs, &rhs, merge).map(|(merged, _)| merged) 177 recursive_merge(&lhs, &rhs, merge)
178} 178}
179 179
180/// Recursively "zips" together lhs and rhs. 180/// Recursively "zips" together lhs and rhs.
@@ -182,20 +182,22 @@ fn recursive_merge(
182 lhs: &ast::UseTree, 182 lhs: &ast::UseTree,
183 rhs: &ast::UseTree, 183 rhs: &ast::UseTree,
184 merge: MergeBehaviour, 184 merge: MergeBehaviour,
185) -> Option<(ast::UseTree, bool)> { 185) -> Option<ast::UseTree> {
186 let mut use_trees = lhs 186 let mut use_trees = lhs
187 .use_tree_list() 187 .use_tree_list()
188 .into_iter() 188 .into_iter()
189 .flat_map(|list| list.use_trees()) 189 .flat_map(|list| list.use_trees())
190 // check if any of the use trees are nested, if they are and the behaviour is `last` we are not allowed to merge this 190 // we use Option here to early return from this function(this is not the same as a `filter` op)
191 // so early exit the iterator by using Option's Intoiterator impl 191 .map(|tree| match merge.is_tree_allowed(&tree) {
192 .map(|tree| match merge == MergeBehaviour::Last && tree.use_tree_list().is_some() { 192 true => Some(tree),
193 true => None, 193 false => None,
194 false => Some(tree),
195 }) 194 })
196 .collect::<Option<Vec<_>>>()?; 195 .collect::<Option<Vec<_>>>()?;
197 use_trees.sort_unstable_by(|a, b| path_cmp_opt(a.path(), b.path())); 196 use_trees.sort_unstable_by(|a, b| path_cmp_opt(a.path(), b.path()));
198 for rhs_t in rhs.use_tree_list().into_iter().flat_map(|list| list.use_trees()) { 197 for rhs_t in rhs.use_tree_list().into_iter().flat_map(|list| list.use_trees()) {
198 if !merge.is_tree_allowed(&rhs_t) {
199 return None;
200 }
199 let rhs_path = rhs_t.path(); 201 let rhs_path = rhs_t.path();
200 match use_trees.binary_search_by(|p| path_cmp_opt(p.path(), rhs_path.clone())) { 202 match use_trees.binary_search_by(|p| path_cmp_opt(p.path(), rhs_path.clone())) {
201 Ok(idx) => { 203 Ok(idx) => {
@@ -239,17 +241,9 @@ fn recursive_merge(
239 } 241 }
240 let lhs = lhs_t.split_prefix(&lhs_prefix); 242 let lhs = lhs_t.split_prefix(&lhs_prefix);
241 let rhs = rhs_t.split_prefix(&rhs_prefix); 243 let rhs = rhs_t.split_prefix(&rhs_prefix);
242 let this_has_children = use_trees.len() > 0;
243 match recursive_merge(&lhs, &rhs, merge) { 244 match recursive_merge(&lhs, &rhs, merge) {
244 Some((_, has_multiple_children)) 245 Some(use_tree) => use_trees[idx] = use_tree,
245 if merge == MergeBehaviour::Last 246 None => return None,
246 && this_has_children
247 && has_multiple_children =>
248 {
249 return None
250 }
251 Some((use_tree, _)) => use_trees[idx] = use_tree,
252 None => use_trees.insert(idx, rhs_t),
253 } 247 }
254 } 248 }
255 Err(_) 249 Err(_)
@@ -264,8 +258,7 @@ fn recursive_merge(
264 } 258 }
265 } 259 }
266 } 260 }
267 let has_multiple_children = use_trees.len() > 1; 261 Some(lhs.with_use_tree_list(make::use_tree_list(use_trees)))
268 Some((lhs.with_use_tree_list(make::use_tree_list(use_trees)), has_multiple_children))
269} 262}
270 263
271/// Traverses both paths until they differ, returning the common prefix of both. 264/// Traverses both paths until they differ, returning the common prefix of both.
@@ -308,6 +301,10 @@ fn segment_iter(path: &ast::Path) -> impl Iterator<Item = ast::PathSegment> + Cl
308 successors(first_segment(path), |p| p.parent_path().parent_path().and_then(|p| p.segment())) 301 successors(first_segment(path), |p| p.parent_path().parent_path().and_then(|p| p.segment()))
309} 302}
310 303
304fn path_len(path: ast::Path) -> usize {
305 segment_iter(&path).count()
306}
307
311/// Orders paths in the following way: 308/// Orders paths in the following way:
312/// the sole self token comes first, after that come uppercase identifiers, then lowercase identifiers 309/// the sole self token comes first, after that come uppercase identifiers, then lowercase identifiers
313// FIXME: rustfmt sort lowercase idents before uppercase, in general we want to have the same ordering rustfmt has 310// FIXME: rustfmt sort lowercase idents before uppercase, in general we want to have the same ordering rustfmt has
@@ -352,6 +349,19 @@ pub enum MergeBehaviour {
352 Last, 349 Last,
353} 350}
354 351
352impl MergeBehaviour {
353 #[inline]
354 fn is_tree_allowed(&self, tree: &ast::UseTree) -> bool {
355 match self {
356 MergeBehaviour::Full => true,
357 // only simple single segment paths are allowed
358 MergeBehaviour::Last => {
359 tree.use_tree_list().is_none() && tree.path().map(path_len) <= Some(1)
360 }
361 }
362 }
363}
364
355#[derive(Eq, PartialEq, PartialOrd, Ord)] 365#[derive(Eq, PartialEq, PartialOrd, Ord)]
356enum ImportGroup { 366enum ImportGroup {
357 // the order here defines the order of new group inserts 367 // the order here defines the order of new group inserts
@@ -676,6 +686,11 @@ use std::io;",
676 } 686 }
677 687
678 #[test] 688 #[test]
689 fn merge_last_into_self() {
690 check_last("foo::bar::baz", r"use foo::bar;", r"use foo::bar::{self, baz};");
691 }
692
693 #[test]
679 fn merge_groups_full() { 694 fn merge_groups_full() {
680 check_full( 695 check_full(
681 "std::io", 696 "std::io",
@@ -819,8 +834,24 @@ use std::io;",
819 } 834 }
820 835
821 #[test] 836 #[test]
822 fn merge_last_too_long() { 837 fn skip_merge_last_too_long() {
823 check_last("foo::bar", r"use foo::bar::baz::Qux;", r"use foo::bar::{self, baz::Qux};"); 838 check_last(
839 "foo::bar",
840 r"use foo::bar::baz::Qux;",
841 r"use foo::bar;
842use foo::bar::baz::Qux;",
843 );
844 }
845
846 #[test]
847 #[ignore] // FIXME: Order changes when switching lhs and rhs
848 fn skip_merge_last_too_long2() {
849 check_last(
850 "foo::bar::baz::Qux",
851 r"use foo::bar;",
852 r"use foo::bar;
853use foo::bar::baz::Qux;",
854 );
824 } 855 }
825 856
826 #[test] 857 #[test]