Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ExternalCrate import grouping style #5723

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion Configurations.md
Original file line number Diff line number Diff line change
Expand Up @@ -2229,7 +2229,7 @@ Controls the strategy for how consecutive imports are grouped together.
Controls the strategy for grouping sets of consecutive imports. Imports may contain newlines between imports and still be grouped together as a single set, but other statements between imports will result in different grouping sets.

- **Default value**: `Preserve`
- **Possible values**: `Preserve`, `StdExternalCrate`, `One`
- **Possible values**: `Preserve`, `StdExternalCrate`, `ExternalCrate`, `One`
- **Stable**: No (tracking issue: [#5083](https://github.com/rust-lang/rustfmt/issues/5083))

Each set of imports (one or more `use` statements, optionally separated by newlines) will be formatted independently. Other statements such as `mod ...` or `extern crate ...` will cause imports to not be grouped together.
Expand Down Expand Up @@ -2277,6 +2277,26 @@ use super::update::convert_publish_payload;
use crate::models::Event;
```

#### `ExternalCrate`:

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

```rust
use alloc::alloc::Layout;
use broker::database::PooledConnection;
use chrono::Utc;
use core::f32;
use juniper::{FieldError, FieldResult};
use std::sync::Arc;
use uuid::Uuid;

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

#### `One`:

Discard existing import groups, and create a single group for everything
Expand Down
4 changes: 4 additions & 0 deletions src/config/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ pub enum GroupImportsTactic {
/// 2. other imports
/// 3. `self` / `crate` / `super` imports
StdExternalCrate,
/// Discard existing groups, and create new groups for
/// 1. `std` / `core` / `alloc` / other imports
/// 2. `self` / `crate` / `super` imports
ExternalCrate,
/// Discard existing groups, and create a single group for everything
One,
}
Expand Down
31 changes: 29 additions & 2 deletions src/reorder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,10 @@ fn rewrite_reorderable_or_regroupable_items(
GroupImportsTactic::Preserve | GroupImportsTactic::One => {
vec![normalized_items]
}
GroupImportsTactic::StdExternalCrate => group_imports(normalized_items),
GroupImportsTactic::StdExternalCrate => {
group_imports_std_external_crate(normalized_items)
}
GroupImportsTactic::ExternalCrate => group_imports_external_crate(normalized_items),
};

if context.config.reorder_imports() {
Expand Down Expand Up @@ -172,7 +175,7 @@ fn contains_macro_use_attr(item: &ast::Item) -> bool {

/// Divides imports into three groups, corresponding to standard, external
/// and local imports. Sorts each subgroup.
fn group_imports(uts: Vec<UseTree>) -> Vec<Vec<UseTree>> {
fn group_imports_std_external_crate(uts: Vec<UseTree>) -> Vec<Vec<UseTree>> {
let mut std_imports = Vec::new();
let mut external_imports = Vec::new();
let mut local_imports = Vec::new();
Expand All @@ -198,6 +201,30 @@ fn group_imports(uts: Vec<UseTree>) -> Vec<Vec<UseTree>> {
vec![std_imports, external_imports, local_imports]
}

/// Divides imports into two groups, corresponding to external crates
/// and local imports. Sorts each subgroup.
fn group_imports_external_crate(uts: Vec<UseTree>) -> Vec<Vec<UseTree>> {
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].kind {
UseSegmentKind::Ident(..) => external_imports.push(ut),
UseSegmentKind::Slf(_) | UseSegmentKind::Super(_) | UseSegmentKind::Crate(_) => {
local_imports.push(ut)
}
// These are probably illegal here
UseSegmentKind::Glob | UseSegmentKind::List(_) => external_imports.push(ut),
}
}

vec![external_imports, local_imports]
}

Comment on lines +204 to +227
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There could be a way to deduplicate this and StdExternalCrate, by either calling group_imports_std_external_crate in this function or calling this function in the other function.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been a while since I was in the code, but I have a vague recollection that there wasn't much that was useful to DRY up here. That being said, WDYT about something that results in:

    for ut in uts {
        match UseCategory::categorize(ut) {
            UseCategory::Std(ut) => std_imports.push(ut),
            UseCategory::External(ut) => external_imports.push(ut),
            UseCategory::Local(ut) => local_imports.push(ut),
        }
    }

and

    for ut in uts {
        match UseCategory::categorize(ut) {
            UseCategory::Std(ut) | UseCategory::External(ut) => external_imports.push(ut),
            UseCategory::Local(ut) => local_imports.push(ut),
        }
    }

/// A simplified version of `ast::ItemKind`.
#[derive(Debug, PartialEq, Eq, Copy, Clone)]
enum ReorderableItemKind {
Expand Down
17 changes: 17 additions & 0 deletions tests/source/configs/group_imports/ExternalCrate-merge_imports.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// rustfmt-group_imports: ExternalCrate
// rustfmt-imports_granularity: Crate
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;
7 changes: 7 additions & 0 deletions tests/source/configs/group_imports/ExternalCrate-nested.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// rustfmt-group_imports: ExternalCrate
mod test {
use crate::foo::bar;
use std::path;
use crate::foo::bar2;
use uuid::Uuid;
}
17 changes: 17 additions & 0 deletions tests/source/configs/group_imports/ExternalCrate-no_reorder.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// rustfmt-group_imports: ExternalCrate
// 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;
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// rustfmt-group_imports: ExternalCrate
use chrono::Utc;
use super::update::convert_publish_payload;





use juniper::{FieldError, FieldResult};

use uuid::Uuid;
use alloc::alloc::Layout;

extern crate uuid;





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/ExternalCrate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// rustfmt-group_imports: ExternalCrate
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/target/configs/group_imports/ExternalCrate-merge_imports.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// rustfmt-group_imports: ExternalCrate
// rustfmt-imports_granularity: Crate
use alloc::{alloc::Layout, vec::Vec};
use broker::database::PooledConnection;
use chrono::Utc;
use core::f32;
use juniper::{FieldError, FieldResult};
use std::sync::Arc;
use uuid::Uuid;

use super::{
schema::{Context, Payload},
update::convert_publish_payload,
};
use crate::models::Event;
8 changes: 8 additions & 0 deletions tests/target/configs/group_imports/ExternalCrate-nested.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// rustfmt-group_imports: ExternalCrate
mod test {
use std::path;
use uuid::Uuid;

use crate::foo::bar;
use crate::foo::bar2;
}
14 changes: 14 additions & 0 deletions tests/target/configs/group_imports/ExternalCrate-no_reorder.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// rustfmt-group_imports: ExternalCrate
// rustfmt-reorder_imports: false

use chrono::Utc;
use juniper::{FieldError, FieldResult};
use uuid::Uuid;
use alloc::alloc::Layout;
use std::sync::Arc;
use broker::database::PooledConnection;
use core::f32;

use super::update::convert_publish_payload;
use super::schema::{Context, Payload};
use crate::models::Event;
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// rustfmt-group_imports: ExternalCrate
use alloc::alloc::Layout;
use chrono::Utc;
use juniper::{FieldError, FieldResult};
use uuid::Uuid;

use super::update::convert_publish_payload;

extern crate uuid;

use broker::database::PooledConnection;
use core::f32;
use std::sync::Arc;

use super::schema::{Context, Payload};
use crate::models::Event;
12 changes: 12 additions & 0 deletions tests/target/configs/group_imports/ExternalCrate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// rustfmt-group_imports: ExternalCrate
use alloc::alloc::Layout;
use broker::database::PooledConnection;
use chrono::Utc;
use core::f32;
use juniper::{FieldError, FieldResult};
use std::sync::Arc;
use uuid::Uuid;

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