Skip to content

Commit

Permalink
refactor: improve mapping of legacy 'version' to 'style_edition'
Browse files Browse the repository at this point in the history
  • Loading branch information
calebcartwright committed Aug 28, 2024
1 parent 0652586 commit 46cb7d3
Show file tree
Hide file tree
Showing 8 changed files with 82 additions and 54 deletions.
21 changes: 20 additions & 1 deletion 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, StyleEdition, Verbosity,
FormatReportFormatterBuilder, Input, Session, StyleEdition, Verbosity, Version,
};

const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rustfmt/issues/new?labels=bug";
Expand Down Expand Up @@ -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<Version> {
self.inline_config
.get("version")
.map(|version| Version::from_str(version).ok())
.flatten()
}
}

fn edition_from_edition_str(edition_str: &str) -> Result<Edition> {
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down
25 changes: 0 additions & 25 deletions src/config/config_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
&_ => (),
}
}
Expand Down Expand Up @@ -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(),
&_ => (),
}
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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(),
&_ => (),
}
}
Expand Down Expand Up @@ -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 {
Expand Down
58 changes: 39 additions & 19 deletions src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,13 @@ impl PartialConfig {
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)
}
Expand All @@ -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 {
Expand Down Expand Up @@ -275,6 +286,7 @@ impl Config {
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();
Expand All @@ -284,6 +296,7 @@ impl Config {
file_path.parent().unwrap(),
edition,
style_edition,
version,
)
.map_err(|err| Error::new(ErrorKind::InvalidData, err))
}
Expand All @@ -301,6 +314,7 @@ impl Config {
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,
Expand Down Expand Up @@ -347,24 +361,25 @@ 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<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(
toml: &str,
dir: &Path,
edition: Option<Edition>,
style_edition: Option<StyleEdition>,
version: Option<Version>,
) -> Result<Config, String> {
let parsed: ::toml::Value = toml
.parse()
Expand All @@ -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");
Expand All @@ -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,
))
};
Expand Down
1 change: 1 addition & 0 deletions src/config/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
7 changes: 6 additions & 1 deletion src/git-rustfmt/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -95,6 +97,9 @@ impl CliOptions for NullOptions {
fn style_edition(&self) -> Option<rustfmt_nightly::StyleEdition> {
unreachable!();
}
fn version(&self) -> Option<Version> {
unreachable!();
}
}

fn uncommitted_files() -> Vec<String> {
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
21 changes: 14 additions & 7 deletions src/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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,
)
};

Expand Down Expand Up @@ -761,14 +763,17 @@ enum IdempotentCheckError {

fn get_editions_from_comments(
comments: &HashMap<String, String>,
) -> (Option<Edition>, Option<StyleEdition>) {
) -> (Option<Edition>, Option<StyleEdition>, Option<Version>) {
(
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))
}),
comments
.get("version")
.map(|v| Version::from_str(v).expect(&format!("invalid version value: '{}'", v))),
)
}

Expand All @@ -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)
Expand Down Expand Up @@ -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
}
Expand All @@ -832,6 +838,7 @@ fn get_config(
Path::new("tests/config/"),
edition,
style_edition,
version,
)
.expect("invalid TOML")
}
Expand Down
1 change: 1 addition & 0 deletions tests/config/style-edition/just-version/rustfmt.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
version = "Two"

0 comments on commit 46cb7d3

Please sign in to comment.