diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2020-09-16 21:03:41 +0100 |
---|---|---|
committer | GitHub <[email protected]> | 2020-09-16 21:03:41 +0100 |
commit | 3877421373ef1439e53b7f7274865af287dbb47f (patch) | |
tree | a7180b968598a3ef53ef9587d43fb7842a98dc1a | |
parent | 4bc8015370e3698248bc93184ef7ec5fefd2c1d4 (diff) | |
parent | 45298b5d2a8e7d1f962f3117de27957e393c03e2 (diff) |
Merge #5989
5989: Rewrite import merging r=jonas-schievink a=Veykril
Rewrites how import merging is being handled. It is now a recursive function to properly handle merging of intermediate levels in the import trees. With this ordering the imports is also now possible tho it doesn't quite order it the same way as `rustfmt` does yet, namely it orders lowercase identifiers after uppercase identifiers as that is the standard character order that rust uses. This also fixes a few weird behaviors that were visible in some of the `replace_qualified_name_with_use.rs` tests.
This really took longer than I was hoping for, fighting with import trees is quite the exhausting task 😅
Co-authored-by: Lukas Wirth <[email protected]>
-rw-r--r-- | crates/assists/src/handlers/merge_imports.rs | 14 | ||||
-rw-r--r-- | crates/assists/src/handlers/replace_qualified_name_with_use.rs | 20 | ||||
-rw-r--r-- | crates/assists/src/utils/insert_use.rs | 356 | ||||
-rw-r--r-- | crates/syntax/src/ast/edit.rs | 1 | ||||
-rw-r--r-- | crates/syntax/src/ast/make.rs | 4 |
5 files changed, 292 insertions, 103 deletions
diff --git a/crates/assists/src/handlers/merge_imports.rs b/crates/assists/src/handlers/merge_imports.rs index 0bd679260..fe33cee53 100644 --- a/crates/assists/src/handlers/merge_imports.rs +++ b/crates/assists/src/handlers/merge_imports.rs | |||
@@ -95,7 +95,7 @@ use std::fmt::Debug; | |||
95 | use std::fmt<|>::Display; | 95 | use std::fmt<|>::Display; |
96 | ", | 96 | ", |
97 | r" | 97 | r" |
98 | use std::fmt::{Display, Debug}; | 98 | use std::fmt::{Debug, Display}; |
99 | ", | 99 | ", |
100 | ); | 100 | ); |
101 | } | 101 | } |
@@ -122,7 +122,7 @@ use std::fmt::{self, Display}; | |||
122 | use std::{fmt, <|>fmt::Display}; | 122 | use std::{fmt, <|>fmt::Display}; |
123 | ", | 123 | ", |
124 | r" | 124 | r" |
125 | use std::{fmt::{Display, self}}; | 125 | use std::{fmt::{self, Display}}; |
126 | ", | 126 | ", |
127 | ); | 127 | ); |
128 | } | 128 | } |
@@ -210,13 +210,17 @@ use std::{fmt<|>::Debug, fmt::Display}; | |||
210 | use std::{fmt::{Debug, Display}}; | 210 | use std::{fmt::{Debug, Display}}; |
211 | ", | 211 | ", |
212 | ); | 212 | ); |
213 | } | ||
214 | |||
215 | #[test] | ||
216 | fn test_merge_nested2() { | ||
213 | check_assist( | 217 | check_assist( |
214 | merge_imports, | 218 | merge_imports, |
215 | r" | 219 | r" |
216 | use std::{fmt::Debug, fmt<|>::Display}; | 220 | use std::{fmt::Debug, fmt<|>::Display}; |
217 | ", | 221 | ", |
218 | r" | 222 | r" |
219 | use std::{fmt::{Display, Debug}}; | 223 | use std::{fmt::{Debug, Display}}; |
220 | ", | 224 | ", |
221 | ); | 225 | ); |
222 | } | 226 | } |
@@ -310,9 +314,7 @@ use foo::<|>{ | |||
310 | }; | 314 | }; |
311 | ", | 315 | ", |
312 | r" | 316 | r" |
313 | use foo::{ | 317 | use foo::{FooBar, bar::baz}; |
314 | FooBar, | ||
315 | bar::baz}; | ||
316 | ", | 318 | ", |
317 | ) | 319 | ) |
318 | } | 320 | } |
diff --git a/crates/assists/src/handlers/replace_qualified_name_with_use.rs b/crates/assists/src/handlers/replace_qualified_name_with_use.rs index e48407fcc..8ac907707 100644 --- a/crates/assists/src/handlers/replace_qualified_name_with_use.rs +++ b/crates/assists/src/handlers/replace_qualified_name_with_use.rs | |||
@@ -312,7 +312,7 @@ impl std::fmt<|> for Foo { | |||
312 | } | 312 | } |
313 | ", | 313 | ", |
314 | r" | 314 | r" |
315 | use std::fmt::{Debug, self}; | 315 | use std::fmt::{self, Debug}; |
316 | 316 | ||
317 | impl fmt for Foo { | 317 | impl fmt for Foo { |
318 | } | 318 | } |
@@ -330,9 +330,8 @@ use std::fmt::{Debug, nested::{Display}}; | |||
330 | impl std::fmt::nested<|> for Foo { | 330 | impl std::fmt::nested<|> for Foo { |
331 | } | 331 | } |
332 | ", | 332 | ", |
333 | // FIXME(veykril): should be nested::{self, Display} here | ||
334 | r" | 333 | r" |
335 | use std::fmt::{Debug, nested::{Display}, nested}; | 334 | use std::fmt::{Debug, nested::{self, Display}}; |
336 | 335 | ||
337 | impl nested for Foo { | 336 | impl nested for Foo { |
338 | } | 337 | } |
@@ -350,9 +349,8 @@ use std::fmt::{Debug, nested::{self, Display}}; | |||
350 | impl std::fmt::nested<|> for Foo { | 349 | impl std::fmt::nested<|> for Foo { |
351 | } | 350 | } |
352 | ", | 351 | ", |
353 | // FIXME(veykril): nested is duplicated now | ||
354 | r" | 352 | r" |
355 | use std::fmt::{Debug, nested::{self, Display}, nested}; | 353 | use std::fmt::{Debug, nested::{self, Display}}; |
356 | 354 | ||
357 | impl nested for Foo { | 355 | impl nested for Foo { |
358 | } | 356 | } |
@@ -371,7 +369,7 @@ impl std::fmt::nested::Debug<|> for Foo { | |||
371 | } | 369 | } |
372 | ", | 370 | ", |
373 | r" | 371 | r" |
374 | use std::fmt::{Debug, nested::{Display}, nested::Debug}; | 372 | use std::fmt::{Debug, nested::{Debug, Display}}; |
375 | 373 | ||
376 | impl Debug for Foo { | 374 | impl Debug for Foo { |
377 | } | 375 | } |
@@ -409,7 +407,7 @@ impl std::fmt::Display<|> for Foo { | |||
409 | } | 407 | } |
410 | ", | 408 | ", |
411 | r" | 409 | r" |
412 | use std::fmt::{nested::Debug, Display}; | 410 | use std::fmt::{Display, nested::Debug}; |
413 | 411 | ||
414 | impl Display for Foo { | 412 | impl Display for Foo { |
415 | } | 413 | } |
@@ -429,12 +427,8 @@ use crate::{ | |||
429 | 427 | ||
430 | fn foo() { crate::ty::lower<|>::trait_env() } | 428 | fn foo() { crate::ty::lower<|>::trait_env() } |
431 | ", | 429 | ", |
432 | // FIXME(veykril): formatting broke here | ||
433 | r" | 430 | r" |
434 | use crate::{ | 431 | use crate::{AssocItem, ty::{Substs, Ty, lower}}; |
435 | ty::{Substs, Ty}, | ||
436 | AssocItem, | ||
437 | ty::lower}; | ||
438 | 432 | ||
439 | fn foo() { lower::trait_env() } | 433 | fn foo() { lower::trait_env() } |
440 | ", | 434 | ", |
@@ -633,7 +627,7 @@ fn main() { | |||
633 | } | 627 | } |
634 | ", | 628 | ", |
635 | r" | 629 | r" |
636 | use std::fmt::{Display, self}; | 630 | use std::fmt::{self, Display}; |
637 | 631 | ||
638 | fn main() { | 632 | fn main() { |
639 | fmt; | 633 | fmt; |
diff --git a/crates/assists/src/utils/insert_use.rs b/crates/assists/src/utils/insert_use.rs index 6d110aaaf..09f4a2224 100644 --- a/crates/assists/src/utils/insert_use.rs +++ b/crates/assists/src/utils/insert_use.rs | |||
@@ -1,7 +1,9 @@ | |||
1 | //! Handle syntactic aspects of inserting a new `use`. | 1 | //! Handle syntactic aspects of inserting a new `use`. |
2 | use std::iter::{self, successors}; | 2 | use std::{ |
3 | cmp::Ordering, | ||
4 | iter::{self, successors}, | ||
5 | }; | ||
3 | 6 | ||
4 | use algo::skip_trivia_token; | ||
5 | use ast::{ | 7 | use ast::{ |
6 | edit::{AstNodeEdit, IndentLevel}, | 8 | edit::{AstNodeEdit, IndentLevel}, |
7 | PathSegmentKind, VisibilityOwner, | 9 | PathSegmentKind, VisibilityOwner, |
@@ -9,9 +11,8 @@ use ast::{ | |||
9 | use syntax::{ | 11 | use syntax::{ |
10 | algo, | 12 | algo, |
11 | ast::{self, make, AstNode}, | 13 | ast::{self, make, AstNode}, |
12 | Direction, InsertPosition, SyntaxElement, SyntaxNode, T, | 14 | InsertPosition, SyntaxElement, SyntaxNode, |
13 | }; | 15 | }; |
14 | use test_utils::mark; | ||
15 | 16 | ||
16 | #[derive(Debug)] | 17 | #[derive(Debug)] |
17 | pub enum ImportScope { | 18 | pub enum ImportScope { |
@@ -119,7 +120,6 @@ pub(crate) fn insert_use( | |||
119 | } | 120 | } |
120 | 121 | ||
121 | if let ident_level @ 1..=usize::MAX = scope.indent_level().0 as usize { | 122 | if let ident_level @ 1..=usize::MAX = scope.indent_level().0 as usize { |
122 | // FIXME: this alone doesnt properly re-align all cases | ||
123 | buf.push(make::tokens::whitespace(&" ".repeat(4 * ident_level)).into()); | 123 | buf.push(make::tokens::whitespace(&" ".repeat(4 * ident_level)).into()); |
124 | } | 124 | } |
125 | buf.push(use_item.syntax().clone().into()); | 125 | buf.push(use_item.syntax().clone().into()); |
@@ -149,66 +149,123 @@ fn eq_visibility(vis0: Option<ast::Visibility>, vis1: Option<ast::Visibility>) - | |||
149 | } | 149 | } |
150 | 150 | ||
151 | pub(crate) fn try_merge_imports( | 151 | pub(crate) fn try_merge_imports( |
152 | old: &ast::Use, | 152 | lhs: &ast::Use, |
153 | new: &ast::Use, | 153 | rhs: &ast::Use, |
154 | merge_behaviour: MergeBehaviour, | 154 | merge_behaviour: MergeBehaviour, |
155 | ) -> Option<ast::Use> { | 155 | ) -> Option<ast::Use> { |
156 | // don't merge imports with different visibilities | 156 | // don't merge imports with different visibilities |
157 | if !eq_visibility(old.visibility(), new.visibility()) { | 157 | if !eq_visibility(lhs.visibility(), rhs.visibility()) { |
158 | return None; | 158 | return None; |
159 | } | 159 | } |
160 | let old_tree = old.use_tree()?; | 160 | let lhs_tree = lhs.use_tree()?; |
161 | let new_tree = new.use_tree()?; | 161 | let rhs_tree = rhs.use_tree()?; |
162 | let merged = try_merge_trees(&old_tree, &new_tree, merge_behaviour)?; | 162 | let merged = try_merge_trees(&lhs_tree, &rhs_tree, merge_behaviour)?; |
163 | Some(old.with_use_tree(merged)) | 163 | Some(lhs.with_use_tree(merged)) |
164 | } | ||
165 | |||
166 | /// Simple function that checks if a UseTreeList is deeper than one level | ||
167 | fn use_tree_list_is_nested(tl: &ast::UseTreeList) -> bool { | ||
168 | tl.use_trees().any(|use_tree| { | ||
169 | use_tree.use_tree_list().is_some() || use_tree.path().and_then(|p| p.qualifier()).is_some() | ||
170 | }) | ||
171 | } | 164 | } |
172 | 165 | ||
173 | // FIXME: currently this merely prepends the new tree into old, ideally it would insert the items in a sorted fashion | ||
174 | pub(crate) fn try_merge_trees( | 166 | pub(crate) fn try_merge_trees( |
175 | old: &ast::UseTree, | 167 | lhs: &ast::UseTree, |
176 | new: &ast::UseTree, | 168 | rhs: &ast::UseTree, |
177 | merge_behaviour: MergeBehaviour, | 169 | merge: MergeBehaviour, |
178 | ) -> Option<ast::UseTree> { | 170 | ) -> Option<ast::UseTree> { |
179 | let lhs_path = old.path()?; | 171 | let lhs_path = lhs.path()?; |
180 | let rhs_path = new.path()?; | 172 | let rhs_path = rhs.path()?; |
181 | 173 | ||
182 | let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?; | 174 | let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?; |
183 | let lhs = old.split_prefix(&lhs_prefix); | 175 | let lhs = lhs.split_prefix(&lhs_prefix); |
184 | let rhs = new.split_prefix(&rhs_prefix); | 176 | let rhs = rhs.split_prefix(&rhs_prefix); |
185 | let lhs_tl = lhs.use_tree_list()?; | 177 | recursive_merge(&lhs, &rhs, merge).map(|(merged, _)| merged) |
186 | let rhs_tl = rhs.use_tree_list()?; | 178 | } |
187 | |||
188 | // if we are only allowed to merge the last level check if the split off paths are only one level deep | ||
189 | if merge_behaviour == MergeBehaviour::Last | ||
190 | && (use_tree_list_is_nested(&lhs_tl) || use_tree_list_is_nested(&rhs_tl)) | ||
191 | { | ||
192 | mark::hit!(test_last_merge_too_long); | ||
193 | return None; | ||
194 | } | ||
195 | 179 | ||
196 | let should_insert_comma = lhs_tl | 180 | /// Recursively "zips" together lhs and rhs. |
197 | .r_curly_token() | 181 | fn recursive_merge( |
198 | .and_then(|it| skip_trivia_token(it.prev_token()?, Direction::Prev)) | 182 | lhs: &ast::UseTree, |
199 | .map(|it| it.kind()) | 183 | rhs: &ast::UseTree, |
200 | != Some(T![,]); | 184 | merge: MergeBehaviour, |
201 | let mut to_insert: Vec<SyntaxElement> = Vec::new(); | 185 | ) -> Option<(ast::UseTree, bool)> { |
202 | if should_insert_comma { | 186 | let mut use_trees = lhs |
203 | to_insert.push(make::token(T![,]).into()); | 187 | .use_tree_list() |
204 | to_insert.push(make::tokens::single_space().into()); | 188 | .into_iter() |
205 | } | 189 | .flat_map(|list| list.use_trees()) |
206 | to_insert.extend( | 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 |
207 | rhs_tl.syntax().children_with_tokens().filter(|it| !matches!(it.kind(), T!['{'] | T!['}'])), | 191 | // so early exit the iterator by using Option's Intoiterator impl |
208 | ); | 192 | .map(|tree| match merge == MergeBehaviour::Last && tree.use_tree_list().is_some() { |
209 | let pos = InsertPosition::Before(lhs_tl.r_curly_token()?.into()); | 193 | true => None, |
210 | let use_tree_list = lhs_tl.insert_children(pos, to_insert); | 194 | false => Some(tree), |
211 | Some(lhs.with_use_tree_list(use_tree_list)) | 195 | }) |
196 | .collect::<Option<Vec<_>>>()?; | ||
197 | 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()) { | ||
199 | let rhs_path = rhs_t.path(); | ||
200 | match use_trees.binary_search_by(|p| path_cmp_opt(p.path(), rhs_path.clone())) { | ||
201 | Ok(idx) => { | ||
202 | let lhs_t = &mut use_trees[idx]; | ||
203 | let lhs_path = lhs_t.path()?; | ||
204 | let rhs_path = rhs_path?; | ||
205 | let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?; | ||
206 | if lhs_prefix == lhs_path && rhs_prefix == rhs_path { | ||
207 | let tree_is_self = |tree: ast::UseTree| { | ||
208 | tree.path().as_ref().map(path_is_self).unwrap_or(false) | ||
209 | }; | ||
210 | // check if only one of the two trees has a tree list, and whether that then contains `self` or not. | ||
211 | // If this is the case we can skip this iteration since the path without the list is already included in the other one via `self` | ||
212 | let tree_contains_self = |tree: &ast::UseTree| { | ||
213 | tree.use_tree_list() | ||
214 | .map(|tree_list| tree_list.use_trees().any(tree_is_self)) | ||
215 | .unwrap_or(false) | ||
216 | }; | ||
217 | match (tree_contains_self(&lhs_t), tree_contains_self(&rhs_t)) { | ||
218 | (true, false) => continue, | ||
219 | (false, true) => { | ||
220 | *lhs_t = rhs_t; | ||
221 | continue; | ||
222 | } | ||
223 | _ => (), | ||
224 | } | ||
225 | |||
226 | // glob imports arent part of the use-tree lists so we need to special handle them here as well | ||
227 | // this special handling is only required for when we merge a module import into a glob import of said module | ||
228 | // see the `merge_self_glob` or `merge_mod_into_glob` tests | ||
229 | if lhs_t.star_token().is_some() || rhs_t.star_token().is_some() { | ||
230 | *lhs_t = make::use_tree( | ||
231 | make::path_unqualified(make::path_segment_self()), | ||
232 | None, | ||
233 | None, | ||
234 | false, | ||
235 | ); | ||
236 | use_trees.insert(idx, make::glob_use_tree()); | ||
237 | continue; | ||
238 | } | ||
239 | } | ||
240 | let lhs = lhs_t.split_prefix(&lhs_prefix); | ||
241 | 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 | Some((_, has_multiple_children)) | ||
245 | if merge == MergeBehaviour::Last | ||
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 | } | ||
254 | } | ||
255 | Err(_) | ||
256 | if merge == MergeBehaviour::Last | ||
257 | && use_trees.len() > 0 | ||
258 | && rhs_t.use_tree_list().is_some() => | ||
259 | { | ||
260 | return None | ||
261 | } | ||
262 | Err(idx) => { | ||
263 | use_trees.insert(idx, rhs_t); | ||
264 | } | ||
265 | } | ||
266 | } | ||
267 | let has_multiple_children = use_trees.len() > 1; | ||
268 | Some((lhs.with_use_tree_list(make::use_tree_list(use_trees)), has_multiple_children)) | ||
212 | } | 269 | } |
213 | 270 | ||
214 | /// Traverses both paths until they differ, returning the common prefix of both. | 271 | /// Traverses both paths until they differ, returning the common prefix of both. |
@@ -219,7 +276,7 @@ fn common_prefix(lhs: &ast::Path, rhs: &ast::Path) -> Option<(ast::Path, ast::Pa | |||
219 | loop { | 276 | loop { |
220 | match (lhs_curr.segment(), rhs_curr.segment()) { | 277 | match (lhs_curr.segment(), rhs_curr.segment()) { |
221 | (Some(lhs), Some(rhs)) if lhs.syntax().text() == rhs.syntax().text() => (), | 278 | (Some(lhs), Some(rhs)) if lhs.syntax().text() == rhs.syntax().text() => (), |
222 | _ => break, | 279 | _ => break res, |
223 | } | 280 | } |
224 | res = Some((lhs_curr.clone(), rhs_curr.clone())); | 281 | res = Some((lhs_curr.clone(), rhs_curr.clone())); |
225 | 282 | ||
@@ -228,11 +285,62 @@ fn common_prefix(lhs: &ast::Path, rhs: &ast::Path) -> Option<(ast::Path, ast::Pa | |||
228 | lhs_curr = lhs; | 285 | lhs_curr = lhs; |
229 | rhs_curr = rhs; | 286 | rhs_curr = rhs; |
230 | } | 287 | } |
231 | _ => break, | 288 | _ => break res, |
232 | } | 289 | } |
233 | } | 290 | } |
291 | } | ||
234 | 292 | ||
235 | res | 293 | fn path_is_self(path: &ast::Path) -> bool { |
294 | path.segment().and_then(|seg| seg.self_token()).is_some() && path.qualifier().is_none() | ||
295 | } | ||
296 | |||
297 | #[inline] | ||
298 | fn first_segment(path: &ast::Path) -> Option<ast::PathSegment> { | ||
299 | first_path(path).segment() | ||
300 | } | ||
301 | |||
302 | fn first_path(path: &ast::Path) -> ast::Path { | ||
303 | successors(Some(path.clone()), ast::Path::qualifier).last().unwrap() | ||
304 | } | ||
305 | |||
306 | fn segment_iter(path: &ast::Path) -> impl Iterator<Item = ast::PathSegment> + Clone { | ||
307 | // cant make use of SyntaxNode::siblings, because the returned Iterator is not clone | ||
308 | successors(first_segment(path), |p| p.parent_path().parent_path().and_then(|p| p.segment())) | ||
309 | } | ||
310 | |||
311 | /// Orders paths in the following way: | ||
312 | /// 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 | ||
314 | // which is `self` and `super` first, then identifier imports with lowercase ones first, then glob imports and at last list imports. | ||
315 | // Example foo::{self, foo, baz, Baz, Qux, *, {Bar}} | ||
316 | fn path_cmp(a: &ast::Path, b: &ast::Path) -> Ordering { | ||
317 | match (path_is_self(a), path_is_self(b)) { | ||
318 | (true, true) => Ordering::Equal, | ||
319 | (true, false) => Ordering::Less, | ||
320 | (false, true) => Ordering::Greater, | ||
321 | (false, false) => { | ||
322 | let a = segment_iter(a); | ||
323 | let b = segment_iter(b); | ||
324 | // cmp_by would be useful for us here but that is currently unstable | ||
325 | // cmp doesnt work due the lifetimes on text's return type | ||
326 | a.zip(b) | ||
327 | .flat_map(|(seg, seg2)| seg.name_ref().zip(seg2.name_ref())) | ||
328 | .find_map(|(a, b)| match a.text().cmp(b.text()) { | ||
329 | ord @ Ordering::Greater | ord @ Ordering::Less => Some(ord), | ||
330 | Ordering::Equal => None, | ||
331 | }) | ||
332 | .unwrap_or(Ordering::Equal) | ||
333 | } | ||
334 | } | ||
335 | } | ||
336 | |||
337 | fn path_cmp_opt(a: Option<ast::Path>, b: Option<ast::Path>) -> Ordering { | ||
338 | match (a, b) { | ||
339 | (None, None) => Ordering::Equal, | ||
340 | (None, Some(_)) => Ordering::Less, | ||
341 | (Some(_), None) => Ordering::Greater, | ||
342 | (Some(a), Some(b)) => path_cmp(&a, &b), | ||
343 | } | ||
236 | } | 344 | } |
237 | 345 | ||
238 | /// What type of merges are allowed. | 346 | /// What type of merges are allowed. |
@@ -279,19 +387,6 @@ impl ImportGroup { | |||
279 | } | 387 | } |
280 | } | 388 | } |
281 | 389 | ||
282 | fn first_segment(path: &ast::Path) -> Option<ast::PathSegment> { | ||
283 | first_path(path).segment() | ||
284 | } | ||
285 | |||
286 | fn first_path(path: &ast::Path) -> ast::Path { | ||
287 | successors(Some(path.clone()), ast::Path::qualifier).last().unwrap() | ||
288 | } | ||
289 | |||
290 | fn segment_iter(path: &ast::Path) -> impl Iterator<Item = ast::PathSegment> + Clone { | ||
291 | // cant make use of SyntaxNode::siblings, because the returned Iterator is not clone | ||
292 | successors(first_segment(path), |p| p.parent_path().parent_path().and_then(|p| p.segment())) | ||
293 | } | ||
294 | |||
295 | #[derive(PartialEq, Eq)] | 390 | #[derive(PartialEq, Eq)] |
296 | enum AddBlankLine { | 391 | enum AddBlankLine { |
297 | Before, | 392 | Before, |
@@ -594,7 +689,7 @@ use std::io;", | |||
594 | check_full( | 689 | check_full( |
595 | "std::foo::bar::Baz", | 690 | "std::foo::bar::Baz", |
596 | r"use std::foo::bar::Qux;", | 691 | r"use std::foo::bar::Qux;", |
597 | r"use std::foo::bar::{Qux, Baz};", | 692 | r"use std::foo::bar::{Baz, Qux};", |
598 | ) | 693 | ) |
599 | } | 694 | } |
600 | 695 | ||
@@ -603,7 +698,7 @@ use std::io;", | |||
603 | check_last( | 698 | check_last( |
604 | "std::foo::bar::Baz", | 699 | "std::foo::bar::Baz", |
605 | r"use std::foo::bar::Qux;", | 700 | r"use std::foo::bar::Qux;", |
606 | r"use std::foo::bar::{Qux, Baz};", | 701 | r"use std::foo::bar::{Baz, Qux};", |
607 | ) | 702 | ) |
608 | } | 703 | } |
609 | 704 | ||
@@ -612,7 +707,7 @@ use std::io;", | |||
612 | check_full( | 707 | check_full( |
613 | "std::foo::bar::Baz", | 708 | "std::foo::bar::Baz", |
614 | r"use std::foo::bar::{Qux, Quux};", | 709 | r"use std::foo::bar::{Qux, Quux};", |
615 | r"use std::foo::bar::{Qux, Quux, Baz};", | 710 | r"use std::foo::bar::{Baz, Quux, Qux};", |
616 | ) | 711 | ) |
617 | } | 712 | } |
618 | 713 | ||
@@ -621,7 +716,7 @@ use std::io;", | |||
621 | check_last( | 716 | check_last( |
622 | "std::foo::bar::Baz", | 717 | "std::foo::bar::Baz", |
623 | r"use std::foo::bar::{Qux, Quux};", | 718 | r"use std::foo::bar::{Qux, Quux};", |
624 | r"use std::foo::bar::{Qux, Quux, Baz};", | 719 | r"use std::foo::bar::{Baz, Quux, Qux};", |
625 | ) | 720 | ) |
626 | } | 721 | } |
627 | 722 | ||
@@ -630,7 +725,7 @@ use std::io;", | |||
630 | check_full( | 725 | check_full( |
631 | "std::foo::bar::Baz", | 726 | "std::foo::bar::Baz", |
632 | r"use std::foo::bar::{Qux, quux::{Fez, Fizz}};", | 727 | r"use std::foo::bar::{Qux, quux::{Fez, Fizz}};", |
633 | r"use std::foo::bar::{Qux, quux::{Fez, Fizz}, Baz};", | 728 | r"use std::foo::bar::{Baz, Qux, quux::{Fez, Fizz}};", |
634 | ) | 729 | ) |
635 | } | 730 | } |
636 | 731 | ||
@@ -645,6 +740,15 @@ use std::foo::bar::{Qux, quux::{Fez, Fizz}};", | |||
645 | } | 740 | } |
646 | 741 | ||
647 | #[test] | 742 | #[test] |
743 | fn merge_groups_full_nested_deep() { | ||
744 | check_full( | ||
745 | "std::foo::bar::quux::Baz", | ||
746 | r"use std::foo::bar::{Qux, quux::{Fez, Fizz}};", | ||
747 | r"use std::foo::bar::{Qux, quux::{Baz, Fez, Fizz}};", | ||
748 | ) | ||
749 | } | ||
750 | |||
751 | #[test] | ||
648 | fn merge_groups_skip_pub() { | 752 | fn merge_groups_skip_pub() { |
649 | check_full( | 753 | check_full( |
650 | "std::io", | 754 | "std::io", |
@@ -670,34 +774,63 @@ use std::io;", | |||
670 | check_last( | 774 | check_last( |
671 | "std::fmt::Result", | 775 | "std::fmt::Result", |
672 | r"use std::{fmt, io};", | 776 | r"use std::{fmt, io};", |
673 | r"use std::{self, fmt::Result}; | 777 | r"use std::fmt::{self, Result}; |
674 | use std::io;", | 778 | use std::io;", |
675 | ) | 779 | ) |
676 | } | 780 | } |
677 | 781 | ||
678 | #[test] | 782 | #[test] |
783 | fn merge_into_module_import() { | ||
784 | check_full( | ||
785 | "std::fmt::Result", | ||
786 | r"use std::{fmt, io};", | ||
787 | r"use std::{fmt::{self, Result}, io};", | ||
788 | ) | ||
789 | } | ||
790 | |||
791 | #[test] | ||
679 | fn merge_groups_self() { | 792 | fn merge_groups_self() { |
680 | check_full("std::fmt::Debug", r"use std::fmt;", r"use std::fmt::{self, Debug};") | 793 | check_full("std::fmt::Debug", r"use std::fmt;", r"use std::fmt::{self, Debug};") |
681 | } | 794 | } |
682 | 795 | ||
683 | #[test] | 796 | #[test] |
684 | fn merge_self_glob() { | 797 | fn merge_mod_into_glob() { |
685 | check_full( | 798 | check_full( |
686 | "token::TokenKind", | 799 | "token::TokenKind", |
687 | r"use token::TokenKind::*;", | 800 | r"use token::TokenKind::*;", |
688 | r"use token::TokenKind::{self::*, self};", | 801 | r"use token::TokenKind::{*, self};", |
802 | ) | ||
803 | // FIXME: have it emit `use token::TokenKind::{self, *}`? | ||
804 | } | ||
805 | |||
806 | #[test] | ||
807 | fn merge_self_glob() { | ||
808 | check_full("self", r"use self::*;", r"use self::{*, self};") | ||
809 | // FIXME: have it emit `use {self, *}`? | ||
810 | } | ||
811 | |||
812 | #[test] | ||
813 | #[ignore] // FIXME: Support this | ||
814 | fn merge_partial_path() { | ||
815 | check_full( | ||
816 | "ast::Foo", | ||
817 | r"use syntax::{ast, algo};", | ||
818 | r"use syntax::{ast::{self, Foo}, algo};", | ||
819 | ) | ||
820 | } | ||
821 | |||
822 | #[test] | ||
823 | fn merge_glob_nested() { | ||
824 | check_full( | ||
825 | "foo::bar::quux::Fez", | ||
826 | r"use foo::bar::{Baz, quux::*};", | ||
827 | r"use foo::bar::{Baz, quux::{self::*, Fez}};", | ||
689 | ) | 828 | ) |
690 | } | 829 | } |
691 | 830 | ||
692 | #[test] | 831 | #[test] |
693 | fn merge_last_too_long() { | 832 | fn merge_last_too_long() { |
694 | mark::check!(test_last_merge_too_long); | 833 | check_last("foo::bar", r"use foo::bar::baz::Qux;", r"use foo::bar::{self, baz::Qux};"); |
695 | check_last( | ||
696 | "foo::bar", | ||
697 | r"use foo::bar::baz::Qux;", | ||
698 | r"use foo::bar; | ||
699 | use foo::bar::baz::Qux;", | ||
700 | ); | ||
701 | } | 834 | } |
702 | 835 | ||
703 | #[test] | 836 | #[test] |
@@ -710,6 +843,42 @@ use foo::bar::baz::Qux;", | |||
710 | ); | 843 | ); |
711 | } | 844 | } |
712 | 845 | ||
846 | #[test] | ||
847 | fn merge_last_fail() { | ||
848 | check_merge_only_fail( | ||
849 | r"use foo::bar::{baz::{Qux, Fez}};", | ||
850 | r"use foo::bar::{baaz::{Quux, Feez}};", | ||
851 | MergeBehaviour::Last, | ||
852 | ); | ||
853 | } | ||
854 | |||
855 | #[test] | ||
856 | fn merge_last_fail1() { | ||
857 | check_merge_only_fail( | ||
858 | r"use foo::bar::{baz::{Qux, Fez}};", | ||
859 | r"use foo::bar::baaz::{Quux, Feez};", | ||
860 | MergeBehaviour::Last, | ||
861 | ); | ||
862 | } | ||
863 | |||
864 | #[test] | ||
865 | fn merge_last_fail2() { | ||
866 | check_merge_only_fail( | ||
867 | r"use foo::bar::baz::{Qux, Fez};", | ||
868 | r"use foo::bar::{baaz::{Quux, Feez}};", | ||
869 | MergeBehaviour::Last, | ||
870 | ); | ||
871 | } | ||
872 | |||
873 | #[test] | ||
874 | fn merge_last_fail3() { | ||
875 | check_merge_only_fail( | ||
876 | r"use foo::bar::baz::{Qux, Fez};", | ||
877 | r"use foo::bar::baaz::{Quux, Feez};", | ||
878 | MergeBehaviour::Last, | ||
879 | ); | ||
880 | } | ||
881 | |||
713 | fn check( | 882 | fn check( |
714 | path: &str, | 883 | path: &str, |
715 | ra_fixture_before: &str, | 884 | ra_fixture_before: &str, |
@@ -742,4 +911,23 @@ use foo::bar::baz::Qux;", | |||
742 | fn check_none(path: &str, ra_fixture_before: &str, ra_fixture_after: &str) { | 911 | fn check_none(path: &str, ra_fixture_before: &str, ra_fixture_after: &str) { |
743 | check(path, ra_fixture_before, ra_fixture_after, None) | 912 | check(path, ra_fixture_before, ra_fixture_after, None) |
744 | } | 913 | } |
914 | |||
915 | fn check_merge_only_fail(ra_fixture0: &str, ra_fixture1: &str, mb: MergeBehaviour) { | ||
916 | let use0 = ast::SourceFile::parse(ra_fixture0) | ||
917 | .tree() | ||
918 | .syntax() | ||
919 | .descendants() | ||
920 | .find_map(ast::Use::cast) | ||
921 | .unwrap(); | ||
922 | |||
923 | let use1 = ast::SourceFile::parse(ra_fixture1) | ||
924 | .tree() | ||
925 | .syntax() | ||
926 | .descendants() | ||
927 | .find_map(ast::Use::cast) | ||
928 | .unwrap(); | ||
929 | |||
930 | let result = try_merge_imports(&use0, &use1, mb); | ||
931 | assert_eq!(result.map(|u| u.to_string()), None); | ||
932 | } | ||
745 | } | 933 | } |
diff --git a/crates/syntax/src/ast/edit.rs b/crates/syntax/src/ast/edit.rs index 8b1c65dd6..45cf31f13 100644 --- a/crates/syntax/src/ast/edit.rs +++ b/crates/syntax/src/ast/edit.rs | |||
@@ -347,6 +347,7 @@ impl ast::UseTree { | |||
347 | self.clone() | 347 | self.clone() |
348 | } | 348 | } |
349 | 349 | ||
350 | /// Splits off the given prefix, making it the path component of the use tree, appending the rest of the path to all UseTreeList items. | ||
350 | #[must_use] | 351 | #[must_use] |
351 | pub fn split_prefix(&self, prefix: &ast::Path) -> ast::UseTree { | 352 | pub fn split_prefix(&self, prefix: &ast::Path) -> ast::UseTree { |
352 | let suffix = if self.path().as_ref() == Some(prefix) && self.use_tree_list().is_none() { | 353 | let suffix = if self.path().as_ref() == Some(prefix) && self.use_tree_list().is_none() { |
diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 25e8a359d..6868feed9 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs | |||
@@ -38,6 +38,10 @@ pub fn path_from_text(text: &str) -> ast::Path { | |||
38 | ast_from_text(text) | 38 | ast_from_text(text) |
39 | } | 39 | } |
40 | 40 | ||
41 | pub fn glob_use_tree() -> ast::UseTree { | ||
42 | ast_from_text("use *;") | ||
43 | } | ||
44 | |||
41 | pub fn use_tree( | 45 | pub fn use_tree( |
42 | path: ast::Path, | 46 | path: ast::Path, |
43 | use_tree_list: Option<ast::UseTreeList>, | 47 | use_tree_list: Option<ast::UseTreeList>, |