Skip to content

Commit

Permalink
Auto merge of rust-lang#16372 - davidsemakula:import-granularity-one,…
Browse files Browse the repository at this point in the history 
… r=Veykril

feat: Add "One" import granularity

Adds a new import granularity option "One" that merges all imports into a single use statement as long as they have the same visibility and attributes.

This is similar to [rustfmt's `imports_granularity = "One"`](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=import#imports_granularity).

Fixes: rust-lang#11361
  • Loading branch information
@bors
bors committed Jan 18, 2024
2 parents 48af3ef + 67c1c2b commit 3f4c6da
Show file tree
Hide file tree
Showing 10 changed files with 473 additions and 85 deletions.
2 changes: 2 additions & 0 deletions crates/ide-assists/src/handlers/auto_import.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ use crate::{AssistContext, AssistId, AssistKind, Assists, GroupLabel};
// - `item`: Don't merge imports at all, creating one import per item.
// - `preserve`: Do not change the granularity of any imports. For auto-import this has the same
// effect as `item`.
// - `one`: Merge all imports into a single use statement as long as they have the same visibility
// and attributes.
//
// In `VS Code` the configuration for this is `rust-analyzer.imports.granularity.group`.
//
Expand Down
190 changes: 169 additions & 21 deletions crates/ide-assists/src/handlers/merge_imports.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use either::Either;
use ide_db::imports::merge_imports::{try_merge_imports, try_merge_trees, MergeBehavior};
use ide_db::imports::{
insert_use::{ImportGranularity, InsertUseConfig},
merge_imports::{try_merge_imports, try_merge_trees, MergeBehavior},
};
use syntax::{
algo::neighbor,
ast::{self, edit_in_place::Removable},
Expand All @@ -16,7 +19,7 @@ use Edit::*;

// Assist: merge_imports
//
// Merges two imports with a common prefix.
// Merges neighbor imports with a common prefix.
//
// ```
// use std::$0fmt::Formatter;
Expand All @@ -29,15 +32,23 @@ use Edit::*;
pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
let (target, edits) = if ctx.has_empty_selection() {
// Merge a neighbor
let tree: ast::UseTree = ctx.find_node_at_offset()?;
let mut tree: ast::UseTree = ctx.find_node_at_offset()?;
if ctx.config.insert_use.granularity == ImportGranularity::One
&& tree.parent_use_tree_list().is_some()
{
cov_mark::hit!(resolve_top_use_tree_for_import_one);
tree = tree.top_use_tree();
}
let target = tree.syntax().text_range();

let edits = if let Some(use_item) = tree.syntax().parent().and_then(ast::Use::cast) {
cov_mark::hit!(merge_with_use_item_neighbors);
let mut neighbor = next_prev().find_map(|dir| neighbor(&use_item, dir)).into_iter();
use_item.try_merge_from(&mut neighbor)
use_item.try_merge_from(&mut neighbor, &ctx.config.insert_use)
} else {
cov_mark::hit!(merge_with_use_tree_neighbors);
let mut neighbor = next_prev().find_map(|dir| neighbor(&tree, dir)).into_iter();
tree.try_merge_from(&mut neighbor)
tree.clone().try_merge_from(&mut neighbor, &ctx.config.insert_use)
};
(target, edits?)
} else {
Expand All @@ -54,10 +65,12 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Optio
let edits = match_ast! {
match first_selected {
ast::Use(use_item) => {
use_item.try_merge_from(&mut selected_nodes.filter_map(ast::Use::cast))
cov_mark::hit!(merge_with_selected_use_item_neighbors);
use_item.try_merge_from(&mut selected_nodes.filter_map(ast::Use::cast), &ctx.config.insert_use)
},
ast::UseTree(use_tree) => {
use_tree.try_merge_from(&mut selected_nodes.filter_map(ast::UseTree::cast))
cov_mark::hit!(merge_with_selected_use_tree_neighbors);
use_tree.try_merge_from(&mut selected_nodes.filter_map(ast::UseTree::cast), &ctx.config.insert_use)
},
_ => return None,
}
Expand Down Expand Up @@ -89,11 +102,15 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Optio
}

trait Merge: AstNode + Clone {
fn try_merge_from(self, items: &mut dyn Iterator<Item = Self>) -> Option<Vec<Edit>> {
fn try_merge_from(
self,
items: &mut dyn Iterator<Item = Self>,
cfg: &InsertUseConfig,
) -> Option<Vec<Edit>> {
let mut edits = Vec::new();
let mut merged = self.clone();
for item in items {
merged = merged.try_merge(&item)?;
merged = merged.try_merge(&item, cfg)?;
edits.push(Edit::Remove(item.into_either()));
}
if !edits.is_empty() {
Expand All @@ -103,21 +120,25 @@ trait Merge: AstNode + Clone {
None
}
}
fn try_merge(&self, other: &Self) -> Option<Self>;
fn try_merge(&self, other: &Self, cfg: &InsertUseConfig) -> Option<Self>;
fn into_either(self) -> Either<ast::Use, ast::UseTree>;
}

impl Merge for ast::Use {
fn try_merge(&self, other: &Self) -> Option<Self> {
try_merge_imports(self, other, MergeBehavior::Crate)
fn try_merge(&self, other: &Self, cfg: &InsertUseConfig) -> Option<Self> {
let mb = match cfg.granularity {
ImportGranularity::One => MergeBehavior::One,
_ => MergeBehavior::Crate,
};
try_merge_imports(self, other, mb)
}
fn into_either(self) -> Either<ast::Use, ast::UseTree> {
Either::Left(self)
}
}

impl Merge for ast::UseTree {
fn try_merge(&self, other: &Self) -> Option<Self> {
fn try_merge(&self, other: &Self, _: &InsertUseConfig) -> Option<Self> {
try_merge_trees(self, other, MergeBehavior::Crate)
}
fn into_either(self) -> Either<ast::Use, ast::UseTree> {
Expand All @@ -138,12 +159,41 @@ impl Edit {

#[cfg(test)]
mod tests {
use crate::tests::{check_assist, check_assist_not_applicable};
use crate::tests::{
check_assist, check_assist_import_one, check_assist_not_applicable,
check_assist_not_applicable_for_import_one,
};

use super::*;

macro_rules! check_assist_import_one_variations {
($first: literal, $second: literal, $expected: literal) => {
check_assist_import_one(
merge_imports,
concat!(concat!("use ", $first, ";"), concat!("use ", $second, ";")),
$expected,
);
check_assist_import_one(
merge_imports,
concat!(concat!("use {", $first, "};"), concat!("use ", $second, ";")),
$expected,
);
check_assist_import_one(
merge_imports,
concat!(concat!("use ", $first, ";"), concat!("use {", $second, "};")),
$expected,
);
check_assist_import_one(
merge_imports,
concat!(concat!("use {", $first, "};"), concat!("use {", $second, "};")),
$expected,
);
};
}

#[test]
fn test_merge_equal() {
cov_mark::check!(merge_with_use_item_neighbors);
check_assist(
merge_imports,
r"
Expand All @@ -153,7 +203,19 @@ use std::fmt::{Display, Debug};
r"
use std::fmt::{Display, Debug};
",
)
);

// The assist macro below calls `check_assist_import_one` 4 times with different input
// use item variations based on the first 2 input parameters, but only 2 calls
// contain `use {std::fmt$0::{Display, Debug}};` for which the top use tree will need
// to be resolved.
cov_mark::check_count!(resolve_top_use_tree_for_import_one, 2);
cov_mark::check_count!(merge_with_use_item_neighbors, 4);
check_assist_import_one_variations!(
"std::fmt$0::{Display, Debug}",
"std::fmt::{Display, Debug}",
"use {std::fmt::{Display, Debug}};"
);
}

#[test]
Expand All @@ -167,7 +229,12 @@ use std::fmt::Display;
r"
use std::fmt::{Debug, Display};
",
)
);
check_assist_import_one_variations!(
"std::fmt$0::Debug",
"std::fmt::Display",
"use {std::fmt::{Debug, Display}};"
);
}

#[test]
Expand All @@ -182,6 +249,11 @@ use std::fmt$0::Display;
use std::fmt::{Debug, Display};
",
);
check_assist_import_one_variations!(
"std::fmt::Debug",
"std::fmt$0::Display",
"use {std::fmt::{Debug, Display}};"
);
}

#[test]
Expand All @@ -196,6 +268,11 @@ use std::fmt::Display;
use std::fmt::{self, Display};
",
);
check_assist_import_one_variations!(
"std::fmt$0",
"std::fmt::Display",
"use {std::fmt::{self, Display}};"
);
}

#[test]
Expand All @@ -211,6 +288,15 @@ use std::{fmt::{self, Display}};
);
}

#[test]
fn not_applicable_to_single_one_style_import() {
cov_mark::check!(resolve_top_use_tree_for_import_one);
check_assist_not_applicable_for_import_one(
merge_imports,
"use {std::{fmt, $0fmt::Display}};",
);
}

#[test]
fn skip_pub1() {
check_assist_not_applicable(
Expand Down Expand Up @@ -299,6 +385,7 @@ pub(in this::path) use std::fmt::{Debug, Display};

#[test]
fn test_merge_nested() {
cov_mark::check!(merge_with_use_tree_neighbors);
check_assist(
merge_imports,
r"
Expand Down Expand Up @@ -335,6 +422,11 @@ use std::{fmt::{self, Debug}};
use std::{fmt::{self, Debug, Display, Write}};
",
);
check_assist_import_one_variations!(
"std$0::{fmt::{Write, Display}}",
"std::{fmt::{self, Debug}}",
"use {std::{fmt::{self, Debug, Display, Write}}};"
);
}

#[test]
Expand All @@ -349,6 +441,11 @@ use std::{fmt::{Write, Display}};
use std::{fmt::{self, Debug, Display, Write}};
",
);
check_assist_import_one_variations!(
"std$0::{fmt::{self, Debug}}",
"std::{fmt::{Write, Display}}",
"use {std::{fmt::{self, Debug, Display, Write}}};"
);
}

#[test]
Expand All @@ -375,7 +472,12 @@ use foo::{bar};
r"
use foo::{bar::{self}};
",
)
);
check_assist_import_one_variations!(
"foo::$0{bar::{self}}",
"foo::{bar}",
"use {foo::{bar::{self}}};"
);
}

