diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2020-11-03 17:34:59 +0000 |
---|---|---|
committer | GitHub <[email protected]> | 2020-11-03 17:34:59 +0000 |
commit | 060c8b2c96a0de4a131c4d780d2aac80afe13de8 (patch) | |
tree | 1c9c4d8b4134418dcf0dcfaf97d43c97d391f246 /crates/assists/src/utils/insert_use.rs | |
parent | 5e622332774fbd57c12addd46b058c8feb2b08a6 (diff) | |
parent | cd349dbbc4a39342fd54e46fc9d70e3e649a2fda (diff) |
Merge #6287
6287: Don't replace entire module and file nodes when inserting imports r=matklad a=Veykril
This change minifies the resulting diff of import insertions by inserting or replacing the produced use tree directly through an `action` return value instead replacing the entire container node. This action has to be applied by the caller now. This unfortunately pulls the `AssistBuilder` into scope of `insert_use` back again but I tried to at least keep it away from the `insert_use` fn itself.
I'm open to more/better ideas regarding this :)
Fixes #6196
Co-authored-by: Lukas Wirth <[email protected]>
Diffstat (limited to 'crates/assists/src/utils/insert_use.rs')
-rw-r--r-- | crates/assists/src/utils/insert_use.rs | 31 |
1 files changed, 18 insertions, 13 deletions
diff --git a/crates/assists/src/utils/insert_use.rs b/crates/assists/src/utils/insert_use.rs index a76bd5ebf..84a0dffdd 100644 --- a/crates/assists/src/utils/insert_use.rs +++ b/crates/assists/src/utils/insert_use.rs | |||
@@ -1,12 +1,9 @@ | |||
1 | //! Handle syntactic aspects of inserting a new `use`. | 1 | //! Handle syntactic aspects of inserting a new `use`. |
2 | use std::{ | 2 | use std::{cmp::Ordering, iter::successors}; |
3 | cmp::Ordering, | ||
4 | iter::{self, successors}, | ||
5 | }; | ||
6 | 3 | ||
7 | use itertools::{EitherOrBoth, Itertools}; | 4 | use itertools::{EitherOrBoth, Itertools}; |
8 | use syntax::{ | 5 | use syntax::{ |
9 | algo, | 6 | algo::SyntaxRewriter, |
10 | ast::{ | 7 | ast::{ |
11 | self, | 8 | self, |
12 | edit::{AstNodeEdit, IndentLevel}, | 9 | edit::{AstNodeEdit, IndentLevel}, |
@@ -88,20 +85,19 @@ impl ImportScope { | |||
88 | } | 85 | } |
89 | 86 | ||
90 | /// Insert an import path into the given file/node. A `merge` value of none indicates that no import merging is allowed to occur. | 87 | /// Insert an import path into the given file/node. A `merge` value of none indicates that no import merging is allowed to occur. |
91 | pub(crate) fn insert_use( | 88 | pub(crate) fn insert_use<'a>( |
92 | scope: &ImportScope, | 89 | scope: &ImportScope, |
93 | path: ast::Path, | 90 | path: ast::Path, |
94 | merge: Option<MergeBehaviour>, | 91 | merge: Option<MergeBehaviour>, |
95 | ) -> SyntaxNode { | 92 | ) -> SyntaxRewriter<'a> { |
93 | let mut rewriter = SyntaxRewriter::default(); | ||
96 | let use_item = make::use_(make::use_tree(path.clone(), None, None, false)); | 94 | let use_item = make::use_(make::use_tree(path.clone(), None, None, false)); |
97 | // merge into existing imports if possible | 95 | // merge into existing imports if possible |
98 | if let Some(mb) = merge { | 96 | if let Some(mb) = merge { |
99 | for existing_use in scope.as_syntax_node().children().filter_map(ast::Use::cast) { | 97 | for existing_use in scope.as_syntax_node().children().filter_map(ast::Use::cast) { |
100 | if let Some(merged) = try_merge_imports(&existing_use, &use_item, mb) { | 98 | if let Some(merged) = try_merge_imports(&existing_use, &use_item, mb) { |
101 | let to_delete: SyntaxElement = existing_use.syntax().clone().into(); | 99 | rewriter.replace(existing_use.syntax(), merged.syntax()); |
102 | let to_delete = to_delete.clone()..=to_delete; | 100 | return rewriter; |
103 | let to_insert = iter::once(merged.syntax().clone().into()); | ||
104 | return algo::replace_children(scope.as_syntax_node(), to_delete, to_insert); | ||
105 | } | 101 | } |
106 | } | 102 | } |
107 | } | 103 | } |
@@ -157,7 +153,15 @@ pub(crate) fn insert_use( | |||
157 | buf | 153 | buf |
158 | }; | 154 | }; |
159 | 155 | ||
160 | algo::insert_children(scope.as_syntax_node(), insert_position, to_insert) | 156 | match insert_position { |
157 | InsertPosition::First => { | ||
158 | rewriter.insert_many_as_first_children(scope.as_syntax_node(), to_insert) | ||
159 | } | ||
160 | InsertPosition::Last => return rewriter, // actually unreachable | ||
161 | InsertPosition::Before(anchor) => rewriter.insert_many_before(&anchor, to_insert), | ||
162 | InsertPosition::After(anchor) => rewriter.insert_many_after(&anchor, to_insert), | ||
163 | } | ||
164 | rewriter | ||
161 | } | 165 | } |
162 | 166 | ||
163 | fn eq_visibility(vis0: Option<ast::Visibility>, vis1: Option<ast::Visibility>) -> bool { | 167 | fn eq_visibility(vis0: Option<ast::Visibility>, vis1: Option<ast::Visibility>) -> bool { |
@@ -1101,7 +1105,8 @@ use foo::bar::baz::Qux;", | |||
1101 | .find_map(ast::Path::cast) | 1105 | .find_map(ast::Path::cast) |
1102 | .unwrap(); | 1106 | .unwrap(); |
1103 | 1107 | ||
1104 | let result = insert_use(&file, path, mb).to_string(); | 1108 | let rewriter = insert_use(&file, path, mb); |
1109 | let result = rewriter.rewrite(file.as_syntax_node()).to_string(); | ||
1105 | assert_eq_text!(&result, ra_fixture_after); | 1110 | assert_eq_text!(&result, ra_fixture_after); |
1106 | } | 1111 | } |
1107 | 1112 | ||