Skip to content

Commit

Permalink
Option to create groups for std, external crates, and other imports
Browse files Browse the repository at this point in the history
Backport of 17d90ca.
  • Loading branch information
MattX authored and calebcartwright committed Nov 19, 2020
1 parent 580d826 commit 073cc38
Show file tree
Hide file tree
Showing 12 changed files with 254 additions and 22 deletions.
50 changes: 50 additions & 0 deletions Configurations.md
Expand Up @@ -1889,6 +1889,56 @@ use dolor;
use sit;
```

## `group_imports`

Controls the strategy for how imports are grouped together.

- **Default value**: `Preserve`
- **Possible values**: `Preserve`, `StdExternalCrate`
- **Stable**: No

#### `Preserve` (default):

Preserve the source file's import groups.

```rust
use super::update::convert_publish_payload;
use chrono::Utc;

use alloc::alloc::Layout;
use juniper::{FieldError, FieldResult};
use uuid::Uuid;

use std::sync::Arc;

use broker::database::PooledConnection;

use super::schema::{Context, Payload};
use crate::models::Event;
use core::f32;
```

#### `StdExternalCrate`:

Discard existing import groups, and create three groups for:
1. `std`, `core` and `alloc`,
2. external crates,
3. `self`, `super` and `crate` imports.

```rust
use alloc::alloc::Layout;
use core::f32;
use std::sync::Arc;

use broker::database::PooledConnection;
use chrono::Utc;
use juniper::{FieldError, FieldResult};
use uuid::Uuid;

