From 1c044fb5d0e9648ec2aca09f5a3422112cadaf20 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Sat, 16 May 2020 13:12:48 -0700 Subject: [PATCH 1/6] Merge configs from parent directories Currently, `rustfmt` stops building its configuration after one configuration file is found. However, users may expect that `rustfmt` includes configuration options inhereted by configuration files in parent directories of the directory `rustfmt` is run in. The motivation for this commit is for a use case involving files that should be ignored by `rustfmt`. Consider the directory structure ``` a |-- rustfmt.toml |-- b |-- rustfmt.toml |-- main.rs ``` Suppose `a/rustfmt.toml` has the option `ignore = ["b/main.rs"]`. A user may expect that running `rusfmt` in either directory `a/` or `b/` will ignore `main.rs`. Today, though, if `rustfmt` is run in `b/`, `main.rs` will be formatted because only `b/rustfmt.toml` will be used for configuration. Although motivated by the use case for ignored files, this change sets up `rustfmt` to incrementally populate all configuration options from parent config files. The priority of population is closest config file to most transient config file. To avoid churn of the existing implementation for ignoring files, populating the ignored files config option with multiple config files is done by computing a join. Given config files "outer" and "inner", where "inner" is of higher priority (nested in the directory of) "outer", we merge their `ignore` configurations by finding the ignore files specified in "outer" that are present in the "inner" directory, and appending this to the files ignored by "inner". This means that printing an `ignore` configuration may not be entirely accurate, as it could be missing files that are not in the current directory specified by transient configurations. These files are out of the scope the `rustfmt` instance's operation, though, so for practical purposes I don't think this matters. Closes #3881 --- rustfmt-core/rustfmt-bin/src/bin/main.rs | 22 ++- rustfmt-core/rustfmt-lib/src/config.rs | 182 +++++++++++++----- .../rustfmt-lib/src/config/config_type.rs | 27 ++- .../rustfmt-lib/src/config/options.rs | 64 ++++++ 4 files changed, 233 insertions(+), 62 deletions(-) diff --git a/rustfmt-core/rustfmt-bin/src/bin/main.rs b/rustfmt-core/rustfmt-bin/src/bin/main.rs index a36f63b58c9..39edfc8fe1a 100644 --- a/rustfmt-core/rustfmt-bin/src/bin/main.rs +++ b/rustfmt-core/rustfmt-bin/src/bin/main.rs @@ -425,7 +425,7 @@ fn format_string(input: String, opt: Opt) -> Result { enum FileConfig { Default, - Local(Config, Option), + Local(Config, Option>), } struct FileConfigPair<'a> { @@ -457,9 +457,9 @@ impl<'a> Iterator for FileConfigPairIter<'a> { let config = if self.has_config_from_commandline { FileConfig::Default } else { - let (local_config, config_path) = + let (local_config, config_paths) = load_config(Some(file.parent()?), Some(self.opt)).ok()?; - FileConfig::Local(local_config, config_path) + FileConfig::Local(local_config, config_paths) }; Some(FileConfigPair { file, config }) @@ -483,11 +483,19 @@ fn format(opt: Opt) -> Result { return Err(format_err!("Error: `{}` is a directory", dir.display())); } - let (default_config, config_path) = load_config(None, Some(&opt))?; + let (default_config, config_paths) = load_config(None, Some(&opt))?; if opt.verbose { - if let Some(path) = config_path.as_ref() { - println!("Using rustfmt config file {}", path.display()); + if let Some(paths) = config_paths.as_ref() { + println!( + "Using rustfmt config files {} for {}", + paths + .into_iter() + .map(|p| p.display().to_string()) + .collect::>() + .join(","), + file.display() + ); } } @@ -496,7 +504,7 @@ fn format(opt: Opt) -> Result { verbosity: opt.verbosity(), }; - let inputs = FileConfigPairIter::new(&opt, config_path.is_some()).collect::>(); + let inputs = FileConfigPairIter::new(&opt, config_paths.is_some()).collect::>(); let format_report = format_inputs( inputs.iter().map(|p| { ( diff --git a/rustfmt-core/rustfmt-lib/src/config.rs b/rustfmt-core/rustfmt-lib/src/config.rs index 76bd0d89055..78678d05418 100644 --- a/rustfmt-core/rustfmt-lib/src/config.rs +++ b/rustfmt-core/rustfmt-lib/src/config.rs @@ -170,6 +170,43 @@ impl PartialConfig { ::toml::to_string(&cloned).map_err(ToTomlError) } + + pub fn from_toml_path(file_path: &Path) -> Result { + let mut file = File::open(&file_path)?; + let mut toml = String::new(); + file.read_to_string(&mut toml)?; + PartialConfig::from_toml(&toml).map_err(|err| Error::new(ErrorKind::InvalidData, err)) + } + + fn from_toml(toml: &str) -> Result { + let parsed: ::toml::Value = toml + .parse() + .map_err(|e| format!("Could not parse TOML: {}", e))?; + let mut err = String::new(); + let table = parsed + .as_table() + .ok_or_else(|| String::from("Parsed config was not table"))?; + for key in table.keys() { + if !Config::is_valid_name(key) { + let msg = &format!("Warning: Unknown configuration option `{}`\n", key); + err.push_str(msg) + } + } + match parsed.try_into() { + Ok(parsed_config) => { + if !err.is_empty() { + eprint!("{}", err); + } + Ok(parsed_config) + } + Err(e) => { + err.push_str("Error: Decoding config file failed:\n"); + err.push_str(format!("{}\n", e).as_str()); + err.push_str("Please check your config file."); + Err(err) + } + } + } } impl Config { @@ -197,11 +234,8 @@ impl Config { /// Returns a `Config` if the config could be read and parsed from /// the file, otherwise errors. pub fn from_toml_path(file_path: &Path) -> 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)) + let partial_config = PartialConfig::from_toml_path(file_path)?; + Ok(Config::default().fill_from_parsed_config(partial_config, file_path.parent().unwrap())) } /// Resolves the config for input in `dir`. @@ -213,11 +247,11 @@ impl Config { /// /// Returns the `Config` to use, and the path of the project file if there was /// one. - pub fn from_resolved_toml_path(dir: &Path) -> Result<(Config, Option), Error> { + pub fn from_resolved_toml_path(dir: &Path) -> Result<(Config, Option>), Error> { /// Try to find a project file in the given directory and its parents. /// Returns the path of a the nearest project file if one exists, /// or `None` if no project file was found. - fn resolve_project_file(dir: &Path) -> Result, Error> { + fn resolve_project_files(dir: &Path) -> Result>, Error> { let mut current = if dir.is_relative() { env::current_dir()?.join(dir) } else { @@ -225,13 +259,11 @@ impl Config { }; current = dunce::canonicalize(current)?; + let mut paths = Vec::new(); loop { - match get_toml_path(¤t) { - Ok(Some(path)) => return Ok(Some(path)), - Err(e) => return Err(e), - _ => (), - } + let current_toml_path = get_toml_path(¤t)?; + paths.push(current_toml_path); // If the current directory has no parent, we're done searching. if !current.pop() { @@ -239,10 +271,14 @@ impl Config { } } + if !paths.is_empty() { + return Ok(paths.into_iter().filter(|p| p.is_some()).collect()); + } + // If nothing was found, check in the home directory. if let Some(home_dir) = dirs::home_dir() { if let Some(path) = get_toml_path(&home_dir)? { - return Ok(Some(path)); + return Ok(Some(vec![path])); } } @@ -250,48 +286,35 @@ impl Config { if let Some(mut config_dir) = dirs::config_dir() { config_dir.push("rustfmt"); if let Some(path) = get_toml_path(&config_dir)? { - return Ok(Some(path)); + return Ok(Some(vec![path])); } } Ok(None) } - match resolve_project_file(dir)? { + let files = resolve_project_files(dir); + + match files? { None => Ok((Config::default(), None)), - Some(path) => Config::from_toml_path(&path).map(|config| (config, Some(path))), + Some(paths) => { + let mut config = Config::default(); + let mut used_paths = Vec::with_capacity(paths.len()); + for path in paths.into_iter().rev() { + let partial_config = PartialConfig::from_toml_path(&path)?; + config = config.fill_from_parsed_config(partial_config, &path); + used_paths.push(path); + } + + Ok((config, Some(used_paths))) + } } } pub fn from_toml(toml: &str, dir: &Path) -> Result { - let parsed: ::toml::Value = toml - .parse() - .map_err(|e| format!("Could not parse TOML: {}", e))?; - let mut err = String::new(); - let table = parsed - .as_table() - .ok_or_else(|| String::from("Parsed config was not table"))?; - for key in table.keys() { - if !Config::is_valid_name(key) { - let msg = &format!("Warning: Unknown configuration option `{}`\n", key); - err.push_str(msg) - } - } - match parsed.try_into() { - Ok(parsed_config) => { - if !err.is_empty() { - eprint!("{}", err); - } - let config = Config::default().fill_from_parsed_config(parsed_config, dir); - Ok(config) - } - Err(e) => { - err.push_str("Error: Decoding config file failed:\n"); - err.push_str(format!("{}\n", e).as_str()); - err.push_str("Please check your config file."); - Err(err) - } - } + let partial_config = PartialConfig::from_toml(toml)?; + let config = Config::default().fill_from_parsed_config(partial_config, dir); + Ok(config) } } @@ -300,14 +323,14 @@ impl Config { pub fn load_config( file_path: Option<&Path>, options: Option<&O>, -) -> Result<(Config, Option), Error> { +) -> Result<(Config, Option>), Error> { let over_ride = match options { Some(opts) => config_path(opts)?, 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()).map(|p| (p, Some(vec![over_ride.to_owned()]))) } else if let Some(file_path) = file_path { Config::from_resolved_toml_path(file_path) } else { @@ -417,6 +440,42 @@ mod test { } } + struct TempFile { + path: PathBuf, + } + + fn make_temp_file(file_name: &'static str, content: &'static str) -> TempFile { + use std::env::var; + + // Used in the Rust build system. + let target_dir = var("RUSTFMT_TEST_DIR").map_or_else(|_| env::temp_dir(), PathBuf::from); + let path = target_dir.join(file_name); + + fs::create_dir_all(path.parent().unwrap()).expect("couldn't create temp file"); + let mut file = File::create(&path).expect("couldn't create temp file"); + file.write_all(content.as_bytes()) + .expect("couldn't write temp file"); + TempFile { path } + } + + impl Drop for TempFile { + fn drop(&mut self) { + use std::fs::remove_file; + remove_file(&self.path).expect("couldn't delete temp file"); + } + } + + struct NullOptions; + + impl CliOptions for NullOptions { + fn apply_to(&self, _: &mut Config) { + unreachable!(); + } + fn config_path(&self) -> Option<&Path> { + unreachable!(); + } + } + #[test] fn test_config_set() { let mut config = Config::default(); @@ -568,6 +627,37 @@ ignore = [] assert_eq!(&toml, &default_config); } + #[test] + fn test_merged_config() { + let _outer_config = make_temp_file( + "a/rustfmt.toml", + r#" +tab_spaces = 2 +fn_call_width = 50 +ignore = ["b/main.rs", "util.rs"] +"#, + ); + + let inner_config = make_temp_file( + "a/b/rustfmt.toml", + r#" +tab_spaces = 3 +ignore = [] +"#, + ); + + let inner_dir = inner_config.path.parent().unwrap(); + let (config, paths) = load_config::(Some(inner_dir), None).unwrap(); + + assert_eq!(config.tab_spaces(), 3); + assert_eq!(config.fn_call_width(), 50); + assert_eq!(config.ignore().to_string(), r#"["main.rs"]"#); + + let paths = paths.unwrap(); + assert!(paths[0].ends_with("a/rustfmt.toml")); + assert!(paths[1].ends_with("a/b/rustfmt.toml")); + } + mod unstable_features { use super::super::*; diff --git a/rustfmt-core/rustfmt-lib/src/config/config_type.rs b/rustfmt-core/rustfmt-lib/src/config/config_type.rs index 5abb8ccc93f..837e8116fe4 100644 --- a/rustfmt-core/rustfmt-lib/src/config/config_type.rs +++ b/rustfmt-core/rustfmt-lib/src/config/config_type.rs @@ -50,6 +50,22 @@ impl ConfigType for IgnoreList { } } +macro_rules! update { + ($self:ident, ignore = $val:ident, $dir:ident) => { + $self.ignore.1 = true; + + let mut new_ignored = $val; + new_ignored.add_prefix($dir); + let old_ignored = $self.ignore.2; + $self.ignore.2 = old_ignored.merge_into(new_ignored); + }; + + ($self:ident, $i:ident = $val:ident, $dir:ident) => { + $self.$i.1 = true; + $self.$i.2 = $val; + }; +} + macro_rules! create_config { ($($i:ident: $Ty:ty, $def:expr, $is_stable:literal, $dstring:literal;)+) => ( use std::io::Write; @@ -149,12 +165,10 @@ macro_rules! create_config { $( if let Some(val) = parsed.$i { if self.$i.3 { - self.$i.1 = true; - self.$i.2 = val; + update!(self, $i = val, dir); } else { if is_nightly_channel!() { - self.$i.1 = true; - self.$i.2 = val; + update!(self, $i = val, dir); } else { eprintln!("Warning: can't set `{} = {:?}`, unstable features are only \ available in nightly channel.", stringify!($i), val); @@ -164,7 +178,6 @@ macro_rules! create_config { )+ self.set_heuristics(); self.set_license_template(); - self.set_ignore(dir); self } @@ -397,10 +410,6 @@ macro_rules! create_config { } } - fn set_ignore(&mut self, dir: &Path) { - self.ignore.2.add_prefix(dir); - } - #[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/rustfmt-core/rustfmt-lib/src/config/options.rs b/rustfmt-core/rustfmt-lib/src/config/options.rs index 23b4ba83e7a..22e07b2fe05 100644 --- a/rustfmt-core/rustfmt-lib/src/config/options.rs +++ b/rustfmt-core/rustfmt-lib/src/config/options.rs @@ -276,6 +276,30 @@ impl IgnoreList { pub fn rustfmt_toml_path(&self) -> &Path { &self.rustfmt_toml_path } + + pub fn merge_into(self, other: Self) -> Self { + let old_rustfmt_toml_path = self.rustfmt_toml_path; + let new_rustfmt_toml_path = other.rustfmt_toml_path.clone(); + let relocalized_old_paths: HashSet = self + .path_set + .into_iter() + .map(|p| old_rustfmt_toml_path.parent().unwrap().join(p)) + .map(|p| { + p.strip_prefix(new_rustfmt_toml_path.parent().unwrap()) + .map(PathBuf::from) + }) + .filter(|p| p.is_ok()) + .map(|p| p.unwrap()) + .collect(); + + let mut path_set = other.path_set; + path_set.extend(relocalized_old_paths); + + Self { + path_set, + rustfmt_toml_path: new_rustfmt_toml_path, + } + } } impl std::str::FromStr for IgnoreList { @@ -331,3 +355,43 @@ pub enum MatchArmLeadingPipe { /// Maintain any existing leading pipes KeepExisting, } + +#[cfg(test)] +mod test { + use std::path::PathBuf; + + use crate::config::IgnoreList; + + #[test] + fn test_ignore_list_merge_into() { + let ignore_list_outer = IgnoreList { + path_set: vec!["b/c/d.rs", "b/c/d/e.rs", "b/other.rs"] + .into_iter() + .map(PathBuf::from) + .collect(), + rustfmt_toml_path: PathBuf::from("a/rustfmt.toml"), + }; + + let ignore_list_inner = IgnoreList { + path_set: vec!["f.rs", "g/h.rs"] + .into_iter() + .map(PathBuf::from) + .collect(), + rustfmt_toml_path: PathBuf::from("a/b/c/rustfmt.toml"), + }; + + let ignore_list_merged = ignore_list_outer.merge_into(ignore_list_inner); + + assert_eq!( + ignore_list_merged.rustfmt_toml_path, + PathBuf::from("a/b/c/rustfmt.toml") + ); + assert_eq!( + ignore_list_merged.path_set, + vec!["d.rs", "d/e.rs", "f.rs", "g/h.rs"] + .into_iter() + .map(PathBuf::from) + .collect() + ); + } +} From 98748cbeba86f0de2754577fbc64c2303a674eca Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Sat, 16 May 2020 18:36:11 -0700 Subject: [PATCH 2/6] fixup! Merge configs from parent directories --- rustfmt-core/rustfmt-lib/src/config.rs | 4 +--- .../rustfmt-lib/src/config/config_type.rs | 20 +++++++++---------- .../rustfmt-lib/src/config/options.rs | 6 +++++- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/rustfmt-core/rustfmt-lib/src/config.rs b/rustfmt-core/rustfmt-lib/src/config.rs index 78678d05418..37a7bb8a6c2 100644 --- a/rustfmt-core/rustfmt-lib/src/config.rs +++ b/rustfmt-core/rustfmt-lib/src/config.rs @@ -293,9 +293,7 @@ impl Config { Ok(None) } - let files = resolve_project_files(dir); - - match files? { + match resolve_project_files(dir)? { None => Ok((Config::default(), None)), Some(paths) => { let mut config = Config::default(); diff --git a/rustfmt-core/rustfmt-lib/src/config/config_type.rs b/rustfmt-core/rustfmt-lib/src/config/config_type.rs index 837e8116fe4..6a4c3191d5d 100644 --- a/rustfmt-core/rustfmt-lib/src/config/config_type.rs +++ b/rustfmt-core/rustfmt-lib/src/config/config_type.rs @@ -50,19 +50,19 @@ impl ConfigType for IgnoreList { } } -macro_rules! update { - ($self:ident, ignore = $val:ident, $dir:ident) => { - $self.ignore.1 = true; +macro_rules! update_config { + ($config:ident, ignore = $val:ident, $dir:ident) => { + $config.ignore.1 = true; let mut new_ignored = $val; new_ignored.add_prefix($dir); - let old_ignored = $self.ignore.2; - $self.ignore.2 = old_ignored.merge_into(new_ignored); + let old_ignored = $config.ignore.2; + $config.ignore.2 = old_ignored.merge_into(new_ignored); }; - ($self:ident, $i:ident = $val:ident, $dir:ident) => { - $self.$i.1 = true; - $self.$i.2 = $val; + ($config:ident, $i:ident = $val:ident, $dir:ident) => { + $config.$i.1 = true; + $config.$i.2 = $val; }; } @@ -165,10 +165,10 @@ macro_rules! create_config { $( if let Some(val) = parsed.$i { if self.$i.3 { - update!(self, $i = val, dir); + update_config!(self, $i = val, dir); } else { if is_nightly_channel!() { - update!(self, $i = val, dir); + update_config!(self, $i = val, dir); } else { eprintln!("Warning: can't set `{} = {:?}`, unstable features are only \ available in nightly channel.", stringify!($i), val); diff --git a/rustfmt-core/rustfmt-lib/src/config/options.rs b/rustfmt-core/rustfmt-lib/src/config/options.rs index 22e07b2fe05..fbaa2e026e7 100644 --- a/rustfmt-core/rustfmt-lib/src/config/options.rs +++ b/rustfmt-core/rustfmt-lib/src/config/options.rs @@ -277,14 +277,18 @@ impl IgnoreList { &self.rustfmt_toml_path } + /// Merges `self` into `other`, returning a new `IgnoreList`. The resulting `IgnoreList` uses + /// the `rustfmt_toml_path` of `other`, and only contains paths that are in `other`'s + /// `rustfmt_toml_path`. pub fn merge_into(self, other: Self) -> Self { let old_rustfmt_toml_path = self.rustfmt_toml_path; - let new_rustfmt_toml_path = other.rustfmt_toml_path.clone(); + let new_rustfmt_toml_path = other.rustfmt_toml_path; let relocalized_old_paths: HashSet = self .path_set .into_iter() .map(|p| old_rustfmt_toml_path.parent().unwrap().join(p)) .map(|p| { + // Only keep old paths that are also in the new config path p.strip_prefix(new_rustfmt_toml_path.parent().unwrap()) .map(PathBuf::from) }) From 2a9648c6662baa7c804d7b4f280f1e89cf595949 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 20 May 2020 10:44:18 -0700 Subject: [PATCH 3/6] fixup! Merge configs from parent directories --- rustfmt-core/rustfmt-bin/src/bin/main.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rustfmt-core/rustfmt-bin/src/bin/main.rs b/rustfmt-core/rustfmt-bin/src/bin/main.rs index 39edfc8fe1a..62be94da77c 100644 --- a/rustfmt-core/rustfmt-bin/src/bin/main.rs +++ b/rustfmt-core/rustfmt-bin/src/bin/main.rs @@ -488,13 +488,12 @@ fn format(opt: Opt) -> Result { if opt.verbose { if let Some(paths) = config_paths.as_ref() { println!( - "Using rustfmt config files {} for {}", + "Using rustfmt config files {}", paths .into_iter() .map(|p| p.display().to_string()) .collect::>() .join(","), - file.display() ); } } From fc9e0bc8978a8e8973d6ca0d16bcb10c2cd91bfa Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 20 May 2020 18:17:03 -0700 Subject: [PATCH 4/6] fixup! Merge configs from parent directories --- ci/integration.sh | 14 +++--- rustfmt-core/rustfmt-lib/src/config.rs | 63 ++++++++++++++++++-------- 2 files changed, 53 insertions(+), 24 deletions(-) diff --git a/ci/integration.sh b/ci/integration.sh index 09d0fb959f5..86980eb11bc 100755 --- a/ci/integration.sh +++ b/ci/integration.sh @@ -71,25 +71,27 @@ function show_head { echo "Head commit of ${INTEGRATION}: $head" } +tempdir=$(mktemp -d) + case ${INTEGRATION} in cargo) - git clone --depth=1 https://github.com/rust-lang/${INTEGRATION}.git - cd ${INTEGRATION} + git clone --depth=1 https://github.com/rust-lang/${INTEGRATION}.git ${tempdir} + cd ${tempdir} show_head export CFG_DISABLE_CROSS_TESTS=1 check_fmt_with_all_tests cd - ;; crater) - git clone --depth=1 https://github.com/rust-lang-nursery/${INTEGRATION}.git - cd ${INTEGRATION} + git clone --depth=1 https://github.com/rust-lang-nursery/${INTEGRATION}.git ${tempdir} + cd ${tempdir} show_head check_fmt_with_lib_tests cd - ;; *) - git clone --depth=1 https://github.com/rust-lang-nursery/${INTEGRATION}.git - cd ${INTEGRATION} + git clone --depth=1 https://github.com/rust-lang-nursery/${INTEGRATION}.git ${tempdir} + cd ${tempdir} show_head check_fmt_with_all_tests cd - diff --git a/rustfmt-core/rustfmt-lib/src/config.rs b/rustfmt-core/rustfmt-lib/src/config.rs index 37a7bb8a6c2..cacaa014385 100644 --- a/rustfmt-core/rustfmt-lib/src/config.rs +++ b/rustfmt-core/rustfmt-lib/src/config.rs @@ -271,8 +271,17 @@ impl Config { } } - if !paths.is_empty() { - return Ok(paths.into_iter().filter(|p| p.is_some()).collect()); + // List of closest -> most distant rustfmt config from the current directory. + let config_paths: Option> = paths.into_iter().filter(|p| p.is_some()).collect(); + let has_paths = config_paths.as_ref().and_then(|paths| { + if paths.is_empty() { + None + } else { + Some(paths.len()) + } + }); + if has_paths.is_some() { + return Ok(config_paths); } // If nothing was found, check in the home directory. @@ -296,6 +305,17 @@ impl Config { match resolve_project_files(dir)? { None => Ok((Config::default(), None)), Some(paths) => { + // If this branch is hit, there must be at least one config file available. + let most_local_path = &paths[0]; + if let Ok(partial_config) = PartialConfig::from_toml_path(most_local_path) { + let config = + Config::default().fill_from_parsed_config(partial_config, most_local_path); + if config.version() == Version::One { + // In v1, don't merge transient config files. Just use the most local one. + return Ok((config, Some(vec![most_local_path.clone()]))); + } + } + let mut config = Config::default(); let mut used_paths = Vec::with_capacity(paths.len()); for path in paths.into_iter().rev() { @@ -627,33 +647,40 @@ ignore = [] #[test] fn test_merged_config() { - let _outer_config = make_temp_file( - "a/rustfmt.toml", - r#" + match option_env!("CFG_RELEASE_CHANNEL") { + // this test requires nightly + None | Some("nightly") => { + let _outer_config = make_temp_file( + "a/rustfmt.toml", + r#" tab_spaces = 2 fn_call_width = 50 ignore = ["b/main.rs", "util.rs"] "#, - ); + ); - let inner_config = make_temp_file( - "a/b/rustfmt.toml", - r#" + let inner_config = make_temp_file( + "a/b/rustfmt.toml", + r#" +version = "two" tab_spaces = 3 ignore = [] "#, - ); + ); - let inner_dir = inner_config.path.parent().unwrap(); - let (config, paths) = load_config::(Some(inner_dir), None).unwrap(); + let inner_dir = inner_config.path.parent().unwrap(); + let (config, paths) = load_config::(Some(inner_dir), None).unwrap(); - assert_eq!(config.tab_spaces(), 3); - assert_eq!(config.fn_call_width(), 50); - assert_eq!(config.ignore().to_string(), r#"["main.rs"]"#); + assert_eq!(config.tab_spaces(), 3); + assert_eq!(config.fn_call_width(), 50); + assert_eq!(config.ignore().to_string(), r#"["main.rs"]"#); - let paths = paths.unwrap(); - assert!(paths[0].ends_with("a/rustfmt.toml")); - assert!(paths[1].ends_with("a/b/rustfmt.toml")); + let paths = paths.unwrap(); + assert!(paths[0].ends_with("a/rustfmt.toml")); + assert!(paths[1].ends_with("a/b/rustfmt.toml")); + } + _ => (), + }; } mod unstable_features { From 0b17a7503a47bfa1e14f8289957938391fb3293b Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 22 May 2020 09:52:38 -0700 Subject: [PATCH 5/6] fixup! Merge configs from parent directories --- rustfmt-core/rustfmt-bin/src/bin/main.rs | 2 +- rustfmt-core/rustfmt-lib/src/config.rs | 12 ++++-------- rustfmt-core/rustfmt-lib/src/config/options.rs | 3 +-- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/rustfmt-core/rustfmt-bin/src/bin/main.rs b/rustfmt-core/rustfmt-bin/src/bin/main.rs index 62be94da77c..9839dfa7ad3 100644 --- a/rustfmt-core/rustfmt-bin/src/bin/main.rs +++ b/rustfmt-core/rustfmt-bin/src/bin/main.rs @@ -488,7 +488,7 @@ fn format(opt: Opt) -> Result { if opt.verbose { if let Some(paths) = config_paths.as_ref() { println!( - "Using rustfmt config files {}", + "Using rustfmt config file(s) {}", paths .into_iter() .map(|p| p.display().to_string()) diff --git a/rustfmt-core/rustfmt-lib/src/config.rs b/rustfmt-core/rustfmt-lib/src/config.rs index cacaa014385..36c991f8147 100644 --- a/rustfmt-core/rustfmt-lib/src/config.rs +++ b/rustfmt-core/rustfmt-lib/src/config.rs @@ -273,14 +273,10 @@ impl Config { // List of closest -> most distant rustfmt config from the current directory. let config_paths: Option> = paths.into_iter().filter(|p| p.is_some()).collect(); - let has_paths = config_paths.as_ref().and_then(|paths| { - if paths.is_empty() { - None - } else { - Some(paths.len()) - } - }); - if has_paths.is_some() { + let has_paths = config_paths + .as_ref() + .map_or(false, |paths| !paths.is_empty()); + if has_paths { return Ok(config_paths); } diff --git a/rustfmt-core/rustfmt-lib/src/config/options.rs b/rustfmt-core/rustfmt-lib/src/config/options.rs index fbaa2e026e7..8a0f548a708 100644 --- a/rustfmt-core/rustfmt-lib/src/config/options.rs +++ b/rustfmt-core/rustfmt-lib/src/config/options.rs @@ -292,8 +292,7 @@ impl IgnoreList { p.strip_prefix(new_rustfmt_toml_path.parent().unwrap()) .map(PathBuf::from) }) - .filter(|p| p.is_ok()) - .map(|p| p.unwrap()) + .flatten() .collect(); let mut path_set = other.path_set; From 098cc37f626649d8a81fa74e8447d0535b8f1530 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Sat, 23 May 2020 10:27:11 -0700 Subject: [PATCH 6/6] fixup! Merge configs from parent directories --- rustfmt-core/rustfmt-lib/src/config.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/rustfmt-core/rustfmt-lib/src/config.rs b/rustfmt-core/rustfmt-lib/src/config.rs index 36c991f8147..1f0a48d05f2 100644 --- a/rustfmt-core/rustfmt-lib/src/config.rs +++ b/rustfmt-core/rustfmt-lib/src/config.rs @@ -301,17 +301,6 @@ impl Config { match resolve_project_files(dir)? { None => Ok((Config::default(), None)), Some(paths) => { - // If this branch is hit, there must be at least one config file available. - let most_local_path = &paths[0]; - if let Ok(partial_config) = PartialConfig::from_toml_path(most_local_path) { - let config = - Config::default().fill_from_parsed_config(partial_config, most_local_path); - if config.version() == Version::One { - // In v1, don't merge transient config files. Just use the most local one. - return Ok((config, Some(vec![most_local_path.clone()]))); - } - } - let mut config = Config::default(); let mut used_paths = Vec::with_capacity(paths.len()); for path in paths.into_iter().rev() {