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

Author: ayazhafiz

rustfmt-core/rustfmt-bin/src/bin/main.rs

@@ -424,7 +424,7 @@ fn format_string(input: String, opt: Opt) -> Result<i32> {
 
 enum FileConfig {
     Default,
-    Local(Config, Option<PathBuf>),
+    Local(Config, Option<Vec<PathBuf>>),
 }
 
 struct FileConfigPair<'a> {
@@ -456,9 +456,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 })
@@ -482,26 +482,32 @@ fn format(opt: Opt) -> Result<i32> {
         return Err(format_err!("Error: `{}` is a directory", dir.display()));
     }
 
-    let (config, config_path) = load_config(None, Some(&opt))?;
+    let (config, config_paths) = load_config(None, Some(&opt))?;
 
     if config.verbose() == Verbosity::Verbose {
-        if let Some(path) = config_path.as_ref() {
-            println!("Using rustfmt config file {}", path.display());
+        if let Some(paths) = config_paths.as_ref() {
+            for path in paths.iter() {
+                println!("Using rustfmt config file {}", path.display());
+            }
         }
     }
 
     let out = &mut stdout();
     let mut session = Session::new(config, Some(out));
 
-    for pair in FileConfigPairIter::new(&opt, config_path.is_some()) {
+    for pair in FileConfigPairIter::new(&opt, config_paths.is_some()) {
         let file = pair.file;
 
         if let FileConfig::Local(local_config, config_path) = pair.config {
-            if let Some(path) = config_path {
+            if let Some(paths) = config_path {
                 if local_config.verbose() == Verbosity::Verbose {
                     println!(
-                        "Using rustfmt config file {} for {}",
-                        path.display(),
+                        "Using rustfmt config files {} for {}",
+                        paths
+                            .into_iter()
+                            .map(|p| p.display().to_string())
+                            .collect::<Vec<_>>()
+                            .join(","),
                         file.display()
                     );
                 }

rustfmt-core/rustfmt-lib/src/config.rs

@@ -184,6 +184,43 @@ impl PartialConfig {
 
         ::toml::to_string(&cloned).map_err(ToTomlError)
     }
+
+    pub fn from_toml_path(file_path: &Path) -> Result<PartialConfig, Error> {
+        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<PartialConfig, String> {
+        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 {
@@ -211,11 +248,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<Config, Error> {
-        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`.
@@ -227,85 +261,74 @@ 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<PathBuf>), Error> {
+    pub fn from_resolved_toml_path(dir: &Path) -> Result<(Config, Option<Vec<PathBuf>>), 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<Option<PathBuf>, Error> {
+        fn resolve_project_files(dir: &Path) -> Result<Option<Vec<PathBuf>>, Error> {
             let mut current = if dir.is_relative() {
                 env::current_dir()?.join(dir)
             } else {
                 dir.to_path_buf()
             };
 
             current = dunce::canonicalize(current)?;
+            let mut paths = Vec::new();
 
             loop {
-                match get_toml_path(&current) {
-                    Ok(Some(path)) => return Ok(Some(path)),
-                    Err(e) => return Err(e),
-                    _ => (),
-                }
+                let current_toml_path = get_toml_path(&current)?;
+                paths.push(current_toml_path);
 
                 // If the current directory has no parent, we're done searching.
                 if !current.pop() {
                     break;
                 }
             }
 
+            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]));
                 }
             }
 
             // If none was found ther either, check in the user's configuration directory.
             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<Config, String> {
-        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)
     }
 }
 
@@ -314,14 +337,14 @@ impl Config {
 pub fn load_config<O: CliOptions>(
     file_path: Option<&Path>,
     options: Option<&O>,
-) -> Result<(Config, Option<PathBuf>), Error> {
+) -> Result<(Config, Option<Vec<PathBuf>>), 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 {
@@ -433,6 +456,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();
@@ -585,6 +644,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::<NullOptions>(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::*;
 

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
             }
 
@@ -398,10 +411,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 {