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

1.x: Rename merge_imports to imports_granularity and add a Module option. #4634

Merged
Merged
Show file tree
Hide file tree
Changes from 2 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
47 changes: 45 additions & 2 deletions Configurations.md
Original file line number Diff line number Diff line change
Expand Up @@ -1615,13 +1615,56 @@ pub enum Foo {}
pub enum Foo {}
```

## `imports_granularity`

Merge together related imports based on their paths.
Copy link
Member

Choose a reason for hiding this comment

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

Oops, think we missed updating this text in the other PR. Should tweak this to better reflect that the option isn't strictly a merge


- **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.
Copy link
Member

Choose a reason for hiding this comment

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

Same theme here, maybe something more like:

Suggested change
Do not perform any merging and preserve the original structure written by the developer.
Do not change the granularity of any imports 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):

Expand Down
23 changes: 21 additions & 2 deletions src/config/config_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
&_ => (),
}
}
Expand Down Expand Up @@ -156,6 +157,7 @@ macro_rules! create_config {
self.set_heuristics();
self.set_license_template();
self.set_ignore(dir);
self.set_merge_imports();
self
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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 {
Expand Down
72 changes: 70 additions & 2 deletions src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -615,4 +622,65 @@ 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() {
if !crate::is_nightly_channel!() {
return;
}
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() {
if !crate::is_nightly_channel!() {
return;
}
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() {
if !crate::is_nightly_channel!() {
return;
}
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() {
if !crate::is_nightly_channel!() {
return;
}
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 changes: 13 additions & 1 deletion src/config/options.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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> {
Expand Down
Loading