#[test]
Expand All @@ -389,7 +491,12 @@ use foo::{bar::{self}};
r"
use foo::{bar::{self}};
",
)
);
check_assist_import_one_variations!(
"foo::$0{bar}",
"foo::{bar::{self}}",
"use {foo::{bar::{self}}};"
);
}

#[test]
Expand All @@ -403,7 +510,12 @@ use std::{fmt::{self, Display}};
r"
use std::{fmt::{self, Display, *}};
",
)
);
check_assist_import_one_variations!(
"std$0::{fmt::*}",
"std::{fmt::{self, Display}}",
"use {std::{fmt::{self, Display, *}}};"
);
}

#[test]
Expand All @@ -417,7 +529,12 @@ use std::str;
r"
use std::{cell::*, str};
",
)
);
check_assist_import_one_variations!(
"std$0::cell::*",
"std::str",
"use {std::{cell::*, str}};"
);
}

#[test]
Expand All @@ -431,7 +548,12 @@ use std::str::*;
r"
use std::{cell::*, str::*};
",
)
);
check_assist_import_one_variations!(
"std$0::cell::*",
"std::str::*",
"use {std::{cell::*, str::*}};"
);
}

#[test]
Expand Down Expand Up @@ -524,10 +646,16 @@ use foo::bar::Baz;
use foo::{bar::Baz, *};
",
);
check_assist_import_one_variations!(
"foo::$0*",
"foo::bar::Baz",
"use {foo::{bar::Baz, *}};"
);
}

