squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Rename merge_imports to imports_granularity and add a Module option.

Browse source 
This renames the existing `true`/`false` options to `Crate`/`Never`, then adds a
new `Module` option which causes imports to be grouped together by their
originating module.
This commit is contained in:
Geoffry Song 2020-12-15 17:19:54 -08:00 committed by Caleb Cartwright
parent 216a643005
commit 71863753bd
18 changed files with 291 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

47
Configurations.md
View file

@ -1615,13 +1615,56 @@ pub enum Foo {}
pub enum Foo {}
```
## `imports_granularity`
Merge together related imports based on their paths.
- **Default value**: `Preserve`
- **Possible values**: `Preserve`, `Crate`, `Module`
- **Stable**: No
#### `Preserve` (default):
Do not perform any merging and preserve the original structure written by the developer.
```rust
use foo::b;
use foo::b::{f, g};
use foo::{a, c, d::e};
use qux::{h, i};
```
#### `Crate`:
Merge imports from the same crate into a single `use` statement. Conversely, imports from different crates are split into separate statements.
```rust
use foo::{
a, b,
b::{f, g},
c,
d::e,
};
use qux::{h, i};
```
#### `Module`:
Merge imports from the same module into a single `use` statement. Conversely, imports from different modules are split into separate statements.
```rust
use foo::b::{f, g};
use foo::d::e;
use foo::{a, b, c};
use qux::{h, i};
```
## `merge_imports`
Merge multiple imports into a single nested import.
This option is deprecated. Use `imports_granularity = "Crate"` instead.
- **Default value**: `false`
- **Possible values**: `true`, `false`
- **Stable**: No (tracking issue: #3362)
#### `false` (default):

23
src/config/config_type.rs
View file

@ -97,6 +97,7 @@ macro_rules! create_config {
match stringify!($i) {
"max_width" | "use_small_heuristics" => self.0.set_heuristics(),
"license_template_path" => self.0.set_license_template(),
"merge_imports" => self.0.set_merge_imports(),
&_ => (),
}
}
@ -156,6 +157,7 @@ macro_rules! create_config {
self.set_heuristics();
self.set_license_template();
self.set_ignore(dir);
self.set_merge_imports();
self
}
@ -230,14 +232,15 @@ macro_rules! create_config {
match key {
"max_width" | "use_small_heuristics" => self.set_heuristics(),
"license_template_path" => self.set_license_template(),
"merge_imports" => self.set_merge_imports(),
&_ => (),
}
}
#[allow(unreachable_pub)]
pub fn is_hidden_option(name: &str) -> bool {
const HIDE_OPTIONS: [&str; 4] =
["verbose", "verbose_diff", "file_lines", "width_heuristics"];
const HIDE_OPTIONS: [&str; 5] =
["verbose", "verbose_diff", "file_lines", "width_heuristics", "merge_imports"];
HIDE_OPTIONS.contains(&name)
}
@ -309,6 +312,22 @@ macro_rules! create_config {
self.ignore.2.add_prefix(dir);
}
fn set_merge_imports(&mut self) {
if self.was_set().merge_imports() {
eprintln!(
"Warning: the `merge_imports` option is deprecated. \
Use `imports_granularity=Crate` instead"
);
if !self.was_set().imports_granularity() {
self.imports_granularity.2 = if self.merge_imports() {
ImportGranularity::Crate
} else {
ImportGranularity::Preserve
};
}
}
}
#[allow(unreachable_pub)]
/// Returns `true` if the config key was explicitly set and is the default value.
pub fn is_default(&self, key: &str) -> bool {

60
src/config/mod.rs
View file

@ -64,9 +64,11 @@ create_config! {
// Imports
imports_indent: IndentStyle, IndentStyle::Block, false, "Indent of imports";
imports_layout: ListTactic, ListTactic::Mixed, false, "Item layout inside a import block";
merge_imports: bool, false, false, "Merge imports";
imports_granularity: ImportGranularity, ImportGranularity::Preserve, false,
"Merge or split imports to the provided granularity";
group_imports: GroupImportsTactic, GroupImportsTactic::Preserve, false,
"Controls the strategy for how imports are grouped together";
merge_imports: bool, false, false, "(deprecated: use imports_granularity instead)";
// Ordering
reorder_imports: bool, true, true, "Reorder import and extern crate statements alphabetically";
@ -174,6 +176,7 @@ impl PartialConfig {
cloned.verbose = None;
cloned.width_heuristics = None;
cloned.print_misformatted_file_names = None;
cloned.merge_imports = None;
::toml::to_string(&cloned).map_err(ToTomlError)
}
@ -407,6 +410,10 @@ mod test {
via the --file-lines option";
width_heuristics: WidthHeuristics, WidthHeuristics::scaled(100), false,
"'small' heuristic values";
// merge_imports deprecation
imports_granularity: ImportGranularity, ImportGranularity::Preserve, false,
"Merge imports";
merge_imports: bool, false, false, "(deprecated: use imports_granularity instead)";
// Options that are used by the tests
stable_option: bool, false, true, "A stable option";
@ -529,7 +536,7 @@ fn_single_line = false
where_single_line = false
imports_indent = "Block"
imports_layout = "Mixed"
merge_imports = false
imports_granularity = "Preserve"
group_imports = "Preserve"
reorder_imports = true
reorder_modules = true
@ -615,4 +622,53 @@ make_backup = false
// assert_eq!(config.unstable_features(), true);
// ::std::env::set_var("CFG_RELEASE_CHANNEL", v);
// }
#[cfg(test)]
mod deprecated_option_merge_imports {
use super::*;
#[test]
fn test_old_option_set() {
let toml = r#"
unstable_features = true
merge_imports = true
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
assert_eq!(config.imports_granularity(), ImportGranularity::Crate);
}
#[test]
fn test_both_set() {
let toml = r#"
unstable_features = true
merge_imports = true
imports_granularity = "Preserve"
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
assert_eq!(config.imports_granularity(), ImportGranularity::Preserve);
}
#[test]
fn test_new_overridden() {
let toml = r#"
unstable_features = true
merge_imports = true
"#;
let mut config = Config::from_toml(toml, Path::new("")).unwrap();
config.override_value("imports_granularity", "Preserve");
assert_eq!(config.imports_granularity(), ImportGranularity::Preserve);
}
#[test]
fn test_old_overridden() {
let toml = r#"
unstable_features = true
imports_granularity = "Module"
"#;
let mut config = Config::from_toml(toml, Path::new("")).unwrap();
config.override_value("merge_imports", "true");
// no effect: the new option always takes precedence
assert_eq!(config.imports_granularity(), ImportGranularity::Module);
}
}
}

14
src/config/options.rs
View file

@ -1,6 +1,7 @@
use std::collections::{hash_set, HashSet};
use std::fmt;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use itertools::Itertools;
use rustfmt_config_proc_macro::config_type;
@ -111,6 +112,17 @@ pub enum GroupImportsTactic {
StdExternalCrate,
}
#[config_type]
/// How to merge imports.
pub enum ImportGranularity {
/// Do not merge imports.
Preserve,
/// Use one `use` statement per crate.
Crate,
/// Use one `use` statement per module.
Module,
}
#[config_type]
pub enum ReportTactic {
Always,
@ -362,7 +374,7 @@ impl IgnoreList {
}
}
impl ::std::str::FromStr for IgnoreList {
impl FromStr for IgnoreList {
type Err = &'static str;
fn from_str(_: &str) -> Result<Self, Self::Err> {

118
src/imports.rs
View file

@ -159,7 +159,7 @@ impl UseSegment {
}
}
pub(crate) fn merge_use_trees(use_trees: Vec<UseTree>) -> Vec<UseTree> {
pub(crate) fn merge_use_trees(use_trees: Vec<UseTree>, merge_by: SharedPrefix) -> Vec<UseTree> {
let mut result = Vec::with_capacity(use_trees.len());
for use_tree in use_trees {
if use_tree.has_comment() || use_tree.attrs.is_some() {
@ -168,8 +168,11 @@ pub(crate) fn merge_use_trees(use_trees: Vec<UseTree>) -> Vec<UseTree> {
}
for flattened in use_tree.flatten() {
if let Some(tree) = result.iter_mut().find(|tree| tree.share_prefix(&flattened)) {
tree.merge(&flattened);
if let Some(tree) = result
.iter_mut()
.find(|tree| tree.share_prefix(&flattened, merge_by))
{
tree.merge(&flattened, merge_by);
} else {
result.push(flattened);
}
@ -527,7 +530,7 @@ impl UseTree {
}
}
fn share_prefix(&self, other: &UseTree) -> bool {
fn share_prefix(&self, other: &UseTree, shared_prefix: SharedPrefix) -> bool {
if self.path.is_empty()
|| other.path.is_empty()
|| self.attrs.is_some()
@ -535,7 +538,12 @@ impl UseTree {
{
false
} else {
self.path[0] == other.path[0]
match shared_prefix {
SharedPrefix::Crate => self.path[0] == other.path[0],
SharedPrefix::Module => {
self.path[..self.path.len() - 1] == other.path[..other.path.len() - 1]
}
}
}
}
@ -573,7 +581,7 @@ impl UseTree {
}
}
fn merge(&mut self, other: &UseTree) {
fn merge(&mut self, other: &UseTree, merge_by: SharedPrefix) {
let mut prefix = 0;
for (a, b) in self.path.iter().zip(other.path.iter()) {
if *a == *b {
@ -582,20 +590,30 @@ impl UseTree {
break;
}
}
if let Some(new_path) = merge_rest(&self.path, &other.path, prefix) {
if let Some(new_path) = merge_rest(&self.path, &other.path, prefix, merge_by) {
self.path = new_path;
self.span = self.span.to(other.span);
}
}
}
fn merge_rest(a: &[UseSegment], b: &[UseSegment], mut len: usize) -> Option<Vec<UseSegment>> {
fn merge_rest(
a: &[UseSegment],
b: &[UseSegment],
mut len: usize,
merge_by: SharedPrefix,
) -> Option<Vec<UseSegment>> {
if a.len() == len && b.len() == len {
return None;
}
if a.len() != len && b.len() != len {
if let UseSegment::List(mut list) = a[len].clone() {
merge_use_trees_inner(&mut list, UseTree::from_path(b[len..].to_vec(), DUMMY_SP));
if let UseSegment::List(ref list) = a[len] {
let mut list = list.clone();
merge_use_trees_inner(
&mut list,
UseTree::from_path(b[len..].to_vec(), DUMMY_SP),
merge_by,
);
let mut new_path = b[..len].to_vec();
new_path.push(UseSegment::List(list));
return Some(new_path);
@ -622,20 +640,20 @@ fn merge_rest(a: &[UseSegment], b: &[UseSegment], mut len: usize) -> Option<Vec<
Some(new_path)
}
fn merge_use_trees_inner(trees: &mut Vec<UseTree>, use_tree: UseTree) {
let similar_trees = trees.iter_mut().filter(|tree| tree.share_prefix(&use_tree));
if use_tree.path.len() == 1 {
fn merge_use_trees_inner(trees: &mut Vec<UseTree>, use_tree: UseTree, merge_by: SharedPrefix) {
let similar_trees = trees
.iter_mut()
.filter(|tree| tree.share_prefix(&use_tree, merge_by));
if use_tree.path.len() == 1 && merge_by == SharedPrefix::Crate {
if let Some(tree) = similar_trees.min_by_key(|tree| tree.path.len()) {
if tree.path.len() == 1 {
return;
}
}
} else {
if let Some(tree) = similar_trees.max_by_key(|tree| tree.path.len()) {
if tree.path.len() > 1 {
tree.merge(&use_tree);
return;
}
} else if let Some(tree) = similar_trees.max_by_key(|tree| tree.path.len()) {
if tree.path.len() > 1 {
tree.merge(&use_tree, merge_by);
return;
}
}
trees.push(use_tree);
@ -848,6 +866,12 @@ impl Rewrite for UseTree {
}
}
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub(crate) enum SharedPrefix {
Crate,
Module,
}
#[cfg(test)]
mod test {
use super::*;
@ -994,44 +1018,72 @@ mod test {
}
}
#[test]
fn test_use_tree_merge() {
macro_rules! test_merge {
([$($input:expr),* $(,)*], [$($output:expr),* $(,)*]) => {
assert_eq!(
merge_use_trees(parse_use_trees!($($input,)*)),
parse_use_trees!($($output,)*),
);
}
macro_rules! test_merge {
($by:ident, [$($input:expr),* $(,)*], [$($output:expr),* $(,)*]) => {
assert_eq!(
merge_use_trees(parse_use_trees!($($input,)*), SharedPrefix::$by),
parse_use_trees!($($output,)*),
);
}
}
test_merge!(["a::b::{c, d}", "a::b::{e, f}"], ["a::b::{c, d, e, f}"]);
test_merge!(["a::b::c", "a::b"], ["a::{b, b::c}"]);
test_merge!(["a::b", "a::b"], ["a::b"]);
test_merge!(["a", "a::b", "a::b::c"], ["a::{self, b, b::c}"]);
#[test]
fn test_use_tree_merge_crate() {
test_merge!(
Crate,
["a::b::{c, d}", "a::b::{e, f}"],
["a::b::{c, d, e, f}"]
);
test_merge!(Crate, ["a::b::c", "a::b"], ["a::{b, b::c}"]);
test_merge!(Crate, ["a::b", "a::b"], ["a::b"]);
test_merge!(Crate, ["a", "a::b", "a::b::c"], ["a::{self, b, b::c}"]);
test_merge!(
Crate,
["a", "a::b", "a::b::c", "a::b::c::d"],
["a::{self, b, b::{c, c::d}}"]
);
test_merge!(["a", "a::b", "a::b::c", "a::b"], ["a::{self, b, b::c}"]);
test_merge!(
Crate,
["a", "a::b", "a::b::c", "a::b"],
["a::{self, b, b::c}"]
);
test_merge!(
Crate,
["a::{b::{self, c}, d::e}", "a::d::f"],
["a::{b::{self, c}, d::{e, f}}"]
);
test_merge!(
Crate,
["a::d::f", "a::{b::{self, c}, d::e}"],
["a::{b::{self, c}, d::{e, f}}"]
);
test_merge!(
Crate,
["a::{c, d, b}", "a::{d, e, b, a, f}", "a::{f, g, c}"],
["a::{a, b, c, d, e, f, g}"]
);
test_merge!(
Crate,
["a::{self}", "b::{self as foo}"],
["a::{self}", "b::{self as foo}"]
);
}
#[test]
fn test_use_tree_merge_module() {
test_merge!(
Module,
["foo::b", "foo::{a, c, d::e}"],
["foo::{a, b, c}", "foo::d::e"]
);
test_merge!(
Module,
["foo::{a::b, a::c, d::e, d::f}"],
["foo::a::{b, c}", "foo::d::{e, f}"]
);
}
#[test]
fn test_use_tree_flatten() {
assert_eq!(

14
src/reorder.rs
View file

@ -11,8 +11,8 @@ use std::cmp::{Ord, Ordering};
use rustc_ast::ast;
use rustc_span::{symbol::sym, Span};
use crate::config::{Config, GroupImportsTactic};
use crate::imports::{merge_use_trees, UseSegment, UseTree};
use crate::config::{Config, GroupImportsTactic, ImportGranularity};
use crate::imports::{merge_use_trees, SharedPrefix, UseSegment, UseTree};
use crate::items::{is_mod_decl, rewrite_extern_crate, rewrite_mod};
use crate::lists::{itemize_list, write_list, ListFormatting, ListItem};
use crate::rewrite::RewriteContext;
@ -107,8 +107,14 @@ fn rewrite_reorderable_or_regroupable_items(
for (item, list_item) in normalized_items.iter_mut().zip(list_items) {
item.list_item = Some(list_item.clone());
}
if context.config.merge_imports() {
normalized_items = merge_use_trees(normalized_items);
match context.config.imports_granularity() {
ImportGranularity::Crate => {
normalized_items = merge_use_trees(normalized_items, SharedPrefix::Crate)
}
ImportGranularity::Module => {
normalized_items = merge_use_trees(normalized_items, SharedPrefix::Module)
}
ImportGranularity::Preserve => {}
}
let mut regrouped_items = match context.config.group_imports() {

2
tests/source/configs/group_imports/StdExternalCrate-merge_imports.rs
View file

@ -1,5 +1,5 @@
// rustfmt-group_imports: StdExternalCrate
// rustfmt-merge_imports: true
// rustfmt-imports_granularity: Crate
use chrono::Utc;
use super::update::convert_publish_payload;

2
tests/source/configs/imports_layout/merge_mixed.rs
View file

@ -1,5 +1,5 @@
// rustfmt-imports_indent: Block
// rustfmt-merge_imports: true
// rustfmt-imports_granularity: Crate
// rustfmt-imports_layout: Mixed
use std::{fmt, io, str};

5
tests/source/merge_imports.rs → tests/source/imports_granularity_crate.rs
View file

@ -1,4 +1,4 @@
// rustfmt-merge_imports: true
// rustfmt-imports_granularity: Crate
use a::{c,d,b};
use a::{d, e, b, a, f};
@ -32,3 +32,6 @@ use g::{self, b};
use h::{a};
use i::a::{self};
use j::{a::{self}};
use {k::{a, b}, l::{a, b}};
use {k::{c, d}, l::{c, d}};

18
tests/source/imports_granularity_module.rs Normal file
View file

@ -0,0 +1,18 @@
// rustfmt-imports_granularity: Module
use a::{b::c, d::e};
use a::{f, g::{h, i}};
use a::{j::{self, k::{self, l}, m}, n::{o::p, q}};
pub use a::{r::s, t};
#[cfg(test)]
use foo::{a::b, c::d};
use foo::e;
use bar::{
// comment
a::b,
// more comment
c::d,
e::f,
};

2
tests/source/issue-3750.rs
View file

@ -1,4 +1,4 @@
// rustfmt-merge_imports: true
// rustfmt-imports_granularity: Crate
pub mod foo {
pub mod bar {

4
tests/source/merge_imports_true_compat.rs Normal file
View file

@ -0,0 +1,4 @@
// rustfmt-merge_imports: true
use a::b;
use a::c;

2
tests/target/configs/group_imports/StdExternalCrate-merge_imports.rs
View file

@ -1,5 +1,5 @@
// rustfmt-group_imports: StdExternalCrate
// rustfmt-merge_imports: true
// rustfmt-imports_granularity: Crate
use alloc::{alloc::Layout, vec::Vec};
use core::f32;
use std::sync::Arc;

2
tests/target/configs/imports_layout/merge_mixed.rs
View file

@ -1,5 +1,5 @@
// rustfmt-imports_indent: Block
// rustfmt-merge_imports: true
// rustfmt-imports_granularity: Crate
// rustfmt-imports_layout: Mixed
use std::{fmt, io, str, str::FromStr};

5
tests/target/merge_imports.rs → tests/target/imports_granularity_crate.rs
View file

@ -1,4 +1,4 @@
// rustfmt-merge_imports: true
// rustfmt-imports_granularity: Crate
use a::{a, b, c, d, e, f, g};
@ -23,3 +23,6 @@ use g::{self, a, b};
use h::a;
use i::a::{self};
use j::a::{self};
use k::{a, b, c, d};
use l::{a, b, c, d};

20
tests/target/imports_granularity_module.rs Normal file
View file

@ -0,0 +1,20 @@
// rustfmt-imports_granularity: Module
use a::b::c;
use a::d::e;
use a::f;
use a::g::{h, i};
use a::j::k::{self, l};
use a::j::{self, m};
use a::n::o::p;
use a::n::q;
pub use a::r::s;
pub use a::t;
use foo::e;
#[cfg(test)]
use foo::{a::b, c::d};
use bar::a::b;
use bar::c::d;
use bar::e::f;

2
tests/target/issue-3750.rs
View file

@ -1,4 +1,4 @@
// rustfmt-merge_imports: true
// rustfmt-imports_granularity: Crate
pub mod foo {
pub mod bar {

3
tests/target/merge_imports_true_compat.rs Normal file
View file

@ -0,0 +1,3 @@
// rustfmt-merge_imports: true
use a::{b, c};