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/handlers/merge_imports.rs | |
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/handlers/merge_imports.rs')
-rw-r--r-- | crates/assists/src/handlers/merge_imports.rs | 161 |
1 files changed, 86 insertions, 75 deletions
diff --git a/crates/assists/src/handlers/merge_imports.rs b/crates/assists/src/handlers/merge_imports.rs index 35b884206..0bd679260 100644 --- a/crates/assists/src/handlers/merge_imports.rs +++ b/crates/assists/src/handlers/merge_imports.rs | |||
@@ -1,14 +1,14 @@ | |||
1 | use std::iter::successors; | ||
2 | |||
3 | use syntax::{ | 1 | use syntax::{ |
4 | algo::{neighbor, skip_trivia_token, SyntaxRewriter}, | 2 | algo::{neighbor, SyntaxRewriter}, |
5 | ast::{self, edit::AstNodeEdit, make}, | 3 | ast, AstNode, |
6 | AstNode, Direction, InsertPosition, SyntaxElement, T, | ||
7 | }; | 4 | }; |
8 | 5 | ||
9 | use crate::{ | 6 | use crate::{ |
10 | assist_context::{AssistContext, Assists}, | 7 | assist_context::{AssistContext, Assists}, |
11 | utils::next_prev, | 8 | utils::{ |
9 | insert_use::{try_merge_imports, try_merge_trees}, | ||
10 | next_prev, MergeBehaviour, | ||
11 | }, | ||
12 | AssistId, AssistKind, | 12 | AssistId, AssistKind, |
13 | }; | 13 | }; |
14 | 14 | ||
@@ -30,23 +30,22 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<() | |||
30 | let mut offset = ctx.offset(); | 30 | let mut offset = ctx.offset(); |
31 | 31 | ||
32 | if let Some(use_item) = tree.syntax().parent().and_then(ast::Use::cast) { | 32 | if let Some(use_item) = tree.syntax().parent().and_then(ast::Use::cast) { |
33 | let (merged, to_delete) = next_prev() | 33 | let (merged, to_delete) = |
34 | .filter_map(|dir| neighbor(&use_item, dir)) | 34 | next_prev().filter_map(|dir| neighbor(&use_item, dir)).find_map(|use_item2| { |
35 | .filter_map(|it| Some((it.clone(), it.use_tree()?))) | 35 | try_merge_imports(&use_item, &use_item2, MergeBehaviour::Full).zip(Some(use_item2)) |
36 | .find_map(|(use_item, use_tree)| { | ||
37 | Some((try_merge_trees(&tree, &use_tree)?, use_item)) | ||
38 | })?; | 36 | })?; |
39 | 37 | ||
40 | rewriter.replace_ast(&tree, &merged); | 38 | rewriter.replace_ast(&use_item, &merged); |
41 | rewriter += to_delete.remove(); | 39 | rewriter += to_delete.remove(); |
42 | 40 | ||
43 | if to_delete.syntax().text_range().end() < offset { | 41 | if to_delete.syntax().text_range().end() < offset { |
44 | offset -= to_delete.syntax().text_range().len(); | 42 | offset -= to_delete.syntax().text_range().len(); |
45 | } | 43 | } |
46 | } else { | 44 | } else { |
47 | let (merged, to_delete) = next_prev() | 45 | let (merged, to_delete) = |
48 | .filter_map(|dir| neighbor(&tree, dir)) | 46 | next_prev().filter_map(|dir| neighbor(&tree, dir)).find_map(|use_tree| { |
49 | .find_map(|use_tree| Some((try_merge_trees(&tree, &use_tree)?, use_tree.clone())))?; | 47 | try_merge_trees(&tree, &use_tree, MergeBehaviour::Full).zip(Some(use_tree)) |
48 | })?; | ||
50 | 49 | ||
51 | rewriter.replace_ast(&tree, &merged); | 50 | rewriter.replace_ast(&tree, &merged); |
52 | rewriter += to_delete.remove(); | 51 | rewriter += to_delete.remove(); |
@@ -67,66 +66,6 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<() | |||
67 | ) | 66 | ) |
68 | } | 67 | } |
69 | 68 | ||
70 | fn try_merge_trees(old: &ast::UseTree, new: &ast::UseTree) -> Option<ast::UseTree> { | ||
71 | let lhs_path = old.path()?; | ||
72 | let rhs_path = new.path()?; | ||
73 | |||
74 | let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?; | ||
75 | |||
76 | let lhs = old.split_prefix(&lhs_prefix); | ||
77 | let rhs = new.split_prefix(&rhs_prefix); | ||
78 | |||
79 | let should_insert_comma = lhs | ||
80 | .use_tree_list()? | ||
81 | .r_curly_token() | ||
82 | .and_then(|it| skip_trivia_token(it.prev_token()?, Direction::Prev)) | ||
83 | .map(|it| it.kind() != T![,]) | ||
84 | .unwrap_or(true); | ||
85 | |||
86 | let mut to_insert: Vec<SyntaxElement> = Vec::new(); | ||
87 | if should_insert_comma { | ||
88 | to_insert.push(make::token(T![,]).into()); | ||
89 | to_insert.push(make::tokens::single_space().into()); | ||
90 | } | ||
91 | to_insert.extend( | ||
92 | rhs.use_tree_list()? | ||
93 | .syntax() | ||
94 | .children_with_tokens() | ||
95 | .filter(|it| it.kind() != T!['{'] && it.kind() != T!['}']), | ||
96 | ); | ||
97 | let use_tree_list = lhs.use_tree_list()?; | ||
98 | let pos = InsertPosition::Before(use_tree_list.r_curly_token()?.into()); | ||
99 | let use_tree_list = use_tree_list.insert_children(pos, to_insert); | ||
100 | Some(lhs.with_use_tree_list(use_tree_list)) | ||
101 | } | ||
102 | |||
103 | fn common_prefix(lhs: &ast::Path, rhs: &ast::Path) -> Option<(ast::Path, ast::Path)> { | ||
104 | let mut res = None; | ||
105 | let mut lhs_curr = first_path(&lhs); | ||
106 | let mut rhs_curr = first_path(&rhs); | ||
107 | loop { | ||
108 | match (lhs_curr.segment(), rhs_curr.segment()) { | ||
109 | (Some(lhs), Some(rhs)) if lhs.syntax().text() == rhs.syntax().text() => (), | ||
110 | _ => break, | ||
111 | } | ||
112 | res = Some((lhs_curr.clone(), rhs_curr.clone())); | ||
113 | |||
114 | match (lhs_curr.parent_path(), rhs_curr.parent_path()) { | ||
115 | (Some(lhs), Some(rhs)) => { | ||
116 | lhs_curr = lhs; | ||
117 | rhs_curr = rhs; | ||
118 | } | ||
119 | _ => break, | ||
120 | } | ||
121 | } | ||
122 | |||
123 | res | ||
124 | } | ||
125 | |||
126 | fn first_path(path: &ast::Path) -> ast::Path { | ||
127 | successors(Some(path.clone()), |it| it.qualifier()).last().unwrap() | ||
128 | } | ||
129 | |||
130 | #[cfg(test)] | 69 | #[cfg(test)] |
131 | mod tests { | 70 | mod tests { |
132 | use crate::tests::{check_assist, check_assist_not_applicable}; | 71 | use crate::tests::{check_assist, check_assist_not_applicable}; |
@@ -189,6 +128,78 @@ use std::{fmt::{Display, self}}; | |||
189 | } | 128 | } |
190 | 129 | ||
191 | #[test] | 130 | #[test] |
131 | fn skip_pub1() { | ||
132 | check_assist_not_applicable( | ||
133 | merge_imports, | ||
134 | r" | ||
135 | pub use std::fmt<|>::Debug; | ||
136 | use std::fmt::Display; | ||
137 | ", | ||
138 | ); | ||
139 | } | ||
140 | |||
141 | #[test] | ||
142 | fn skip_pub_last() { | ||
143 | check_assist_not_applicable( | ||
144 | merge_imports, | ||
145 | r" | ||
146 | use std::fmt<|>::Debug; | ||
147 | pub use std::fmt::Display; | ||
148 | ", | ||
149 | ); | ||
150 | } | ||
151 | |||
152 | #[test] | ||
153 | fn skip_pub_crate_pub() { | ||
154 | check_assist_not_applicable( | ||
155 | merge_imports, | ||
156 | r" | ||
157 | pub(crate) use std::fmt<|>::Debug; | ||
158 | pub use std::fmt::Display; | ||
159 | ", | ||
160 | ); | ||
161 | } | ||
162 | |||
163 | #[test] | ||
164 | fn skip_pub_pub_crate() { | ||
165 | check_assist_not_applicable( | ||
166 | merge_imports, | ||
167 | r" | ||
168 | pub use std::fmt<|>::Debug; | ||
169 | pub(crate) use std::fmt::Display; | ||
170 | ", | ||
171 | ); | ||
172 | } | ||
173 | |||
174 | #[test] | ||
175 | fn merge_pub() { | ||
176 | check_assist( | ||
177 | merge_imports, | ||
178 | r" | ||
179 | pub use std::fmt<|>::Debug; | ||
180 | pub use std::fmt::Display; | ||
181 | ", | ||
182 | r" | ||
183 | pub use std::fmt::{Debug, Display}; | ||
184 | ", | ||
185 | ) | ||
186 | } | ||
187 | |||
188 | #[test] | ||
189 | fn merge_pub_crate() { | ||
190 | check_assist( | ||
191 | merge_imports, | ||
192 | r" | ||
193 | pub(crate) use std::fmt<|>::Debug; | ||
194 | pub(crate) use std::fmt::Display; | ||
195 | ", | ||
196 | r" | ||
197 | pub(crate) use std::fmt::{Debug, Display}; | ||
198 | ", | ||
199 | ) | ||
200 | } | ||
201 | |||
202 | #[test] | ||
192 | fn test_merge_nested() { | 203 | fn test_merge_nested() { |
193 | check_assist( | 204 | check_assist( |
194 | merge_imports, | 205 | merge_imports, |