aboutsummaryrefslogtreecommitdiff
path: root/crates
diff options
context:
space:
mode:
authorLukas Wirth <[email protected]>2020-09-03 15:23:33 +0100
committerLukas Wirth <[email protected]>2020-09-03 17:36:08 +0100
commit7de2a30f4080d0e6d8790882238bed1cb6ccbc21 (patch)
tree450b363b8f6646ca7cda39325e4524af7b9389e8 /crates
parent98e2f674e9e736720d1cd0a5b8c24e1fb10f64a1 (diff)
Fix import insertion breaking nested modules
Diffstat (limited to 'crates')
-rw-r--r--crates/assists/src/handlers/auto_import.rs11
-rw-r--r--crates/assists/src/handlers/extract_struct_from_enum_variant.rs8
-rw-r--r--crates/assists/src/handlers/replace_qualified_name_with_use.rs20
-rw-r--r--crates/assists/src/utils.rs2
-rw-r--r--crates/assists/src/utils/insert_use.rs138
5 files changed, 117 insertions, 62 deletions
diff --git a/crates/assists/src/handlers/auto_import.rs b/crates/assists/src/handlers/auto_import.rs
index f3b4c5708..66e819154 100644
--- a/crates/assists/src/handlers/auto_import.rs
+++ b/crates/assists/src/handlers/auto_import.rs
@@ -1,11 +1,13 @@
1use std::collections::BTreeSet; 1use std::collections::BTreeSet;
2 2
3use ast::make;
3use either::Either; 4use either::Either;
4use hir::{ 5use hir::{
5 AsAssocItem, AssocItemContainer, ModPath, Module, ModuleDef, PathResolution, Semantics, Trait, 6 AsAssocItem, AssocItemContainer, ModPath, Module, ModuleDef, PathResolution, Semantics, Trait,
6 Type, 7 Type,
7}; 8};
8use ide_db::{imports_locator, RootDatabase}; 9use ide_db::{imports_locator, RootDatabase};
10use insert_use::ImportScope;
9use rustc_hash::FxHashSet; 11use rustc_hash::FxHashSet;
10use syntax::{ 12use syntax::{
11 ast::{self, AstNode}, 13 ast::{self, AstNode},
@@ -16,8 +18,6 @@ use crate::{
16 utils::{insert_use, MergeBehaviour}, 18 utils::{insert_use, MergeBehaviour},
17 AssistContext, AssistId, AssistKind, Assists, GroupLabel, 19 AssistContext, AssistId, AssistKind, Assists, GroupLabel,
18}; 20};
19use ast::make;
20use insert_use::find_insert_use_container;
21 21
22// Assist: auto_import 22// Assist: auto_import
23// 23//
@@ -47,8 +47,9 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
47 47
48 let range = ctx.sema.original_range(&auto_import_assets.syntax_under_caret).range; 48 let range = ctx.sema.original_range(&auto_import_assets.syntax_under_caret).range;
49 let group = auto_import_assets.get_import_group_message(); 49 let group = auto_import_assets.get_import_group_message();
50 let container = find_insert_use_container(&auto_import_assets.syntax_under_caret, ctx)?; 50 let scope =
51 let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone()); 51 ImportScope::find_insert_use_container(&auto_import_assets.syntax_under_caret, ctx)?;
52 let syntax = scope.as_syntax_node();
52 for import in proposed_imports { 53 for import in proposed_imports {
53 acc.add_group( 54 acc.add_group(
54 &group, 55 &group,
@@ -57,7 +58,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
57 range, 58 range,
58 |builder| { 59 |builder| {
59 let new_syntax = insert_use( 60 let new_syntax = insert_use(
60 &syntax, 61 &scope,
61 make::path_from_text(&import.to_string()), 62 make::path_from_text(&import.to_string()),
62 Some(MergeBehaviour::Full), 63 Some(MergeBehaviour::Full),
63 ); 64 );
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 d9c50f25f..a50a57f1a 100644
--- a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs
+++ b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs
@@ -15,7 +15,7 @@ use crate::{
15 AssistContext, AssistId, AssistKind, Assists, 15 AssistContext, AssistId, AssistKind, Assists,
16}; 16};
17use ast::make; 17use ast::make;
18use insert_use::find_insert_use_container; 18use insert_use::ImportScope;
19 19
20// Assist: extract_struct_from_enum_variant 20// Assist: extract_struct_from_enum_variant
21// 21//
@@ -110,11 +110,11 @@ fn insert_import(
110 if let Some(mut mod_path) = mod_path { 110 if let Some(mut mod_path) = mod_path {
111 mod_path.segments.pop(); 111 mod_path.segments.pop();
112 mod_path.segments.push(variant_hir_name.clone()); 112 mod_path.segments.push(variant_hir_name.clone());
113 let container = find_insert_use_container(path.syntax(), ctx)?; 113 let scope = ImportScope::find_insert_use_container(path.syntax(), ctx)?;
114 let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone()); 114 let syntax = scope.as_syntax_node();
115 115
116 let new_syntax = insert_use( 116 let new_syntax = insert_use(
117 &syntax, 117 &scope,
118 make::path_from_text(&mod_path.to_string()), 118 make::path_from_text(&mod_path.to_string()),
119 Some(MergeBehaviour::Full), 119 Some(MergeBehaviour::Full),
120 ); 120 );
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 597bc268c..85c70d16b 100644
--- a/crates/assists/src/handlers/replace_qualified_name_with_use.rs
+++ b/crates/assists/src/handlers/replace_qualified_name_with_use.rs
@@ -2,7 +2,7 @@ use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SyntaxNode, TextRang
2use test_utils::mark; 2use test_utils::mark;
3 3
4use crate::{ 4use crate::{
5 utils::{find_insert_use_container, insert_use, MergeBehaviour}, 5 utils::{insert_use, ImportScope, MergeBehaviour},
6 AssistContext, AssistId, AssistKind, Assists, 6 AssistContext, AssistId, AssistKind, Assists,
7}; 7};
8use ast::make; 8use ast::make;
@@ -44,8 +44,8 @@ pub(crate) fn replace_qualified_name_with_use(
44 }; 44 };
45 45
46 let target = path.syntax().text_range(); 46 let target = path.syntax().text_range();
47 let container = find_insert_use_container(path.syntax(), ctx)?; 47 let scope = ImportScope::find_insert_use_container(path.syntax(), ctx)?;
48 let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone()); 48 let syntax = scope.as_syntax_node();
49 acc.add( 49 acc.add(
50 AssistId("replace_qualified_name_with_use", AssistKind::RefactorRewrite), 50 AssistId("replace_qualified_name_with_use", AssistKind::RefactorRewrite),
51 "Replace qualified path with use", 51 "Replace qualified path with use",
@@ -56,12 +56,14 @@ pub(crate) fn replace_qualified_name_with_use(
56 let mut rewriter = SyntaxRewriter::default(); 56 let mut rewriter = SyntaxRewriter::default();
57 shorten_paths(&mut rewriter, syntax.clone(), path); 57 shorten_paths(&mut rewriter, syntax.clone(), path);
58 let rewritten_syntax = rewriter.rewrite(&syntax); 58 let rewritten_syntax = rewriter.rewrite(&syntax);
59 let new_syntax = insert_use( 59 if let Some(ref import_scope) = ImportScope::from(rewritten_syntax) {
60 &rewritten_syntax, 60 let new_syntax = insert_use(
61 make::path_from_text(path_to_import), 61 import_scope,
62 Some(MergeBehaviour::Full), 62 make::path_from_text(path_to_import),
63 ); 63 Some(MergeBehaviour::Full),
64 builder.replace(syntax.text_range(), new_syntax.to_string()) 64 );
65 builder.replace(syntax.text_range(), new_syntax.to_string())
66 }
65 }, 67 },
66 ) 68 )
67} 69}
diff --git a/crates/assists/src/utils.rs b/crates/assists/src/utils.rs
index 07afa64ff..7559ddd63 100644
--- a/crates/assists/src/utils.rs
+++ b/crates/assists/src/utils.rs
@@ -16,7 +16,7 @@ use syntax::{
16 16
17use crate::assist_config::SnippetCap; 17use crate::assist_config::SnippetCap;
18 18
19pub(crate) use insert_use::{find_insert_use_container, insert_use, MergeBehaviour}; 19pub(crate) use insert_use::{insert_use, ImportScope, MergeBehaviour};
20 20
21pub(crate) fn unwrap_trivial_block(block: ast::BlockExpr) -> ast::Expr { 21pub(crate) fn unwrap_trivial_block(block: ast::BlockExpr) -> ast::Expr {
22 extract_trivial_expression(&block) 22 extract_trivial_expression(&block)
diff --git a/crates/assists/src/utils/insert_use.rs b/crates/assists/src/utils/insert_use.rs
index 1ddd72197..40ff31075 100644
--- a/crates/assists/src/utils/insert_use.rs
+++ b/crates/assists/src/utils/insert_use.rs
@@ -1,8 +1,10 @@
1use std::iter::{self, successors}; 1use std::iter::{self, successors};
2 2
3use algo::skip_trivia_token; 3use algo::skip_trivia_token;
4use ast::{edit::AstNodeEdit, PathSegmentKind, VisibilityOwner}; 4use ast::{
5use either::Either; 5 edit::{AstNodeEdit, IndentLevel},
6 PathSegmentKind, VisibilityOwner,
7};
6use syntax::{ 8use syntax::{
7 algo, 9 algo,
8 ast::{self, make, AstNode}, 10 ast::{self, make, AstNode},
@@ -11,56 +13,121 @@ use syntax::{
11 13
12use test_utils::mark; 14use test_utils::mark;
13 15
14/// Determines the containing syntax node in which to insert a `use` statement affecting `position`. 16#[derive(Debug)]
15pub(crate) fn find_insert_use_container( 17pub enum ImportScope {
16 position: &SyntaxNode, 18 File(ast::SourceFile),
17 ctx: &crate::assist_context::AssistContext, 19 Module(ast::ItemList),
18) -> Option<Either<ast::ItemList, ast::SourceFile>> { 20}
19 ctx.sema.ancestors_with_macros(position.clone()).find_map(|n| { 21
20 if let Some(module) = ast::Module::cast(n.clone()) { 22impl ImportScope {
21 module.item_list().map(Either::Left) 23 pub fn from(syntax: SyntaxNode) -> Option<Self> {
24 if let Some(module) = ast::Module::cast(syntax.clone()) {
25 module.item_list().map(ImportScope::Module)
26 } else if let this @ Some(_) = ast::SourceFile::cast(syntax.clone()) {
27 this.map(ImportScope::File)
22 } else { 28 } else {
23 Some(Either::Right(ast::SourceFile::cast(n)?)) 29 ast::ItemList::cast(syntax).map(ImportScope::Module)
24 } 30 }
25 }) 31 }
32
33 /// Determines the containing syntax node in which to insert a `use` statement affecting `position`.
34 pub(crate) fn find_insert_use_container(
35 position: &SyntaxNode,
36 ctx: &crate::assist_context::AssistContext,
37 ) -> Option<Self> {
38 ctx.sema.ancestors_with_macros(position.clone()).find_map(Self::from)
39 }
40
41 pub(crate) fn as_syntax_node(&self) -> &SyntaxNode {
42 match self {
43 ImportScope::File(file) => file.syntax(),
44 ImportScope::Module(item_list) => item_list.syntax(),
45 }
46 }
47
48 fn indent_level(&self) -> IndentLevel {
49 match self {
50 ImportScope::File(file) => file.indent_level(),
51 ImportScope::Module(item_list) => item_list.indent_level() + 1,
52 }
53 }
54
55 fn first_insert_pos(&self) -> (InsertPosition<SyntaxElement>, AddBlankLine) {
56 match self {
57 ImportScope::File(_) => (InsertPosition::First, AddBlankLine::AfterTwice),
58 // don't insert the impotrs before the item lists curly brace
59 ImportScope::Module(item_list) => item_list
60 .l_curly_token()
61 .map(|b| (InsertPosition::After(b.into()), AddBlankLine::Around))
62 .unwrap_or((InsertPosition::First, AddBlankLine::AfterTwice)),
63 }
64 }
65
66 fn insert_pos_after_inner_attribute(&self) -> (InsertPosition<SyntaxElement>, AddBlankLine) {
67 // check if the scope has a inner attributes, we dont want to insert in front of it
68 match self
69 .as_syntax_node()
70 .children()
71 // no flat_map here cause we want to short circuit the iterator
72 .map(ast::Attr::cast)
73 .take_while(|attr| {
74 attr.as_ref().map(|attr| attr.kind() == ast::AttrKind::Inner).unwrap_or(false)
75 })
76 .last()
77 .flatten()
78 {
79 Some(attr) => {
80 (InsertPosition::After(attr.syntax().clone().into()), AddBlankLine::BeforeTwice)
81 }
82 None => self.first_insert_pos(),
83 }
84 }
26} 85}
27 86
28/// 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.
29pub fn insert_use( 88pub(crate) fn insert_use(
30 where_: &SyntaxNode, 89 scope: &ImportScope,
31 path: ast::Path, 90 path: ast::Path,
32 merge: Option<MergeBehaviour>, 91 merge: Option<MergeBehaviour>,
33) -> SyntaxNode { 92) -> SyntaxNode {
34 let use_item = make::use_(make::use_tree(path.clone(), None, None, false)); 93 let use_item = make::use_(make::use_tree(path.clone(), None, None, false));
35 // merge into existing imports if possible 94 // merge into existing imports if possible
36 if let Some(mb) = merge { 95 if let Some(mb) = merge {
37 for existing_use in where_.children().filter_map(ast::Use::cast) { 96 for existing_use in scope.as_syntax_node().children().filter_map(ast::Use::cast) {
38 if let Some(merged) = try_merge_imports(&existing_use, &use_item, mb) { 97 if let Some(merged) = try_merge_imports(&existing_use, &use_item, mb) {
39 let to_delete: SyntaxElement = existing_use.syntax().clone().into(); 98 let to_delete: SyntaxElement = existing_use.syntax().clone().into();
40 let to_delete = to_delete.clone()..=to_delete; 99 let to_delete = to_delete.clone()..=to_delete;
41 let to_insert = iter::once(merged.syntax().clone().into()); 100 let to_insert = iter::once(merged.syntax().clone().into());
42 return algo::replace_children(where_, to_delete, to_insert); 101 return algo::replace_children(scope.as_syntax_node(), to_delete, to_insert);
43 } 102 }
44 } 103 }
45 } 104 }
46 105
47 // either we weren't allowed to merge or there is no import that fits the merge conditions 106 // either we weren't allowed to merge or there is no import that fits the merge conditions
48 // so look for the place we have to insert to 107 // so look for the place we have to insert to
49 let (insert_position, add_blank) = find_insert_position(where_, path); 108 let (insert_position, add_blank) = find_insert_position(scope, path);
50 109
51 let to_insert: Vec<SyntaxElement> = { 110 let to_insert: Vec<SyntaxElement> = {
52 let mut buf = Vec::new(); 111 let mut buf = Vec::new();
53 112
54 match add_blank { 113 match add_blank {
55 AddBlankLine::Before => buf.push(make::tokens::single_newline().into()), 114 AddBlankLine::Before | AddBlankLine::Around => {
115 buf.push(make::tokens::single_newline().into())
116 }
56 AddBlankLine::BeforeTwice => buf.push(make::tokens::blank_line().into()), 117 AddBlankLine::BeforeTwice => buf.push(make::tokens::blank_line().into()),
57 _ => (), 118 _ => (),
58 } 119 }
59 120
121 if let ident_level @ 1..=usize::MAX = scope.indent_level().0 as usize {
122 // TODO: this alone doesnt properly re-align all cases
123 buf.push(make::tokens::whitespace(&" ".repeat(4 * ident_level)).into());
124 }
60 buf.push(use_item.syntax().clone().into()); 125 buf.push(use_item.syntax().clone().into());
61 126
62 match add_blank { 127 match add_blank {
63 AddBlankLine::After => buf.push(make::tokens::single_newline().into()), 128 AddBlankLine::After | AddBlankLine::Around => {
129 buf.push(make::tokens::single_newline().into())
130 }
64 AddBlankLine::AfterTwice => buf.push(make::tokens::blank_line().into()), 131 AddBlankLine::AfterTwice => buf.push(make::tokens::blank_line().into()),
65 _ => (), 132 _ => (),
66 } 133 }
@@ -68,7 +135,7 @@ pub fn insert_use(
68 buf 135 buf
69 }; 136 };
70 137
71 algo::insert_children(where_, insert_position, to_insert) 138 algo::insert_children(scope.as_syntax_node(), insert_position, to_insert)
72} 139}
73 140
74fn try_merge_imports( 141fn try_merge_imports(
@@ -218,16 +285,18 @@ fn segment_iter(path: &ast::Path) -> impl Iterator<Item = ast::PathSegment> + Cl
218enum AddBlankLine { 285enum AddBlankLine {
219 Before, 286 Before,
220 BeforeTwice, 287 BeforeTwice,
288 Around,
221 After, 289 After,
222 AfterTwice, 290 AfterTwice,
223} 291}
224 292
225fn find_insert_position( 293fn find_insert_position(
226 scope: &SyntaxNode, 294 scope: &ImportScope,
227 insert_path: ast::Path, 295 insert_path: ast::Path,
228) -> (InsertPosition<SyntaxElement>, AddBlankLine) { 296) -> (InsertPosition<SyntaxElement>, AddBlankLine) {
229 let group = ImportGroup::new(&insert_path); 297 let group = ImportGroup::new(&insert_path);
230 let path_node_iter = scope 298 let path_node_iter = scope
299 .as_syntax_node()
231 .children() 300 .children()
232 .filter_map(|node| ast::Use::cast(node.clone()).zip(Some(node))) 301 .filter_map(|node| ast::Use::cast(node.clone()).zip(Some(node)))
233 .flat_map(|(use_, node)| use_.use_tree().and_then(|tree| tree.path()).zip(Some(node))); 302 .flat_map(|(use_, node)| use_.use_tree().and_then(|tree| tree.path()).zip(Some(node)));
@@ -275,27 +344,7 @@ fn find_insert_position(
275 (InsertPosition::After(node.into()), AddBlankLine::BeforeTwice) 344 (InsertPosition::After(node.into()), AddBlankLine::BeforeTwice)
276 } 345 }
277 // there are no imports in this file at all 346 // there are no imports in this file at all
278 None => { 347 None => scope.insert_pos_after_inner_attribute(),
279 // check if the scope has a inner attributes, we dont want to insert in front of it
280 match scope
281 .children()
282 // no flat_map here cause we want to short circuit the iterator
283 .map(ast::Attr::cast)
284 .take_while(|attr| {
285 attr.as_ref()
286 .map(|attr| attr.kind() == ast::AttrKind::Inner)
287 .unwrap_or(false)
288 })
289 .last()
290 .flatten()
291 {
292 Some(attr) => (
293 InsertPosition::After(attr.syntax().clone().into()),
294 AddBlankLine::BeforeTwice,
295 ),
296 None => (InsertPosition::First, AddBlankLine::AfterTwice),
297 }
298 }
299 }, 348 },
300 } 349 }
301 } 350 }
@@ -640,7 +689,10 @@ use foo::bar;",
640 ra_fixture_after: &str, 689 ra_fixture_after: &str,
641 mb: Option<MergeBehaviour>, 690 mb: Option<MergeBehaviour>,
642 ) { 691 ) {
643 let file = ast::SourceFile::parse(ra_fixture_before).tree().syntax().clone(); 692 let file = super::ImportScope::from(
693 ast::SourceFile::parse(ra_fixture_before).tree().syntax().clone(),
694 )
695 .unwrap();
644 let path = ast::SourceFile::parse(&format!("use {};", path)) 696 let path = ast::SourceFile::parse(&format!("use {};", path))
645 .tree() 697 .tree()
646 .syntax() 698 .syntax()