#[test]
fn merge_selection_uses() {
cov_mark::check!(merge_with_selected_use_item_neighbors);
check_assist(
merge_imports,
r"
Expand All @@ -541,12 +669,30 @@ $0use std::fmt::Result;
use std::fmt::Error;
use std::fmt::{Debug, Display, Write};
use std::fmt::Result;
",
);

cov_mark::check!(merge_with_selected_use_item_neighbors);
check_assist_import_one(
merge_imports,
r"
use std::fmt::Error;
$0use std::fmt::Display;
use std::fmt::Debug;
use std::fmt::Write;
$0use std::fmt::Result;
",
r"
use std::fmt::Error;
use {std::fmt::{Debug, Display, Write}};
use std::fmt::Result;
",
);
}

#[test]
fn merge_selection_use_trees() {
cov_mark::check!(merge_with_selected_use_tree_neighbors);
check_assist(
merge_imports,
r"
Expand All @@ -564,7 +710,9 @@ use std::{
fmt::Result,
};",
);

// FIXME: Remove redundant braces. See also unnecessary-braces diagnostic.
cov_mark::check!(merge_with_selected_use_tree_neighbors);
check_assist(
merge_imports,
r"use std::$0{fmt::Display, fmt::Debug}$0;",
Expand Down
Loading

0 comments on commit 3f4c6da

Please sign in to comment.