From 64f7072c255bd97a58b8344d0beeae281b8f7e13 Mon Sep 17 00:00:00 2001 From: Lukas Tobias Wirth Date: Tue, 18 May 2021 19:49:15 +0200 Subject: MergeBehavior -> ImportGranularity --- crates/ide_db/src/helpers/insert_use.rs | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) (limited to 'crates/ide_db/src/helpers/insert_use.rs') diff --git a/crates/ide_db/src/helpers/insert_use.rs b/crates/ide_db/src/helpers/insert_use.rs index 55cdc4da3..a4eb31815 100644 --- a/crates/ide_db/src/helpers/insert_use.rs +++ b/crates/ide_db/src/helpers/insert_use.rs @@ -15,9 +15,32 @@ use crate::{ 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, +} + +impl ImportGranularity { + pub fn merge_behavior(self) -> Option { + match self { + ImportGranularity::Crate => Some(MergeBehavior::Crate), + ImportGranularity::Module => Some(MergeBehavior::Module), + ImportGranularity::Preserve | ImportGranularity::Item => None, + } + } +} + #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct InsertUseConfig { - pub merge: Option, + pub granularity: ImportGranularity, pub prefix_kind: PrefixKind, pub group: bool, } @@ -73,7 +96,7 @@ pub fn insert_use<'a>(scope: &ImportScope, path: ast::Path, cfg: InsertUseConfig 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) = cfg.granularity.merge_behavior() { 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()); -- cgit v1.2.3 From b8a99692d12189406cad7215530fb5103e6c4b5d Mon Sep 17 00:00:00 2001 From: Lukas Tobias Wirth Date: Tue, 18 May 2021 20:10:39 +0200 Subject: Implement import-granularity guessing --- crates/ide_db/src/helpers/insert_use.rs | 35 +++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) (limited to 'crates/ide_db/src/helpers/insert_use.rs') diff --git a/crates/ide_db/src/helpers/insert_use.rs b/crates/ide_db/src/helpers/insert_use.rs index a4eb31815..4852121a1 100644 --- a/crates/ide_db/src/helpers/insert_use.rs +++ b/crates/ide_db/src/helpers/insert_use.rs @@ -4,7 +4,7 @@ use std::cmp::Ordering; use hir::Semantics; use syntax::{ algo, - ast::{self, make, AstNode, PathSegmentKind}, + ast::{self, make, AstNode, ModuleItemOwner, PathSegmentKind}, ted, AstToken, Direction, NodeOrToken, SyntaxNode, SyntaxToken, }; @@ -88,15 +88,46 @@ impl ImportScope { ImportScope::Module(item_list) => ImportScope::Module(item_list.clone_for_update()), } } + + fn guess_merge_behavior_from_scope(&self) -> Option { + let use_stmt = |item| match item { + ast::Item::Use(use_) => use_.use_tree(), + _ => None, + }; + let 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); + } + // has to be at least a module style based import, might be crate style tho so look further + res = Some(MergeBehavior::Module); + } + } + res + } } /// 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::Preserve => scope.guess_merge_behavior_from_scope(), + ImportGranularity::Crate => Some(MergeBehavior::Crate), + ImportGranularity::Module => Some(MergeBehavior::Module), + ImportGranularity::Item => None, + }; + 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.granularity.merge_behavior() { + 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()); -- cgit v1.2.3 From 5fd9f6c7b9944638e4781e3d9384638942f84456 Mon Sep 17 00:00:00 2001 From: Lukas Tobias Wirth Date: Tue, 18 May 2021 20:21:47 +0200 Subject: Add ImportGranularity::Guess --- crates/ide_db/src/helpers/insert_use.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) (limited to 'crates/ide_db/src/helpers/insert_use.rs') diff --git a/crates/ide_db/src/helpers/insert_use.rs b/crates/ide_db/src/helpers/insert_use.rs index 4852121a1..1fdd110cc 100644 --- a/crates/ide_db/src/helpers/insert_use.rs +++ b/crates/ide_db/src/helpers/insert_use.rs @@ -18,6 +18,8 @@ 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. @@ -28,16 +30,6 @@ pub enum ImportGranularity { Item, } -impl ImportGranularity { - pub fn merge_behavior(self) -> Option { - match self { - ImportGranularity::Crate => Some(MergeBehavior::Crate), - ImportGranularity::Module => Some(MergeBehavior::Module), - ImportGranularity::Preserve | ImportGranularity::Item => None, - } - } -} - #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct InsertUseConfig { pub granularity: ImportGranularity, @@ -118,10 +110,10 @@ impl ImportScope { pub fn insert_use<'a>(scope: &ImportScope, path: ast::Path, cfg: InsertUseConfig) { let _p = profile::span("insert_use"); let mb = match cfg.granularity { - ImportGranularity::Preserve => scope.guess_merge_behavior_from_scope(), + ImportGranularity::Guess => scope.guess_merge_behavior_from_scope(), ImportGranularity::Crate => Some(MergeBehavior::Crate), ImportGranularity::Module => Some(MergeBehavior::Module), - ImportGranularity::Item => None, + ImportGranularity::Item | ImportGranularity::Preserve => None, }; let use_item = -- cgit v1.2.3 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_db/src/helpers/insert_use.rs | 85 ++++++++++++++++++++++++++------- 1 file changed, 67 insertions(+), 18 deletions(-) (limited to 'crates/ide_db/src/helpers/insert_use.rs') 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(); -- cgit v1.2.3 From 2bf720900f94e36969af44ff8ac52470faf9af4b Mon Sep 17 00:00:00 2001 From: Lukas Tobias Wirth Date: Thu, 20 May 2021 10:25:04 +0200 Subject: Check for differing attributes in granularity guessing --- crates/ide_db/src/helpers/insert_use.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'crates/ide_db/src/helpers/insert_use.rs') diff --git a/crates/ide_db/src/helpers/insert_use.rs b/crates/ide_db/src/helpers/insert_use.rs index e7ae69302..aa61c5bcb 100644 --- a/crates/ide_db/src/helpers/insert_use.rs +++ b/crates/ide_db/src/helpers/insert_use.rs @@ -4,13 +4,13 @@ use std::cmp::Ordering; use hir::Semantics; use syntax::{ algo, - ast::{self, make, AstNode, ModuleItemOwner, PathSegmentKind, VisibilityOwner}, + ast::{self, make, AstNode, AttrsOwner, ModuleItemOwner, PathSegmentKind, VisibilityOwner}, ted, AstToken, Direction, NodeOrToken, SyntaxNode, SyntaxToken, }; use crate::{ helpers::merge_imports::{ - common_prefix, eq_visibility, try_merge_imports, use_tree_path_cmp, MergeBehavior, + common_prefix, eq_attrs, eq_visibility, try_merge_imports, use_tree_path_cmp, MergeBehavior, }, RootDatabase, }; @@ -88,7 +88,7 @@ impl ImportScope { let use_stmt = |item| match item { ast::Item::Use(use_) => { let use_tree = use_.use_tree()?; - Some((use_tree, use_.visibility())) + Some((use_tree, use_.visibility(), use_.attrs())) } _ => None, }; @@ -98,7 +98,7 @@ impl ImportScope { } .filter_map(use_stmt); let mut res = ImportGranularityGuess::Unknown; - let (mut prev, mut prev_vis) = match use_stmts.next() { + let (mut prev, mut prev_vis, mut prev_attrs) = match use_stmts.next() { Some(it) => it, None => return res, }; @@ -113,11 +113,12 @@ impl ImportScope { } } - let (curr, curr_vis) = match use_stmts.next() { + let (curr, curr_vis, curr_attrs) = match use_stmts.next() { Some(it) => it, None => break res, }; - if eq_visibility(prev_vis, curr_vis.clone()) { + 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() { @@ -133,6 +134,7 @@ impl ImportScope { } prev = curr; prev_vis = curr_vis; + prev_attrs = curr_attrs; } } } -- cgit v1.2.3