diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs index 0d3969c36d..1739302bf0 100644 --- a/crates/ide_assists/src/tests.rs +++ b/crates/ide_assists/src/tests.rs @@ -4,7 +4,10 @@ use expect_test::expect; use hir::Semantics; use ide_db::{ base_db::{fixture::WithFixture, FileId, FileRange, SourceDatabaseExt}, - helpers::{insert_use::InsertUseConfig, merge_imports::MergeBehavior, SnippetCap}, + helpers::{ + insert_use::{ImportGranularity, InsertUseConfig}, + SnippetCap, + }, source_change::FileSystemEdit, RootDatabase, }; @@ -21,8 +24,9 @@ pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig { snippet_cap: SnippetCap::new(true), allowed: None, insert_use: InsertUseConfig { - merge: Some(MergeBehavior::Crate), + granularity: ImportGranularity::Crate, prefix_kind: hir::PrefixKind::Plain, + enforce_granularity: true, group: true, }, }; diff --git a/crates/ide_completion/src/test_utils.rs b/crates/ide_completion/src/test_utils.rs index 939fb2d470..37be575e55 100644 --- a/crates/ide_completion/src/test_utils.rs +++ b/crates/ide_completion/src/test_utils.rs @@ -3,7 +3,10 @@ use hir::{PrefixKind, Semantics}; use ide_db::{ base_db::{fixture::ChangeFixture, FileLoader, FilePosition}, - helpers::{insert_use::InsertUseConfig, merge_imports::MergeBehavior, SnippetCap}, + helpers::{ + insert_use::{ImportGranularity, InsertUseConfig}, + SnippetCap, + }, RootDatabase, }; use itertools::Itertools; @@ -20,8 +23,9 @@ pub(crate) const TEST_CONFIG: CompletionConfig = CompletionConfig { add_call_argument_snippets: true, snippet_cap: SnippetCap::new(true), insert_use: InsertUseConfig { - merge: Some(MergeBehavior::Crate), + granularity: ImportGranularity::Crate, prefix_kind: PrefixKind::Plain, + enforce_granularity: true, group: true, }, }; diff --git a/crates/ide_db/src/helpers/insert_use.rs b/crates/ide_db/src/helpers/insert_use.rs index 55cdc4da3f..aa61c5bcb4 100644 --- a/crates/ide_db/src/helpers/insert_use.rs +++ b/crates/ide_db/src/helpers/insert_use.rs @@ -4,20 +4,36 @@ use std::cmp::Ordering; use hir::Semantics; use syntax::{ algo, - ast::{self, make, AstNode, PathSegmentKind}, + ast::{self, make, AstNode, AttrsOwner, ModuleItemOwner, PathSegmentKind, VisibilityOwner}, ted, AstToken, Direction, NodeOrToken, SyntaxNode, SyntaxToken, }; use crate::{ - helpers::merge_imports::{try_merge_imports, use_tree_path_cmp, MergeBehavior}, + helpers::merge_imports::{ + common_prefix, eq_attrs, eq_visibility, try_merge_imports, use_tree_path_cmp, MergeBehavior, + }, RootDatabase, }; pub use hir::PrefixKind; +/// How imports should be grouped into use statements. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum ImportGranularity { + /// Do not change the granularity of any imports and preserve the original structure written by the developer. + Preserve, + /// Merge imports from the same crate into a single use statement. + Crate, + /// Merge imports from the same module into a single use statement. + Module, + /// Flatten imports so that each has its own use statement. + Item, +} + #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct InsertUseConfig { - pub merge: Option, + pub granularity: ImportGranularity, + pub enforce_granularity: bool, pub prefix_kind: PrefixKind, pub group: bool, } @@ -65,15 +81,96 @@ impl ImportScope { ImportScope::Module(item_list) => ImportScope::Module(item_list.clone_for_update()), } } + + fn guess_granularity_from_scope(&self) -> ImportGranularityGuess { + // The idea is simple, just check each import as well as the import and its precedent together for + // whether they fulfill a granularity criteria. + let use_stmt = |item| match item { + ast::Item::Use(use_) => { + let use_tree = use_.use_tree()?; + Some((use_tree, use_.visibility(), use_.attrs())) + } + _ => None, + }; + let mut use_stmts = match self { + ImportScope::File(f) => f.items(), + ImportScope::Module(m) => m.items(), + } + .filter_map(use_stmt); + let mut res = ImportGranularityGuess::Unknown; + let (mut prev, mut prev_vis, mut prev_attrs) = match use_stmts.next() { + Some(it) => it, + None => return res, + }; + loop { + if let Some(use_tree_list) = prev.use_tree_list() { + if use_tree_list.use_trees().any(|tree| tree.use_tree_list().is_some()) { + // Nested tree lists can only occur in crate style, or with no proper style being enforced in the file. + break ImportGranularityGuess::Crate; + } else { + // Could still be crate-style so continue looking. + res = ImportGranularityGuess::CrateOrModule; + } + } + + let (curr, curr_vis, curr_attrs) = match use_stmts.next() { + Some(it) => it, + None => break res, + }; + if eq_visibility(prev_vis, curr_vis.clone()) && eq_attrs(prev_attrs, curr_attrs.clone()) + { + if let Some((prev_path, curr_path)) = prev.path().zip(curr.path()) { + if let Some(_) = common_prefix(&prev_path, &curr_path) { + if prev.use_tree_list().is_none() && curr.use_tree_list().is_none() { + // Same prefix but no use tree lists so this has to be of item style. + break ImportGranularityGuess::Item; // this overwrites CrateOrModule, technically the file doesn't adhere to anything here. + } else { + // Same prefix with item tree lists, has to be module style as it + // can't be crate style since the trees wouldn't share a prefix then. + break ImportGranularityGuess::Module; + } + } + } + } + prev = curr; + prev_vis = curr_vis; + prev_attrs = curr_attrs; + } + } +} + +#[derive(PartialEq, PartialOrd, Debug, Clone, Copy)] +enum ImportGranularityGuess { + Unknown, + Item, + Module, + Crate, + CrateOrModule, } /// 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) { let _p = profile::span("insert_use"); + let mut mb = match cfg.granularity { + ImportGranularity::Crate => Some(MergeBehavior::Crate), + ImportGranularity::Module => Some(MergeBehavior::Module), + ImportGranularity::Item | ImportGranularity::Preserve => None, + }; + if !cfg.enforce_granularity { + let file_granularity = scope.guess_granularity_from_scope(); + mb = match file_granularity { + ImportGranularityGuess::Unknown => mb, + ImportGranularityGuess::Item => None, + ImportGranularityGuess::Module => Some(MergeBehavior::Module), + ImportGranularityGuess::Crate => Some(MergeBehavior::Crate), + ImportGranularityGuess::CrateOrModule => mb.or(Some(MergeBehavior::Crate)), + }; + } + let use_item = make::use_(None, make::use_tree(path.clone(), None, None, false)).clone_for_update(); // merge into existing imports if possible - if let Some(mb) = cfg.merge { + if let Some(mb) = mb { 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) { ted::replace(existing_use.syntax(), merged.syntax()); diff --git a/crates/ide_db/src/helpers/insert_use/tests.rs b/crates/ide_db/src/helpers/insert_use/tests.rs index 248227d295..78a2a87b38 100644 --- a/crates/ide_db/src/helpers/insert_use/tests.rs +++ b/crates/ide_db/src/helpers/insert_use/tests.rs @@ -21,7 +21,7 @@ use crate::bar::A; use self::bar::A; use super::bar::A; use external_crate2::bar::A;", - None, + ImportGranularity::Item, false, false, ); @@ -36,7 +36,7 @@ fn insert_not_group_empty() { r"use external_crate2::bar::A; ", - None, + ImportGranularity::Item, false, false, ); @@ -281,7 +281,7 @@ fn insert_empty_module() { r"{ use foo::bar; }", - None, + ImportGranularity::Item, true, true, ) @@ -631,11 +631,121 @@ fn merge_last_fail3() { ); } +#[test] +fn guess_empty() { + check_guess("", ImportGranularityGuess::Unknown); +} + +#[test] +fn guess_single() { + check_guess(r"use foo::{baz::{qux, quux}, bar};", ImportGranularityGuess::Crate); + check_guess(r"use foo::bar;", ImportGranularityGuess::Unknown); + check_guess(r"use foo::bar::{baz, qux};", ImportGranularityGuess::CrateOrModule); +} + +#[test] +fn guess_unknown() { + check_guess( + r" +use foo::bar::baz; +use oof::rab::xuq; +", + ImportGranularityGuess::Unknown, + ); +} + +#[test] +fn guess_item() { + check_guess( + r" +use foo::bar::baz; +use foo::bar::qux; +", + ImportGranularityGuess::Item, + ); +} + +#[test] +fn guess_module() { + check_guess( + r" +use foo::bar::baz; +use foo::bar::{qux, quux}; +", + ImportGranularityGuess::Module, + ); + // this is a rather odd case, technically this file isn't following any style properly. + check_guess( + r" +use foo::bar::baz; +use foo::{baz::{qux, quux}, bar}; +", + ImportGranularityGuess::Module, + ); +} + +#[test] +fn guess_crate_or_module() { + check_guess( + r" +use foo::bar::baz; +use oof::bar::{qux, quux}; +", + ImportGranularityGuess::CrateOrModule, + ); +} + +#[test] +fn guess_crate() { + check_guess( + r" +use frob::bar::baz; +use foo::{baz::{qux, quux}, bar}; +", + ImportGranularityGuess::Crate, + ); +} + +#[test] +fn guess_skips_differing_vis() { + check_guess( + r" +use foo::bar::baz; +pub use foo::bar::qux; +", + ImportGranularityGuess::Unknown, + ); +} + +#[test] +fn guess_skips_differing_attrs() { + check_guess( + r" +pub use foo::bar::baz; +#[doc(hidden)] +pub use foo::bar::qux; +", + ImportGranularityGuess::Unknown, + ); +} + +#[test] +fn guess_grouping_matters() { + check_guess( + r" +use foo::bar::baz; +use oof::bar::baz; +use foo::bar::qux; +", + ImportGranularityGuess::Unknown, + ); +} + fn check( path: &str, ra_fixture_before: &str, ra_fixture_after: &str, - mb: Option, + granularity: ImportGranularity, module: bool, group: bool, ) { @@ -651,21 +761,30 @@ fn check( .find_map(ast::Path::cast) .unwrap(); - insert_use(&file, path, InsertUseConfig { merge: mb, prefix_kind: PrefixKind::Plain, group }); + insert_use( + &file, + path, + InsertUseConfig { + granularity, + enforce_granularity: true, + prefix_kind: PrefixKind::Plain, + group, + }, + ); 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) { - check(path, ra_fixture_before, ra_fixture_after, Some(MergeBehavior::Crate), false, true) + check(path, ra_fixture_before, ra_fixture_after, ImportGranularity::Crate, false, true) } fn check_module(path: &str, ra_fixture_before: &str, ra_fixture_after: &str) { - check(path, ra_fixture_before, ra_fixture_after, Some(MergeBehavior::Module), false, true) + check(path, ra_fixture_before, ra_fixture_after, ImportGranularity::Module, 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, true) + check(path, ra_fixture_before, ra_fixture_after, ImportGranularity::Item, false, true) } fn check_merge_only_fail(ra_fixture0: &str, ra_fixture1: &str, mb: MergeBehavior) { @@ -686,3 +805,9 @@ fn check_merge_only_fail(ra_fixture0: &str, ra_fixture1: &str, mb: MergeBehavior let result = try_merge_imports(&use0, &use1, mb); assert_eq!(result.map(|u| u.to_string()), None); } + +fn check_guess(ra_fixture: &str, expected: ImportGranularityGuess) { + let syntax = ast::SourceFile::parse(ra_fixture).tree().syntax().clone(); + let file = super::ImportScope::from(syntax).unwrap(); + assert_eq!(file.guess_granularity_from_scope(), expected); +} diff --git a/crates/ide_db/src/helpers/merge_imports.rs b/crates/ide_db/src/helpers/merge_imports.rs index 8fb40e8371..697e8bcffa 100644 --- a/crates/ide_db/src/helpers/merge_imports.rs +++ b/crates/ide_db/src/helpers/merge_imports.rs @@ -181,7 +181,7 @@ fn recursive_merge( } /// Traverses both paths until they differ, returning the common prefix of both. -fn common_prefix(lhs: &ast::Path, rhs: &ast::Path) -> Option<(ast::Path, ast::Path)> { +pub fn common_prefix(lhs: &ast::Path, rhs: &ast::Path) -> Option<(ast::Path, ast::Path)> { let mut res = None; let mut lhs_curr = lhs.first_qualifier_or_self(); let mut rhs_curr = rhs.first_qualifier_or_self(); @@ -289,7 +289,7 @@ fn path_segment_cmp(a: &ast::PathSegment, b: &ast::PathSegment) -> Ordering { a.as_ref().map(ast::NameRef::text).cmp(&b.as_ref().map(ast::NameRef::text)) } -fn eq_visibility(vis0: Option, vis1: Option) -> bool { +pub fn eq_visibility(vis0: Option, vis1: Option) -> bool { match (vis0, vis1) { (None, None) => true, // FIXME: Don't use the string representation to check for equality @@ -299,7 +299,7 @@ fn eq_visibility(vis0: Option, vis1: Option) - } } -fn eq_attrs( +pub fn eq_attrs( attrs0: impl Iterator, attrs1: impl Iterator, ) -> bool { diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 339014fd31..b700d025f1 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -12,8 +12,7 @@ use std::{ffi::OsString, iter, path::PathBuf}; use flycheck::FlycheckConfig; use ide::{AssistConfig, CompletionConfig, DiagnosticsConfig, HoverConfig, InlayHintsConfig}; use ide_db::helpers::{ - insert_use::{InsertUseConfig, PrefixKind}, - merge_imports::MergeBehavior, + insert_use::{ImportGranularity, InsertUseConfig, PrefixKind}, SnippetCap, }; use lsp_types::{ClientCapabilities, MarkupKind}; @@ -35,15 +34,18 @@ use crate::{ // be specified directly in `package.json`. config_data! { struct ConfigData { - /// The strategy to use when inserting new imports or merging imports. + /// How imports should be grouped into use statements. + assist_importGranularity | assist_importMergeBehavior | - assist_importMergeBehaviour: MergeBehaviorDef = "\"crate\"", + assist_importMergeBehaviour: ImportGranularityDef = "\"crate\"", + /// Whether to enforce the import granularity setting for all files. If set to false rust-analyzer will try to keep import styles consistent per file. + assist_importEnforceGranularity: bool = "false", /// The path structure for newly inserted paths to use. - assist_importPrefix: ImportPrefixDef = "\"plain\"", + 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", + assist_importGroup: bool = "true", /// Show function name and docs in parameter hints. - callInfo_full: bool = "true", + callInfo_full: bool = "true", /// Automatically refresh project info via `cargo metadata` on /// `Cargo.toml` changes. @@ -624,11 +626,13 @@ impl Config { } fn insert_use_config(&self) -> InsertUseConfig { InsertUseConfig { - merge: match self.data.assist_importMergeBehavior { - MergeBehaviorDef::None => None, - MergeBehaviorDef::Crate => Some(MergeBehavior::Crate), - MergeBehaviorDef::Module => Some(MergeBehavior::Module), + granularity: match self.data.assist_importGranularity { + ImportGranularityDef::Preserve => ImportGranularity::Preserve, + ImportGranularityDef::Item => ImportGranularity::Item, + ImportGranularityDef::Crate => ImportGranularity::Crate, + ImportGranularityDef::Module => ImportGranularity::Module, }, + enforce_granularity: self.data.assist_importEnforceGranularity, prefix_kind: match self.data.assist_importPrefix { ImportPrefixDef::Plain => PrefixKind::Plain, ImportPrefixDef::ByCrate => PrefixKind::ByCrate, @@ -748,8 +752,10 @@ enum ManifestOrProjectJson { #[derive(Deserialize, Debug, Clone)] #[serde(rename_all = "snake_case")] -enum MergeBehaviorDef { - None, +enum ImportGranularityDef { + Preserve, + #[serde(alias = "none")] + Item, #[serde(alias = "full")] Crate, #[serde(alias = "last")] @@ -782,7 +788,7 @@ macro_rules! _config_data { (struct $name:ident { $( $(#[doc=$doc:literal])* - $field:ident $(| $alias:ident)?: $ty:ty = $default:expr, + $field:ident $(| $alias:ident)*: $ty:ty = $default:expr, )* }) => { #[allow(non_snake_case)] @@ -794,7 +800,7 @@ macro_rules! _config_data { $field: get_field( &mut json, stringify!($field), - None$(.or(Some(stringify!($alias))))?, + None$(.or(Some(stringify!($alias))))*, $default, ), )*} @@ -931,6 +937,16 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json "Merge imports from the same module into a single `use` statement." ], }, + "ImportGranularityDef" => set! { + "type": "string", + "enum": ["preserve", "crate", "module", "item"], + "enumDescriptions": [ + "Do not change the granularity of any imports and preserve the original structure written by the developer.", + "Merge imports from the same crate into a single use statement. Conversely, imports from different crates are split into separate statements.", + "Merge imports from the same module into a single use statement. Conversely, imports from different modules are split into separate statements.", + "Flatten imports so that each has its own use statement." + ], + }, "ImportPrefixDef" => set! { "type": "string", "enum": [ diff --git a/crates/rust-analyzer/src/integrated_benchmarks.rs b/crates/rust-analyzer/src/integrated_benchmarks.rs index ba2790acbd..781073fe5b 100644 --- a/crates/rust-analyzer/src/integrated_benchmarks.rs +++ b/crates/rust-analyzer/src/integrated_benchmarks.rs @@ -13,7 +13,10 @@ use std::{convert::TryFrom, sync::Arc}; use ide::{Change, CompletionConfig, FilePosition, TextSize}; -use ide_db::helpers::{insert_use::InsertUseConfig, merge_imports::MergeBehavior, SnippetCap}; +use ide_db::helpers::{ + insert_use::{ImportGranularity, InsertUseConfig}, + SnippetCap, +}; use test_utils::project_root; use vfs::{AbsPathBuf, VfsPath}; @@ -133,8 +136,9 @@ fn integrated_completion_benchmark() { add_call_argument_snippets: true, snippet_cap: SnippetCap::new(true), insert_use: InsertUseConfig { - merge: Some(MergeBehavior::Crate), + granularity: ImportGranularity::Crate, prefix_kind: hir::PrefixKind::ByCrate, + enforce_granularity: true, group: true, }, }; @@ -166,8 +170,9 @@ fn integrated_completion_benchmark() { add_call_argument_snippets: true, snippet_cap: SnippetCap::new(true), insert_use: InsertUseConfig { - merge: Some(MergeBehavior::Crate), + granularity: ImportGranularity::Crate, prefix_kind: hir::PrefixKind::ByCrate, + enforce_granularity: true, group: true, }, }; diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 9dec46c789..ef9e0aee9b 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -1145,7 +1145,7 @@ mod tests { use ide::Analysis; use ide_db::helpers::{ - insert_use::{InsertUseConfig, PrefixKind}, + insert_use::{ImportGranularity, InsertUseConfig, PrefixKind}, SnippetCap, }; @@ -1177,8 +1177,9 @@ mod tests { add_call_argument_snippets: true, snippet_cap: SnippetCap::new(true), insert_use: InsertUseConfig { - merge: None, + granularity: ImportGranularity::Item, prefix_kind: PrefixKind::Plain, + enforce_granularity: true, group: true, }, }, diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc index b324118875..c02bab7cc3 100644 --- a/docs/user/generated_config.adoc +++ b/docs/user/generated_config.adoc @@ -1,7 +1,12 @@ -[[rust-analyzer.assist.importMergeBehavior]]rust-analyzer.assist.importMergeBehavior (default: `"crate"`):: +[[rust-analyzer.assist.importGranularity]]rust-analyzer.assist.importGranularity (default: `"crate"`):: + -- -The strategy to use when inserting new imports or merging imports. +How imports should be grouped into use statements. +-- +[[rust-analyzer.assist.importEnforceGranularity]]rust-analyzer.assist.importEnforceGranularity (default: `false`):: ++ +-- +Whether to enforce the import granularity setting for all files. If set to false rust-analyzer will try to keep import styles consistent per file. -- [[rust-analyzer.assist.importPrefix]]rust-analyzer.assist.importPrefix (default: `"plain"`):: + diff --git a/editors/code/package.json b/editors/code/package.json index 99223c4e8c..1743b374c0 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -385,21 +385,28 @@ "markdownDescription": "Optional settings passed to the debug engine. Example: `{ \"lldb\": { \"terminal\":\"external\"} }`" }, "$generated-start": false, - "rust-analyzer.assist.importMergeBehavior": { - "markdownDescription": "The strategy to use when inserting new imports or merging imports.", + "rust-analyzer.assist.importGranularity": { + "markdownDescription": "How imports should be grouped into use statements.", "default": "crate", "type": "string", "enum": [ - "none", + "preserve", "crate", - "module" + "module", + "item" ], "enumDescriptions": [ - "Do not merge imports at all.", - "Merge imports from the same crate into a single `use` statement.", - "Merge imports from the same module into a single `use` statement." + "Do not change the granularity of any imports and preserve the original structure written by the developer.", + "Merge imports from the same crate into a single use statement. Conversely, imports from different crates are split into separate statements.", + "Merge imports from the same module into a single use statement. Conversely, imports from different modules are split into separate statements.", + "Flatten imports so that each has its own use statement." ] }, + "rust-analyzer.assist.importEnforceGranularity": { + "markdownDescription": "Whether to enforce the import granularity setting for all files. If set to false rust-analyzer will try to keep import styles consistent per file.", + "default": false, + "type": "boolean" + }, "rust-analyzer.assist.importPrefix": { "markdownDescription": "The path structure for newly inserted paths to use.", "default": "plain",