Auto merge of #16417 - davidsemakula:normalize-use-trees, r=Veykril

feat: "Normalize import" assist and utilities for normalizing use trees

- Add import/use tree normalization utilities
- Add "normalize import" assist
- Update "merge imports" assist to always apply to the covering use item except for nested use tree selections
- Update "merge imports" assist to avoid adding unnecessary braces when merging nested use tree selections

See [this discussion](https://github.com/rust-lang/rust-analyzer/pull/16372#discussion_r1457244321) for the motivation for the new "normalize import" assist and changes to the "merge imports" assist.
This commit is contained in:
bors 2024-01-30 14:55:08 +00:00
commit 63123ab408
9 changed files with 645 additions and 150 deletions

View file

@ -1,8 +1,9 @@
use either::Either; use either::Either;
use ide_db::imports::{ use ide_db::imports::{
insert_use::{ImportGranularity, InsertUseConfig}, insert_use::{ImportGranularity, InsertUseConfig},
merge_imports::{try_merge_imports, try_merge_trees, MergeBehavior}, merge_imports::{try_merge_imports, try_merge_trees, try_normalize_use_tree, MergeBehavior},
}; };
use itertools::Itertools;
use syntax::{ use syntax::{
algo::neighbor, algo::neighbor,
ast::{self, edit_in_place::Removable}, ast::{self, edit_in_place::Removable},
@ -32,24 +33,13 @@ use Edit::*;
pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
let (target, edits) = if ctx.has_empty_selection() { let (target, edits) = if ctx.has_empty_selection() {
// Merge a neighbor // Merge a neighbor
let mut tree: ast::UseTree = ctx.find_node_at_offset()?; cov_mark::hit!(merge_with_use_item_neighbors);
if ctx.config.insert_use.granularity == ImportGranularity::One let tree = ctx.find_node_at_offset::<ast::UseTree>()?.top_use_tree();
&& 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 target = tree.syntax().text_range();
let edits = if let Some(use_item) = tree.syntax().parent().and_then(ast::Use::cast) { let 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(); let mut neighbor = next_prev().find_map(|dir| neighbor(&use_item, dir)).into_iter();
use_item.try_merge_from(&mut neighbor, &ctx.config.insert_use) let edits = 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.clone().try_merge_from(&mut neighbor, &ctx.config.insert_use)
};
(target, edits?) (target, edits?)
} else { } else {
// Merge selected // Merge selected
@ -94,7 +84,35 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Optio
for edit in edits_mut { for edit in edits_mut {
match edit { match edit {
Remove(it) => it.as_ref().either(Removable::remove, Removable::remove), Remove(it) => it.as_ref().either(Removable::remove, Removable::remove),
Replace(old, new) => ted::replace(old, new), Replace(old, new) => {
ted::replace(old, &new);
// If there's a selection and we're replacing a use tree in a tree list,
// normalize the parent use tree if it only contains the merged subtree.
if !ctx.has_empty_selection() {
let normalized_use_tree = ast::UseTree::cast(new)
.as_ref()
.and_then(ast::UseTree::parent_use_tree_list)
.and_then(|use_tree_list| {
if use_tree_list.use_trees().collect_tuple::<(_,)>().is_some() {
Some(use_tree_list.parent_use_tree())
} else {
None
}
})
.and_then(|target_tree| {
try_normalize_use_tree(
&target_tree,
ctx.config.insert_use.granularity.into(),
)
.map(|top_use_tree_flat| (target_tree, top_use_tree_flat))
});
if let Some((old_tree, new_tree)) = normalized_use_tree {
cov_mark::hit!(replace_parent_with_normalized_use_tree);
ted::replace(old_tree.syntax(), new_tree.syntax());
}
}
}
} }
} }
}, },
@ -201,20 +219,17 @@ use std::fmt$0::{Display, Debug};
use std::fmt::{Display, Debug}; use std::fmt::{Display, Debug};
", ",
r" r"
use std::fmt::{Display, Debug}; use std::fmt::{Debug, Display};
", ",
); );
// The assist macro below calls `check_assist_import_one` 4 times with different input // 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 // use item variations based on the first 2 input parameters.
// 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); cov_mark::check_count!(merge_with_use_item_neighbors, 4);
check_assist_import_one_variations!( check_assist_import_one_variations!(
"std::fmt$0::{Display, Debug}", "std::fmt$0::{Display, Debug}",
"std::fmt::{Display, Debug}", "std::fmt::{Display, Debug}",
"use {std::fmt::{Display, Debug}};" "use {std::fmt::{Debug, Display}};"
); );
} }
@ -257,7 +272,7 @@ use std::fmt::{Debug, Display};
} }
#[test] #[test]
fn merge_self1() { fn merge_self() {
check_assist( check_assist(
merge_imports, merge_imports,
r" r"
@ -276,21 +291,8 @@ use std::fmt::{self, Display};
} }
#[test] #[test]
fn merge_self2() { fn not_applicable_to_single_import() {
check_assist( check_assist_not_applicable(merge_imports, "use std::{fmt, $0fmt::Display};");
merge_imports,
r"
use std::{fmt, $0fmt::Display};
",
r"
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( check_assist_not_applicable_for_import_one(
merge_imports, merge_imports,
"use {std::{fmt, $0fmt::Display}};", "use {std::{fmt, $0fmt::Display}};",
@ -385,14 +387,14 @@ pub(in this::path) use std::fmt::{Debug, Display};
#[test] #[test]
fn test_merge_nested() { fn test_merge_nested() {
cov_mark::check!(merge_with_use_tree_neighbors);
check_assist( check_assist(
merge_imports, merge_imports,
r" r"
use std::{fmt$0::Debug, fmt::Display}; use std::{fmt$0::Debug, fmt::Error};
use std::{fmt::Write, fmt::Display};
", ",
r" r"
use std::{fmt::{Debug, Display}}; use std::fmt::{Debug, Display, Error, Write};
", ",
); );
} }
@ -402,10 +404,11 @@ use std::{fmt::{Debug, Display}};
check_assist( check_assist(
merge_imports, merge_imports,
r" r"
use std::{fmt::Debug, fmt$0::Display}; use std::{fmt::Debug, fmt$0::Error};
use std::{fmt::Write, fmt::Display};
", ",
r" r"
use std::{fmt::{Debug, Display}}; use std::fmt::{Debug, Display, Error, Write};
", ",
); );
} }
@ -419,13 +422,13 @@ use std$0::{fmt::{Write, Display}};
use std::{fmt::{self, Debug}}; use std::{fmt::{self, Debug}};
", ",
r" r"
use std::{fmt::{self, Debug, Display, Write}}; use std::fmt::{self, Debug, Display, Write};
", ",
); );
check_assist_import_one_variations!( check_assist_import_one_variations!(
"std$0::{fmt::{Write, Display}}", "std$0::{fmt::{Write, Display}}",
"std::{fmt::{self, Debug}}", "std::{fmt::{self, Debug}}",
"use {std::{fmt::{self, Debug, Display, Write}}};" "use {std::fmt::{self, Debug, Display, Write}};"
); );
} }
@ -438,26 +441,13 @@ use std$0::{fmt::{self, Debug}};
use std::{fmt::{Write, Display}}; use std::{fmt::{Write, Display}};
", ",
r" r"
use std::{fmt::{self, Debug, Display, Write}}; use std::fmt::{self, Debug, Display, Write};
", ",
); );
check_assist_import_one_variations!( check_assist_import_one_variations!(
"std$0::{fmt::{self, Debug}}", "std$0::{fmt::{self, Debug}}",
"std::{fmt::{Write, Display}}", "std::{fmt::{Write, Display}}",
"use {std::{fmt::{self, Debug, Display, Write}}};" "use {std::fmt::{self, Debug, Display, Write}};"
);
}
#[test]
fn test_merge_self_with_nested_self_item() {
check_assist(
merge_imports,
r"
use std::{fmt$0::{self, Debug}, fmt::{Write, Display}};
",
r"
use std::{fmt::{self, Debug, Display, Write}};
",
); );
} }
@ -470,13 +460,13 @@ use foo::$0{bar::{self}};
use foo::{bar}; use foo::{bar};
", ",
r" r"
use foo::{bar::{self}}; use foo::bar;
", ",
); );
check_assist_import_one_variations!( check_assist_import_one_variations!(
"foo::$0{bar::{self}}", "foo::$0{bar::{self}}",
"foo::{bar}", "foo::{bar}",
"use {foo::{bar::{self}}};" "use {foo::bar};"
); );
} }
@ -489,13 +479,13 @@ use foo::$0{bar};
use foo::{bar::{self}}; use foo::{bar::{self}};
", ",
r" r"
use foo::{bar::{self}}; use foo::bar;
", ",
); );
check_assist_import_one_variations!( check_assist_import_one_variations!(
"foo::$0{bar}", "foo::$0{bar}",
"foo::{bar::{self}}", "foo::{bar::{self}}",
"use {foo::{bar::{self}}};" "use {foo::bar};"
); );
} }
@ -508,13 +498,13 @@ use std$0::{fmt::*};
use std::{fmt::{self, Display}}; use std::{fmt::{self, Display}};
", ",
r" r"
use std::{fmt::{self, Display, *}}; use std::fmt::{self, Display, *};
", ",
); );
check_assist_import_one_variations!( check_assist_import_one_variations!(
"std$0::{fmt::*}", "std$0::{fmt::*}",
"std::{fmt::{self, Display}}", "std::{fmt::{self, Display}}",
"use {std::{fmt::{self, Display, *}}};" "use {std::fmt::{self, Display, *}};"
); );
} }
@ -579,29 +569,27 @@ use foo::{bar, baz};
check_assist( check_assist(
merge_imports, merge_imports,
r" r"
use { use foo$0::{
foo$0::bar, bar, baz,
foo::baz,
}; };
use foo::qux;
", ",
r" r"
use { use foo::{
foo::{bar, baz}, bar, baz, qux,
}; };
", ",
); );
check_assist( check_assist(
merge_imports, merge_imports,
r" r"
use { use foo::{
foo::baz, baz, bar,
foo$0::bar,
}; };
use foo$0::qux;
", ",
r" r"
use { use foo::{bar, baz, qux};
foo::{bar, baz},
};
", ",
); );
} }
@ -711,12 +699,19 @@ use std::{
};", };",
); );
// FIXME: Remove redundant braces. See also unnecessary-braces diagnostic.
cov_mark::check!(merge_with_selected_use_tree_neighbors); cov_mark::check!(merge_with_selected_use_tree_neighbors);
check_assist(
merge_imports,
r"use std::{fmt::Result, $0fmt::Display, fmt::Debug$0};",
r"use std::{fmt::Result, fmt::{Debug, Display}};",
);
cov_mark::check!(merge_with_selected_use_tree_neighbors);
cov_mark::check!(replace_parent_with_normalized_use_tree);
check_assist( check_assist(
merge_imports, merge_imports,
r"use std::$0{fmt::Display, fmt::Debug}$0;", r"use std::$0{fmt::Display, fmt::Debug}$0;",
r"use std::{fmt::{Debug, Display}};", r"use std::fmt::{Debug, Display};",
); );
} }
} }

