config: Introduce a mergeable config

This commit replaces `ParsedConfig` with a `PartialConfig` that can be
merged into a `Config` or another `PartialConfig`. This provides a
unified place for configuration that is passed through rustfmt, and a
mechanism for overriding settings from other sources.

Expected uses:
 - overriding config file options from command line options
 - adding options that do not make sense in the config file, such as
   line ranges to restrict formatting to; see rust-lang#434

refs rust-lang#434

Author: kamalmarhubi

src/bin/rustfmt.rs

@@ -18,7 +18,7 @@ extern crate env_logger;
 extern crate getopts;
 
 use rustfmt::{run, run_from_stdin};
-use rustfmt::config::{Config, WriteMode};
+use rustfmt::config::{Config, PartialConfig, WriteMode};
 
 use std::env;
 use std::fs::{self, File};
@@ -92,7 +92,8 @@ fn resolve_config(dir: &Path) -> io::Result<(Config, Option<PathBuf>)> {
     let mut file = try!(File::open(&path));
     let mut toml = String::new();
     try!(file.read_to_string(&mut toml));
-    Ok((Config::from_toml(&toml), Some(path)))
+    let parsed_config: PartialConfig = toml::decode_str(&toml).expect("Failed to parse config");
+    Ok((Config::from(parsed_config), Some(path)))
 }
 
 fn update_config(config: &mut Config, matches: &Matches) {

src/config.rs

@@ -196,41 +196,97 @@ macro_rules! create_config {
             $(pub $i: $ty),+
         }
 
-        // Just like the Config struct but with each property wrapped
-        // as Option<T>. This is used to parse a rustfmt.toml that doesn't
-        // specity all properties of `Config`.
-        // We first parse into `ParsedConfig`, then create a default `Config`
-        // and overwrite the properties with corresponding values from `ParsedConfig`
+        /// Equivalent to `Config` except that each field is wrapped in an `Option`.
+        ///
+        /// This can be decoded into from TOML, and then later merged into a `Config` or another
+        /// `PartialConfig`.
+        ///
+        /// # Examples
+        ///
+        /// Decode a TOML value into a `PartialConfig`:
+        ///
+        /// ```ignore
+        /// extern crate toml;
+        /// let toml_str = r#"
+        ///     ideal_width = 72
+        /// "#;
+        ///
+        /// let partial: PartialConfig = toml::decode_str(toml_str);
+        /// ```
+        ///
+        /// Later, merge the `PartialConfig` into the default `Config`:
+        ///
+        /// ```ignore
+        /// # extern crate toml;
+        /// # use config::{Config, PartialConfig};
+        ///
+        /// # let toml_value = "abc = 3".parse().expect("Could not parse TOML");
+        /// # let partial: PartialConfig = toml::decode(toml_value);
+        /// let config = Config::Default().merge(partial);
+        /// assert_eq!(72, config.ideal_width);
+        /// ```
         #[derive(RustcDecodable, Clone)]
-        pub struct ParsedConfig {
+        pub struct PartialConfig {
             $(pub $i: Option<$ty>),+
         }
 
+        impl PartialConfig {
+
+            /// Create a `PartialConfig` with all fields set to `None`.
+            pub fn new() -> PartialConfig {
+                PartialConfig {
+                $(
+                    $i: None,
+                )+
+                }
+
+            }
+
+            /// Merge `other` into `self, overwriting fields in `self` with any non-`None` fields
+            /// in `other`.
+            pub fn merge(&mut self, other: &PartialConfig) -> &mut PartialConfig {
+            $(
+                if other.$i.is_some() {
+                    self.$i = other.$i.clone();
+                }
+             )+
+                self
+            }
+        }
+
+        impl Default for PartialConfig {
+            fn default() -> PartialConfig {
+                PartialConfig::new()
+            }
+        }
+
+        /// Applies settings in `partial` on top of the default `Config`.
+        impl From<PartialConfig> for Config {
+            fn from(partial: PartialConfig) -> Config {
+                Config::default().merge(&partial)
+            }
+        }
+
+        /// Applies settings in `partial` on top of the default `Config`.
+        impl<'a> From<&'a PartialConfig> for Config {
+            fn from(partial: &'a PartialConfig) -> Config {
+                Config::default().merge(partial)
+            }
+        }
+
         impl Config {
 
-            fn fill_from_parsed_config(mut self, parsed: ParsedConfig) -> Config {
+            /// Merge `partial` into `self, overwriting fields in `self` with any non-`None` fields
+            /// in `partial`.
+            pub fn merge(mut self, partial: &PartialConfig) -> Config {
             $(
-                if let Some(val) = parsed.$i {
+                if let Some(val) = partial.$i {
                     self.$i = val;
                 }
             )+
                 self
             }
 
-            pub fn from_toml(toml: &str) -> Config {
-                let parsed = toml.parse().unwrap();
-                let parsed_config:ParsedConfig = match toml::decode(parsed) {
-                    Some(decoded) => decoded,
-                    None => {
-                        println!("Decoding config file failed. Config:\n{}", toml);
-                        let parsed: toml::Value = toml.parse().unwrap();
-                        println!("\n\nParsed:\n{:?}", parsed);
-                        panic!();
-                    }
-                };
-                Config::default().fill_from_parsed_config(parsed_config)
-            }
-
             pub fn override_value(&mut self, key: &str, val: &str) {
                 match key {
                     $(
@@ -347,3 +403,37 @@ create_config! {
     write_mode: WriteMode, WriteMode::Default,
         "What Write Mode to use when none is supplied: Replace, Overwrite, Display, Diff, Coverage";
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    #[test]
+    fn test_config_merge_overrides() {
+        let config = Config::default().merge(&PartialConfig {
+            ideal_width: Some(37),
+            ..PartialConfig::default()
+        });
+        assert_eq!(37, config.ideal_width);
+    }
+
+    #[test]
+    fn test_partial_config_merge_overrides() {
+        let mut config = PartialConfig::default();
+        config.merge(&PartialConfig { ideal_width: Some(37), ..PartialConfig::default() });
+        assert_eq!(Some(37), config.ideal_width);
+    }
+
+    #[test]
+    fn test_config_merge_does_not_override_if_none() {
+        let mut config = Config { ideal_width: 37, ..Config::default() };
+        config = config.merge(&PartialConfig::new());
+        assert_eq!(37, config.ideal_width);
+    }
+
+    #[test]
+    fn test_partial_config_merge_does_not_override_if_none() {
+        let mut config = PartialConfig { ideal_width: Some(37), ..PartialConfig::default() };
+        config.merge(&PartialConfig::new());
+        assert_eq!(Some(37), config.ideal_width);
+    }
+}

tests/system.rs

@@ -12,6 +12,7 @@ extern crate rustfmt;
 extern crate diff;
 extern crate regex;
 extern crate term;
+extern crate toml;
 
 use std::collections::HashMap;
 use std::fs;
@@ -20,7 +21,7 @@ use std::path::Path;
 
 use rustfmt::*;
 use rustfmt::filemap::{write_system_newlines, FileMap};
-use rustfmt::config::{Config, ReportTactic, WriteMode};
+use rustfmt::config::{Config, PartialConfig, ReportTactic, WriteMode};
 use rustfmt::rustfmt_diff::*;
 
 static DIFF_CONTEXT_SIZE: usize = 3;
@@ -232,10 +233,11 @@ fn get_config(config_file: Option<&str>) -> Config {
     };
 
     let mut def_config_file = fs::File::open(config_file_name).expect("Couldn't open config");
-    let mut def_config = String::new();
-    def_config_file.read_to_string(&mut def_config).expect("Couldn't read config");
+    let mut buf = String::new();
+    def_config_file.read_to_string(&mut buf).expect("Couldn't read config");
 
-    Config::from_toml(&def_config)
+    let def_config: PartialConfig = toml::decode_str(&buf).expect("Failed to parse config");
+    Config::from(def_config)
 }
 
 // Reads significant comments of the form: // rustfmt-key: value