Skip to content

Commit 4a6b16b

Browse files
committed
order use trees following rustfmt's algorithm for ordering imports
1 parent 9d8889c commit 4a6b16b

File tree

2 files changed

+141
-77
lines changed

2 files changed

+141
-77
lines changed

crates/ide-db/src/imports/insert_use.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use syntax::{
1616

1717
use crate::{
1818
imports::merge_imports::{
19-
common_prefix, eq_attrs, eq_visibility, try_merge_imports, use_tree_path_cmp, MergeBehavior,
19+
common_prefix, eq_attrs, eq_visibility, try_merge_imports, use_tree_cmp, MergeBehavior,
2020
},
2121
RootDatabase,
2222
};
@@ -357,15 +357,16 @@ fn insert_use_(
357357
use_item: ast::Use,
358358
) {
359359
let scope_syntax = scope.as_syntax_node();
360+
let insert_use_tree =
361+
use_item.use_tree().expect("`use_item` should have a use tree for `insert_path`");
360362
let group = ImportGroup::new(insert_path);
361363
let path_node_iter = scope_syntax
362364
.children()
363365
.filter_map(|node| ast::Use::cast(node.clone()).zip(Some(node)))
364366
.flat_map(|(use_, node)| {
365367
let tree = use_.use_tree()?;
366368
let path = tree.path()?;
367-
let has_tl = tree.use_tree_list().is_some();
368-
Some((path, has_tl, node))
369+
Some((path, tree, node))
369370
});
370371

371372
if group_imports {
@@ -381,8 +382,8 @@ fn insert_use_(
381382
// find the element that would come directly after our new import
382383
let post_insert: Option<(_, _, SyntaxNode)> = group_iter
383384
.inspect(|(.., node)| last = Some(node.clone()))
384-
.find(|&(ref path, has_tl, _)| {
385-
use_tree_path_cmp(insert_path, false, path, has_tl) != Ordering::Greater
385+
.find(|&(_, ref use_tree, _)| {
386+
use_tree_cmp(&insert_use_tree, use_tree) != Ordering::Greater
386387
});
387388

388389
if let Some((.., node)) = post_insert {

crates/ide-db/src/imports/merge_imports.rs

Lines changed: 135 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ use std::cmp::Ordering;
33

44
use itertools::{EitherOrBoth, Itertools};
55
use syntax::{
6-
ast::{self, AstNode, HasAttrs, HasVisibility, PathSegmentKind},
7-
ted,
6+
ast::{self, AstNode, HasAttrs, HasName, HasVisibility, PathSegmentKind},
7+
ted, TokenText,
88
};
99

1010
use crate::syntax_helpers::node_ext::vis_eq;
@@ -97,7 +97,7 @@ fn recursive_merge(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehavior)
9797
// same as a `filter` op).
9898
.map(|tree| merge.is_tree_allowed(&tree).then_some(tree))
9999
.collect::<Option<_>>()?;
100-
use_trees.sort_unstable_by(|a, b| path_cmp_for_sort(a.path(), b.path()));
100+
use_trees.sort_unstable_by(|a, b| use_tree_cmp(a, b));
101101
for rhs_t in rhs.use_tree_list().into_iter().flat_map(|list| list.use_trees()) {
102102
if !merge.is_tree_allowed(&rhs_t) {
103103
return None;
@@ -190,25 +190,6 @@ pub fn common_prefix(lhs: &ast::Path, rhs: &ast::Path) -> Option<(ast::Path, ast
190190
}
191191
}
192192

193-
/// Orders paths in the following way:
194-
/// the sole self token comes first, after that come uppercase identifiers, then lowercase identifiers
195-
// FIXME: rustfmt sorts lowercase idents before uppercase, in general we want to have the same ordering rustfmt has
196-
// which is `self` and `super` first, then identifier imports with lowercase ones first, then glob imports and at last list imports.
197-
// Example foo::{self, foo, baz, Baz, Qux, *, {Bar}}
198-
fn path_cmp_for_sort(a: Option<ast::Path>, b: Option<ast::Path>) -> Ordering {
199-
match (a, b) {
200-
(None, None) => Ordering::Equal,
201-
(None, Some(_)) => Ordering::Less,
202-
(Some(_), None) => Ordering::Greater,
203-
(Some(ref a), Some(ref b)) => match (path_is_self(a), path_is_self(b)) {
204-
(true, true) => Ordering::Equal,
205-
(true, false) => Ordering::Less,
206-
(false, true) => Ordering::Greater,
207-
(false, false) => path_cmp_short(a, b),
208-
},
209-
}
210-
}
211-
212193
/// Path comparison func for binary searching for merging.
213194
fn path_cmp_bin_search(lhs: Option<ast::Path>, rhs: Option<&ast::Path>) -> Ordering {
214195
match (lhs.as_ref().and_then(ast::Path::first_segment), rhs.and_then(ast::Path::first_segment))
@@ -220,59 +201,141 @@ fn path_cmp_bin_search(lhs: Option<ast::Path>, rhs: Option<&ast::Path>) -> Order
220201
}
221202
}
222203

223-
/// Short circuiting comparison, if both paths are equal until one of them ends they are considered
224-
/// equal
225-
fn path_cmp_short(a: &ast::Path, b: &ast::Path) -> Ordering {
226-
let a = a.segments();
227-
let b = b.segments();
228-
// cmp_by would be useful for us here but that is currently unstable
229-
// cmp doesn't work due the lifetimes on text's return type
230-
a.zip(b)
231-
.find_map(|(a, b)| match path_segment_cmp(&a, &b) {
232-
Ordering::Equal => None,
233-
ord => Some(ord),
234-
})
235-
.unwrap_or(Ordering::Equal)
236-
}
237-
238-
/// Compares two paths, if one ends earlier than the other the has_tl parameters decide which is
239-
/// greater as a path that has a tree list should be greater, while one that just ends without
240-
/// a tree list should be considered less.
241-
pub(super) fn use_tree_path_cmp(
242-
a: &ast::Path,
243-
a_has_tl: bool,
244-
b: &ast::Path,
245-
b_has_tl: bool,
246-
) -> Ordering {
247-
let a_segments = a.segments();
248-
let b_segments = b.segments();
249-
// cmp_by would be useful for us here but that is currently unstable
250-
// cmp doesn't work due the lifetimes on text's return type
251-
a_segments
252-
.zip_longest(b_segments)
253-
.find_map(|zipped| match zipped {
254-
EitherOrBoth::Both(ref a, ref b) => match path_segment_cmp(a, b) {
255-
Ordering::Equal => None,
256-
ord => Some(ord),
257-
},
258-
EitherOrBoth::Left(_) if !b_has_tl => Some(Ordering::Greater),
259-
EitherOrBoth::Left(_) => Some(Ordering::Less),
260-
EitherOrBoth::Right(_) if !a_has_tl => Some(Ordering::Less),
261-
EitherOrBoth::Right(_) => Some(Ordering::Greater),
262-
})
263-
.unwrap_or(Ordering::Equal)
204+
/// Orders use trees following `rustfmt`'s algorithm for ordering imports, which is `self`, `super` and `crate` first,
205+
/// then identifier imports with lowercase ones first and upper snake case (e.g. UPPER_SNAKE_CASE) ones last,
206+
/// then glob imports, and at last list imports.
207+
///
208+
/// Example foo::{self, foo, baz, Baz, Qux, FOO_BAZ, *, {Bar}}
209+
/// Ref: <https://github.com/rust-lang/rustfmt/blob/6356fca675bd756d71f5c123cd053d17b16c573e/src/imports.rs#L83-L86>.
210+
pub(super) fn use_tree_cmp(a: &ast::UseTree, b: &ast::UseTree) -> Ordering {
211+
let tiebreak_by_glob_or_alias = || match (a.star_token().is_some(), b.star_token().is_some()) {
212+
(true, false) => Ordering::Greater,
213+
(false, true) => Ordering::Less,
214+
_ => match (a.rename(), b.rename()) {
215+
(None, None) => Ordering::Equal,
216+
(Some(_), None) => Ordering::Greater,
217+
(None, Some(_)) => Ordering::Less,
218+
(Some(a_rename), Some(b_rename)) => a_rename
219+
.name()
220+
.as_ref()
221+
.map(ast::Name::text)
222+
.as_ref()
223+
.map_or("_", TokenText::as_str)
224+
.cmp(
225+
b_rename
226+
.name()
227+
.as_ref()
228+
.map(ast::Name::text)
229+
.as_ref()
230+
.map_or("_", TokenText::as_str),
231+
),
232+
},
233+
};
234+
let tiebreak_by_tree_list_glob_or_alias = || match (a.use_tree_list(), b.use_tree_list()) {
235+
(None, None) => tiebreak_by_glob_or_alias(),
236+
(Some(_), None) => Ordering::Greater,
237+
(None, Some(_)) => Ordering::Less,
238+
(Some(a_list), Some(b_list)) => a_list
239+
.use_trees()
240+
.zip_longest(b_list.use_trees())
241+
.find_map(|zipped| match zipped {
242+
EitherOrBoth::Both(ref a_tree, ref b_tree) => match use_tree_cmp(a_tree, b_tree) {
243+
Ordering::Equal => None,
244+
ord => Some(ord),
245+
},
246+
EitherOrBoth::Left(_) => Some(Ordering::Greater),
247+
EitherOrBoth::Right(_) => Some(Ordering::Less),
248+
})
249+
.unwrap_or_else(tiebreak_by_glob_or_alias),
250+
};
251+
match (a.path(), b.path()) {
252+
(None, None) => match (a.is_simple_path(), b.is_simple_path()) {
253+
(true, true) => Ordering::Equal,
254+
(true, false) => Ordering::Less,
255+
(false, true) => Ordering::Greater,
256+
(false, false) => tiebreak_by_tree_list_glob_or_alias(),
257+
},
258+
(Some(_), None) if !b.is_simple_path() => Ordering::Less,
259+
(Some(_), None) => Ordering::Greater,
260+
(None, Some(_)) if !a.is_simple_path() => Ordering::Greater,
261+
(None, Some(_)) => Ordering::Less,
262+
(Some(ref a_path), Some(ref b_path)) => {
263+
// cmp_by would be useful for us here but that is currently unstable
264+
// cmp doesn't work due the lifetimes on text's return type
265+
a_path
266+
.segments()
267+
.zip_longest(b_path.segments())
268+
.find_map(|zipped| match zipped {
269+
EitherOrBoth::Both(ref a_segment, ref b_segment) => {
270+
match path_segment_cmp(a_segment, b_segment) {
271+
Ordering::Equal => None,
272+
ord => Some(ord),
273+
}
274+
}
275+
EitherOrBoth::Left(_) if b.is_simple_path() => Some(Ordering::Greater),
276+
EitherOrBoth::Left(_) => Some(Ordering::Less),
277+
EitherOrBoth::Right(_) if a.is_simple_path() => Some(Ordering::Less),
278+
EitherOrBoth::Right(_) => Some(Ordering::Greater),
279+
})
280+
.unwrap_or_else(tiebreak_by_tree_list_glob_or_alias)
281+
}
282+
}
264283
}
265284

266285
fn path_segment_cmp(a: &ast::PathSegment, b: &ast::PathSegment) -> Ordering {
267-
let a = a.kind().and_then(|kind| match kind {
268-
PathSegmentKind::Name(name_ref) => Some(name_ref),
269-
_ => None,
270-
});
271-
let b = b.kind().and_then(|kind| match kind {
272-
PathSegmentKind::Name(name_ref) => Some(name_ref),
273-
_ => None,
274-
});
275-
a.as_ref().map(ast::NameRef::text).cmp(&b.as_ref().map(ast::NameRef::text))
286+
match (a.kind(), b.kind()) {
287+
(None, None) => Ordering::Equal,
288+
(Some(_), None) => Ordering::Greater,
289+
(None, Some(_)) => Ordering::Less,
290+
// self
291+
(Some(PathSegmentKind::SelfKw), Some(PathSegmentKind::SelfKw)) => Ordering::Equal,
292+
(Some(PathSegmentKind::SelfKw), _) => Ordering::Less,
293+
(_, Some(PathSegmentKind::SelfKw)) => Ordering::Greater,
294+
// super
295+
(Some(PathSegmentKind::SuperKw), Some(PathSegmentKind::SuperKw)) => Ordering::Equal,
296+
(Some(PathSegmentKind::SuperKw), _) => Ordering::Less,
297+
(_, Some(PathSegmentKind::SuperKw)) => Ordering::Greater,
298+
// crate
299+
(Some(PathSegmentKind::CrateKw), Some(PathSegmentKind::CrateKw)) => Ordering::Equal,
300+
(Some(PathSegmentKind::CrateKw), _) => Ordering::Less,
301+
(_, Some(PathSegmentKind::CrateKw)) => Ordering::Greater,
302+
// identifiers (everything else is treated as an identifier).
303+
_ => {
304+
match (
305+
a.name_ref().as_ref().map(ast::NameRef::text),
306+
b.name_ref().as_ref().map(ast::NameRef::text),
307+
) {
308+
(None, None) => Ordering::Equal,
309+
(Some(_), None) => Ordering::Greater,
310+
(None, Some(_)) => Ordering::Less,
311+
(Some(a_name), Some(b_name)) => {
312+
// snake_case < CamelCase < UPPER_SNAKE_CASE
313+
let a_text = a_name.as_str();
314+
let b_text = b_name.as_str();
315+
fn is_upper_snake_case(s: &str) -> bool {
316+
s.chars().all(|c| c.is_uppercase() || c == '_' || c.is_numeric())
317+
}
318+
if a_text.starts_with(char::is_lowercase)
319+
&& b_text.starts_with(char::is_uppercase)
320+
{
321+
return Ordering::Less;
322+
}
323+
if a_text.starts_with(char::is_uppercase)
324+
&& b_text.starts_with(char::is_lowercase)
325+
{
326+
return Ordering::Greater;
327+
}
328+
if !is_upper_snake_case(a_text) && is_upper_snake_case(b_text) {
329+
return Ordering::Less;
330+
}
331+
if is_upper_snake_case(a_text) && !is_upper_snake_case(b_text) {
332+
return Ordering::Greater;
333+
}
334+
a_text.cmp(&b_text)
335+
}
336+
}
337+
}
338+
}
276339
}
277340

278341
pub fn eq_visibility(vis0: Option<ast::Visibility>, vis1: Option<ast::Visibility>) -> bool {

0 commit comments

Comments
 (0)