View file

@ -0,0 +1,219 @@
use ide_db::imports::merge_imports::try_normalize_import;
use syntax::{ast, AstNode};
use crate::{
assist_context::{AssistContext, Assists},
AssistId, AssistKind,
};
// Assist: normalize_import
//
// Normalizes an import.
//
// ```
// use$0 std::{io, {fmt::Formatter}};
// ```
// ->
// ```
// use std::{fmt::Formatter, io};
// ```
pub(crate) fn normalize_import(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
let use_item = if ctx.has_empty_selection() {
ctx.find_node_at_offset()?
} else {
ctx.covering_element().ancestors().find_map(ast::Use::cast)?
};
let target = use_item.syntax().text_range();
let normalized_use_item =
try_normalize_import(&use_item, ctx.config.insert_use.granularity.into())?;
acc.add(
AssistId("normalize_import", AssistKind::RefactorRewrite),
"Normalize import",
target,
|builder| {
builder.replace_ast(use_item, normalized_use_item);
},
)
}
#[cfg(test)]
mod tests {
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_variations {
($fixture: literal, $expected: literal) => {
check_assist(
normalize_import,
concat!("use $0", $fixture, ";"),
concat!("use ", $expected, ";"),
);
check_assist(
normalize_import,
concat!("$0use ", $fixture, ";"),
concat!("use ", $expected, ";"),
);
check_assist_import_one(
normalize_import,
concat!("use $0", $fixture, ";"),
concat!("use {", $expected, "};"),
);
check_assist_import_one(
normalize_import,
concat!("$0use ", $fixture, ";"),
concat!("use {", $expected, "};"),
);
check_assist_import_one(
normalize_import,
concat!("use $0{", $fixture, "};"),
concat!("use {", $expected, "};"),
);
check_assist_import_one(
normalize_import,
concat!("$0use {", $fixture, "};"),
concat!("use {", $expected, "};"),
);
check_assist(
normalize_import,
concat!("use $0", $fixture, "$0;"),
concat!("use ", $expected, ";"),
);
check_assist(
normalize_import,
concat!("$0use ", $fixture, ";$0"),
concat!("use ", $expected, ";"),
);
};
}
macro_rules! check_assist_not_applicable_variations {
($fixture: literal) => {
check_assist_not_applicable(normalize_import, concat!("use $0", $fixture, ";"));
check_assist_not_applicable(normalize_import, concat!("$0use ", $fixture, ";"));
check_assist_not_applicable_for_import_one(
normalize_import,
concat!("use $0{", $fixture, "};"),
);
check_assist_not_applicable_for_import_one(
normalize_import,
concat!("$0use {", $fixture, "};"),
);
};
}
#[test]
fn test_order() {
check_assist_variations!(
"foo::{*, Qux, bar::{Quux, Bar}, baz, FOO_BAZ, self, Baz}",
"foo::{self, bar::{Bar, Quux}, baz, Baz, Qux, FOO_BAZ, *}"
);
}
#[test]
fn test_redundant_braces() {
check_assist_variations!("foo::{bar::{baz, Qux}}", "foo::bar::{baz, Qux}");
check_assist_variations!("foo::{bar::{self}}", "foo::bar");
check_assist_variations!("foo::{bar::{*}}", "foo::bar::*");
check_assist_variations!("foo::{bar::{Qux as Quux}}", "foo::bar::Qux as Quux");
check_assist_variations!(
"foo::bar::{{FOO_BAZ, Qux, self}, {*, baz}}",
"foo::bar::{self, baz, Qux, FOO_BAZ, *}"
);
check_assist_variations!(
"foo::bar::{{{FOO_BAZ}, {{Qux}, {self}}}, {{*}, {baz}}}",
"foo::bar::{self, baz, Qux, FOO_BAZ, *}"
);
}
#[test]
fn test_merge() {
check_assist_variations!(
"foo::{*, bar, {FOO_BAZ, qux}, bar::{*, baz}, {Quux}}",
"foo::{bar::{self, baz, *}, qux, Quux, FOO_BAZ, *}"
);
check_assist_variations!(
"foo::{*, bar, {FOO_BAZ, qux}, bar::{*, baz}, {Quux, bar::{baz::Foo}}}",
"foo::{bar::{self, baz::{self, Foo}, *}, qux, Quux, FOO_BAZ, *}"
);
}
#[test]
fn test_merge_self() {
check_assist_variations!("std::{fmt, fmt::Display}", "std::fmt::{self, Display}");
}
#[test]
fn test_merge_nested() {
check_assist_variations!("std::{fmt::Debug, fmt::Display}", "std::fmt::{Debug, Display}");
}
#[test]
fn test_merge_nested2() {
check_assist_variations!("std::{fmt::Debug, fmt::Display}", "std::fmt::{Debug, Display}");
}
#[test]
fn test_merge_self_with_nested_self_item() {
check_assist_variations!(
"std::{fmt::{self, Debug}, fmt::{Write, Display}}",
"std::fmt::{self, Debug, Display, Write}"
);
}
#[test]
fn works_with_trailing_comma() {
check_assist(
normalize_import,
r"
use $0{
foo::bar,
foo::baz,
};
",
r"
use foo::{bar, baz};
",
);
check_assist_import_one(
normalize_import,
r"
use $0{
foo::bar,
foo::baz,
};
",
r"
use {
foo::{bar, baz},
};
",
);
}
#[test]
fn not_applicable_to_normalized_import() {
check_assist_not_applicable_variations!("foo::bar");
check_assist_not_applicable_variations!("foo::bar::*");
check_assist_not_applicable_variations!("foo::bar::Qux as Quux");
check_assist_not_applicable_variations!("foo::bar::{self, baz, Qux, FOO_BAZ, *}");
check_assist_not_applicable_variations!(
"foo::{self, bar::{Bar, Quux}, baz, Baz, Qux, FOO_BAZ, *}"
);
check_assist_not_applicable_variations!(
"foo::{bar::{self, baz, *}, qux, Quux, FOO_BAZ, *}"
);
check_assist_not_applicable_variations!(
"foo::{bar::{self, baz::{self, Foo}, *}, qux, Quux, FOO_BAZ, *}"
);
}
}

