8873: Implement import-granularity guessing  r=matklad a=Veykril

This renames our `MergeBehavior` to `ImportGranularity` as rustfmt has it as the purpose of them are basically the same. `ImportGranularity::Preserve` currently has no specific purpose for us as we don't have an organize imports assist yet, so it currently acts the same as `ImportGranularity::Item`.

We now try to guess the import style on a per file basis and fall back to the user granularity setting if the file has no specific style yet or where it is ambiguous. This can be turned off by setting `import.enforceGranularity` to `true`.

Closes https://github.com/rust-analyzer/rust-analyzer/issues/8870

Co-authored-by: Lukas Tobias Wirth <lukastw97@gmail.com>
This commit is contained in:
bors[bot] 2021-05-20 08:27:16 +00:00 committed by GitHub
commit 8bb37737c9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 312 additions and 48 deletions

View file

@ -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,
},
};

View file

@ -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,
},
};

View file

@ -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<MergeBehavior>,
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());

View file

@ -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<MergeBehavior>,
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);
}

View file

@ -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
@ -299,7 +299,7 @@ fn eq_visibility(vis0: Option<ast::Visibility>, vis1: Option<ast::Visibility>) -
}
}
fn eq_attrs(
pub fn eq_attrs(
attrs0: impl Iterator<Item = ast::Attr>,
attrs1: impl Iterator<Item = ast::Attr>,
) -> bool {

View file

@ -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": [

View file

@ -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,
},
};

View file

@ -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,
},
},

View file

@ -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"`)::
+

View file

@ -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",