aboutsummaryrefslogtreecommitdiff
path: root/crates
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2020-11-03 17:34:59 +0000
committerGitHub <[email protected]>2020-11-03 17:34:59 +0000
commit060c8b2c96a0de4a131c4d780d2aac80afe13de8 (patch)
tree1c9c4d8b4134418dcf0dcfaf97d43c97d391f246 /crates
parent5e622332774fbd57c12addd46b058c8feb2b08a6 (diff)
parentcd349dbbc4a39342fd54e46fc9d70e3e649a2fda (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')
-rw-r--r--crates/assists/src/handlers/auto_import.rs5
-rw-r--r--crates/assists/src/handlers/extract_struct_from_enum_variant.rs156
-rw-r--r--crates/assists/src/handlers/replace_qualified_name_with_use.rs7
-rw-r--r--crates/assists/src/utils/insert_use.rs31
-rw-r--r--crates/syntax/src/ast/make.rs37
5 files changed, 129 insertions, 107 deletions
diff --git a/crates/assists/src/handlers/auto_import.rs b/crates/assists/src/handlers/auto_import.rs
index e49e641b3..37dd61266 100644
--- a/crates/assists/src/handlers/auto_import.rs
+++ b/crates/assists/src/handlers/auto_import.rs
@@ -99,7 +99,6 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
99 let range = ctx.sema.original_range(import_assets.syntax_under_caret()).range; 99 let range = ctx.sema.original_range(import_assets.syntax_under_caret()).range;
100 let group = import_group_message(import_assets.import_candidate()); 100 let group = import_group_message(import_assets.import_candidate());
101 let scope = ImportScope::find_insert_use_container(import_assets.syntax_under_caret(), ctx)?; 101 let scope = ImportScope::find_insert_use_container(import_assets.syntax_under_caret(), ctx)?;
102 let syntax = scope.as_syntax_node();
103 for (import, _) in proposed_imports { 102 for (import, _) in proposed_imports {
104 acc.add_group( 103 acc.add_group(
105 &group, 104 &group,
@@ -107,9 +106,9 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
107 format!("Import `{}`", &import), 106 format!("Import `{}`", &import),
108 range, 107 range,
109 |builder| { 108 |builder| {
110 let new_syntax = 109 let rewriter =
111 insert_use(&scope, mod_path_to_ast(&import), ctx.config.insert_use.merge); 110 insert_use(&scope, mod_path_to_ast(&import), ctx.config.insert_use.merge);
112 builder.replace(syntax.text_range(), new_syntax.to_string()) 111 builder.rewrite(rewriter);
113 }, 112 },
114 ); 113 );
115 } 114 }
diff --git a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs
index 178718c5e..dddab255e 100644
--- a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs
+++ b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs
@@ -1,16 +1,14 @@
1use hir::{EnumVariant, Module, ModuleDef, Name}; 1use hir::{AsName, EnumVariant, Module, ModuleDef, Name};
2use ide_db::base_db::FileId;
3use ide_db::{defs::Definition, search::Reference, RootDatabase}; 2use ide_db::{defs::Definition, search::Reference, RootDatabase};
4use itertools::Itertools; 3use rustc_hash::{FxHashMap, FxHashSet};
5use rustc_hash::FxHashSet;
6use syntax::{ 4use syntax::{
7 algo::find_node_at_offset, 5 algo::find_node_at_offset,
8 ast::{self, edit::IndentLevel, ArgListOwner, AstNode, NameOwner, VisibilityOwner}, 6 algo::SyntaxRewriter,
9 SourceFile, TextRange, TextSize, 7 ast::{self, edit::IndentLevel, make, ArgListOwner, AstNode, NameOwner, VisibilityOwner},
8 SourceFile, SyntaxElement,
10}; 9};
11 10
12use crate::{ 11use crate::{
13 assist_context::AssistBuilder,
14 utils::{insert_use, mod_path_to_ast, ImportScope}, 12 utils::{insert_use, mod_path_to_ast, ImportScope},
15 AssistContext, AssistId, AssistKind, Assists, 13 AssistContext, AssistId, AssistKind, Assists,
16}; 14};
@@ -43,7 +41,7 @@ pub(crate) fn extract_struct_from_enum_variant(
43 return None; 41 return None;
44 } 42 }
45 43
46 let variant_name = variant.name()?.to_string(); 44 let variant_name = variant.name()?;
47 let variant_hir = ctx.sema.to_def(&variant)?; 45 let variant_hir = ctx.sema.to_def(&variant)?;
48 if existing_struct_def(ctx.db(), &variant_name, &variant_hir) { 46 if existing_struct_def(ctx.db(), &variant_name, &variant_hir) {
49 return None; 47 return None;
@@ -62,14 +60,18 @@ pub(crate) fn extract_struct_from_enum_variant(
62 |builder| { 60 |builder| {
63 let definition = Definition::ModuleDef(ModuleDef::EnumVariant(variant_hir)); 61 let definition = Definition::ModuleDef(ModuleDef::EnumVariant(variant_hir));
64 let res = definition.usages(&ctx.sema).all(); 62 let res = definition.usages(&ctx.sema).all();
65 let start_offset = variant.parent_enum().syntax().text_range().start(); 63
66 let mut visited_modules_set = FxHashSet::default(); 64 let mut visited_modules_set = FxHashSet::default();
67 visited_modules_set.insert(current_module); 65 visited_modules_set.insert(current_module);
66 let mut rewriters = FxHashMap::default();
68 for reference in res { 67 for reference in res {
68 let rewriter = rewriters
69 .entry(reference.file_range.file_id)
70 .or_insert_with(SyntaxRewriter::default);
69 let source_file = ctx.sema.parse(reference.file_range.file_id); 71 let source_file = ctx.sema.parse(reference.file_range.file_id);
70 update_reference( 72 update_reference(
71 ctx, 73 ctx,
72 builder, 74 rewriter,
73 reference, 75 reference,
74 &source_file, 76 &source_file,
75 &enum_module_def, 77 &enum_module_def,
@@ -77,34 +79,39 @@ pub(crate) fn extract_struct_from_enum_variant(
77 &mut visited_modules_set, 79 &mut visited_modules_set,
78 ); 80 );
79 } 81 }
82 let mut rewriter =
83 rewriters.remove(&ctx.frange.file_id).unwrap_or_else(SyntaxRewriter::default);
84 for (file_id, rewriter) in rewriters {
85 builder.edit_file(file_id);
86 builder.rewrite(rewriter);
87 }
88 builder.edit_file(ctx.frange.file_id);
89 update_variant(&mut rewriter, &variant_name, &field_list);
80 extract_struct_def( 90 extract_struct_def(
81 builder, 91 &mut rewriter,
82 &enum_ast, 92 &enum_ast,
83 &variant_name, 93 variant_name.clone(),
84 &field_list.to_string(), 94 &field_list,
85 start_offset, 95 &variant.parent_enum().syntax().clone().into(),
86 ctx.frange.file_id, 96 visibility,
87 &visibility,
88 ); 97 );
89 let list_range = field_list.syntax().text_range(); 98 builder.rewrite(rewriter);
90 update_variant(builder, &variant_name, ctx.frange.file_id, list_range);
91 }, 99 },
92 ) 100 )
93} 101}
94 102
95fn existing_struct_def(db: &RootDatabase, variant_name: &str, variant: &EnumVariant) -> bool { 103fn existing_struct_def(db: &RootDatabase, variant_name: &ast::Name, variant: &EnumVariant) -> bool {
96 variant 104 variant
97 .parent_enum(db) 105 .parent_enum(db)
98 .module(db) 106 .module(db)
99 .scope(db, None) 107 .scope(db, None)
100 .into_iter() 108 .into_iter()
101 .any(|(name, _)| name.to_string() == variant_name) 109 .any(|(name, _)| name == variant_name.as_name())
102} 110}
103 111
104#[allow(dead_code)]
105fn insert_import( 112fn insert_import(
106 ctx: &AssistContext, 113 ctx: &AssistContext,
107 builder: &mut AssistBuilder, 114 rewriter: &mut SyntaxRewriter,
108 path: &ast::PathExpr, 115 path: &ast::PathExpr,
109 module: &Module, 116 module: &Module,
110 enum_module_def: &ModuleDef, 117 enum_module_def: &ModuleDef,
@@ -116,69 +123,59 @@ fn insert_import(
116 mod_path.segments.pop(); 123 mod_path.segments.pop();
117 mod_path.segments.push(variant_hir_name.clone()); 124 mod_path.segments.push(variant_hir_name.clone());
118 let scope = ImportScope::find_insert_use_container(path.syntax(), ctx)?; 125 let scope = ImportScope::find_insert_use_container(path.syntax(), ctx)?;
119 let syntax = scope.as_syntax_node();
120 126
121 let new_syntax = 127 *rewriter += insert_use(&scope, mod_path_to_ast(&mod_path), ctx.config.insert_use.merge);
122 insert_use(&scope, mod_path_to_ast(&mod_path), ctx.config.insert_use.merge);
123 // FIXME: this will currently panic as multiple imports will have overlapping text ranges
124 builder.replace(syntax.text_range(), new_syntax.to_string())
125 } 128 }
126 Some(()) 129 Some(())
127} 130}
128 131
129// FIXME: this should use strongly-typed `make`, rather than string manipulation.
130fn extract_struct_def( 132fn extract_struct_def(
131 builder: &mut AssistBuilder, 133 rewriter: &mut SyntaxRewriter,
132 enum_: &ast::Enum, 134 enum_: &ast::Enum,
133 variant_name: &str, 135 variant_name: ast::Name,
134 variant_list: &str, 136 variant_list: &ast::TupleFieldList,
135 start_offset: TextSize, 137 start_offset: &SyntaxElement,
136 file_id: FileId, 138 visibility: Option<ast::Visibility>,
137 visibility: &Option<ast::Visibility>,
138) -> Option<()> { 139) -> Option<()> {
139 let visibility_string = if let Some(visibility) = visibility { 140 let variant_list = make::tuple_field_list(
140 format!("{} ", visibility.to_string()) 141 variant_list
141 } else { 142 .fields()
142 "".to_string() 143 .flat_map(|field| Some(make::tuple_field(Some(make::visibility_pub()), field.ty()?))),
143 };
144 let indent = IndentLevel::from_node(enum_.syntax());
145 let struct_def = format!(
146 r#"{}struct {}{};
147
148{}"#,
149 visibility_string,
150 variant_name,
151 list_with_visibility(variant_list),
152 indent
153 ); 144 );
154 builder.edit_file(file_id); 145
155 builder.insert(start_offset, struct_def); 146 rewriter.insert_before(
147 start_offset,
148 make::struct_(visibility, variant_name, None, variant_list.into()).syntax(),
149 );
150 rewriter.insert_before(start_offset, &make::tokens::blank_line());
151
152 if let indent_level @ 1..=usize::MAX = IndentLevel::from_node(enum_.syntax()).0 as usize {
153 rewriter
154 .insert_before(start_offset, &make::tokens::whitespace(&" ".repeat(4 * indent_level)));
155 }
156 Some(()) 156 Some(())
157} 157}
158 158
159fn update_variant( 159fn update_variant(
160 builder: &mut AssistBuilder, 160 rewriter: &mut SyntaxRewriter,
161 variant_name: &str, 161 variant_name: &ast::Name,
162 file_id: FileId, 162 field_list: &ast::TupleFieldList,
163 list_range: TextRange,
164) -> Option<()> { 163) -> Option<()> {
165 let inside_variant_range = TextRange::new( 164 let (l, r): (SyntaxElement, SyntaxElement) =
166 list_range.start().checked_add(TextSize::from(1))?, 165 (field_list.l_paren_token()?.into(), field_list.r_paren_token()?.into());
167 list_range.end().checked_sub(TextSize::from(1))?, 166 let replacement = vec![l, variant_name.syntax().clone().into(), r];
168 ); 167 rewriter.replace_with_many(field_list.syntax(), replacement);
169 builder.edit_file(file_id);
170 builder.replace(inside_variant_range, variant_name);
171 Some(()) 168 Some(())
172} 169}
173 170
174fn update_reference( 171fn update_reference(
175 ctx: &AssistContext, 172 ctx: &AssistContext,
176 builder: &mut AssistBuilder, 173 rewriter: &mut SyntaxRewriter,
177 reference: Reference, 174 reference: Reference,
178 source_file: &SourceFile, 175 source_file: &SourceFile,
179 _enum_module_def: &ModuleDef, 176 enum_module_def: &ModuleDef,
180 _variant_hir_name: &Name, 177 variant_hir_name: &Name,
181 _visited_modules_set: &mut FxHashSet<Module>, 178 visited_modules_set: &mut FxHashSet<Module>,
182) -> Option<()> { 179) -> Option<()> {
183 let path_expr: ast::PathExpr = find_node_at_offset::<ast::PathExpr>( 180 let path_expr: ast::PathExpr = find_node_at_offset::<ast::PathExpr>(
184 source_file.syntax(), 181 source_file.syntax(),
@@ -187,35 +184,21 @@ fn update_reference(
187 let call = path_expr.syntax().parent().and_then(ast::CallExpr::cast)?; 184 let call = path_expr.syntax().parent().and_then(ast::CallExpr::cast)?;
188 let list = call.arg_list()?; 185 let list = call.arg_list()?;
189 let segment = path_expr.path()?.segment()?; 186 let segment = path_expr.path()?.segment()?;
190 let _module = ctx.sema.scope(&path_expr.syntax()).module()?; 187 let module = ctx.sema.scope(&path_expr.syntax()).module()?;
191 let list_range = list.syntax().text_range();
192 let inside_list_range = TextRange::new(
193 list_range.start().checked_add(TextSize::from(1))?,
194 list_range.end().checked_sub(TextSize::from(1))?,
195 );
196 builder.edit_file(reference.file_range.file_id);
197 /* FIXME: this most likely requires AST-based editing, see `insert_import`
198 if !visited_modules_set.contains(&module) { 188 if !visited_modules_set.contains(&module) {
199 if insert_import(ctx, builder, &path_expr, &module, enum_module_def, variant_hir_name) 189 if insert_import(ctx, rewriter, &path_expr, &module, enum_module_def, variant_hir_name)
200 .is_some() 190 .is_some()
201 { 191 {
202 visited_modules_set.insert(module); 192 visited_modules_set.insert(module);
203 } 193 }
204 } 194 }
205 */
206 builder.replace(inside_list_range, format!("{}{}", segment, list));
207 Some(())
208}
209 195
210fn list_with_visibility(list: &str) -> String { 196 let lparen = syntax::SyntaxElement::from(list.l_paren_token()?);
211 list.split(',') 197 let rparen = syntax::SyntaxElement::from(list.r_paren_token()?);
212 .map(|part| { 198 rewriter.insert_after(&lparen, segment.syntax());
213 let index = if part.chars().next().unwrap() == '(' { 1usize } else { 0 }; 199 rewriter.insert_after(&lparen, &lparen);
214 let mut mod_part = part.trim().to_string(); 200 rewriter.insert_before(&rparen, &rparen);
215 mod_part.insert_str(index, "pub "); 201 Some(())
216 mod_part
217 })
218 .join(", ")
219} 202}
220 203
221#[cfg(test)] 204#[cfg(test)]
@@ -250,7 +233,6 @@ pub enum A { One(One) }"#,
250 } 233 }
251 234
252 #[test] 235 #[test]
253 #[ignore] // FIXME: this currently panics if `insert_import` is used
254 fn test_extract_struct_with_complex_imports() { 236 fn test_extract_struct_with_complex_imports() {
255 check_assist( 237 check_assist(
256 extract_struct_from_enum_variant, 238 extract_struct_from_enum_variant,
diff --git a/crates/assists/src/handlers/replace_qualified_name_with_use.rs b/crates/assists/src/handlers/replace_qualified_name_with_use.rs
index c50bc7604..d7e1d9580 100644
--- a/crates/assists/src/handlers/replace_qualified_name_with_use.rs
+++ b/crates/assists/src/handlers/replace_qualified_name_with_use.rs
@@ -45,10 +45,9 @@ pub(crate) fn replace_qualified_name_with_use(
45 // affected (that is, all paths inside the node we added the `use` to). 45 // affected (that is, all paths inside the node we added the `use` to).
46 let mut rewriter = SyntaxRewriter::default(); 46 let mut rewriter = SyntaxRewriter::default();
47 shorten_paths(&mut rewriter, syntax.clone(), &path); 47 shorten_paths(&mut rewriter, syntax.clone(), &path);
48 let rewritten_syntax = rewriter.rewrite(&syntax); 48 if let Some(ref import_scope) = ImportScope::from(syntax.clone()) {
49 if let Some(ref import_scope) = ImportScope::from(rewritten_syntax) { 49 rewriter += insert_use(import_scope, path, ctx.config.insert_use.merge);
50 let new_syntax = insert_use(import_scope, path, ctx.config.insert_use.merge); 50 builder.rewrite(rewriter);
51 builder.replace(syntax.text_range(), new_syntax.to_string())
52 } 51 }
53 }, 52 },
54 ) 53 )
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`.
2use std::{ 2use std::{cmp::Ordering, iter::successors};
3 cmp::Ordering,
4 iter::{self, successors},
5};
6 3
7use itertools::{EitherOrBoth, Itertools}; 4use itertools::{EitherOrBoth, Itertools};
8use syntax::{ 5use 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.
91pub(crate) fn insert_use( 88pub(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
163fn eq_visibility(vis0: Option<ast::Visibility>, vis1: Option<ast::Visibility>) -> bool { 167fn 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
diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs
index 5b06cb767..2cf436e7a 100644
--- a/crates/syntax/src/ast/make.rs
+++ b/crates/syntax/src/ast/make.rs
@@ -351,6 +351,23 @@ pub fn visibility_pub_crate() -> ast::Visibility {
351 ast_from_text("pub(crate) struct S") 351 ast_from_text("pub(crate) struct S")
352} 352}
353 353
354pub fn visibility_pub() -> ast::Visibility {
355 ast_from_text("pub struct S")
356}
357
358pub fn tuple_field_list(fields: impl IntoIterator<Item = ast::TupleField>) -> ast::TupleFieldList {
359 let fields = fields.into_iter().join(", ");
360 ast_from_text(&format!("struct f({});", fields))
361}
362
363pub fn tuple_field(visibility: Option<ast::Visibility>, ty: ast::Type) -> ast::TupleField {
364 let visibility = match visibility {
365 None => String::new(),
366 Some(it) => format!("{} ", it),
367 };
368 ast_from_text(&format!("struct f({}{});", visibility, ty))
369}
370
354pub fn fn_( 371pub fn fn_(
355 visibility: Option<ast::Visibility>, 372 visibility: Option<ast::Visibility>,
356 fn_name: ast::Name, 373 fn_name: ast::Name,
@@ -373,6 +390,26 @@ pub fn fn_(
373 )) 390 ))
374} 391}
375 392
393pub fn struct_(
394 visibility: Option<ast::Visibility>,
395 strukt_name: ast::Name,
396 type_params: Option<ast::GenericParamList>,
397 field_list: ast::FieldList,
398) -> ast::Struct {
399 let semicolon = if matches!(field_list, ast::FieldList::TupleFieldList(_)) { ";" } else { "" };
400 let type_params =
401 if let Some(type_params) = type_params { format!("<{}>", type_params) } else { "".into() };
402 let visibility = match visibility {
403 None => String::new(),
404 Some(it) => format!("{} ", it),
405 };
406
407 ast_from_text(&format!(
408 "{}struct {}{}{}{}",
409 visibility, strukt_name, type_params, field_list, semicolon
410 ))
411}
412
376fn ast_from_text<N: AstNode>(text: &str) -> N { 413fn ast_from_text<N: AstNode>(text: &str) -> N {
377 let parse = SourceFile::parse(text); 414 let parse = SourceFile::parse(text);
378 let node = match parse.tree().syntax().descendants().find_map(N::cast) { 415 let node = match parse.tree().syntax().descendants().find_map(N::cast) {