Don't insert imports outside of cfg attributed items

This commit is contained in:
Lukas Wirth 2021-06-18 23:53:41 +02:00
parent d9666ce509
commit 344cb5e76a
4 changed files with 118 additions and 32 deletions

View file

@ -103,6 +103,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
let scope = match scope.clone() { let scope = match scope.clone() {
ImportScope::File(it) => ImportScope::File(builder.make_mut(it)), ImportScope::File(it) => ImportScope::File(builder.make_mut(it)),
ImportScope::Module(it) => ImportScope::Module(builder.make_mut(it)), ImportScope::Module(it) => ImportScope::Module(builder.make_mut(it)),
ImportScope::Block(it) => ImportScope::Block(builder.make_mut(it)),
}; };
insert_use(&scope, mod_path_to_ast(&import.import_path), &ctx.config.insert_use); insert_use(&scope, mod_path_to_ast(&import.import_path), &ctx.config.insert_use);
}, },
@ -991,6 +992,64 @@ mod foo {}
const _: () = { const _: () = {
Foo Foo
}; };
"#,
);
}
#[test]
fn respects_cfg_attr() {
check_assist(
auto_import,
r#"
mod bar {
pub struct Bar;
}
#[cfg(test)]
fn foo() {
Bar$0
}
"#,
r#"
mod bar {
pub struct Bar;
}
#[cfg(test)]
fn foo() {
use bar::Bar;
Bar
}
"#,
);
}
#[test]
fn respects_cfg_attr2() {
check_assist(
auto_import,
r#"
mod bar {
pub struct Bar;
}
#[cfg(test)]
const FOO: Bar = {
Bar$0
}
"#,
r#"
mod bar {
pub struct Bar;
}
#[cfg(test)]
const FOO: Bar = {
use bar::Bar;
Bar
}
"#, "#,
); );
} }

View file

@ -32,7 +32,6 @@ pub(crate) fn replace_qualified_name_with_use(
let target = path.syntax().text_range(); let target = path.syntax().text_range();
let scope = ImportScope::find_insert_use_container_with_macros(path.syntax(), &ctx.sema)?; let scope = ImportScope::find_insert_use_container_with_macros(path.syntax(), &ctx.sema)?;
let syntax = scope.as_syntax_node();
acc.add( acc.add(
AssistId("replace_qualified_name_with_use", AssistKind::RefactorRewrite), AssistId("replace_qualified_name_with_use", AssistKind::RefactorRewrite),
"Replace qualified path with use", "Replace qualified path with use",
@ -40,11 +39,13 @@ pub(crate) fn replace_qualified_name_with_use(
|builder| { |builder| {
// Now that we've brought the name into scope, re-qualify all paths that could be // Now that we've brought the name into scope, re-qualify all paths that could be
// affected (that is, all paths inside the node we added the `use` to). // affected (that is, all paths inside the node we added the `use` to).
let syntax = builder.make_syntax_mut(syntax.clone()); let scope = match scope {
if let Some(ref import_scope) = ImportScope::from(syntax.clone()) { ImportScope::File(it) => ImportScope::File(builder.make_mut(it)),
shorten_paths(&syntax, &path.clone_for_update()); ImportScope::Module(it) => ImportScope::Module(builder.make_mut(it)),
insert_use(import_scope, path, &ctx.config.insert_use); ImportScope::Block(it) => ImportScope::Block(builder.make_mut(it)),
} };
shorten_paths(scope.as_syntax_node(), &path.clone_for_update());
insert_use(&scope, path, &ctx.config.insert_use);
}, },
) )
} }

View file

