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

implement Style Edition support #6247

Merged
merged 9 commits into from
Jul 29, 2024
17 changes: 13 additions & 4 deletions Configurations.md
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ Note that this option may be soft-deprecated in the future once the [ignore](#ig
Specifies which edition is used by the parser.

- **Default value**: `"2015"`
- **Possible values**: `"2015"`, `"2018"`, `"2021"`
- **Possible values**: `"2015"`, `"2018"`, `"2021"`, `"2024"`
- **Stable**: Yes

Rustfmt is able to pick up the edition used by reading the `Cargo.toml` file if executed
Expand Down Expand Up @@ -2692,6 +2692,17 @@ By default this option is set as a percentage of [`max_width`](#max_width) provi

See also [`max_width`](#max_width) and [`use_small_heuristics`](#use_small_heuristics)

## `style_edition`

Controls the edition of the [Rust Style Guide] to use for formatting ([RFC 3338])

- **Default value**: `"2015"`
- **Possible values**: `"2015"`, `"2018"`, `"2021"`, `"2024"` (unstable variant)
ytmimi marked this conversation as resolved.
Show resolved Hide resolved
- **Stable**: No

[Rust Style Guide]: https://doc.rust-lang.org/nightly/style-guide/
[RFC 3338]: https://rust-lang.github.io/rfcs/3338-style-evolution.html

## `tab_spaces`

Number of spaces per tab
Expand Down Expand Up @@ -3051,9 +3062,7 @@ fn main() {

## `version`

Which version of the formatting rules to use. `Version::One` is backwards-compatible
with Rustfmt 1.0. Other versions are only backwards compatible within a major
version number.
This option is deprecated and has been replaced by [`style_edition`](#style_edition)

- **Default value**: `One`
- **Possible values**: `One`, `Two`
Expand Down
21 changes: 11 additions & 10 deletions Contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,17 @@ If you want to test modified `cargo-fmt`, or run `rustfmt` on the whole project
RUSTFMT="./target/debug/rustfmt" cargo run --bin cargo-fmt -- --manifest-path path/to/project/you/want2test/Cargo.toml
```

### Version-gate formatting changes
### Gate formatting changes

A change that introduces a different code-formatting should be gated on the
`version` configuration. This is to ensure the formatting of the current major
release is preserved, while allowing fixes to be implemented for the next
release.
A change that introduces a different code-formatting must be gated on the
`style_edition` configuration. This is to ensure rustfmt upholds its formatting
stability guarantees and adheres to the Style Edition process set in [RFC 3338]

This is done by conditionally guarding the change like so:
This can be done by conditionally guarding the formatting change, e.g.:

```rust
if config.version() == Version::One { // if the current major release is 1.x
// if the current stable Style Edition is Edition 2024
if config.style_edition() <= StyleEdition::Edition2024 {
// current formatting
} else {
// new formatting
Expand All @@ -129,13 +129,14 @@ if config.version() == Version::One { // if the current major release is 1.x
This allows the user to apply the next formatting explicitly via the
configuration, while being stable by default.

When the next major release is done, the code block of the previous formatting
can be deleted, e.g., the first block in the example above when going from `1.x`
to `2.x`.
This can then be enhanced as needed if and when there are
new Style Editions with differing formatting prescriptions.

| Note: Only formatting changes with default options need to be gated. |
| --- |

[RFC 3338]: https://rust-lang.github.io/rfcs/3338-style-evolution.html

### A quick tour of Rustfmt

Rustfmt is basically a pretty printer - that is, its mode of operation is to
Expand Down
2 changes: 1 addition & 1 deletion rustfmt.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
error_on_line_overflow = true
error_on_unformatted = true
version = "Two"
style_edition = "2024"
174 changes: 172 additions & 2 deletions src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use getopts::{Matches, Options};

use crate::rustfmt::{
load_config, CliOptions, Color, Config, Edition, EmitMode, FileLines, FileName,
FormatReportFormatterBuilder, Input, Session, Verbosity,
FormatReportFormatterBuilder, Input, Session, StyleEdition, Verbosity,
};

const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rustfmt/issues/new?labels=bug";
Expand Down Expand Up @@ -129,7 +129,12 @@ fn make_opts() -> Options {
found reverts to the input file path",
"[Path for the configuration file]",
);
opts.optopt("", "edition", "Rust edition to use", "[2015|2018|2021]");
opts.optopt(
"",
"edition",
"Rust edition to use",
"[2015|2018|2021|2024]",
ytmimi marked this conversation as resolved.
Show resolved Hide resolved
);
opts.optopt(
"",
"color",
Expand Down Expand Up @@ -181,6 +186,12 @@ fn make_opts() -> Options {
"skip-children",
"Don't reformat child modules (unstable).",
);
opts.optopt(
"",
"style-edition",
"The edition of the Style Guide (unstable).",
"[2015|2018|2021|2024]",
);
}

opts.optflag("v", "verbose", "Print verbose output");
Expand Down Expand Up @@ -525,6 +536,7 @@ struct GetOptsOptions {
backup: bool,
check: bool,
edition: Option<Edition>,
style_edition: Option<StyleEdition>,
color: Option<Color>,
file_lines: FileLines, // Default is all lines in all files.
unstable_features: bool,
Expand Down Expand Up @@ -556,6 +568,10 @@ impl GetOptsOptions {
if let Some(ref file_lines) = matches.opt_str("file-lines") {
options.file_lines = file_lines.parse()?;
}
if let Some(ref edition_str) = matches.opt_str("style-edition") {
options.style_edition =
Some(style_edition_from_style_edition_str(edition_str)?);
}
} else {
let mut unstable_options = vec![];
if matches.opt_present("skip-children") {
Expand All @@ -567,6 +583,9 @@ impl GetOptsOptions {
if matches.opt_present("file-lines") {
unstable_options.push("`--file-lines`");
}
if matches.opt_present("style-edition") {
unstable_options.push("`--style-edition`");
}
if !unstable_options.is_empty() {
let s = if unstable_options.len() == 1 { "" } else { "s" };
return Err(format_err!(
Expand Down Expand Up @@ -688,6 +707,9 @@ impl CliOptions for GetOptsOptions {
if let Some(edition) = self.edition {
config.set_cli().edition(edition);
}
if let Some(edition) = self.style_edition {
config.set_cli().style_edition(edition);
}
if self.check {
config.set_cli().emit_mode(EmitMode::Diff);
} else if let Some(emit_mode) = self.emit_mode {
Expand Down Expand Up @@ -723,6 +745,16 @@ fn edition_from_edition_str(edition_str: &str) -> Result<Edition> {
}
}

fn style_edition_from_style_edition_str(edition_str: &str) -> Result<StyleEdition> {
match edition_str {
"2015" => Ok(StyleEdition::Edition2015),
"2018" => Ok(StyleEdition::Edition2018),
"2021" => Ok(StyleEdition::Edition2021),
"2024" => Ok(StyleEdition::Edition2024),
_ => Err(format_err!("Invalid value for `--style-edition`")),
}
}
ytmimi marked this conversation as resolved.
Show resolved Hide resolved

fn emit_mode_from_emit_str(emit_str: &str) -> Result<EmitMode> {
match emit_str {
"files" => Ok(EmitMode::Files),
Expand All @@ -733,3 +765,141 @@ fn emit_mode_from_emit_str(emit_str: &str) -> Result<EmitMode> {
_ => Err(format_err!("Invalid value for `--emit`")),
}
}

#[cfg(test)]
#[allow(dead_code)]
mod test {
use super::*;
use rustfmt_config_proc_macro::nightly_only_test;

fn get_config<O: CliOptions>(path: Option<&Path>, options: Option<O>) -> Config {
load_config(path, options).unwrap().0
}

#[nightly_only_test]
#[test]
fn flag_sets_style_edition_override_correctly() {
let mut options = GetOptsOptions::default();
options.style_edition = Some(StyleEdition::Edition2024);
let config = get_config(None, Some(options));
assert_eq!(config.style_edition(), StyleEdition::Edition2024);
}

#[nightly_only_test]
#[test]
fn edition_sets_style_edition_override_correctly() {
let mut options = GetOptsOptions::default();
options.edition = Some(Edition::Edition2024);
let config = get_config(None, Some(options));
assert_eq!(config.style_edition(), StyleEdition::Edition2024);
}

#[nightly_only_test]
#[test]
fn version_sets_style_edition_override_correctly() {
let mut options = GetOptsOptions::default();
options.inline_config = HashMap::from([("version".to_owned(), "Two".to_owned())]);
let config = get_config(None, Some(options));
assert_eq!(config.style_edition(), StyleEdition::Edition2024);
}

#[nightly_only_test]
#[test]
fn style_edition_flag_has_correct_precedence_over_edition() {
let mut options = GetOptsOptions::default();
options.style_edition = Some(StyleEdition::Edition2021);
options.edition = Some(Edition::Edition2024);
let config = get_config(None, Some(options));
assert_eq!(config.style_edition(), StyleEdition::Edition2021);
}

#[nightly_only_test]
#[test]
fn style_edition_flag_has_correct_precedence_over_version() {
let mut options = GetOptsOptions::default();
options.style_edition = Some(StyleEdition::Edition2018);
options.inline_config = HashMap::from([("version".to_owned(), "Two".to_owned())]);
let config = get_config(None, Some(options));
assert_eq!(config.style_edition(), StyleEdition::Edition2018);
}

#[nightly_only_test]
#[test]
fn style_edition_flag_has_correct_precedence_over_edition_version() {
let mut options = GetOptsOptions::default();
options.style_edition = Some(StyleEdition::Edition2021);
options.edition = Some(Edition::Edition2018);
options.inline_config = HashMap::from([("version".to_owned(), "Two".to_owned())]);
let config = get_config(None, Some(options));
assert_eq!(config.style_edition(), StyleEdition::Edition2021);
}

#[nightly_only_test]
#[test]
fn style_edition_inline_has_correct_precedence_over_edition_version() {
let mut options = GetOptsOptions::default();
options.edition = Some(Edition::Edition2018);
options.inline_config = HashMap::from([
("version".to_owned(), "One".to_owned()),
("style_edition".to_owned(), "2024".to_owned()),
]);
let config = get_config(None, Some(options));
assert_eq!(config.style_edition(), StyleEdition::Edition2024);
}

#[nightly_only_test]
#[test]
fn style_edition_config_file_trumps_edition_flag_version_inline() {
let mut options = GetOptsOptions::default();
let config_file = Some(Path::new("tests/config/style-edition/just-style-edition"));
options.edition = Some(Edition::Edition2018);
options.inline_config = HashMap::from([("version".to_owned(), "One".to_owned())]);
let config = get_config(config_file, Some(options));
assert_eq!(config.style_edition(), StyleEdition::Edition2024);
}

#[nightly_only_test]
#[test]
fn style_edition_config_file_trumps_edition_config_and_version_inline() {
let mut options = GetOptsOptions::default();
let config_file = Some(Path::new(
"tests/config/style-edition/style-edition-and-edition",
));
options.inline_config = HashMap::from([("version".to_owned(), "Two".to_owned())]);
let config = get_config(config_file, Some(options));
assert_eq!(config.style_edition(), StyleEdition::Edition2021);
assert_eq!(config.edition(), Edition::Edition2024);
}

#[nightly_only_test]
#[test]
fn version_config_trumps_edition_config_and_flag() {
let mut options = GetOptsOptions::default();
let config_file = Some(Path::new("tests/config/style-edition/version-edition"));
options.edition = Some(Edition::Edition2018);
let config = get_config(config_file, Some(options));
assert_eq!(config.style_edition(), StyleEdition::Edition2024);
}

#[nightly_only_test]
#[test]
fn style_edition_config_file_trumps_version_config() {
let options = GetOptsOptions::default();
let config_file = Some(Path::new(
"tests/config/style-edition/version-style-edition",
));
let config = get_config(config_file, Some(options));
assert_eq!(config.style_edition(), StyleEdition::Edition2021);
}

#[nightly_only_test]
#[test]
fn style_edition_config_file_trumps_edition_version_config() {
let options = GetOptsOptions::default();
let config_file = Some(Path::new(
"tests/config/style-edition/version-style-edition-and-edition",
));
let config = get_config(config_file, Some(options));
assert_eq!(config.style_edition(), StyleEdition::Edition2021);
}
}
4 changes: 2 additions & 2 deletions src/chains.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ use rustc_ast::{ast, ptr};
use rustc_span::{symbol, BytePos, Span};

use crate::comment::{rewrite_comment, CharClasses, FullCodeCharKind, RichChar};
use crate::config::{IndentStyle, Version};
use crate::config::{IndentStyle, StyleEdition};
use crate::expr::rewrite_call;
use crate::lists::extract_pre_comment;
use crate::macros::convert_try_mac;
Expand Down Expand Up @@ -285,7 +285,7 @@ impl Rewrite for ChainItem {
ChainItemKind::StructField(ident) => format!(".{}", rewrite_ident(context, ident)),
ChainItemKind::TupleField(ident, nested) => format!(
"{}.{}",
if nested && context.config.version() == Version::One {
if nested && context.config.style_edition() <= StyleEdition::Edition2021 {
calebcartwright marked this conversation as resolved.
Show resolved Hide resolved
" "
} else {
""
Expand Down
10 changes: 5 additions & 5 deletions src/closures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use thin_vec::thin_vec;

use crate::attr::get_attrs_from_stmt;
use crate::config::lists::*;
use crate::config::Version;
use crate::config::StyleEdition;
use crate::expr::{block_contains_comment, is_simple_block, is_unsafe_block, rewrite_cond};
use crate::items::{span_hi_for_param, span_lo_for_param};
use crate::lists::{definitive_tactic, itemize_list, write_list, ListFormatting, Separator};
Expand Down Expand Up @@ -457,18 +457,18 @@ fn is_block_closure_forced(context: &RewriteContext<'_>, expr: &ast::Expr) -> bo
if context.inside_macro() {
false
} else {
is_block_closure_forced_inner(expr, context.config.version())
is_block_closure_forced_inner(expr, context.config.style_edition())
}
}

fn is_block_closure_forced_inner(expr: &ast::Expr, version: Version) -> bool {
fn is_block_closure_forced_inner(expr: &ast::Expr, style_edition: StyleEdition) -> bool {
match expr.kind {
ast::ExprKind::If(..) | ast::ExprKind::While(..) | ast::ExprKind::ForLoop { .. } => true,
ast::ExprKind::Loop(..) if version == Version::Two => true,
ast::ExprKind::Loop(..) if style_edition >= StyleEdition::Edition2024 => true,
ast::ExprKind::AddrOf(_, _, ref expr)
| ast::ExprKind::Try(ref expr)
| ast::ExprKind::Unary(_, ref expr)
| ast::ExprKind::Cast(ref expr, _) => is_block_closure_forced_inner(expr, version),
| ast::ExprKind::Cast(ref expr, _) => is_block_closure_forced_inner(expr, style_edition),
_ => false,
}
}
Expand Down
Loading
Loading