use super::schema::{Context, Payload};
use super::update::convert_publish_payload;
use crate::models::Event;
```

## `reorder_modules`

Expand Down
3 changes: 3 additions & 0 deletions src/config/mod.rs
Expand Up @@ -65,6 +65,8 @@ create_config! {
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";
group_imports: GroupImportsTactic, GroupImportsTactic::Preserve, false,
"Controls the strategy for how imports are grouped together";

// Ordering
reorder_imports: bool, true, true, "Reorder import and extern crate statements alphabetically";
Expand Down Expand Up @@ -528,6 +530,7 @@ where_single_line = false
imports_indent = "Block"
imports_layout = "Mixed"
merge_imports = false
group_imports = "Preserve"
reorder_imports = true
reorder_modules = true
reorder_impl_items = false
Expand Down
12 changes: 12 additions & 0 deletions src/config/options.rs
Expand Up @@ -99,6 +99,18 @@ impl Density {
}
}

#[config_type]
/// Configuration for import groups, i.e. sets of imports separated by newlines.
pub enum GroupImportsTactic {
/// Keep groups as they are.
Preserve,
/// Discard existing groups, and create new groups for
/// 1. `std` / `core` / `alloc` imports
/// 2. other imports
/// 3. `self` / `crate` / `super` imports
StdExternalCrate,
}

#[config_type]
pub enum ReportTactic {
Always,
Expand Down
105 changes: 83 additions & 22 deletions src/reorder.rs
Expand Up @@ -11,8 +11,8 @@ use std::cmp::{Ord, Ordering};
use rustc_ast::ast;
use rustc_span::{symbol::sym, Span};

use crate::config::Config;
use crate::imports::{merge_use_trees, UseTree};
use crate::config::{Config, GroupImportsTactic};
use crate::imports::{merge_use_trees, 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;
Expand Down Expand Up @@ -74,9 +74,10 @@ fn rewrite_reorderable_item(
}
}

/// Rewrite a list of items with reordering. Every item in `items` must have
/// the same `ast::ItemKind`.
fn rewrite_reorderable_items(
/// Rewrite a list of items with reordering and/or regrouping. Every item
/// in `items` must have the same `ast::ItemKind`. Whether reordering, regrouping,
/// or both are done is determined from the `context`.
fn rewrite_reorderable_or_regroupable_items(
context: &RewriteContext<'_>,
reorderable_items: &[&ast::Item],
shape: Shape,
Expand Down Expand Up @@ -109,19 +110,35 @@ fn rewrite_reorderable_items(
if context.config.merge_imports() {
normalized_items = merge_use_trees(normalized_items);
}
normalized_items.sort();

let mut regrouped_items = match context.config.group_imports() {
GroupImportsTactic::Preserve => vec![normalized_items],
GroupImportsTactic::StdExternalCrate => group_imports(normalized_items),
};

if context.config.reorder_imports() {
regrouped_items.iter_mut().for_each(|items| items.sort())
}

// 4 = "use ", 1 = ";"
let nested_shape = shape.offset_left(4)?.sub_width(1)?;
let item_vec: Vec<_> = normalized_items
let item_vec: Vec<_> = regrouped_items
.into_iter()
.map(|use_tree| ListItem {
item: use_tree.rewrite_top_level(context, nested_shape),
..use_tree.list_item.unwrap_or_else(ListItem::empty)
.filter(|use_group| !use_group.is_empty())
.map(|use_group| {
let item_vec: Vec<_> = use_group
.into_iter()
.map(|use_tree| ListItem {
item: use_tree.rewrite_top_level(context, nested_shape),
..use_tree.list_item.unwrap_or_else(ListItem::empty)
})
.collect();
wrap_reorderable_items(context, &item_vec, nested_shape)
})
.collect();
.collect::<Option<Vec<_>>>()?;

wrap_reorderable_items(context, &item_vec, nested_shape)
let join_string = format!("\n\n{}", shape.indent.to_string(context.config));
Some(item_vec.join(&join_string))
}
_ => {
let list_items = itemize_list(
Expand Down Expand Up @@ -150,6 +167,34 @@ fn contains_macro_use_attr(item: &ast::Item) -> bool {
crate::attr::contains_name(&item.attrs, sym::macro_use)
}

/// Divides imports into three groups, corresponding to standard, external
/// and local imports. Sorts each subgroup.
fn group_imports(uts: Vec<UseTree>) -> Vec<Vec<UseTree>> {
let mut std_imports = Vec::new();
let mut external_imports = Vec::new();
let mut local_imports = Vec::new();

for ut in uts.into_iter() {
if ut.path.is_empty() {
external_imports.push(ut);
continue;
}
match &ut.path[0] {
UseSegment::Ident(id, _) => match id.as_ref() {
"std" | "alloc" | "core" => std_imports.push(ut),
_ => external_imports.push(ut),
},
UseSegment::Slf(_) | UseSegment::Super(_) | UseSegment::Crate(_) => {
local_imports.push(ut)
}
// These are probably illegal here
UseSegment::Glob | UseSegment::List(_) => external_imports.push(ut),
}
}

vec![std_imports, external_imports, local_imports]
}

/// A simplified version of `ast::ItemKind`.
#[derive(Debug, PartialEq, Eq, Copy, Clone)]
enum ReorderableItemKind {
Expand Down Expand Up @@ -187,21 +232,29 @@ impl ReorderableItemKind {
}
}

fn in_group(self) -> bool {
fn is_regroupable(self, config: &Config) -> bool {
match self {
ReorderableItemKind::ExternCrate
| ReorderableItemKind::Mod
| ReorderableItemKind::Use => true,
| ReorderableItemKind::Other => false,
ReorderableItemKind::Use => config.group_imports() != GroupImportsTactic::Preserve,
}
}

fn in_group(self, config: &Config) -> bool {
match self {
ReorderableItemKind::ExternCrate | ReorderableItemKind::Mod => true,
ReorderableItemKind::Use => config.group_imports() == GroupImportsTactic::Preserve,
ReorderableItemKind::Other => false,
}
}
}

impl<'b, 'a: 'b> FmtVisitor<'a> {
/// Format items with the same item kind and reorder them. If `in_group` is
/// `true`, then the items separated by an empty line will not be reordered
/// together.
fn walk_reorderable_items(
/// Format items with the same item kind and reorder them, regroup them, or
/// both. If `in_group` is `true`, then the items separated by an empty line
/// will not be reordered together.
fn walk_reorderable_or_regroupable_items(
&mut self,
items: &[&ast::Item],
item_kind: ReorderableItemKind,
Expand Down Expand Up @@ -230,7 +283,12 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
let lo = items.first().unwrap().span().lo();
let hi = items.last().unwrap().span().hi();
let span = mk_sp(lo, hi);
let rw = rewrite_reorderable_items(&self.get_context(), items, self.shape(), span);
let rw = rewrite_reorderable_or_regroupable_items(
&self.get_context(),
items,
self.shape(),
span,
);
self.push_rewrite(span, rw);
} else {
for item in items {
Expand All @@ -249,9 +307,12 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
// subsequent items that have the same item kind to be reordered within
// `walk_reorderable_items`. Otherwise, just format the next item for output.
let item_kind = ReorderableItemKind::from(items[0]);
if item_kind.is_reorderable(self.config) {
let visited_items_num =
self.walk_reorderable_items(items, item_kind, item_kind.in_group());
if item_kind.is_reorderable(self.config) || item_kind.is_regroupable(self.config) {
let visited_items_num = self.walk_reorderable_or_regroupable_items(
items,
item_kind,
item_kind.in_group(self.config),
);
let (_, rest) = items.split_at(visited_items_num);
items = rest;
} else {
Expand Down
@@ -0,0 +1,17 @@
// rustfmt-group_imports: StdExternalCrate
// rustfmt-merge_imports: true
use chrono::Utc;
use super::update::convert_publish_payload;

use juniper::{FieldError, FieldResult};
use uuid::Uuid;
use alloc::alloc::Layout;

use std::sync::Arc;
use alloc::vec::Vec;

use broker::database::PooledConnection;

use super::schema::{Context, Payload};
use core::f32;
use crate::models::Event;
6 changes: 6 additions & 0 deletions tests/source/configs/group_imports/StdExternalCrate-nested.rs
@@ -0,0 +1,6 @@
// rustfmt-group_imports: StdExternalCrate
mod test {
use crate::foo::bar;
use std::path;
use crate::foo::bar2;
}
17 changes: 17 additions & 0 deletions tests/source/configs/group_imports/StdExternalCrate-no_reorder.rs
@@ -0,0 +1,17 @@
// rustfmt-group_imports: StdExternalCrate
// rustfmt-reorder_imports: false

use chrono::Utc;
use super::update::convert_publish_payload;

use juniper::{FieldError, FieldResult};
use uuid::Uuid;
use alloc::alloc::Layout;

use std::sync::Arc;

use broker::database::PooledConnection;

use super::schema::{Context, Payload};
use core::f32;
use crate::models::Event;
15 changes: 15 additions & 0 deletions tests/source/configs/group_imports/StdExternalCrate.rs
@@ -0,0 +1,15 @@
// rustfmt-group_imports: StdExternalCrate
use chrono::Utc;
use super::update::convert_publish_payload;

use juniper::{FieldError, FieldResult};
use uuid::Uuid;
use alloc::alloc::Layout;

use std::sync::Arc;

use broker::database::PooledConnection;

use super::schema::{Context, Payload};
use core::f32;
use crate::models::Event;
@@ -0,0 +1,16 @@
// rustfmt-group_imports: StdExternalCrate
// rustfmt-merge_imports: true
use alloc::{alloc::Layout, vec::Vec};
use core::f32;
use std::sync::Arc;

use broker::database::PooledConnection;
use chrono::Utc;
use juniper::{FieldError, FieldResult};
use uuid::Uuid;

use super::{
schema::{Context, Payload},
update::convert_publish_payload,
};
use crate::models::Event;
7 changes: 7 additions & 0 deletions tests/target/configs/group_imports/StdExternalCrate-nested.rs
@@ -0,0 +1,7 @@
// rustfmt-group_imports: StdExternalCrate
mod test {
use std::path;

use crate::foo::bar;
use crate::foo::bar2;
}
15 changes: 15 additions & 0 deletions tests/target/configs/group_imports/StdExternalCrate-no_reorder.rs
@@ -0,0 +1,15 @@
// rustfmt-group_imports: StdExternalCrate
// rustfmt-reorder_imports: false

use alloc::alloc::Layout;
use std::sync::Arc;
use core::f32;

use chrono::Utc;
use juniper::{FieldError, FieldResult};
use uuid::Uuid;
use broker::database::PooledConnection;

use super::update::convert_publish_payload;
use super::schema::{Context, Payload};
use crate::models::Event;
13 changes: 13 additions & 0 deletions tests/target/configs/group_imports/StdExternalCrate.rs
@@ -0,0 +1,13 @@
// rustfmt-group_imports: StdExternalCrate
use alloc::alloc::Layout;
use core::f32;
use std::sync::Arc;

use broker::database::PooledConnection;
use chrono::Utc;
use juniper::{FieldError, FieldResult};
use uuid::Uuid;

use super::schema::{Context, Payload};
use super::update::convert_publish_payload;
use crate::models::Event;

0 comments on commit 073cc38

Please sign in to comment.