diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2020-09-11 15:01:20 +0100 |
---|---|---|
committer | GitHub <[email protected]> | 2020-09-11 15:01:20 +0100 |
commit | 568dc38b7b6474e225533ae63718c24b0561f48d (patch) | |
tree | 8cacf0d45fa3d33e48a1f8126d54f29eadb168ee /crates/assists/src/utils | |
parent | 96e988fcc3be11acc89dc2c1957bc14e8f39c911 (diff) | |
parent | 74b755d23366bcfa5437df25b023f5a2451e1ea6 (diff) |
Merge #5955
5955: Remove merge import code duplication r=jonas-schievink a=Veykril
This removes the code duplication caused by #5935, this also allows the assist to merge imports that have equal visibility and prevents merges of unequal visibility. This PR also fixes an iteration mistake in the mentioned PR:
Turns out I made a mistake when writing the `segment_iter` function, I was assuming that the `children` of a path will just be the segments, which is obviously not the case. This also brings insertion order of shorter paths in line with how `rustfmt` orders them.
Co-authored-by: Lukas Wirth <[email protected]>
Diffstat (limited to 'crates/assists/src/utils')
-rw-r--r-- | crates/assists/src/utils/insert_use.rs | 35 |
1 files changed, 28 insertions, 7 deletions
diff --git a/crates/assists/src/utils/insert_use.rs b/crates/assists/src/utils/insert_use.rs index 8a4c8520d..98553b2e0 100644 --- a/crates/assists/src/utils/insert_use.rs +++ b/crates/assists/src/utils/insert_use.rs | |||
@@ -138,13 +138,23 @@ pub(crate) fn insert_use( | |||
138 | algo::insert_children(scope.as_syntax_node(), insert_position, to_insert) | 138 | algo::insert_children(scope.as_syntax_node(), insert_position, to_insert) |
139 | } | 139 | } |
140 | 140 | ||
141 | fn try_merge_imports( | 141 | fn eq_visibility(vis0: Option<ast::Visibility>, vis1: Option<ast::Visibility>) -> bool { |
142 | match (vis0, vis1) { | ||
143 | (None, None) => true, | ||
144 | // FIXME: Don't use the string representation to check for equality | ||
145 | // spaces inside of the node would break this comparison | ||
146 | (Some(vis0), Some(vis1)) => vis0.to_string() == vis1.to_string(), | ||
147 | _ => false, | ||
148 | } | ||
149 | } | ||
150 | |||
151 | pub(crate) fn try_merge_imports( | ||
142 | old: &ast::Use, | 152 | old: &ast::Use, |
143 | new: &ast::Use, | 153 | new: &ast::Use, |
144 | merge_behaviour: MergeBehaviour, | 154 | merge_behaviour: MergeBehaviour, |
145 | ) -> Option<ast::Use> { | 155 | ) -> Option<ast::Use> { |
146 | // don't merge into re-exports | 156 | // don't merge imports with different visibilities |
147 | if old.visibility().and_then(|vis| vis.pub_token()).is_some() { | 157 | if !eq_visibility(old.visibility(), new.visibility()) { |
148 | return None; | 158 | return None; |
149 | } | 159 | } |
150 | let old_tree = old.use_tree()?; | 160 | let old_tree = old.use_tree()?; |
@@ -161,7 +171,7 @@ fn use_tree_list_is_nested(tl: &ast::UseTreeList) -> bool { | |||
161 | } | 171 | } |
162 | 172 | ||
163 | // FIXME: currently this merely prepends the new tree into old, ideally it would insert the items in a sorted fashion | 173 | // FIXME: currently this merely prepends the new tree into old, ideally it would insert the items in a sorted fashion |
164 | pub fn try_merge_trees( | 174 | pub(crate) fn try_merge_trees( |
165 | old: &ast::UseTree, | 175 | old: &ast::UseTree, |
166 | new: &ast::UseTree, | 176 | new: &ast::UseTree, |
167 | merge_behaviour: MergeBehaviour, | 177 | merge_behaviour: MergeBehaviour, |
@@ -278,7 +288,8 @@ fn first_path(path: &ast::Path) -> ast::Path { | |||
278 | } | 288 | } |
279 | 289 | ||
280 | fn segment_iter(path: &ast::Path) -> impl Iterator<Item = ast::PathSegment> + Clone { | 290 | fn segment_iter(path: &ast::Path) -> impl Iterator<Item = ast::PathSegment> + Clone { |
281 | path.syntax().children().flat_map(ast::PathSegment::cast) | 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())) | ||
282 | } | 293 | } |
283 | 294 | ||
284 | #[derive(PartialEq, Eq)] | 295 | #[derive(PartialEq, Eq)] |
@@ -684,8 +695,18 @@ use std::io;", | |||
684 | check_last( | 695 | check_last( |
685 | "foo::bar", | 696 | "foo::bar", |
686 | r"use foo::bar::baz::Qux;", | 697 | r"use foo::bar::baz::Qux;", |
687 | r"use foo::bar::baz::Qux; | 698 | r"use foo::bar; |
688 | use foo::bar;", | 699 | use foo::bar::baz::Qux;", |
700 | ); | ||
701 | } | ||
702 | |||
703 | #[test] | ||
704 | fn insert_short_before_long() { | ||
705 | check_none( | ||
706 | "foo::bar", | ||
707 | r"use foo::bar::baz::Qux;", | ||
708 | r"use foo::bar; | ||
709 | use foo::bar::baz::Qux;", | ||
689 | ); | 710 | ); |
690 | } | 711 | } |
691 | 712 | ||