From c84b8935e12a6a659ff755d489980742f773a1f6 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Thu, 1 Aug 2024 18:42:45 -0500 Subject: [PATCH 1/5] refactor: use style edition when loading from partial config --- src/config/config_type.rs | 2 +- src/config/mod.rs | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/config/config_type.rs b/src/config/config_type.rs index 91efb71744b..4b83e974932 100644 --- a/src/config/config_type.rs +++ b/src/config/config_type.rs @@ -210,7 +210,7 @@ macro_rules! create_config { )+ #[allow(unreachable_pub)] - pub fn default_with_style_edition(style_edition: StyleEdition) -> Config { + pub(super) fn default_with_style_edition(style_edition: StyleEdition) -> Config { Config { $( $i: ( diff --git a/src/config/mod.rs b/src/config/mod.rs index 5d6047c385d..6462c62a358 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -217,9 +217,37 @@ impl PartialConfig { ::toml::to_string(&cloned).map_err(ToTomlError) } + + pub(super) fn to_parsed_config( + self, + style_edition_override: Option, + edition_override: Option, + dir: &Path, + ) -> Config { + Config::default_for_possible_style_edition( + style_edition_override.or(self.style_edition), + edition_override.or(self.edition), + ) + .fill_from_parsed_config(self, dir) + } } impl Config { + pub fn default_for_possible_style_edition( + style_edition: Option, + edition: Option, + ) -> Config { + style_edition.map_or_else( + || { + edition.map_or_else( + || Config::default(), + |e| Self::default_with_style_edition(e.into()), + ) + }, + |se| Self::default_with_style_edition(se), + ) + } + pub(crate) fn version_meets_requirement(&self) -> bool { if self.was_set().required_version() { let version = env!("CARGO_PKG_VERSION"); @@ -324,12 +352,13 @@ impl Config { err.push_str(msg) } } - match parsed.try_into() { + + match parsed.try_into::() { Ok(parsed_config) => { if !err.is_empty() { eprint!("{err}"); } - Ok(Config::default().fill_from_parsed_config(parsed_config, dir)) + Ok(parsed_config.to_parsed_config(None, None, dir)) } Err(e) => { err.push_str("Error: Decoding config file failed:\n"); From 5a168fff4deebd5bcd440ad093e27a9555cfd392 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Thu, 1 Aug 2024 22:14:23 -0500 Subject: [PATCH 2/5] refactor: include edition & style edition in CliOptions --- src/bin/main.rs | 12 ++++++++++++ src/config/mod.rs | 10 +++++++--- src/config/options.rs | 2 ++ src/git-rustfmt/main.rs | 6 ++++++ 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index 1185454c8e7..2e4877c7569 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -734,6 +734,18 @@ impl CliOptions for GetOptsOptions { fn config_path(&self) -> Option<&Path> { self.config_path.as_deref() } + + fn edition(&self) -> Option { + self.inline_config + .get("edition") + .map_or(self.edition, |e| Edition::from_str(e).ok()) + } + + fn style_edition(&self) -> Option { + self.inline_config + .get("style_edition") + .map_or(self.style_edition, |se| StyleEdition::from_str(se).ok()) + } } fn edition_from_edition_str(edition_str: &str) -> Result { diff --git a/src/config/mod.rs b/src/config/mod.rs index 6462c62a358..3b77902fa2a 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -376,9 +376,13 @@ pub fn load_config( file_path: Option<&Path>, options: Option, ) -> Result<(Config, Option), Error> { - let over_ride = match options { - Some(ref opts) => config_path(opts)?, - None => None, + let (over_ride, _edition, _style_edition) = match options { + Some(ref opts) => ( + config_path(opts)?, + opts.edition(), + opts.style_edition(), + ), + None => (None, None, None), }; let result = if let Some(over_ride) = over_ride { diff --git a/src/config/options.rs b/src/config/options.rs index 40cfafed29d..f7a8c114330 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -419,6 +419,8 @@ pub trait CliOptions { /// It is ok if the returned path doesn't exist or is not canonicalized /// (i.e. the callers are expected to handle such cases). fn config_path(&self) -> Option<&Path>; + fn edition(&self) -> Option; + fn style_edition(&self) -> Option; } /// The edition of the syntax and semantics of code (RFC 2052). diff --git a/src/git-rustfmt/main.rs b/src/git-rustfmt/main.rs index 5674f40bef9..14ac81322b9 100644 --- a/src/git-rustfmt/main.rs +++ b/src/git-rustfmt/main.rs @@ -89,6 +89,12 @@ impl CliOptions for NullOptions { fn config_path(&self) -> Option<&Path> { unreachable!(); } + fn edition(&self) -> Option { + unreachable!(); + } + fn style_edition(&self) -> Option { + unreachable!(); + } } fn uncommitted_files() -> Vec { From 8f728b0e2b280a5f4f4381eea6255e1f06dcff70 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Thu, 1 Aug 2024 23:17:26 -0500 Subject: [PATCH 3/5] feat: support style edition differing defaults in config loading --- src/config/mod.rs | 61 ++++++++++++++++++++++++++++++++++------------- src/test/mod.rs | 55 +++++++++++++++++++++++++++++++++++------- 2 files changed, 90 insertions(+), 26 deletions(-) diff --git a/src/config/mod.rs b/src/config/mod.rs index 3b77902fa2a..da7adea5a74 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -271,12 +271,21 @@ impl Config { /// /// Returns a `Config` if the config could be read and parsed from /// the file, otherwise errors. - pub(super) fn from_toml_path(file_path: &Path) -> Result { + pub(super) fn from_toml_path( + file_path: &Path, + edition: Option, + style_edition: Option, + ) -> Result { let mut file = File::open(&file_path)?; let mut toml = String::new(); file.read_to_string(&mut toml)?; - Config::from_toml(&toml, file_path.parent().unwrap()) - .map_err(|err| Error::new(ErrorKind::InvalidData, err)) + Config::from_toml_for_style_edition( + &toml, + file_path.parent().unwrap(), + edition, + style_edition, + ) + .map_err(|err| Error::new(ErrorKind::InvalidData, err)) } /// Resolves the config for input in `dir`. @@ -288,7 +297,11 @@ impl Config { /// /// Returns the `Config` to use, and the path of the project file if there was /// one. - pub(super) fn from_resolved_toml_path(dir: &Path) -> Result<(Config, Option), Error> { + pub(super) fn from_resolved_toml_path( + dir: &Path, + edition: Option, + style_edition: Option, + ) -> Result<(Config, Option), Error> { /// Try to find a project file in the given directory and its parents. /// Returns the path of the nearest project file if one exists, /// or `None` if no project file was found. @@ -333,12 +346,26 @@ impl Config { } match resolve_project_file(dir)? { - None => Ok((Config::default(), None)), - Some(path) => Config::from_toml_path(&path).map(|config| (config, Some(path))), + None => Ok(( + Config::default_for_possible_style_edition(style_edition, edition), + None, + )), + Some(path) => Config::from_toml_path(&path, edition, style_edition) + .map(|config| (config, Some(path))), } } - pub(crate) fn from_toml(toml: &str, dir: &Path) -> Result { + #[allow(dead_code)] + pub(super) fn from_toml(toml: &str, dir: &Path) -> Result { + Self::from_toml_for_style_edition(toml, dir, None, None) + } + + pub(crate) fn from_toml_for_style_edition( + toml: &str, + dir: &Path, + edition: Option, + style_edition: Option, + ) -> Result { let parsed: ::toml::Value = toml .parse() .map_err(|e| format!("Could not parse TOML: {}", e))?; @@ -358,7 +385,7 @@ impl Config { if !err.is_empty() { eprint!("{err}"); } - Ok(parsed_config.to_parsed_config(None, None, dir)) + Ok(parsed_config.to_parsed_config(style_edition, edition, dir)) } Err(e) => { err.push_str("Error: Decoding config file failed:\n"); @@ -376,21 +403,21 @@ pub fn load_config( file_path: Option<&Path>, options: Option, ) -> Result<(Config, Option), Error> { - let (over_ride, _edition, _style_edition) = match options { - Some(ref opts) => ( - config_path(opts)?, - opts.edition(), - opts.style_edition(), - ), + let (over_ride, edition, style_edition) = match options { + Some(ref opts) => (config_path(opts)?, opts.edition(), opts.style_edition()), None => (None, None, None), }; let result = if let Some(over_ride) = over_ride { - Config::from_toml_path(over_ride.as_ref()).map(|p| (p, Some(over_ride.to_owned()))) + Config::from_toml_path(over_ride.as_ref(), edition, style_edition) + .map(|p| (p, Some(over_ride.to_owned()))) } else if let Some(file_path) = file_path { - Config::from_resolved_toml_path(file_path) + Config::from_resolved_toml_path(file_path, edition, style_edition) } else { - Ok((Config::default(), None)) + Ok(( + Config::default_for_possible_style_edition(style_edition, edition), + None, + )) }; result.map(|(mut c, p)| { diff --git a/src/test/mod.rs b/src/test/mod.rs index 7c563801c32..96706efb161 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -6,14 +6,17 @@ use std::iter::Peekable; use std::mem; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; -use std::str::Chars; +use std::str::{Chars, FromStr}; use std::thread; use crate::config::{Color, Config, EmitMode, FileName, NewlineStyle}; use crate::formatting::{ReportedErrors, SourceFile}; use crate::rustfmt_diff::{make_diff, print_diff, DiffLine, Mismatch, ModifiedChunk, OutputWriter}; use crate::source_file; -use crate::{is_nightly_channel, FormatReport, FormatReportFormatterBuilder, Input, Session}; +use crate::{ + is_nightly_channel, Edition, FormatReport, FormatReportFormatterBuilder, Input, Session, + StyleEdition, +}; use rustfmt_config_proc_macro::nightly_only_test; @@ -710,13 +713,22 @@ fn print_mismatches String>( fn read_config(filename: &Path) -> Config { let sig_comments = read_significant_comments(filename); + let (edition, style_edition) = get_editions_from_comments(&sig_comments); // Look for a config file. If there is a 'config' property in the significant comments, use // that. Otherwise, if there are no significant comments at all, look for a config file with // the same name as the test file. let mut config = if !sig_comments.is_empty() { - get_config(sig_comments.get("config").map(Path::new)) + get_config( + sig_comments.get("config").map(Path::new), + edition, + style_edition, + ) } else { - get_config(filename.with_extension("toml").file_name().map(Path::new)) + get_config( + filename.with_extension("toml").file_name().map(Path::new), + edition, + style_edition, + ) }; for (key, val) in &sig_comments { @@ -747,13 +759,28 @@ enum IdempotentCheckError { Parse, } +fn get_editions_from_comments( + comments: &HashMap, +) -> (Option, Option) { + ( + comments + .get("edition") + .map(|e| Edition::from_str(e).expect(&format!("invalid edition value: '{}'", e))), + comments.get("style_edition").map(|se| { + StyleEdition::from_str(se).expect(&format!("invalid style_edition value: '{}'", se)) + }), + ) +} + fn idempotent_check( filename: &PathBuf, opt_config: &Option, ) -> Result { let sig_comments = read_significant_comments(filename); let config = if let Some(ref config_file_path) = opt_config { - Config::from_toml_path(config_file_path).expect("`rustfmt.toml` not found") + let (edition, style_edition) = get_editions_from_comments(&sig_comments); + Config::from_toml_path(config_file_path, edition, style_edition) + .expect("`rustfmt.toml` not found") } else { read_config(filename) }; @@ -777,14 +804,18 @@ fn idempotent_check( // Reads test config file using the supplied (optional) file name. If there's no file name or the // file doesn't exist, just return the default config. Otherwise, the file must be read // successfully. -fn get_config(config_file: Option<&Path>) -> Config { +fn get_config( + config_file: Option<&Path>, + edition: Option, + style_edition: Option, +) -> Config { let config_file_name = match config_file { - None => return Default::default(), + None => return Config::default_for_possible_style_edition(style_edition, edition), Some(file_name) => { let mut full_path = PathBuf::from("tests/config/"); full_path.push(file_name); if !full_path.exists() { - return Default::default(); + return Config::default_for_possible_style_edition(style_edition, edition); }; full_path } @@ -796,7 +827,13 @@ fn get_config(config_file: Option<&Path>) -> Config { .read_to_string(&mut def_config) .expect("Couldn't read config"); - Config::from_toml(&def_config, Path::new("tests/config/")).expect("invalid TOML") + Config::from_toml_for_style_edition( + &def_config, + Path::new("tests/config/"), + edition, + style_edition, + ) + .expect("invalid TOML") } // Reads significant comments of the form: `// rustfmt-key: value` into a hash map. From b7b383559cb5339073e136c463fe50c983412460 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Thu, 1 Aug 2024 23:26:00 -0500 Subject: [PATCH 4/5] feat: implement 2024 Style Edition for expr overflows --- rustfmt.toml | 1 + src/bin/main.rs | 32 ++++ src/config/mod.rs | 2 +- src/config/options.rs | 2 +- .../style-edition/overrides/rustfmt.toml | 2 + .../style_edition/overflow_delim_expr_2015.rs | 155 ++++++++++++++++++ .../style_edition/overflow_delim_expr_2024.rs | 155 ++++++++++++++++++ .../style_edition/overflow_delim_expr_2015.rs | 135 +++++++++++++++ .../style_edition/overflow_delim_expr_2024.rs | 120 ++++++++++++++ 9 files changed, 602 insertions(+), 2 deletions(-) create mode 100644 tests/config/style-edition/overrides/rustfmt.toml create mode 100644 tests/source/configs/style_edition/overflow_delim_expr_2015.rs create mode 100644 tests/source/configs/style_edition/overflow_delim_expr_2024.rs create mode 100644 tests/target/configs/style_edition/overflow_delim_expr_2015.rs create mode 100644 tests/target/configs/style_edition/overflow_delim_expr_2024.rs diff --git a/rustfmt.toml b/rustfmt.toml index 52e4d728b64..86447eac2e6 100644 --- a/rustfmt.toml +++ b/rustfmt.toml @@ -1,3 +1,4 @@ error_on_line_overflow = true error_on_unformatted = true style_edition = "2024" +overflow_delimited_expr = false diff --git a/src/bin/main.rs b/src/bin/main.rs index 2e4877c7569..9e7476b1f81 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -915,4 +915,36 @@ mod test { let config = get_config(config_file, Some(options)); assert_eq!(config.style_edition(), StyleEdition::Edition2021); } + + #[nightly_only_test] + #[test] + fn correct_defaults_for_style_edition_loaded() { + 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); + assert_eq!(config.overflow_delimited_expr(), true); + } + + #[nightly_only_test] + #[test] + fn style_edition_defaults_overridden_from_config() { + let options = GetOptsOptions::default(); + let config_file = Some(Path::new("tests/config/style-edition/overrides")); + let config = get_config(config_file, Some(options)); + assert_eq!(config.style_edition(), StyleEdition::Edition2024); + assert_eq!(config.overflow_delimited_expr(), false); + } + + #[nightly_only_test] + #[test] + fn style_edition_defaults_overridden_from_cli() { + let mut options = GetOptsOptions::default(); + let config_file = Some(Path::new("tests/config/style-edition/just-style-edition")); + options.inline_config = + HashMap::from([("overflow_delimited_expr".to_owned(), "false".to_owned())]); + let config = get_config(config_file, Some(options)); + assert_eq!(config.style_edition(), StyleEdition::Edition2024); + assert_eq!(config.overflow_delimited_expr(), false); + } } diff --git a/src/config/mod.rs b/src/config/mod.rs index da7adea5a74..cd6870e3890 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -828,7 +828,7 @@ binop_separator = "Front" remove_nested_parens = true combine_control_expr = true short_array_element_width_threshold = 10 -overflow_delimited_expr = false +overflow_delimited_expr = true struct_field_align_threshold = 0 enum_discrim_align_threshold = 0 match_arm_blocks = true diff --git a/src/config/options.rs b/src/config/options.rs index f7a8c114330..46db5186b11 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -627,7 +627,7 @@ config_option_with_style_edition_default!( RemoveNestedParens, bool, _ => true; CombineControlExpr, bool, _ => true; ShortArrayElementWidthThreshold, usize, _ => 10; - OverflowDelimitedExpr, bool, _ => false; + OverflowDelimitedExpr, bool, Edition2024 => true, _ => false; StructFieldAlignThreshold, usize, _ => 0; EnumDiscrimAlignThreshold, usize, _ => 0; MatchArmBlocks, bool, _ => true; diff --git a/tests/config/style-edition/overrides/rustfmt.toml b/tests/config/style-edition/overrides/rustfmt.toml new file mode 100644 index 00000000000..24205692b1f --- /dev/null +++ b/tests/config/style-edition/overrides/rustfmt.toml @@ -0,0 +1,2 @@ +style_edition = "2024" +overflow_delimited_expr = false diff --git a/tests/source/configs/style_edition/overflow_delim_expr_2015.rs b/tests/source/configs/style_edition/overflow_delim_expr_2015.rs new file mode 100644 index 00000000000..5cb4a870fc1 --- /dev/null +++ b/tests/source/configs/style_edition/overflow_delim_expr_2015.rs @@ -0,0 +1,155 @@ +// rustfmt-style_edition: 2015 + +fn combine_blocklike() { + do_thing( + |param| { + action(); + foo(param) + }, + ); + + do_thing( + x, + |param| { + action(); + foo(param) + }, + ); + + do_thing( + x, + + // I'll be discussing the `action` with your para(m)legal counsel + |param| { + action(); + foo(param) + }, + ); + + do_thing( + Bar { + x: value, + y: value2, + }, + ); + + do_thing( + x, + Bar { + x: value, + y: value2, + }, + ); + + do_thing( + x, + + // Let me tell you about that one time at the `Bar` + Bar { + x: value, + y: value2, + }, + ); + + do_thing( + &[ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + x, + &[ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + x, + + // Just admit it; my list is longer than can be folded on to one line + &[ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + vec![ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + x, + vec![ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + x, + + // Just admit it; my list is longer than can be folded on to one line + vec![ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + x, + ( + 1, + 2, + 3, + |param| { + action(); + foo(param) + }, + ), + ); +} + +fn combine_struct_sample() { + let identity = verify( + &ctx, + VerifyLogin { + type_: LoginType::Username, + username: args.username.clone(), + password: Some(args.password.clone()), + domain: None, + }, + )?; +} + +fn combine_macro_sample() { + rocket::ignite() + .mount( + "/", + routes![ + http::auth::login, + http::auth::logout, + http::cors::options, + http::action::dance, + http::action::sleep, + ], + ) + .launch(); +} diff --git a/tests/source/configs/style_edition/overflow_delim_expr_2024.rs b/tests/source/configs/style_edition/overflow_delim_expr_2024.rs new file mode 100644 index 00000000000..66c95e71aa9 --- /dev/null +++ b/tests/source/configs/style_edition/overflow_delim_expr_2024.rs @@ -0,0 +1,155 @@ +// rustfmt-style_edition: 2024 + +fn combine_blocklike() { + do_thing( + |param| { + action(); + foo(param) + }, + ); + + do_thing( + x, + |param| { + action(); + foo(param) + }, + ); + + do_thing( + x, + + // I'll be discussing the `action` with your para(m)legal counsel + |param| { + action(); + foo(param) + }, + ); + + do_thing( + Bar { + x: value, + y: value2, + }, + ); + + do_thing( + x, + Bar { + x: value, + y: value2, + }, + ); + + do_thing( + x, + + // Let me tell you about that one time at the `Bar` + Bar { + x: value, + y: value2, + }, + ); + + do_thing( + &[ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + x, + &[ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + x, + + // Just admit it; my list is longer than can be folded on to one line + &[ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + vec![ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + x, + vec![ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + x, + + // Just admit it; my list is longer than can be folded on to one line + vec![ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + x, + ( + 1, + 2, + 3, + |param| { + action(); + foo(param) + }, + ), + ); +} + +fn combine_struct_sample() { + let identity = verify( + &ctx, + VerifyLogin { + type_: LoginType::Username, + username: args.username.clone(), + password: Some(args.password.clone()), + domain: None, + }, + )?; +} + +fn combine_macro_sample() { + rocket::ignite() + .mount( + "/", + routes![ + http::auth::login, + http::auth::logout, + http::cors::options, + http::action::dance, + http::action::sleep, + ], + ) + .launch(); +} diff --git a/tests/target/configs/style_edition/overflow_delim_expr_2015.rs b/tests/target/configs/style_edition/overflow_delim_expr_2015.rs new file mode 100644 index 00000000000..05d4b8b6d33 --- /dev/null +++ b/tests/target/configs/style_edition/overflow_delim_expr_2015.rs @@ -0,0 +1,135 @@ +// rustfmt-style_edition: 2015 + +fn combine_blocklike() { + do_thing(|param| { + action(); + foo(param) + }); + + do_thing(x, |param| { + action(); + foo(param) + }); + + do_thing( + x, + // I'll be discussing the `action` with your para(m)legal counsel + |param| { + action(); + foo(param) + }, + ); + + do_thing(Bar { + x: value, + y: value2, + }); + + do_thing( + x, + Bar { + x: value, + y: value2, + }, + ); + + do_thing( + x, + // Let me tell you about that one time at the `Bar` + Bar { + x: value, + y: value2, + }, + ); + + do_thing(&[ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ]); + + do_thing( + x, + &[ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + x, + // Just admit it; my list is longer than can be folded on to one line + &[ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing(vec![ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ]); + + do_thing( + x, + vec![ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + x, + // Just admit it; my list is longer than can be folded on to one line + vec![ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + x, + (1, 2, 3, |param| { + action(); + foo(param) + }), + ); +} + +fn combine_struct_sample() { + let identity = verify( + &ctx, + VerifyLogin { + type_: LoginType::Username, + username: args.username.clone(), + password: Some(args.password.clone()), + domain: None, + }, + )?; +} + +fn combine_macro_sample() { + rocket::ignite() + .mount( + "/", + routes![ + http::auth::login, + http::auth::logout, + http::cors::options, + http::action::dance, + http::action::sleep, + ], + ) + .launch(); +} diff --git a/tests/target/configs/style_edition/overflow_delim_expr_2024.rs b/tests/target/configs/style_edition/overflow_delim_expr_2024.rs new file mode 100644 index 00000000000..ecd2e8ca797 --- /dev/null +++ b/tests/target/configs/style_edition/overflow_delim_expr_2024.rs @@ -0,0 +1,120 @@ +// rustfmt-style_edition: 2024 + +fn combine_blocklike() { + do_thing(|param| { + action(); + foo(param) + }); + + do_thing(x, |param| { + action(); + foo(param) + }); + + do_thing( + x, + // I'll be discussing the `action` with your para(m)legal counsel + |param| { + action(); + foo(param) + }, + ); + + do_thing(Bar { + x: value, + y: value2, + }); + + do_thing(x, Bar { + x: value, + y: value2, + }); + + do_thing( + x, + // Let me tell you about that one time at the `Bar` + Bar { + x: value, + y: value2, + }, + ); + + do_thing(&[ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ]); + + do_thing(x, &[ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ]); + + do_thing( + x, + // Just admit it; my list is longer than can be folded on to one line + &[ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing(vec![ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ]); + + do_thing(x, vec![ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ]); + + do_thing( + x, + // Just admit it; my list is longer than can be folded on to one line + vec![ + value_with_longer_name, + value2_with_longer_name, + value3_with_longer_name, + value4_with_longer_name, + ], + ); + + do_thing( + x, + (1, 2, 3, |param| { + action(); + foo(param) + }), + ); +} + +fn combine_struct_sample() { + let identity = verify(&ctx, VerifyLogin { + type_: LoginType::Username, + username: args.username.clone(), + password: Some(args.password.clone()), + domain: None, + })?; +} + +fn combine_macro_sample() { + rocket::ignite() + .mount("/", routes![ + http::auth::login, + http::auth::logout, + http::cors::options, + http::action::dance, + http::action::sleep, + ]) + .launch(); +} From 5a403ebfa92c42c0002bd4cc8ea70e77a5496eee Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Wed, 14 Aug 2024 16:05:24 -0500 Subject: [PATCH 5/5] refactor: improve mapping of legacy 'version' to 'style_edition' --- src/bin/main.rs | 21 ++++++- src/config/config_type.rs | 25 -------- src/config/mod.rs | 58 +++++++++++++------ src/config/options.rs | 1 + src/git-rustfmt/main.rs | 7 ++- src/lib.rs | 2 +- src/test/mod.rs | 21 ++++--- .../style-edition/just-version/rustfmt.toml | 1 + 8 files changed, 82 insertions(+), 54 deletions(-) create mode 100644 tests/config/style-edition/just-version/rustfmt.toml diff --git a/src/bin/main.rs b/src/bin/main.rs index 9e7476b1f81..14299434bc7 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -19,7 +19,7 @@ use getopts::{Matches, Options}; use crate::rustfmt::{ load_config, CliOptions, Color, Config, Edition, EmitMode, FileLines, FileName, - FormatReportFormatterBuilder, Input, Session, StyleEdition, Verbosity, + FormatReportFormatterBuilder, Input, Session, StyleEdition, Verbosity, Version, }; const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rustfmt/issues/new?labels=bug"; @@ -746,6 +746,13 @@ impl CliOptions for GetOptsOptions { .get("style_edition") .map_or(self.style_edition, |se| StyleEdition::from_str(se).ok()) } + + fn version(&self) -> Option { + self.inline_config + .get("version") + .map(|version| Version::from_str(version).ok()) + .flatten() + } } fn edition_from_edition_str(edition_str: &str) -> Result { @@ -814,6 +821,17 @@ mod test { 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); + assert_eq!(config.overflow_delimited_expr(), true); + } + + #[nightly_only_test] + #[test] + fn version_config_file_sets_style_edition_override_correctly() { + let options = GetOptsOptions::default(); + let config_file = Some(Path::new("tests/config/style-edition/just-version")); + let config = get_config(config_file, Some(options)); + assert_eq!(config.style_edition(), StyleEdition::Edition2024); + assert_eq!(config.overflow_delimited_expr(), true); } #[nightly_only_test] @@ -858,6 +876,7 @@ mod test { ]); let config = get_config(None, Some(options)); assert_eq!(config.style_edition(), StyleEdition::Edition2024); + assert_eq!(config.overflow_delimited_expr(), true); } #[nightly_only_test] diff --git a/src/config/config_type.rs b/src/config/config_type.rs index 4b83e974932..14217caba0a 100644 --- a/src/config/config_type.rs +++ b/src/config/config_type.rs @@ -134,7 +134,6 @@ macro_rules! create_config { "fn_args_layout" => self.0.set_fn_args_layout(), "hide_parse_errors" => self.0.set_hide_parse_errors(), "version" => self.0.set_version(), - "edition" => self.0.set_edition(), &_ => (), } } @@ -165,7 +164,6 @@ macro_rules! create_config { "fn_args_layout" => self.0.set_fn_args_layout(), "hide_parse_errors" => self.0.set_hide_parse_errors(), "version" => self.0.set_version(), - "edition" => self.0.set_edition(), &_ => (), } } @@ -264,7 +262,6 @@ macro_rules! create_config { self.set_fn_args_layout(); self.set_hide_parse_errors(); self.set_version(); - self.set_edition(); self } @@ -367,7 +364,6 @@ macro_rules! create_config { "fn_args_layout" => self.set_fn_args_layout(), "hide_parse_errors" => self.set_hide_parse_errors(), "version" => self.set_version(), - "edition" => self.set_edition(), &_ => (), } } @@ -585,30 +581,9 @@ macro_rules! create_config { option which takes precedence. \ The value of the `version` option will be ignored." ); - } else if matches!(self.version(), Version::Two) { - self.style_edition.2 = StyleEdition::Edition2024; - } else { - self.style_edition.2 = StyleEdition::Edition2015; } } - fn set_edition(&mut self) { - let style_edition_set = self.was_set().style_edition() - || self.was_set_cli().style_edition(); - - if style_edition_set || self.was_set().version() { - return; - } - - // User has explicitly specified an Edition value without - // explicitly specifying a Style Edition value, so the Style Edition - // must default to whatever value was provided for Edition - // as per: https://rust-lang.github.io/rfcs/3338-style-evolution.html#explanation - self.style_edition.2 = self.edition().into(); - } - - - #[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 { diff --git a/src/config/mod.rs b/src/config/mod.rs index cd6870e3890..8a95cc55fcb 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -222,11 +222,13 @@ impl PartialConfig { self, style_edition_override: Option, edition_override: Option, + version_override: Option, dir: &Path, ) -> Config { Config::default_for_possible_style_edition( style_edition_override.or(self.style_edition), edition_override.or(self.edition), + version_override.or(self.version), ) .fill_from_parsed_config(self, dir) } @@ -236,16 +238,25 @@ impl Config { pub fn default_for_possible_style_edition( style_edition: Option, edition: Option, + version: Option, ) -> Config { - style_edition.map_or_else( - || { - edition.map_or_else( - || Config::default(), - |e| Self::default_with_style_edition(e.into()), - ) - }, - |se| Self::default_with_style_edition(se), - ) + // Ensures the configuration defaults associated with Style Editions + // follow the precedence set in + // https://rust-lang.github.io/rfcs/3338-style-evolution.html + // 'version' is a legacy alias for 'style_edition' that we'll support + // for some period of time + // FIXME(calebcartwright) - remove 'version' at some point + match (style_edition, version, edition) { + (Some(se), _, _) => Self::default_with_style_edition(se), + (None, Some(Version::Two), _) => { + Self::default_with_style_edition(StyleEdition::Edition2024) + } + (None, Some(Version::One), _) => { + Self::default_with_style_edition(StyleEdition::Edition2015) + } + (None, None, Some(e)) => Self::default_with_style_edition(e.into()), + (None, None, None) => Config::default(), + } } pub(crate) fn version_meets_requirement(&self) -> bool { @@ -275,6 +286,7 @@ impl Config { file_path: &Path, edition: Option, style_edition: Option, + version: Option, ) -> Result { let mut file = File::open(&file_path)?; let mut toml = String::new(); @@ -284,6 +296,7 @@ impl Config { file_path.parent().unwrap(), edition, style_edition, + version, ) .map_err(|err| Error::new(ErrorKind::InvalidData, err)) } @@ -301,6 +314,7 @@ impl Config { dir: &Path, edition: Option, style_edition: Option, + version: Option, ) -> Result<(Config, Option), Error> { /// Try to find a project file in the given directory and its parents. /// Returns the path of the nearest project file if one exists, @@ -347,17 +361,17 @@ impl Config { match resolve_project_file(dir)? { None => Ok(( - Config::default_for_possible_style_edition(style_edition, edition), + Config::default_for_possible_style_edition(style_edition, edition, version), None, )), - Some(path) => Config::from_toml_path(&path, edition, style_edition) + Some(path) => Config::from_toml_path(&path, edition, style_edition, version) .map(|config| (config, Some(path))), } } #[allow(dead_code)] pub(super) fn from_toml(toml: &str, dir: &Path) -> Result { - Self::from_toml_for_style_edition(toml, dir, None, None) + Self::from_toml_for_style_edition(toml, dir, None, None, None) } pub(crate) fn from_toml_for_style_edition( @@ -365,6 +379,7 @@ impl Config { dir: &Path, edition: Option, style_edition: Option, + version: Option, ) -> Result { let parsed: ::toml::Value = toml .parse() @@ -385,7 +400,7 @@ impl Config { if !err.is_empty() { eprint!("{err}"); } - Ok(parsed_config.to_parsed_config(style_edition, edition, dir)) + Ok(parsed_config.to_parsed_config(style_edition, edition, version, dir)) } Err(e) => { err.push_str("Error: Decoding config file failed:\n"); @@ -403,19 +418,24 @@ pub fn load_config( file_path: Option<&Path>, options: Option, ) -> Result<(Config, Option), Error> { - let (over_ride, edition, style_edition) = match options { - Some(ref opts) => (config_path(opts)?, opts.edition(), opts.style_edition()), - None => (None, None, None), + let (over_ride, edition, style_edition, version) = match options { + Some(ref opts) => ( + config_path(opts)?, + opts.edition(), + opts.style_edition(), + opts.version(), + ), + None => (None, None, None, None), }; let result = if let Some(over_ride) = over_ride { - Config::from_toml_path(over_ride.as_ref(), edition, style_edition) + Config::from_toml_path(over_ride.as_ref(), edition, style_edition, version) .map(|p| (p, Some(over_ride.to_owned()))) } else if let Some(file_path) = file_path { - Config::from_resolved_toml_path(file_path, edition, style_edition) + Config::from_resolved_toml_path(file_path, edition, style_edition, version) } else { Ok(( - Config::default_for_possible_style_edition(style_edition, edition), + Config::default_for_possible_style_edition(style_edition, edition, version), None, )) }; diff --git a/src/config/options.rs b/src/config/options.rs index 46db5186b11..b9e79b9a7de 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -421,6 +421,7 @@ pub trait CliOptions { fn config_path(&self) -> Option<&Path>; fn edition(&self) -> Option; fn style_edition(&self) -> Option; + fn version(&self) -> Option; } /// The edition of the syntax and semantics of code (RFC 2052). diff --git a/src/git-rustfmt/main.rs b/src/git-rustfmt/main.rs index 14ac81322b9..b5a71588e9e 100644 --- a/src/git-rustfmt/main.rs +++ b/src/git-rustfmt/main.rs @@ -15,7 +15,9 @@ use getopts::{Matches, Options}; use rustfmt_nightly as rustfmt; use tracing_subscriber::EnvFilter; -use crate::rustfmt::{load_config, CliOptions, FormatReportFormatterBuilder, Input, Session}; +use crate::rustfmt::{ + load_config, CliOptions, FormatReportFormatterBuilder, Input, Session, Version, +}; fn prune_files(files: Vec<&str>) -> Vec<&str> { let prefixes: Vec<_> = files @@ -95,6 +97,9 @@ impl CliOptions for NullOptions { fn style_edition(&self) -> Option { unreachable!(); } + fn version(&self) -> Option { + unreachable!(); + } } fn uncommitted_files() -> Vec { diff --git a/src/lib.rs b/src/lib.rs index 1e9efc5ce94..7dc698dd92c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -48,7 +48,7 @@ use crate::utils::indent_next_line; pub use crate::config::{ load_config, CliOptions, Color, Config, Edition, EmitMode, FileLines, FileName, NewlineStyle, - Range, StyleEdition, Verbosity, + Range, StyleEdition, Verbosity, Version, }; pub use crate::format_report_formatter::{FormatReportFormatter, FormatReportFormatterBuilder}; diff --git a/src/test/mod.rs b/src/test/mod.rs index 96706efb161..4d22f64f352 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -15,7 +15,7 @@ use crate::rustfmt_diff::{make_diff, print_diff, DiffLine, Mismatch, ModifiedChu use crate::source_file; use crate::{ is_nightly_channel, Edition, FormatReport, FormatReportFormatterBuilder, Input, Session, - StyleEdition, + StyleEdition, Version, }; use rustfmt_config_proc_macro::nightly_only_test; @@ -713,7 +713,7 @@ fn print_mismatches String>( fn read_config(filename: &Path) -> Config { let sig_comments = read_significant_comments(filename); - let (edition, style_edition) = get_editions_from_comments(&sig_comments); + let (edition, style_edition, version) = get_editions_from_comments(&sig_comments); // Look for a config file. If there is a 'config' property in the significant comments, use // that. Otherwise, if there are no significant comments at all, look for a config file with // the same name as the test file. @@ -722,12 +722,14 @@ fn read_config(filename: &Path) -> Config { sig_comments.get("config").map(Path::new), edition, style_edition, + version, ) } else { get_config( filename.with_extension("toml").file_name().map(Path::new), edition, style_edition, + version, ) }; @@ -761,7 +763,7 @@ enum IdempotentCheckError { fn get_editions_from_comments( comments: &HashMap, -) -> (Option, Option) { +) -> (Option, Option, Option) { ( comments .get("edition") @@ -769,6 +771,9 @@ fn get_editions_from_comments( comments.get("style_edition").map(|se| { StyleEdition::from_str(se).expect(&format!("invalid style_edition value: '{}'", se)) }), + comments + .get("version") + .map(|v| Version::from_str(v).expect(&format!("invalid version value: '{}'", v))), ) } @@ -778,8 +783,8 @@ fn idempotent_check( ) -> Result { let sig_comments = read_significant_comments(filename); let config = if let Some(ref config_file_path) = opt_config { - let (edition, style_edition) = get_editions_from_comments(&sig_comments); - Config::from_toml_path(config_file_path, edition, style_edition) + let (edition, style_edition, version) = get_editions_from_comments(&sig_comments); + Config::from_toml_path(config_file_path, edition, style_edition, version) .expect("`rustfmt.toml` not found") } else { read_config(filename) @@ -808,14 +813,15 @@ fn get_config( config_file: Option<&Path>, edition: Option, style_edition: Option, + version: Option, ) -> Config { let config_file_name = match config_file { - None => return Config::default_for_possible_style_edition(style_edition, edition), + None => return Config::default_for_possible_style_edition(style_edition, edition, version), Some(file_name) => { let mut full_path = PathBuf::from("tests/config/"); full_path.push(file_name); if !full_path.exists() { - return Config::default_for_possible_style_edition(style_edition, edition); + return Config::default_for_possible_style_edition(style_edition, edition, version); }; full_path } @@ -832,6 +838,7 @@ fn get_config( Path::new("tests/config/"), edition, style_edition, + version, ) .expect("invalid TOML") } diff --git a/tests/config/style-edition/just-version/rustfmt.toml b/tests/config/style-edition/just-version/rustfmt.toml new file mode 100644 index 00000000000..1082fd88889 --- /dev/null +++ b/tests/config/style-edition/just-version/rustfmt.toml @@ -0,0 +1 @@ +version = "Two"