From 96fc01a30b88d95619b26fd96c58627dd54cb339 Mon Sep 17 00:00:00 2001 From: asv Date: Sat, 6 Mar 2021 13:02:26 +0200 Subject: Make group imports configurable --- crates/ide_assists/src/handlers/auto_import.rs | 3 +- .../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 | 11 +++---- crates/ide_completion/src/lib.rs | 2 +- crates/ide_completion/src/test_utils.rs | 3 +- crates/ide_db/src/helpers/insert_use.rs | 17 ++++++++-- crates/ide_db/src/helpers/insert_use/tests.rs | 38 +++++++++++++++++++--- crates/rust-analyzer/src/cli/analysis_bench.rs | 6 +++- crates/rust-analyzer/src/config.rs | 4 ++- crates/rust-analyzer/src/to_proto.rs | 6 +++- docs/user/generated_config.adoc | 2 ++ editors/code/package.json | 5 +++ 14 files changed, 79 insertions(+), 23 deletions(-) diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs index dc38f90e9..1422224ac 100644 --- a/crates/ide_assists/src/handlers/auto_import.rs +++ b/crates/ide_assists/src/handlers/auto_import.rs @@ -99,8 +99,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> format!("Import `{}`", &import), range, |builder| { - let rewriter = - insert_use(&scope, mod_path_to_ast(&import), ctx.config.insert_use.merge); + let rewriter = insert_use(&scope, mod_path_to_ast(&import), ctx.config.insert_use); builder.rewrite(rewriter); }, ); 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 5c7678b53..4f0422e96 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 @@ -154,7 +154,7 @@ fn insert_import( mod_path.pop_segment(); mod_path.push_segment(variant_hir_name.clone()); let scope = ImportScope::find_insert_use_container(scope_node, &ctx.sema)?; - *rewriter += insert_use(&scope, mod_path_to_ast(&mod_path), ctx.config.insert_use.merge); + *rewriter += insert_use(&scope, mod_path_to_ast(&mod_path), ctx.config.insert_use); } Some(()) } 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 f3bc6cf39..55481af34 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 @@ -44,7 +44,7 @@ pub(crate) fn replace_qualified_name_with_use( let mut rewriter = SyntaxRewriter::default(); shorten_paths(&mut rewriter, syntax.clone(), &path); if let Some(ref import_scope) = ImportScope::from(syntax.clone()) { - rewriter += insert_use(import_scope, path, ctx.config.insert_use.merge); + rewriter += insert_use(import_scope, path, ctx.config.insert_use); builder.rewrite(rewriter); } }, diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs index b7f616760..a7a923beb 100644 --- a/crates/ide_assists/src/tests.rs +++ b/crates/ide_assists/src/tests.rs @@ -23,6 +23,7 @@ pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig { insert_use: InsertUseConfig { merge: Some(MergeBehavior::Full), prefix_kind: hir::PrefixKind::Plain, + group: true, }, }; diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index 884711f11..9b2435c4b 100644 --- a/crates/ide_completion/src/item.rs +++ b/crates/ide_completion/src/item.rs @@ -5,7 +5,7 @@ use std::fmt; use hir::{Documentation, ModPath, Mutability}; use ide_db::{ helpers::{ - insert_use::{self, ImportScope, MergeBehavior}, + insert_use::{self, ImportScope, InsertUseConfig}, mod_path_to_ast, SnippetCap, }, SymbolKind, @@ -280,14 +280,11 @@ pub struct ImportEdit { impl ImportEdit { /// Attempts to insert the import to the given scope, producing a text edit. /// May return no edit in edge cases, such as scope already containing the import. - pub fn to_text_edit(&self, merge_behavior: Option) -> Option { + pub fn to_text_edit(&self, cfg: InsertUseConfig) -> Option { let _p = profile::span("ImportEdit::to_text_edit"); - let rewriter = insert_use::insert_use( - &self.import_scope, - mod_path_to_ast(&self.import_path), - merge_behavior, - ); + let rewriter = + insert_use::insert_use(&self.import_scope, mod_path_to_ast(&self.import_path), cfg); let old_ast = rewriter.rewrite_root()?; let mut import_insert = TextEdit::builder(); algo::diff(&old_ast, &rewriter.rewrite(&old_ast)).into_text_edit(&mut import_insert); diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs index 76f31de9e..b0b809791 100644 --- a/crates/ide_completion/src/lib.rs +++ b/crates/ide_completion/src/lib.rs @@ -156,7 +156,7 @@ pub fn resolve_completion_edits( .find(|mod_path| mod_path.to_string() == full_import_path)?; ImportEdit { import_path, import_scope, import_for_trait_assoc_item } - .to_text_edit(config.insert_use.merge) + .to_text_edit(config.insert_use) .map(|edit| vec![edit]) } diff --git a/crates/ide_completion/src/test_utils.rs b/crates/ide_completion/src/test_utils.rs index baff83305..9da844031 100644 --- a/crates/ide_completion/src/test_utils.rs +++ b/crates/ide_completion/src/test_utils.rs @@ -25,6 +25,7 @@ pub(crate) const TEST_CONFIG: CompletionConfig = CompletionConfig { insert_use: InsertUseConfig { merge: Some(MergeBehavior::Full), prefix_kind: PrefixKind::Plain, + group: true, }, }; @@ -119,7 +120,7 @@ pub(crate) fn check_edit_with_config( let mut combined_edit = completion.text_edit().to_owned(); if let Some(import_text_edit) = - completion.import_to_add().and_then(|edit| edit.to_text_edit(config.insert_use.merge)) + completion.import_to_add().and_then(|edit| edit.to_text_edit(config.insert_use)) { combined_edit.union(import_text_edit).expect( "Failed to apply completion resolve changes: change ranges overlap, but should not", diff --git a/crates/ide_db/src/helpers/insert_use.rs b/crates/ide_db/src/helpers/insert_use.rs index fd4035198..f52aee344 100644 --- a/crates/ide_db/src/helpers/insert_use.rs +++ b/crates/ide_db/src/helpers/insert_use.rs @@ -19,6 +19,7 @@ use test_utils::mark; pub struct InsertUseConfig { pub merge: Option, pub prefix_kind: hir::PrefixKind, + pub group: bool, } #[derive(Debug, Clone)] @@ -99,13 +100,13 @@ fn is_inner_comment(token: SyntaxToken) -> bool { pub fn insert_use<'a>( scope: &ImportScope, path: ast::Path, - merge: Option, + cfg: InsertUseConfig, ) -> SyntaxRewriter<'a> { let _p = profile::span("insert_use"); let mut rewriter = SyntaxRewriter::default(); let use_item = make::use_(None, make::use_tree(path.clone(), None, None, false)); // merge into existing imports if possible - if let Some(mb) = merge { + if let Some(mb) = cfg.merge { for existing_use in scope.as_syntax_node().children().filter_map(ast::Use::cast) { if let Some(merged) = try_merge_imports(&existing_use, &use_item, mb) { rewriter.replace(existing_use.syntax(), merged.syntax()); @@ -116,7 +117,7 @@ pub fn insert_use<'a>( // either we weren't allowed to merge or there is no import that fits the merge conditions // so look for the place we have to insert to - let (insert_position, add_blank) = find_insert_position(scope, path); + let (insert_position, add_blank) = find_insert_position(scope, path, cfg.group); let indent = if let ident_level @ 1..=usize::MAX = scope.indent_level().0 as usize { Some(make::tokens::whitespace(&" ".repeat(4 * ident_level)).into()) @@ -538,6 +539,7 @@ impl AddBlankLine { fn find_insert_position( scope: &ImportScope, insert_path: ast::Path, + group_imports: bool, ) -> (InsertPosition, AddBlankLine) { let group = ImportGroup::new(&insert_path); let path_node_iter = scope @@ -550,6 +552,14 @@ fn find_insert_position( let has_tl = tree.use_tree_list().is_some(); Some((path, has_tl, node)) }); + + if !group_imports { + if let Some((_, _, node)) = path_node_iter.last() { + return (InsertPosition::After(node.into()), AddBlankLine::Before); + } + return (InsertPosition::First, AddBlankLine::AfterTwice); + } + // Iterator that discards anything thats not in the required grouping // This implementation allows the user to rearrange their import groups as this only takes the first group that fits let group_iter = path_node_iter @@ -565,6 +575,7 @@ fn find_insert_position( use_tree_path_cmp(&insert_path, false, path, has_tl) != Ordering::Greater }, ); + match post_insert { // insert our import before that element Some((.., node)) => (InsertPosition::Before(node.into()), AddBlankLine::After), diff --git a/crates/ide_db/src/helpers/insert_use/tests.rs b/crates/ide_db/src/helpers/insert_use/tests.rs index 4bbe66f1f..67d0d6fb6 100644 --- a/crates/ide_db/src/helpers/insert_use/tests.rs +++ b/crates/ide_db/src/helpers/insert_use/tests.rs @@ -1,7 +1,31 @@ use super::*; +use hir::PrefixKind; use test_utils::assert_eq_text; +#[test] +fn insert_not_group() { + check( + "use external_crate2::bar::A", + r" +use std::bar::B; +use external_crate::bar::A; +use crate::bar::A; +use self::bar::A; +use super::bar::A;", + r" +use std::bar::B; +use external_crate::bar::A; +use crate::bar::A; +use self::bar::A; +use super::bar::A; +use external_crate2::bar::A;", + None, + false, + false, + ); +} + #[test] fn insert_existing() { check_full("std::fs", "use std::fs;", "use std::fs;") @@ -240,6 +264,7 @@ fn insert_empty_module() { }", None, true, + true, ) } @@ -584,6 +609,7 @@ fn check( ra_fixture_after: &str, mb: Option, module: bool, + group: bool, ) { let mut syntax = ast::SourceFile::parse(ra_fixture_before).tree().syntax().clone(); if module { @@ -597,21 +623,25 @@ fn check( .find_map(ast::Path::cast) .unwrap(); - let rewriter = insert_use(&file, path, mb); + let rewriter = insert_use( + &file, + path, + InsertUseConfig { merge: mb, prefix_kind: PrefixKind::Plain, group }, + ); let result = rewriter.rewrite(file.as_syntax_node()).to_string(); assert_eq_text!(ra_fixture_after, &result); } fn check_full(path: &str, ra_fixture_before: &str, ra_fixture_after: &str) { - check(path, ra_fixture_before, ra_fixture_after, Some(MergeBehavior::Full), false) + check(path, ra_fixture_before, ra_fixture_after, Some(MergeBehavior::Full), false, true) } fn check_last(path: &str, ra_fixture_before: &str, ra_fixture_after: &str) { - check(path, ra_fixture_before, ra_fixture_after, Some(MergeBehavior::Last), false) + check(path, ra_fixture_before, ra_fixture_after, Some(MergeBehavior::Last), false, true) } fn check_none(path: &str, ra_fixture_before: &str, ra_fixture_after: &str) { - check(path, ra_fixture_before, ra_fixture_after, None, false) + check(path, ra_fixture_before, ra_fixture_after, None, false, true) } fn check_merge_only_fail(ra_fixture0: &str, ra_fixture1: &str, mb: MergeBehavior) { diff --git a/crates/rust-analyzer/src/cli/analysis_bench.rs b/crates/rust-analyzer/src/cli/analysis_bench.rs index 3bd7e678d..49994824f 100644 --- a/crates/rust-analyzer/src/cli/analysis_bench.rs +++ b/crates/rust-analyzer/src/cli/analysis_bench.rs @@ -108,7 +108,11 @@ impl BenchCmd { add_call_parenthesis: true, add_call_argument_snippets: true, snippet_cap: SnippetCap::new(true), - insert_use: InsertUseConfig { merge: None, prefix_kind: PrefixKind::Plain }, + insert_use: InsertUseConfig { + merge: None, + prefix_kind: PrefixKind::Plain, + group: true, + }, }; let res = do_work(&mut host, file_id, |analysis| { analysis.completions(&options, file_position) diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 367136702..cac48e911 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -35,7 +35,8 @@ config_data! { assist_importMergeBehaviour: MergeBehaviorDef = "\"full\"", /// The path structure for newly inserted paths to use. 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", /// Show function name and docs in parameter hints. callInfo_full: bool = "true", @@ -574,6 +575,7 @@ impl Config { ImportPrefixDef::ByCrate => PrefixKind::ByCrate, ImportPrefixDef::BySelf => PrefixKind::BySelf, }, + group: self.data.assist_importGroup, } } pub fn completion(&self) -> CompletionConfig { diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index c1ca88df6..4235eb6dd 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -1087,7 +1087,11 @@ mod tests { add_call_parenthesis: true, add_call_argument_snippets: true, snippet_cap: SnippetCap::new(true), - insert_use: InsertUseConfig { merge: None, prefix_kind: PrefixKind::Plain }, + insert_use: InsertUseConfig { + merge: None, + prefix_kind: PrefixKind::Plain, + group: true, + }, }, ide_db::base_db::FilePosition { file_id, offset }, ) diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc index 1dbf2a611..e564c1427 100644 --- a/docs/user/generated_config.adoc +++ b/docs/user/generated_config.adoc @@ -2,6 +2,8 @@ The strategy to use when inserting new imports or merging imports. [[rust-analyzer.assist.importPrefix]]rust-analyzer.assist.importPrefix (default: `"plain"`):: The path structure for newly inserted paths to use. +[[rust-analyzer.assist.importGroup]]rust-analyzer.assist.importGroup (default: `true`):: + Group inserted imports by the [following order](https://rust-analyzer.github.io/manual.html#auto-import). Groups are separated by newlines. [[rust-analyzer.callInfo.full]]rust-analyzer.callInfo.full (default: `true`):: Show function name and docs in parameter hints. [[rust-analyzer.cargo.autoreload]]rust-analyzer.cargo.autoreload (default: `true`):: diff --git a/editors/code/package.json b/editors/code/package.json index 1987364bc..5f41f7803 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -385,6 +385,11 @@ "Force import paths to be absolute by always starting them with `crate` or the crate name they refer to." ] }, + "rust-analyzer.assist.importGroup": { + "markdownDescription": "Group inserted imports by the [following order](https://rust-analyzer.github.io/manual.html#auto-import). Groups are separated by newlines.", + "default": true, + "type": "boolean" + }, "rust-analyzer.callInfo.full": { "markdownDescription": "Show function name and docs in parameter hints.", "default": true, -- cgit v1.2.3