@ -5,7 +5,7 @@ use hir::Semantics;
use syntax::{ use syntax::{
algo, algo,
ast::{self, make, AstNode, AttrsOwner, ModuleItemOwner, PathSegmentKind, VisibilityOwner}, ast::{self, make, AstNode, AttrsOwner, ModuleItemOwner, PathSegmentKind, VisibilityOwner},
ted, AstToken, Direction, NodeOrToken, SyntaxNode, SyntaxToken, match_ast, ted, AstToken, Direction, NodeOrToken, SyntaxNode, SyntaxToken,
}; };
use crate::{ use crate::{
@ -43,16 +43,32 @@ pub struct InsertUseConfig {
pub enum ImportScope { pub enum ImportScope {
File(ast::SourceFile), File(ast::SourceFile),
Module(ast::ItemList), Module(ast::ItemList),
Block(ast::BlockExpr),
} }
impl ImportScope { impl ImportScope {
pub fn from(syntax: SyntaxNode) -> Option<Self> { fn from(syntax: SyntaxNode) -> Option<Self> {
if let Some(module) = ast::Module::cast(syntax.clone()) { fn contains_cfg_attr(attrs: &dyn AttrsOwner) -> bool {
module.item_list().map(ImportScope::Module) attrs
} else if let this @ Some(_) = ast::SourceFile::cast(syntax.clone()) { .attrs()
this.map(ImportScope::File) .any(|attr| attr.as_simple_call().map_or(false, |(ident, _)| ident == "cfg"))
} else { }
ast::ItemList::cast(syntax).map(ImportScope::Module) match_ast! {
match syntax {
ast::Module(module) => module.item_list().map(ImportScope::Module),
ast::SourceFile(file) => Some(ImportScope::File(file)),
ast::Fn(func) => contains_cfg_attr(&func).then(|| func.body().map(ImportScope::Block)).flatten(),
ast::Const(konst) => contains_cfg_attr(&konst).then(|| match konst.body()? {
ast::Expr::BlockExpr(block) => Some(block),
_ => None,
}).flatten().map(ImportScope::Block),
ast::Static(statik) => contains_cfg_attr(&statik).then(|| match statik.body()? {
ast::Expr::BlockExpr(block) => Some(block),
_ => None,
}).flatten().map(ImportScope::Block),
_ => None,
}
} }
} }
@ -73,6 +89,7 @@ impl ImportScope {
match self { match self {
ImportScope::File(file) => file.syntax(), ImportScope::File(file) => file.syntax(),
ImportScope::Module(item_list) => item_list.syntax(), ImportScope::Module(item_list) => item_list.syntax(),
ImportScope::Block(block) => block.syntax(),
} }
} }
@ -80,6 +97,7 @@ impl ImportScope {
match self { match self {
ImportScope::File(file) => ImportScope::File(file.clone_for_update()), ImportScope::File(file) => ImportScope::File(file.clone_for_update()),
ImportScope::Module(item_list) => ImportScope::Module(item_list.clone_for_update()), ImportScope::Module(item_list) => ImportScope::Module(item_list.clone_for_update()),
ImportScope::Block(block) => ImportScope::Block(block.clone_for_update()),
} }
} }
@ -96,6 +114,7 @@ impl ImportScope {
let mut use_stmts = match self { let mut use_stmts = match self {
ImportScope::File(f) => f.items(), ImportScope::File(f) => f.items(),
ImportScope::Module(m) => m.items(), ImportScope::Module(m) => m.items(),
ImportScope::Block(b) => b.items(),
} }
.filter_map(use_stmt); .filter_map(use_stmt);
let mut res = ImportGranularityGuess::Unknown; let mut res = ImportGranularityGuess::Unknown;
@ -319,28 +338,29 @@ fn insert_use_(
ted::insert(ted::Position::after(last_inner_element), make::tokens::single_newline()); ted::insert(ted::Position::after(last_inner_element), make::tokens::single_newline());
return; return;
} }
match scope { let l_curly = match scope {
ImportScope::File(_) => { ImportScope::File(_) => {
cov_mark::hit!(insert_group_empty_file); cov_mark::hit!(insert_group_empty_file);
ted::insert(ted::Position::first_child_of(scope_syntax), make::tokens::blank_line()); ted::insert(ted::Position::first_child_of(scope_syntax), make::tokens::blank_line());
ted::insert(ted::Position::first_child_of(scope_syntax), use_item.syntax()) ted::insert(ted::Position::first_child_of(scope_syntax), use_item.syntax());
return;
} }
// don't insert the imports before the item list/block expr's opening curly brace
ImportScope::Module(item_list) => item_list.l_curly_token(),
// don't insert the imports before the item list's opening curly brace // don't insert the imports before the item list's opening curly brace
ImportScope::Module(item_list) => match item_list.l_curly_token() { ImportScope::Block(block) => block.l_curly_token(),
Some(b) => { };
cov_mark::hit!(insert_group_empty_module); match l_curly {
ted::insert(ted::Position::after(&b), make::tokens::single_newline()); Some(b) => {
ted::insert(ted::Position::after(&b), use_item.syntax()); cov_mark::hit!(insert_group_empty_module);
} ted::insert(ted::Position::after(&b), make::tokens::single_newline());
None => { ted::insert(ted::Position::after(&b), use_item.syntax());
// This should never happens, broken module syntax node }
ted::insert( None => {
ted::Position::first_child_of(scope_syntax), // This should never happens, broken module syntax node
make::tokens::blank_line(), ted::insert(ted::Position::first_child_of(scope_syntax), make::tokens::blank_line());
); ted::insert(ted::Position::first_child_of(scope_syntax), use_item.syntax());
ted::insert(ted::Position::first_child_of(scope_syntax), use_item.syntax()); }
}
},
} }
} }

View file

@ -8,7 +8,7 @@ use parser::SyntaxKind;
use rowan::{GreenNodeData, GreenTokenData}; use rowan::{GreenNodeData, GreenTokenData};
use crate::{ use crate::{
ast::{self, support, AstNode, AstToken, AttrsOwner, NameOwner, SyntaxNode}, ast::{self, support, AstChildren, AstNode, AstToken, AttrsOwner, NameOwner, SyntaxNode},
NodeOrToken, SmolStr, SyntaxElement, SyntaxToken, TokenText, T, NodeOrToken, SmolStr, SyntaxElement, SyntaxToken, TokenText, T,
}; };
@ -45,6 +45,12 @@ fn text_of_first_token(node: &SyntaxNode) -> TokenText<'_> {
} }
} }
impl ast::BlockExpr {
pub fn items(&self) -> AstChildren<ast::Item> {
support::children(self.syntax())
}
}
#[derive(Debug, PartialEq, Eq, Clone)] #[derive(Debug, PartialEq, Eq, Clone)]
pub enum Macro { pub enum Macro {
MacroRules(ast::MacroRules), MacroRules(ast::MacroRules),