Replace ImportGranularity::Guess with guessing boolean flag

8 files changed, 206 insertions, 28 deletions

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<MergeBehavior> {
+    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<ast::Visibility>, vis1: Option<ast::Visibility>) -> bool {
+pub fn eq_visibility(vis0: Option<ast::Visibility>, vis1: Option<ast::Visibility>) -> 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,
                     },
                 },