View file

@ -183,6 +183,7 @@ mod handlers {
mod move_guard; mod move_guard;
mod move_module_to_file; mod move_module_to_file;
mod move_to_mod_rs; mod move_to_mod_rs;
mod normalize_import;
mod number_representation; mod number_representation;
mod promote_local_to_const; mod promote_local_to_const;
mod pull_assignment_up; mod pull_assignment_up;
@ -300,6 +301,7 @@ mod handlers {
move_module_to_file::move_module_to_file, move_module_to_file::move_module_to_file,
move_to_mod_rs::move_to_mod_rs, move_to_mod_rs::move_to_mod_rs,
move_from_mod_rs::move_from_mod_rs, move_from_mod_rs::move_from_mod_rs,
normalize_import::normalize_import,
number_representation::reformat_number_literal, number_representation::reformat_number_literal,
pull_assignment_up::pull_assignment_up, pull_assignment_up::pull_assignment_up,
promote_local_to_const::promote_local_to_const, promote_local_to_const::promote_local_to_const,

View file

@ -2217,6 +2217,19 @@ fn t() {}
) )
} }
#[test]
fn doctest_normalize_import() {
check_doc_test(
"normalize_import",
r#####"
use$0 std::{io, {fmt::Formatter}};
"#####,
r#####"
use std::{fmt::Formatter, io};
"#####,
)
}
#[test] #[test]
fn doctest_promote_local_to_const() { fn doctest_promote_local_to_const() {
check_doc_test( check_doc_test(

View file

@ -106,7 +106,7 @@ fn main() {
} }
"#, "#,
r#" r#"
use dep::{FirstStruct, some_module::{SecondStruct, ThirdStruct}}; use dep::{some_module::{SecondStruct, ThirdStruct}, FirstStruct};
fn main() { fn main() {
ThirdStruct ThirdStruct

View file

@ -17,6 +17,7 @@ use syntax::{
use crate::{ use crate::{
imports::merge_imports::{ imports::merge_imports::{
common_prefix, eq_attrs, eq_visibility, try_merge_imports, use_tree_cmp, MergeBehavior, common_prefix, eq_attrs, eq_visibility, try_merge_imports, use_tree_cmp, MergeBehavior,
NormalizationStyle,
}, },
RootDatabase, RootDatabase,
}; };
@ -40,6 +41,15 @@ pub enum ImportGranularity {
One, One,
} }
impl From<ImportGranularity> for NormalizationStyle {
fn from(granularity: ImportGranularity) -> Self {
match granularity {
ImportGranularity::One => NormalizationStyle::One,
_ => NormalizationStyle::Default,
}
}
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)] #[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct InsertUseConfig { pub struct InsertUseConfig {
pub granularity: ImportGranularity, pub granularity: ImportGranularity,

View file

@ -635,7 +635,7 @@ use std::io;",
check_one( check_one(
"std::io", "std::io",
r"use {std::fmt::{Result, Display}};", r"use {std::fmt::{Result, Display}};",
r"use {std::{fmt::{Result, Display}, io}};", r"use {std::{fmt::{Display, Result}, io}};",
); );
} }
@ -650,12 +650,12 @@ fn merge_groups_full() {
check_crate( check_crate(
"std::io", "std::io",
r"use std::fmt::{Result, Display};", r"use std::fmt::{Result, Display};",
r"use std::{fmt::{Result, Display}, io};", r"use std::{fmt::{Display, Result}, io};",
); );
check_one( check_one(
"std::io", "std::io",
r"use {std::fmt::{Result, Display}};", r"use {std::fmt::{Result, Display}};",
r"use {std::{fmt::{Result, Display}, io}};", r"use {std::{fmt::{Display, Result}, io}};",
); );
} }
@ -749,12 +749,12 @@ fn merge_groups_full_nested_deep() {
check_crate( check_crate(
"std::foo::bar::quux::Baz", "std::foo::bar::quux::Baz",
r"use std::foo::bar::{Qux, quux::{Fez, Fizz}};", r"use std::foo::bar::{Qux, quux::{Fez, Fizz}};",
r"use std::foo::bar::{Qux, quux::{Baz, Fez, Fizz}};", r"use std::foo::bar::{quux::{Baz, Fez, Fizz}, Qux};",
); );
check_one( check_one(
"std::foo::bar::quux::Baz", "std::foo::bar::quux::Baz",
r"use {std::foo::bar::{Qux, quux::{Fez, Fizz}}};", r"use {std::foo::bar::{Qux, quux::{Fez, Fizz}}};",
r"use {std::foo::bar::{Qux, quux::{Baz, Fez, Fizz}}};", r"use {std::foo::bar::{quux::{Baz, Fez, Fizz}, Qux}};",
); );
} }
@ -763,7 +763,7 @@ fn merge_groups_full_nested_long() {
check_crate( check_crate(
"std::foo::bar::Baz", "std::foo::bar::Baz",
r"use std::{foo::bar::Qux};", r"use std::{foo::bar::Qux};",
r"use std::{foo::bar::{Baz, Qux}};", r"use std::foo::bar::{Baz, Qux};",
); );
} }
@ -772,12 +772,12 @@ fn merge_groups_last_nested_long() {
check_crate( check_crate(
"std::foo::bar::Baz", "std::foo::bar::Baz",
r"use std::{foo::bar::Qux};", r"use std::{foo::bar::Qux};",
r"use std::{foo::bar::{Baz, Qux}};", r"use std::foo::bar::{Baz, Qux};",
); );
check_one( check_one(
"std::foo::bar::Baz", "std::foo::bar::Baz",
r"use {std::{foo::bar::Qux}};", r"use {std::{foo::bar::Qux}};",
r"use {std::{foo::bar::{Baz, Qux}}};", r"use {std::foo::bar::{Baz, Qux}};",
); );
} }
@ -898,7 +898,7 @@ fn merge_glob() {
r" r"
use syntax::{SyntaxKind::*};", use syntax::{SyntaxKind::*};",
r" r"
use syntax::{SyntaxKind::{self, *}};", use syntax::SyntaxKind::{self, *};",
) )
} }
@ -907,7 +907,7 @@ fn merge_glob_nested() {
check_crate( check_crate(
"foo::bar::quux::Fez", "foo::bar::quux::Fez",
r"use foo::bar::{Baz, quux::*};", r"use foo::bar::{Baz, quux::*};",
r"use foo::bar::{Baz, quux::{Fez, *}};", r"use foo::bar::{quux::{Fez, *}, Baz};",
) )
} }
@ -1211,7 +1211,7 @@ fn insert_with_renamed_import_complex_use() {
use self::foo::{self, Foo as _, Bar}; use self::foo::{self, Foo as _, Bar};
"#, "#,
r#" r#"
use self::foo::{self, Foo, Bar}; use self::foo::{self, Bar, Foo};
"#, "#,
&InsertUseConfig { &InsertUseConfig {
granularity: ImportGranularity::Crate, granularity: ImportGranularity::Crate,

View file

@ -1,15 +1,17 @@
//! Handle syntactic aspects of merging UseTrees. //! Handle syntactic aspects of merging UseTrees.
use std::cmp::Ordering; use std::cmp::Ordering;
use std::iter::empty;
use itertools::{EitherOrBoth, Itertools}; use itertools::{EitherOrBoth, Itertools};
use parser::T; use parser::T;
use stdx::is_upper_snake_case; use stdx::is_upper_snake_case;
use syntax::{ use syntax::{
algo, algo,
ast::{self, make, AstNode, HasAttrs, HasName, HasVisibility, PathSegmentKind}, ast::{
self, edit_in_place::Removable, make, AstNode, HasAttrs, HasName, HasVisibility,
PathSegmentKind,
},
ted::{self, Position}, ted::{self, Position},
Direction, Direction, SyntaxElement,
}; };
use crate::syntax_helpers::node_ext::vis_eq; use crate::syntax_helpers::node_ext::vis_eq;
@ -58,6 +60,10 @@ pub fn try_merge_imports(
let lhs_tree = lhs.use_tree()?; let lhs_tree = lhs.use_tree()?;
let rhs_tree = rhs.use_tree()?; let rhs_tree = rhs.use_tree()?;
try_merge_trees_mut(&lhs_tree, &rhs_tree, merge_behavior)?; try_merge_trees_mut(&lhs_tree, &rhs_tree, merge_behavior)?;
// Ignore `None` result because normalization should not affect the merge result.
try_normalize_use_tree_mut(&lhs_tree, merge_behavior.into());
Some(lhs) Some(lhs)
} }
@ -71,6 +77,10 @@ pub fn try_merge_trees(
let lhs = lhs.clone_subtree().clone_for_update(); let lhs = lhs.clone_subtree().clone_for_update();
let rhs = rhs.clone_subtree().clone_for_update(); let rhs = rhs.clone_subtree().clone_for_update();
try_merge_trees_mut(&lhs, &rhs, merge)?; try_merge_trees_mut(&lhs, &rhs, merge)?;
// Ignore `None` result because normalization should not affect the merge result.
try_normalize_use_tree_mut(&lhs, merge.into());
Some(lhs) Some(lhs)
} }
@ -173,60 +183,301 @@ fn recursive_merge(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehavior)
} }
Err(insert_idx) => { Err(insert_idx) => {
use_trees.insert(insert_idx, rhs_t.clone()); use_trees.insert(insert_idx, rhs_t.clone());
match lhs.use_tree_list() { // We simply add the use tree to the end of tree list. Ordering of use trees
// Creates a new use tree list with the item. // and imports is done by the `try_normalize_*` functions. The sorted `use_trees`
None => lhs.get_or_create_use_tree_list().add_use_tree(rhs_t), // vec is only used for binary search.
// Recreates the use tree list with sorted items (see `use_tree_cmp` doc). lhs.get_or_create_use_tree_list().add_use_tree(rhs_t);
Some(use_tree_list) => {
if use_tree_list.l_curly_token().is_none() {
ted::insert_raw(
Position::first_child_of(use_tree_list.syntax()),
make::token(T!['{']),
);
} }
if use_tree_list.r_curly_token().is_none() { }
ted::insert_raw( }
Position::last_child_of(use_tree_list.syntax()), Some(())
make::token(T!['}']),
);
} }
let mut elements = Vec::new(); /// Style to follow when normalizing a use tree.
for (idx, tree) in use_trees.iter().enumerate() { #[derive(Copy, Clone, Debug, PartialEq, Eq)]
if idx > 0 { pub enum NormalizationStyle {
/// Merges all descendant use tree lists with only one child use tree into their parent use tree.
///
/// Examples:
/// - `foo::{bar::{Qux}}` -> `foo::bar::Qux`
/// - `foo::{bar::{self}}` -> `foo::bar`
/// - `{foo::bar}` -> `foo::bar`
Default,
/// Same as default but wraps the root use tree in a use tree list.
///
/// Examples:
/// - `foo::{bar::{Qux}}` -> `{foo::bar::Qux}`
/// - `foo::{bar::{self}}` -> `{foo::bar}`
/// - `{foo::bar}` -> `{foo::bar}`
One,
}
impl From<MergeBehavior> for NormalizationStyle {
fn from(mb: MergeBehavior) -> Self {
match mb {
MergeBehavior::One => NormalizationStyle::One,
_ => NormalizationStyle::Default,
}
}
}
/// Normalizes a use item by:
/// - Ordering all use trees
/// - Merging use trees with common prefixes
/// - Removing redundant braces based on the specified normalization style
/// (see [`NormalizationStyle`] doc)
///
/// Examples:
///
/// Using the "Default" normalization style
///
/// - `foo::{bar::Qux, bar::{self}}` -> `foo::bar::{self, Qux}`
/// - `foo::bar::{self}` -> `foo::bar`
/// - `{foo::bar}` -> `foo::bar`
///
/// Using the "One" normalization style
///
/// - `foo::{bar::Qux, bar::{self}}` -> `{foo::bar::{self, Qux}}`
/// - `foo::bar::{self}` -> `{foo::bar}`
/// - `foo::bar` -> `{foo::bar}`
pub fn try_normalize_import(use_item: &ast::Use, style: NormalizationStyle) -> Option<ast::Use> {
let use_item = use_item.clone_subtree().clone_for_update();
try_normalize_use_tree_mut(&use_item.use_tree()?, style)?;
Some(use_item)
}
/// Normalizes a use tree (see [`try_normalize_import`] doc).
pub fn try_normalize_use_tree(
use_tree: &ast::UseTree,
style: NormalizationStyle,
) -> Option<ast::UseTree> {
let use_tree = use_tree.clone_subtree().clone_for_update();
try_normalize_use_tree_mut(&use_tree, style)?;
Some(use_tree)
}
pub fn try_normalize_use_tree_mut(
use_tree: &ast::UseTree,
style: NormalizationStyle,
) -> Option<()> {
if style == NormalizationStyle::One {
let mut modified = false;
modified |= use_tree.wrap_in_tree_list().is_some();
modified |= recursive_normalize(use_tree, style).is_some();
if !modified {
// Either the use tree was already normalized or its semantically empty.
return None;
}
} else {
recursive_normalize(use_tree, NormalizationStyle::Default)?;
}
Some(())
}
/// Recursively normalizes a use tree and its subtrees (if any).
fn recursive_normalize(use_tree: &ast::UseTree, style: NormalizationStyle) -> Option<()> {
let use_tree_list = use_tree.use_tree_list()?;
let merge_subtree_into_parent_tree = |single_subtree: &ast::UseTree| {
let merged_path = match (use_tree.path(), single_subtree.path()) {
(None, None) => None,
(Some(outer), None) => Some(outer),
(None, Some(inner)) if path_is_self(&inner) => None,
(None, Some(inner)) => Some(inner),
(Some(outer), Some(inner)) if path_is_self(&inner) => Some(outer),
(Some(outer), Some(inner)) => Some(make::path_concat(outer, inner).clone_for_update()),
};
if merged_path.is_some()
|| single_subtree.use_tree_list().is_some()
|| single_subtree.star_token().is_some()
{
ted::remove_all_iter(use_tree.syntax().children_with_tokens());
if let Some(path) = merged_path {
ted::insert_raw(Position::first_child_of(use_tree.syntax()), path.syntax());
if single_subtree.use_tree_list().is_some() || single_subtree.star_token().is_some()
{
ted::insert_raw(
Position::last_child_of(use_tree.syntax()),
make::token(T![::]),
);
}
}
if let Some(inner_use_tree_list) = single_subtree.use_tree_list() {
ted::insert_raw(
Position::last_child_of(use_tree.syntax()),
inner_use_tree_list.syntax(),
);
} else if single_subtree.star_token().is_some() {
ted::insert_raw(Position::last_child_of(use_tree.syntax()), make::token(T![*]));
} else if let Some(rename) = single_subtree.rename() {
ted::insert_raw(
Position::last_child_of(use_tree.syntax()),
make::tokens::single_space(),
);
ted::insert_raw(Position::last_child_of(use_tree.syntax()), rename.syntax());
}
Some(())
} else {
// Bail on semantically empty use trees.
None
}
};
let one_style_tree_list = |subtree: &ast::UseTree| match (
subtree.path().is_none() && subtree.star_token().is_none() && subtree.rename().is_none(),
subtree.use_tree_list(),
) {
(true, tree_list) => tree_list,
_ => None,
};
let add_element_to_list = |elem: SyntaxElement, elements: &mut Vec<SyntaxElement>| {
if !elements.is_empty() {
elements.push(make::token(T![,]).into()); elements.push(make::token(T![,]).into());
elements.push(make::tokens::single_space().into()); elements.push(make::tokens::single_space().into());
} }
elements.push(tree.syntax().clone().into()); elements.push(elem);
};
if let Some((single_subtree,)) = use_tree_list.use_trees().collect_tuple() {
if style == NormalizationStyle::One {
// Only normalize descendant subtrees if the normalization style is "one".
recursive_normalize(&single_subtree, NormalizationStyle::Default)?;
} else {
// Otherwise, merge the single subtree into it's parent (if possible)
// and then normalize the result.
merge_subtree_into_parent_tree(&single_subtree)?;
recursive_normalize(use_tree, style);
}
} else {
// Tracks whether any changes have been made to the use tree.
let mut modified = false;
// Recursively un-nests (if necessary) and then normalizes each subtree in the tree list.
for subtree in use_tree_list.use_trees() {
if let Some(one_tree_list) = one_style_tree_list(&subtree) {
let mut elements = Vec::new();
let mut one_tree_list_iter = one_tree_list.use_trees();
let mut prev_skipped = Vec::new();
loop {
let mut prev_skipped_iter = prev_skipped.into_iter();
let mut curr_skipped = Vec::new();
while let Some(sub_sub_tree) =
one_tree_list_iter.next().or(prev_skipped_iter.next())
{
if let Some(sub_one_tree_list) = one_style_tree_list(&sub_sub_tree) {
curr_skipped.extend(sub_one_tree_list.use_trees());
} else {
modified |=
recursive_normalize(&sub_sub_tree, NormalizationStyle::Default)
.is_some();
add_element_to_list(
sub_sub_tree.syntax().clone().into(),
&mut elements,
);
}
} }
if curr_skipped.is_empty() {
// Un-nesting is complete.
break;
}
prev_skipped = curr_skipped;
}
// Either removes the subtree (if its semantically empty) or replaces it with
// the un-nested elements.
if elements.is_empty() {
subtree.remove();
} else {
ted::replace_with_many(subtree.syntax(), elements);
}
modified = true;
} else {
modified |= recursive_normalize(&subtree, NormalizationStyle::Default).is_some();
}
}
// Merge all merge-able subtrees.
let mut tree_list_iter = use_tree_list.use_trees();
let mut anchor = tree_list_iter.next()?;
let mut prev_skipped = Vec::new();
loop {
let mut has_merged = false;
let mut prev_skipped_iter = prev_skipped.into_iter();
let mut next_anchor = None;
let mut curr_skipped = Vec::new();
while let Some(candidate) = tree_list_iter.next().or(prev_skipped_iter.next()) {
let result = try_merge_trees_mut(&anchor, &candidate, MergeBehavior::Crate);
if result.is_some() {
// Remove merged subtree.
candidate.remove();
has_merged = true;
} else if next_anchor.is_none() {
next_anchor = Some(candidate);
} else {
curr_skipped.push(candidate);
}
}
if has_merged {
// Normalize the merge result.
recursive_normalize(&anchor, NormalizationStyle::Default);
modified = true;
}
let (Some(next_anchor), true) = (next_anchor, !curr_skipped.is_empty()) else {
// Merging is complete.
break;
};
// Try to merge the remaining subtrees in the next iteration.
anchor = next_anchor;
prev_skipped = curr_skipped;
}
let mut subtrees: Vec<_> = use_tree_list.use_trees().collect();
// Merge the remaining subtree into its parent, if its only one and
// the normalization style is not "one".
if subtrees.len() == 1 && style != NormalizationStyle::One {
modified |= merge_subtree_into_parent_tree(&subtrees[0]).is_some();
}
// Order the remaining subtrees (if necessary).
if subtrees.len() > 1 {
let mut did_sort = false;
subtrees.sort_unstable_by(|a, b| {
let order = use_tree_cmp_bin_search(a, b);
if !did_sort && order == Ordering::Less {
did_sort = true;
}
order
});
if did_sort {
let start = use_tree_list let start = use_tree_list
.l_curly_token() .l_curly_token()
.and_then(|l_curly| { .and_then(|l_curly| algo::non_trivia_sibling(l_curly.into(), Direction::Next))
algo::non_trivia_sibling(l_curly.into(), Direction::Next)
})
.filter(|it| it.kind() != T!['}']); .filter(|it| it.kind() != T!['}']);
let end = use_tree_list let end = use_tree_list
.r_curly_token() .r_curly_token()
.and_then(|r_curly| { .and_then(|r_curly| algo::non_trivia_sibling(r_curly.into(), Direction::Prev))
algo::non_trivia_sibling(r_curly.into(), Direction::Prev)
})
.filter(|it| it.kind() != T!['{']); .filter(|it| it.kind() != T!['{']);
if let Some((start, end)) = start.zip(end) { if let Some((start, end)) = start.zip(end) {
// Attempt to insert elements while preserving preceding and trailing trivia. // Attempt to insert elements while preserving preceding and trailing trivia.
let mut elements = Vec::new();
for subtree in subtrees {
add_element_to_list(subtree.syntax().clone().into(), &mut elements);
}
ted::replace_all(start..=end, elements); ted::replace_all(start..=end, elements);
} else { } else {
let new_use_tree_list = make::use_tree_list(empty()).clone_for_update(); let new_use_tree_list =
let trees_pos = match new_use_tree_list.l_curly_token() { make::use_tree_list(subtrees.into_iter()).clone_for_update();
Some(l_curly) => Position::after(l_curly),
None => Position::last_child_of(new_use_tree_list.syntax()),
};
ted::insert_all_raw(trees_pos, elements);
ted::replace(use_tree_list.syntax(), new_use_tree_list.syntax()); ted::replace(use_tree_list.syntax(), new_use_tree_list.syntax());
} }
modified = true;
} }
} }
}
if !modified {
// Either the use tree was already normalized or its semantically empty.
return None;
} }
} }
Some(()) Some(())
@ -280,7 +531,7 @@ fn use_tree_cmp_bin_search(lhs: &ast::UseTree, rhs: &ast::UseTree) -> Ordering {
/// and `crate` first, then identifier imports with lowercase ones first and upper snake case /// and `crate` first, then identifier imports with lowercase ones first and upper snake case
/// (e.g. UPPER_SNAKE_CASE) ones last, then glob imports, and at last list imports. /// (e.g. UPPER_SNAKE_CASE) ones last, then glob imports, and at last list imports.
/// ///
/// Example foo::{self, foo, baz, Baz, Qux, FOO_BAZ, *, {Bar}} /// Example: `foo::{self, baz, foo, Baz, Qux, FOO_BAZ, *, {Bar}}`
/// Ref: <https://github.com/rust-lang/rustfmt/blob/6356fca675bd756d71f5c123cd053d17b16c573e/src/imports.rs#L83-L86>. /// Ref: <https://github.com/rust-lang/rustfmt/blob/6356fca675bd756d71f5c123cd053d17b16c573e/src/imports.rs#L83-L86>.
pub(super) fn use_tree_cmp(a: &ast::UseTree, b: &ast::UseTree) -> Ordering { pub(super) fn use_tree_cmp(a: &ast::UseTree, b: &ast::UseTree) -> Ordering {
let a_is_simple_path = a.is_simple_path() && a.rename().is_none(); let a_is_simple_path = a.is_simple_path() && a.rename().is_none();

View file

@ -538,9 +538,13 @@ impl ast::UseTree {
/// `foo::bar` -> `{foo::bar}` /// `foo::bar` -> `{foo::bar}`
/// ///
/// `{foo::bar}` -> `{foo::bar}` /// `{foo::bar}` -> `{foo::bar}`
pub fn wrap_in_tree_list(&self) { pub fn wrap_in_tree_list(&self) -> Option<()> {
if self.path().is_none() { if self.use_tree_list().is_some()
return; && self.path().is_none()
&& self.star_token().is_none()
&& self.rename().is_none()
{
return None;
} }
let subtree = self.clone_subtree().clone_for_update(); let subtree = self.clone_subtree().clone_for_update();
ted::remove_all_iter(self.syntax().children_with_tokens()); ted::remove_all_iter(self.syntax().children_with_tokens());
@ -548,6 +552,7 @@ impl ast::UseTree {
self.syntax(), self.syntax(),
make::use_tree_list(once(subtree)).clone_for_update().syntax(), make::use_tree_list(once(subtree)).clone_for_update().syntax(),
); );
Some(())
} }
} }