From 46cb7d322055625c07823dea8fdfad4c07f98754 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Wed, 14 Aug 2024 16:05:24 -0500 Subject: [PATCH] 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 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 @@ fn style_edition(&self) -> Option { .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 @@ fn version_sets_style_edition_override_correctly() { 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 @@ fn style_edition_inline_has_correct_precedence_over_edition_version() { ]); 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 @@ pub fn $i(&mut self, value: <$ty as StyleEditionDefault>::ConfigType) { "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 @@ pub fn $i(&mut self, value: <$ty as StyleEditionDefault>::ConfigType) { "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 @@ fn fill_from_parsed_config(mut self, parsed: PartialConfig, dir: &Path) -> Confi self.set_fn_args_layout(); self.set_hide_parse_errors(); self.set_version(); - self.set_edition(); self } @@ -367,7 +364,6 @@ pub fn override_value(&mut self, key: &str, val: &str) "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 @@ fn set_version(&mut self) { 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 @@ pub(super) fn to_parsed_config( 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 @@ pub(super) fn from_toml_path( 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 @@ pub(super) fn from_toml_path( file_path.parent().unwrap(), edition, style_edition, + version, ) .map_err(|err| Error::new(ErrorKind::InvalidData, err)) } @@ -301,6 +314,7 @@ pub(super) fn from_resolved_toml_path( 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 @@ fn resolve_project_file(dir: &Path) -> Result, Error> { 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 @@ pub(crate) fn from_toml_for_style_edition( dir: &Path, edition: Option, style_edition: Option, + version: Option, ) -> Result { let parsed: ::toml::Value = toml .parse() @@ -385,7 +400,7 @@ pub(crate) fn from_toml_for_style_edition( 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 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 @@ fn edition(&self) -> Option { 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 @@ 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::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"