From 2ee090faaf69474a2baadf0494ef3c6ed4fdbcbc Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 18 Jun 2021 23:11:56 +0200 Subject: Allow to disable import insertion on single path glob imports --- crates/ide_assists/src/handlers/auto_import.rs | 2 +- .../handlers/extract_struct_from_enum_variant.rs | 2 +- .../handlers/replace_qualified_name_with_use.rs | 2 +- crates/ide_assists/src/tests.rs | 1 + crates/ide_completion/src/item.rs | 2 +- crates/ide_completion/src/tests.rs | 1 + crates/ide_db/src/helpers/insert_use.rs | 8 ++- crates/ide_db/src/helpers/insert_use/tests.rs | 72 ++++++++++++++++++---- crates/rust-analyzer/src/config.rs | 4 ++ crates/rust-analyzer/src/integrated_benchmarks.rs | 2 + crates/rust-analyzer/src/to_proto.rs | 1 + crates/syntax/src/ast/node_ext.rs | 9 +++ 12 files changed, 89 insertions(+), 17 deletions(-) (limited to 'crates') diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs index d4748ef3a..6c7348178 100644 --- a/crates/ide_assists/src/handlers/auto_import.rs +++ b/crates/ide_assists/src/handlers/auto_import.rs @@ -104,7 +104,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> ImportScope::File(it) => ImportScope::File(builder.make_mut(it)), ImportScope::Module(it) => ImportScope::Module(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); }, ); } diff --git a/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs index d3ff7b65c..da6df9106 100644 --- a/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ide_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -235,7 +235,7 @@ fn apply_references( import: Option<(ImportScope, hir::ModPath)>, ) { if let Some((scope, path)) = import { - insert_use(&scope, mod_path_to_ast(&path), insert_use_cfg); + insert_use(&scope, mod_path_to_ast(&path), &insert_use_cfg); } // deep clone to prevent cycle let path = make::path_from_segments(iter::once(segment.clone_subtree()), false); diff --git a/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs b/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs index 39f5eb4ff..26019c793 100644 --- a/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs +++ b/crates/ide_assists/src/handlers/replace_qualified_name_with_use.rs @@ -43,7 +43,7 @@ pub(crate) fn replace_qualified_name_with_use( let syntax = builder.make_syntax_mut(syntax.clone()); if let Some(ref import_scope) = ImportScope::from(syntax.clone()) { shorten_paths(&syntax, &path.clone_for_update()); - insert_use(import_scope, path, ctx.config.insert_use); + insert_use(import_scope, path, &ctx.config.insert_use); } }, ) diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs index 29bd4a563..b6f224b21 100644 --- a/crates/ide_assists/src/tests.rs +++ b/crates/ide_assists/src/tests.rs @@ -28,6 +28,7 @@ pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig { prefix_kind: hir::PrefixKind::Plain, enforce_granularity: true, group: true, + skip_glob_imports: true, }, }; diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index 99edb9499..ae63d132e 100644 --- a/crates/ide_completion/src/item.rs +++ b/crates/ide_completion/src/item.rs @@ -378,7 +378,7 @@ impl ImportEdit { let _p = profile::span("ImportEdit::to_text_edit"); let new_ast = self.scope.clone_for_update(); - insert_use::insert_use(&new_ast, mod_path_to_ast(&self.import.import_path), cfg); + insert_use::insert_use(&new_ast, mod_path_to_ast(&self.import.import_path), &cfg); let mut import_insert = TextEdit::builder(); algo::diff(self.scope.as_syntax_node(), new_ast.as_syntax_node()) .into_text_edit(&mut import_insert); diff --git a/crates/ide_completion/src/tests.rs b/crates/ide_completion/src/tests.rs index 1ea6017ce..211c89c40 100644 --- a/crates/ide_completion/src/tests.rs +++ b/crates/ide_completion/src/tests.rs @@ -36,6 +36,7 @@ pub(crate) const TEST_CONFIG: CompletionConfig = CompletionConfig { prefix_kind: PrefixKind::Plain, enforce_granularity: true, group: true, + skip_glob_imports: true, }, }; diff --git a/crates/ide_db/src/helpers/insert_use.rs b/crates/ide_db/src/helpers/insert_use.rs index 10bbafe77..4da058cb2 100644 --- a/crates/ide_db/src/helpers/insert_use.rs +++ b/crates/ide_db/src/helpers/insert_use.rs @@ -36,6 +36,7 @@ pub struct InsertUseConfig { pub enforce_granularity: bool, pub prefix_kind: PrefixKind, pub group: bool, + pub skip_glob_imports: bool, } #[derive(Debug, Clone)] @@ -153,7 +154,7 @@ enum ImportGranularityGuess { } /// Insert an import path into the given file/node. A `merge` value of none indicates that no import merging is allowed to occur. -pub fn insert_use<'a>(scope: &ImportScope, path: ast::Path, cfg: InsertUseConfig) { +pub fn insert_use<'a>(scope: &ImportScope, path: ast::Path, cfg: &InsertUseConfig) { let _p = profile::span("insert_use"); let mut mb = match cfg.granularity { ImportGranularity::Crate => Some(MergeBehavior::Crate), @@ -175,7 +176,10 @@ pub fn insert_use<'a>(scope: &ImportScope, path: ast::Path, cfg: InsertUseConfig make::use_(None, make::use_tree(path.clone(), None, None, false)).clone_for_update(); // merge into existing imports if possible if let Some(mb) = mb { - for existing_use in scope.as_syntax_node().children().filter_map(ast::Use::cast) { + let filter = |it: &_| !(cfg.skip_glob_imports && ast::Use::is_simple_glob(it)); + for existing_use in + scope.as_syntax_node().children().filter_map(ast::Use::cast).filter(filter) + { if let Some(merged) = try_merge_imports(&existing_use, &use_item, mb) { ted::replace(existing_use.syntax(), merged.syntax()); return; diff --git a/crates/ide_db/src/helpers/insert_use/tests.rs b/crates/ide_db/src/helpers/insert_use/tests.rs index 5a88ec742..263edcdc9 100644 --- a/crates/ide_db/src/helpers/insert_use/tests.rs +++ b/crates/ide_db/src/helpers/insert_use/tests.rs @@ -3,6 +3,23 @@ use super::*; use hir::PrefixKind; use test_utils::assert_eq_text; +#[test] +fn insert_skips_lone_glob_imports() { + check( + "use foo::baz::A", + r" +use foo::bar::*; +", + r" +use foo::bar::*; +use foo::baz::A; +", + ImportGranularity::Crate, + false, + false, + ); +} + #[test] fn insert_not_group() { cov_mark::check!(insert_no_grouping_last); @@ -534,17 +551,37 @@ fn merge_groups_self() { #[test] fn merge_mod_into_glob() { - check_crate( + check_with_config( "token::TokenKind", r"use token::TokenKind::*;", r"use token::TokenKind::{*, self};", + false, + &InsertUseConfig { + granularity: ImportGranularity::Crate, + enforce_granularity: true, + prefix_kind: PrefixKind::Plain, + group: false, + skip_glob_imports: false, + }, ) // FIXME: have it emit `use token::TokenKind::{self, *}`? } #[test] fn merge_self_glob() { - check_crate("self", r"use self::*;", r"use self::{*, self};") + check_with_config( + "self", + r"use self::*;", + r"use self::{*, self};", + false, + &InsertUseConfig { + granularity: ImportGranularity::Crate, + enforce_granularity: true, + prefix_kind: PrefixKind::Plain, + group: false, + skip_glob_imports: false, + }, + ) // FIXME: have it emit `use {self, *}`? } @@ -757,13 +794,12 @@ use foo::bar::qux; ); } -fn check( +fn check_with_config( path: &str, ra_fixture_before: &str, ra_fixture_after: &str, - granularity: ImportGranularity, module: bool, - group: bool, + config: &InsertUseConfig, ) { let mut syntax = ast::SourceFile::parse(ra_fixture_before).tree().syntax().clone(); if module { @@ -777,18 +813,32 @@ fn check( .find_map(ast::Path::cast) .unwrap(); - insert_use( - &file, + insert_use(&file, path, config); + let result = file.as_syntax_node().to_string(); + assert_eq_text!(ra_fixture_after, &result); +} + +fn check( + path: &str, + ra_fixture_before: &str, + ra_fixture_after: &str, + granularity: ImportGranularity, + module: bool, + group: bool, +) { + check_with_config( path, - InsertUseConfig { + ra_fixture_before, + ra_fixture_after, + module, + &InsertUseConfig { granularity, enforce_granularity: true, prefix_kind: PrefixKind::Plain, group, + skip_glob_imports: true, }, - ); - let result = file.as_syntax_node().to_string(); - assert_eq_text!(ra_fixture_after, &result); + ) } fn check_crate(path: &str, ra_fixture_before: &str, ra_fixture_after: &str) { diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index c4757a1cb..16c295639 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -44,6 +44,9 @@ config_data! { assist_importPrefix: ImportPrefixDef = "\"plain\"", /// Group inserted imports by the [following order](https://rust-analyzer.github.io/manual.html#auto-import). Groups are separated by newlines. assist_importGroup: bool = "true", + /// Whether to allow import insertion to merge new imports into single path glob imports like `use std::fmt::*;`. + assist_allowMergingIntoGlobImports: bool = "true", + /// Show function name and docs in parameter hints. callInfo_full: bool = "true", @@ -672,6 +675,7 @@ impl Config { ImportPrefixDef::BySelf => PrefixKind::BySelf, }, group: self.data.assist_importGroup, + skip_glob_imports: !self.data.assist_allowMergingIntoGlobImports, } } pub fn completion(&self) -> CompletionConfig { diff --git a/crates/rust-analyzer/src/integrated_benchmarks.rs b/crates/rust-analyzer/src/integrated_benchmarks.rs index 8ddeb59f7..f8afc930a 100644 --- a/crates/rust-analyzer/src/integrated_benchmarks.rs +++ b/crates/rust-analyzer/src/integrated_benchmarks.rs @@ -143,6 +143,7 @@ fn integrated_completion_benchmark() { prefix_kind: hir::PrefixKind::ByCrate, enforce_granularity: true, group: true, + skip_glob_imports: true, }, }; let position = @@ -178,6 +179,7 @@ fn integrated_completion_benchmark() { prefix_kind: hir::PrefixKind::ByCrate, enforce_granularity: true, group: true, + skip_glob_imports: true, }, }; let position = diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index e53cd3c7b..310d8c6d2 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -1187,6 +1187,7 @@ mod tests { prefix_kind: PrefixKind::Plain, enforce_granularity: true, group: true, + skip_glob_imports: true, }, }, ide_db::base_db::FilePosition { file_id, offset }, diff --git a/crates/syntax/src/ast/node_ext.rs b/crates/syntax/src/ast/node_ext.rs index b057e6624..3c92a486f 100644 --- a/crates/syntax/src/ast/node_ext.rs +++ b/crates/syntax/src/ast/node_ext.rs @@ -281,6 +281,15 @@ impl ast::Path { successors(self.qualifier(), |p| p.qualifier()) } } + +impl ast::Use { + pub fn is_simple_glob(&self) -> bool { + self.use_tree() + .map(|use_tree| use_tree.use_tree_list().is_none() && use_tree.star_token().is_some()) + .unwrap_or(false) + } +} + impl ast::UseTree { pub fn is_simple_path(&self) -> bool { self.use_tree_list().is_none() && self.star_token().is_none() -- cgit v1.2.3