diff options
author | Lukas Wirth <[email protected]> | 2020-09-12 22:27:01 +0100 |
---|---|---|
committer | Lukas Wirth <[email protected]> | 2020-09-12 22:27:01 +0100 |
commit | cd6cd91bf345d47cbf22e00fc4cddced4ae67ae6 (patch) | |
tree | cd45394c56072ac51166a57c879b16568add07d5 /crates/assists/src/utils/insert_use.rs | |
parent | a898752881779a328462ad9f2db291073f2f134f (diff) |
Tidy up `recursive_merge` implementation
Diffstat (limited to 'crates/assists/src/utils/insert_use.rs')
-rw-r--r-- | crates/assists/src/utils/insert_use.rs | 120 |
1 files changed, 60 insertions, 60 deletions
diff --git a/crates/assists/src/utils/insert_use.rs b/crates/assists/src/utils/insert_use.rs index 4f5fd317a..99f0b5b75 100644 --- a/crates/assists/src/utils/insert_use.rs +++ b/crates/assists/src/utils/insert_use.rs | |||
@@ -120,7 +120,6 @@ pub(crate) fn insert_use( | |||
120 | } | 120 | } |
121 | 121 | ||
122 | 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 { |
123 | // FIXME: this alone doesnt properly re-align all cases | ||
124 | buf.push(make::tokens::whitespace(&" ".repeat(4 * ident_level)).into()); | 123 | buf.push(make::tokens::whitespace(&" ".repeat(4 * ident_level)).into()); |
125 | } | 124 | } |
126 | buf.push(use_item.syntax().clone().into()); | 125 | buf.push(use_item.syntax().clone().into()); |
@@ -164,44 +163,6 @@ pub(crate) fn try_merge_imports( | |||
164 | Some(old.with_use_tree(merged)) | 163 | Some(old.with_use_tree(merged)) |
165 | } | 164 | } |
166 | 165 | ||
167 | /// Orders paths in the following way: | ||
168 | /// the sole self token comes first, after that come uppercase identifiers, then lowercase identifiers | ||
169 | /// FIXME: rustfmt sort lowercase idents before uppercase | ||
170 | fn path_cmp(a: &ast::Path, b: &ast::Path) -> Ordering { | ||
171 | let a = segment_iter(a); | ||
172 | let b = segment_iter(b); | ||
173 | let mut a_clone = a.clone(); | ||
174 | let mut b_clone = b.clone(); | ||
175 | match ( | ||
176 | a_clone.next().and_then(|ps| ps.self_token()).is_some() && a_clone.next().is_none(), | ||
177 | b_clone.next().and_then(|ps| ps.self_token()).is_some() && b_clone.next().is_none(), | ||
178 | ) { | ||
179 | (true, true) => Ordering::Equal, | ||
180 | (true, false) => Ordering::Less, | ||
181 | (false, true) => Ordering::Greater, | ||
182 | (false, false) => { | ||
183 | // cmp_by would be useful for us here but that is currently unstable | ||
184 | // cmp doesnt work due the lifetimes on text's return type | ||
185 | a.zip(b) | ||
186 | .flat_map(|(seg, seg2)| seg.name_ref().zip(seg2.name_ref())) | ||
187 | .find_map(|(a, b)| match a.text().cmp(b.text()) { | ||
188 | ord @ Ordering::Greater | ord @ Ordering::Less => Some(ord), | ||
189 | Ordering::Equal => None, | ||
190 | }) | ||
191 | .unwrap_or(Ordering::Equal) | ||
192 | } | ||
193 | } | ||
194 | } | ||
195 | |||
196 | fn path_cmp_opt(a: Option<ast::Path>, b: Option<ast::Path>) -> Ordering { | ||
197 | match (a, b) { | ||
198 | (None, None) => Ordering::Equal, | ||
199 | (None, Some(_)) => Ordering::Less, | ||
200 | (Some(_), None) => Ordering::Greater, | ||
201 | (Some(a), Some(b)) => path_cmp(&a, &b), | ||
202 | } | ||
203 | } | ||
204 | |||
205 | pub(crate) fn try_merge_trees( | 166 | pub(crate) fn try_merge_trees( |
206 | old: &ast::UseTree, | 167 | old: &ast::UseTree, |
207 | new: &ast::UseTree, | 168 | new: &ast::UseTree, |
@@ -216,17 +177,18 @@ pub(crate) fn try_merge_trees( | |||
216 | recursive_merge(&lhs, &rhs, merge).map(|(merged, _)| merged) | 177 | recursive_merge(&lhs, &rhs, merge).map(|(merged, _)| merged) |
217 | } | 178 | } |
218 | 179 | ||
219 | /// Returns the merged tree and the number of children it has, which is required to check if the tree is allowed to be used for MergeBehaviour::Last | 180 | /// Recursively "zips" together lhs and rhs. |
220 | fn recursive_merge( | 181 | fn recursive_merge( |
221 | lhs: &ast::UseTree, | 182 | lhs: &ast::UseTree, |
222 | rhs: &ast::UseTree, | 183 | rhs: &ast::UseTree, |
223 | merge: MergeBehaviour, | 184 | merge: MergeBehaviour, |
224 | ) -> Option<(ast::UseTree, usize)> { | 185 | ) -> Option<(ast::UseTree, bool)> { |
225 | let mut use_trees = lhs | 186 | let mut use_trees = lhs |
226 | .use_tree_list() | 187 | .use_tree_list() |
227 | .into_iter() | 188 | .into_iter() |
228 | .flat_map(|list| list.use_trees()) | 189 | .flat_map(|list| list.use_trees()) |
229 | // check if any of the use trees are nested, if they are and the behaviour is last only we are not allowed to merge this | 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 |
191 | // so early exit the iterator by using Option's Intoiterator impl | ||
230 | .map(|tree| match merge == MergeBehaviour::Last && tree.use_tree_list().is_some() { | 192 | .map(|tree| match merge == MergeBehaviour::Last && tree.use_tree_list().is_some() { |
231 | true => None, | 193 | true => None, |
232 | false => Some(tree), | 194 | false => Some(tree), |
@@ -237,15 +199,17 @@ fn recursive_merge( | |||
237 | let rhs_path = rhs_t.path(); | 199 | let rhs_path = rhs_t.path(); |
238 | match use_trees.binary_search_by(|p| path_cmp_opt(p.path(), rhs_path.clone())) { | 200 | match use_trees.binary_search_by(|p| path_cmp_opt(p.path(), rhs_path.clone())) { |
239 | Ok(idx) => { | 201 | Ok(idx) => { |
240 | let lhs_path = use_trees[idx].path()?; | 202 | let lhs_t = &mut use_trees[idx]; |
203 | let lhs_path = lhs_t.path()?; | ||
241 | let rhs_path = rhs_path?; | 204 | let rhs_path = rhs_path?; |
242 | let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?; | 205 | let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?; |
243 | if lhs_prefix == lhs_path && rhs_prefix == rhs_path { | 206 | if lhs_prefix == lhs_path && rhs_prefix == rhs_path { |
244 | let tree_is_self = | 207 | let tree_is_self = |tree: ast::UseTree| { |
245 | |tree: ast::UseTree| tree.path().map(path_is_self).unwrap_or(false); | 208 | tree.path().as_ref().map(path_is_self).unwrap_or(false) |
209 | }; | ||
246 | // check if only one of the two trees has a tree list, and whether that then contains `self` or not. | 210 | // check if only one of the two trees has a tree list, and whether that then contains `self` or not. |
247 | // 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` | 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` |
248 | if use_trees[idx] | 212 | if lhs_t |
249 | .use_tree_list() | 213 | .use_tree_list() |
250 | .xor(rhs_t.use_tree_list()) | 214 | .xor(rhs_t.use_tree_list()) |
251 | .map(|tree_list| tree_list.use_trees().any(tree_is_self)) | 215 | .map(|tree_list| tree_list.use_trees().any(tree_is_self)) |
@@ -257,8 +221,8 @@ fn recursive_merge( | |||
257 | // glob imports arent part of the use-tree lists so we need to special handle them here as well | 221 | // glob imports arent part of the use-tree lists so we need to special handle them here as well |
258 | // this special handling is only required for when we merge a module import into a glob import of said module | 222 | // this special handling is only required for when we merge a module import into a glob import of said module |
259 | // see the `merge_self_glob` or `merge_mod_into_glob` tests | 223 | // see the `merge_self_glob` or `merge_mod_into_glob` tests |
260 | if use_trees[idx].star_token().is_some() || rhs_t.star_token().is_some() { | 224 | if lhs_t.star_token().is_some() || rhs_t.star_token().is_some() { |
261 | use_trees[idx] = make::use_tree( | 225 | *lhs_t = make::use_tree( |
262 | make::path_unqualified(make::path_segment_self()), | 226 | make::path_unqualified(make::path_segment_self()), |
263 | None, | 227 | None, |
264 | None, | 228 | None, |
@@ -276,11 +240,14 @@ fn recursive_merge( | |||
276 | continue; | 240 | continue; |
277 | } | 241 | } |
278 | } | 242 | } |
279 | let lhs = use_trees[idx].split_prefix(&lhs_prefix); | 243 | let lhs = lhs_t.split_prefix(&lhs_prefix); |
280 | let rhs = rhs_t.split_prefix(&rhs_prefix); | 244 | let rhs = rhs_t.split_prefix(&rhs_prefix); |
245 | let this_has_children = use_trees.len() > 0; | ||
281 | match recursive_merge(&lhs, &rhs, merge) { | 246 | match recursive_merge(&lhs, &rhs, merge) { |
282 | Some((_, count)) | 247 | Some((_, has_multiple_children)) |
283 | if merge == MergeBehaviour::Last && use_trees.len() > 1 && count > 1 => | 248 | if merge == MergeBehaviour::Last |
249 | && this_has_children | ||
250 | && has_multiple_children => | ||
284 | { | 251 | { |
285 | return None | 252 | return None |
286 | } | 253 | } |
@@ -293,9 +260,8 @@ fn recursive_merge( | |||
293 | } | 260 | } |
294 | } | 261 | } |
295 | } | 262 | } |
296 | let count = use_trees.len(); | 263 | let has_multiple_children = use_trees.len() > 1; |
297 | let tl = make::use_tree_list(use_trees); | 264 | Some((lhs.with_use_tree_list(make::use_tree_list(use_trees)), has_multiple_children)) |
298 | Some((lhs.with_use_tree_list(tl), count)) | ||
299 | } | 265 | } |
300 | 266 | ||
301 | /// Traverses both paths until they differ, returning the common prefix of both. | 267 | /// Traverses both paths until they differ, returning the common prefix of both. |
@@ -306,7 +272,7 @@ fn common_prefix(lhs: &ast::Path, rhs: &ast::Path) -> Option<(ast::Path, ast::Pa | |||
306 | loop { | 272 | loop { |
307 | match (lhs_curr.segment(), rhs_curr.segment()) { | 273 | match (lhs_curr.segment(), rhs_curr.segment()) { |
308 | (Some(lhs), Some(rhs)) if lhs.syntax().text() == rhs.syntax().text() => (), | 274 | (Some(lhs), Some(rhs)) if lhs.syntax().text() == rhs.syntax().text() => (), |
309 | _ => break, | 275 | _ => break res, |
310 | } | 276 | } |
311 | res = Some((lhs_curr.clone(), rhs_curr.clone())); | 277 | res = Some((lhs_curr.clone(), rhs_curr.clone())); |
312 | 278 | ||
@@ -315,17 +281,16 @@ fn common_prefix(lhs: &ast::Path, rhs: &ast::Path) -> Option<(ast::Path, ast::Pa | |||
315 | lhs_curr = lhs; | 281 | lhs_curr = lhs; |
316 | rhs_curr = rhs; | 282 | rhs_curr = rhs; |
317 | } | 283 | } |
318 | _ => break, | 284 | _ => break res, |
319 | } | 285 | } |
320 | } | 286 | } |
321 | |||
322 | res | ||
323 | } | 287 | } |
324 | 288 | ||
325 | fn path_is_self(path: ast::Path) -> bool { | 289 | fn path_is_self(path: &ast::Path) -> bool { |
326 | path.segment().and_then(|seg| seg.self_token()).is_some() && path.qualifier().is_none() | 290 | path.segment().and_then(|seg| seg.self_token()).is_some() && path.qualifier().is_none() |
327 | } | 291 | } |
328 | 292 | ||
293 | #[inline] | ||
329 | fn first_segment(path: &ast::Path) -> Option<ast::PathSegment> { | 294 | fn first_segment(path: &ast::Path) -> Option<ast::PathSegment> { |
330 | first_path(path).segment() | 295 | first_path(path).segment() |
331 | } | 296 | } |
@@ -339,6 +304,41 @@ fn segment_iter(path: &ast::Path) -> impl Iterator<Item = ast::PathSegment> + Cl | |||
339 | successors(first_segment(path), |p| p.parent_path().parent_path().and_then(|p| p.segment())) | 304 | successors(first_segment(path), |p| p.parent_path().parent_path().and_then(|p| p.segment())) |
340 | } | 305 | } |
341 | 306 | ||
307 | /// Orders paths in the following way: | ||
308 | /// the sole self token comes first, after that come uppercase identifiers, then lowercase identifiers | ||
309 | // FIXME: rustfmt sort lowercase idents before uppercase, in general we want to have the same ordering rustfmt has | ||
310 | // which is `self` and `super` first, then identifier imports with lowercase ones first, then glob imports and at last list imports. | ||
311 | // Example foo::{self, foo, baz, Baz, Qux, *, {Bar}} | ||
312 | fn path_cmp(a: &ast::Path, b: &ast::Path) -> Ordering { | ||
313 | match (path_is_self(a), path_is_self(b)) { | ||
314 | (true, true) => Ordering::Equal, | ||
315 | (true, false) => Ordering::Less, | ||
316 | (false, true) => Ordering::Greater, | ||
317 | (false, false) => { | ||
318 | let a = segment_iter(a); | ||
319 | let b = segment_iter(b); | ||
320 | // cmp_by would be useful for us here but that is currently unstable | ||
321 | // cmp doesnt work due the lifetimes on text's return type | ||
322 | a.zip(b) | ||
323 | .flat_map(|(seg, seg2)| seg.name_ref().zip(seg2.name_ref())) | ||
324 | .find_map(|(a, b)| match a.text().cmp(b.text()) { | ||
325 | ord @ Ordering::Greater | ord @ Ordering::Less => Some(ord), | ||
326 | Ordering::Equal => None, | ||
327 | }) | ||
328 | .unwrap_or(Ordering::Equal) | ||
329 | } | ||
330 | } | ||
331 | } | ||
332 | |||
333 | fn path_cmp_opt(a: Option<ast::Path>, b: Option<ast::Path>) -> Ordering { | ||
334 | match (a, b) { | ||
335 | (None, None) => Ordering::Equal, | ||
336 | (None, Some(_)) => Ordering::Less, | ||
337 | (Some(_), None) => Ordering::Greater, | ||
338 | (Some(a), Some(b)) => path_cmp(&a, &b), | ||
339 | } | ||
340 | } | ||
341 | |||
342 | /// What type of merges are allowed. | 342 | /// What type of merges are allowed. |
343 | #[derive(Copy, Clone, PartialEq, Eq)] | 343 | #[derive(Copy, Clone, PartialEq, Eq)] |
344 | pub enum MergeBehaviour { | 344 | pub enum MergeBehaviour { |
@@ -924,6 +924,6 @@ use foo::bar::baz::Qux;", | |||
924 | .unwrap(); | 924 | .unwrap(); |
925 | 925 | ||
926 | let result = try_merge_imports(&use0, &use1, mb); | 926 | let result = try_merge_imports(&use0, &use1, mb); |
927 | assert_eq!(result, None); | 927 | assert_eq!(result.map(|u| u.to_string()), None); |
928 | } | 928 | } |
929 | } | 929 | } |