From b4fe479236f592fcbfa1422dda54253b77d8b0e1 Mon Sep 17 00:00:00 2001 From: Lukas Tobias Wirth Date: Tue, 18 May 2021 20:21:47 +0200 Subject: Replace ImportGranularity::Guess with guessing boolean flag --- crates/ide_assists/src/tests.rs | 1 + crates/ide_completion/src/test_utils.rs | 1 + crates/ide_db/src/helpers/insert_use.rs | 85 ++++++++++++---- crates/ide_db/src/helpers/insert_use/tests.rs | 115 +++++++++++++++++++++- crates/ide_db/src/helpers/merge_imports.rs | 4 +- crates/rust-analyzer/src/config.rs | 25 +++-- crates/rust-analyzer/src/integrated_benchmarks.rs | 2 + crates/rust-analyzer/src/to_proto.rs | 1 + 8 files changed, 206 insertions(+), 28 deletions(-) (limited to 'crates') diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs index bb1ca0b3d..1739302bf 100644 --- a/crates/ide_assists/src/tests.rs +++ b/crates/ide_assists/src/tests.rs @@ -26,6 +26,7 @@ pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig { insert_use: InsertUseConfig { 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 b150a5c3f..37be575e5 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 { 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 1fdd110cc..e7ae69302 100644 --- a/crates/ide_db/src/helpers/insert_use.rs +++ b/crates/ide_db/src/helpers/insert_use.rs @@ -4,12 +4,14 @@ use std::cmp::Ordering; use hir::Semantics; use syntax::{ algo, - ast::{self, make, AstNode, ModuleItemOwner, PathSegmentKind}, + ast::{self, make, AstNode, 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_visibility, try_merge_imports, use_tree_path_cmp, MergeBehavior, + }, RootDatabase, }; @@ -18,8 +20,6 @@ pub use hir::PrefixKind; /// How imports should be grouped into use statements. #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum ImportGranularity { - /// Try to guess the granularity of imports on a per module basis by observing the existing imports. - Guess, /// 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. @@ -33,6 +33,7 @@ pub enum ImportGranularity { #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct InsertUseConfig { pub granularity: ImportGranularity, + pub enforce_granularity: bool, pub prefix_kind: PrefixKind, pub group: bool, } @@ -81,40 +82,88 @@ impl ImportScope { } } - fn guess_merge_behavior_from_scope(&self) -> Option { + 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_) => use_.use_tree(), + ast::Item::Use(use_) => { + let use_tree = use_.use_tree()?; + Some((use_tree, use_.visibility())) + } _ => None, }; - let use_stmts = match self { + let mut use_stmts = match self { ImportScope::File(f) => f.items(), ImportScope::Module(m) => m.items(), } .filter_map(use_stmt); - let mut res = None; - for tree in use_stmts { - if let Some(list) = tree.use_tree_list() { - if list.use_trees().any(|tree| tree.use_tree_list().is_some()) { - // double nested tree list, can only be a crate style import at this point - return Some(MergeBehavior::Crate); + let mut res = ImportGranularityGuess::Unknown; + let (mut prev, mut prev_vis) = 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; } - // has to be at least a module style based import, might be crate style tho so look further - res = Some(MergeBehavior::Module); } + + let (curr, curr_vis) = match use_stmts.next() { + Some(it) => it, + None => break res, + }; + if eq_visibility(prev_vis, curr_vis.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; } - res } } +#[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 mb = match cfg.granularity { - ImportGranularity::Guess => scope.guess_merge_behavior_from_scope(), + 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(); diff --git a/crates/ide_db/src/helpers/insert_use/tests.rs b/crates/ide_db/src/helpers/insert_use/tests.rs index f99857a89..f795bbf00 100644 --- a/crates/ide_db/src/helpers/insert_use/tests.rs +++ b/crates/ide_db/src/helpers/insert_use/tests.rs @@ -631,6 +631,104 @@ 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_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, @@ -651,7 +749,16 @@ fn check( .find_map(ast::Path::cast) .unwrap(); - insert_use(&file, path, InsertUseConfig { granularity, 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); } @@ -686,3 +793,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 8fb40e837..546f2d0c4 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 diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index a34ca97ac..867d72ea4 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -33,16 +33,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: ImportGranularityDef = "\"guess\"", + 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. @@ -610,12 +612,12 @@ impl Config { fn insert_use_config(&self) -> InsertUseConfig { InsertUseConfig { granularity: match self.data.assist_importGranularity { - ImportGranularityDef::Guess => ImportGranularity::Guess, 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, @@ -721,7 +723,6 @@ enum ManifestOrProjectJson { #[serde(rename_all = "snake_case")] enum ImportGranularityDef { Preserve, - Guess, #[serde(alias = "none")] Item, #[serde(alias = "full")] @@ -891,6 +892,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 cd0b8d559..781073fe5 100644 --- a/crates/rust-analyzer/src/integrated_benchmarks.rs +++ b/crates/rust-analyzer/src/integrated_benchmarks.rs @@ -138,6 +138,7 @@ fn integrated_completion_benchmark() { insert_use: InsertUseConfig { granularity: ImportGranularity::Crate, prefix_kind: hir::PrefixKind::ByCrate, + enforce_granularity: true, group: true, }, }; @@ -171,6 +172,7 @@ fn integrated_completion_benchmark() { insert_use: InsertUseConfig { 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 64d5f9e2e..ef9e0aee9 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -1179,6 +1179,7 @@ mod tests { insert_use: InsertUseConfig { granularity: ImportGranularity::Item, prefix_kind: PrefixKind::Plain, + enforce_granularity: true, group: true, }, }, -- cgit v1.2.3