refactor: improve mapping of legacy 'version' to 'style_edition'

This commit is contained in:
Caleb Cartwright 2024-08-14 16:05:24 -05:00 committed by Caleb Cartwright
parent 065258659d
commit 46cb7d3220
8 changed files with 82 additions and 54 deletions

View File

@ -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<StyleEdition> {
.get("style_edition")
.map_or(self.style_edition, |se| StyleEdition::from_str(se).ok())
}
fn version(&self) -> Option<Version> {
self.inline_config
.get("version")
.map(|version| Version::from_str(version).ok())
.flatten()
}
}
fn edition_from_edition_str(edition_str: &str) -> Result<Edition> {
@ -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]

View File

@ -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 {

View File

@ -222,11 +222,13 @@ pub(super) fn to_parsed_config(
self,
style_edition_override: Option<StyleEdition>,
edition_override: Option<Edition>,
version_override: Option<Version>,
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<StyleEdition>,
edition: Option<Edition>,
version: Option<Version>,
) -> 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<Edition>,
style_edition: Option<StyleEdition>,
version: Option<Version>,
) -> Result<Config, Error> {
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<Edition>,
style_edition: Option<StyleEdition>,
version: Option<Version>,
) -> Result<(Config, Option<PathBuf>), 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<Option<PathBuf>, 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<Config, String> {
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<Edition>,
style_edition: Option<StyleEdition>,
version: Option<Version>,
) -> Result<Config, String> {
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<O: CliOptions>(
file_path: Option<&Path>,
options: Option<O>,
) -> Result<(Config, Option<PathBuf>), 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,
))
};

View File

@ -421,6 +421,7 @@ pub trait CliOptions {
fn config_path(&self) -> Option<&Path>;
fn edition(&self) -> Option<Edition>;
fn style_edition(&self) -> Option<StyleEdition>;
fn version(&self) -> Option<Version>;
}
/// The edition of the syntax and semantics of code (RFC 2052).

View File

@ -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<rustfmt_nightly::Edition> {
fn style_edition(&self) -> Option<rustfmt_nightly::StyleEdition> {
unreachable!();
}
fn version(&self) -> Option<Version> {
unreachable!();
}
}
fn uncommitted_files() -> Vec<String> {

View File

@ -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};

View File

@ -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<T: Fn(u32) -> 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<String, String>,
) -> (Option<Edition>, Option<StyleEdition>) {
) -> (Option<Edition>, Option<StyleEdition>, Option<Version>) {
(
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<FormatReport, IdempotentCheckError> {
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<Edition>,
style_edition: Option<StyleEdition>,
version: Option<Version>,
) -> 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")
}

View File

@ -0,0 +1 @@
version = "Two"