config: Introduce a mergeable config

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};
@@ -30,7 +30,7 @@ use getopts::{Matches, Options};
 /// Rustfmt operations.
 enum Operation {
     /// Format files and their child modules.
-    Format(Vec<PathBuf>, WriteMode),
+    Format(Vec<PathBuf>),
     /// Print the help message.
     Help,
     // Print version information
@@ -40,7 +40,7 @@ enum Operation {
     /// Invalid program input, including reason.
     InvalidInput(String),
     /// No file specified, read from stdin
-    Stdin(String, WriteMode),
+    Stdin(String),
 }
 
 /// Try to find a project file in the given directory and its parents. Returns the path of a the
@@ -92,12 +92,24 @@ 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) {
-    config.verbose = matches.opt_present("verbose");
-    config.skip_children = matches.opt_present("skip-children");
+fn partial_config_from_options(matches: &Matches) -> PartialConfig {
+    let mut config = PartialConfig::new();
+    if matches.opt_present("skip_children") {
+        config.skip_children = Some(true);
+    }
+    if matches.opt_present("verbose") {
+        config.verbose = Some(true);
+    }
+    if let Some(mode) = matches.opt_str("write-mode") {
+        config.write_mode = Some(mode.parse()
+                                     .expect(&format!("Unrecognized write mode: {}", mode)));
+    }
+
+    config
 }
 
 fn execute() -> i32 {
@@ -142,27 +154,30 @@ fn execute() -> i32 {
             Config::print_docs();
             0
         }
-        Operation::Stdin(input, write_mode) => {
+        Operation::Stdin(input) => {
             // try to read config from local directory
-            let (config, _) = resolve_config(&env::current_dir().unwrap())
-                                  .expect("Error resolving config");
+            let (mut config, _) = resolve_config(&env::current_dir().unwrap())
+                                      .expect("Error resolving config");
 
-            run_from_stdin(input, write_mode, &config);
+            // WriteMode is always Plain for Stdin.
+            config = config.merge(&PartialConfig::from(WriteMode::Plain));
+
+            run_from_stdin(input, &config);
             0
         }
-        Operation::Format(files, write_mode) => {
+        Operation::Format(files) => {
             for file in files {
                 let (mut config, path) = resolve_config(file.parent().unwrap())
                                              .expect(&format!("Error resolving config for {}",
                                                               file.display()));
+                config = config.merge(&partial_config_from_options(&matches));
                 if let Some(path) = path {
                     println!("Using rustfmt config file {} for {}",
                              path.display(),
                              file.display());
                 }
 
-                update_config(&mut config, &matches);
-                run(&file, write_mode, &config);
+                run(&file, &config);
             }
             0
         }
@@ -220,21 +235,11 @@ fn determine_operation(matches: &Matches) -> Operation {
             Err(e) => return Operation::InvalidInput(e.to_string()),
         }
 
-        // WriteMode is always plain for Stdin
-        return Operation::Stdin(buffer, WriteMode::Plain);
+        return Operation::Stdin(buffer);
     }
 
-    let write_mode = match matches.opt_str("write-mode") {
-        Some(mode) => {
-            match mode.parse() {
-                Ok(mode) => mode,
-                Err(..) => return Operation::InvalidInput("Unrecognized write mode".into()),
-            }
-        }
-        None => WriteMode::Default,
-    };
 
     let files: Vec<_> = matches.free.iter().map(PathBuf::from).collect();
 
-    Operation::Format(files, write_mode)
+    Operation::Format(files)
 }

src/config.rs

@@ -121,9 +121,6 @@ configuration_option_enum! { ReportTactic:
 }
 
 configuration_option_enum! { WriteMode:
-    // Used internally to represent when no option is given
-    // Treated as Replace.
-    Default,
     // Backsup the original file and overwrites the orignal.
     Replace,
     // Overwrites original file without backup.
@@ -196,41 +193,118 @@ 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;
+        /// use config::{Config, PartialConfig};
+        /// 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_str = r#"
+        /// #     ideal_width = 72
+        /// # "#;
+        ///
+        /// let partial: PartialConfig = toml::decode_str(toml_str);
+        /// 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()
+            }
+        }
+
+        // Convenience impl.
+        impl From<WriteMode> for PartialConfig {
+            fn from(write_mode: WriteMode) -> PartialConfig {
+                PartialConfig {
+                    write_mode: Some(write_mode), ..PartialConfig::default()
+                }
+            }
+        }
+
+        // Convenience impl.
+        impl From<Option<WriteMode>> for PartialConfig {
+            fn from(write_mode: Option<WriteMode>) -> PartialConfig {
+                PartialConfig {
+                    write_mode: write_mode, ..PartialConfig::default()
+                }
+            }
+        }
+
+        /// 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 {
                     $(
@@ -344,6 +418,40 @@ create_config! {
     match_block_trailing_comma: bool, false,
         "Put a trailing comma after a block based match arm (non-block arms are not affected)";
     match_wildcard_trailing_comma: bool, true, "Put a trailing comma after a wildcard arm";
-    write_mode: WriteMode, WriteMode::Default,
+    write_mode: WriteMode, WriteMode::Replace,
         "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);
+    }
+}

src/expr.rs

@@ -427,7 +427,7 @@ impl Rewrite for ast::Block {
             return Some(user_str);
         }
 
-        let mut visitor = FmtVisitor::from_codemap(context.parse_session, context.config, None);
+        let mut visitor = FmtVisitor::from_codemap(context.parse_session, context.config);
         visitor.block_indent = context.block_indent;
 
         let prefix = match self.rules {
@@ -954,9 +954,7 @@ impl Rewrite for ast::Arm {
         let attr_str = if !attrs.is_empty() {
             // We only use this visitor for the attributes, should we use it for
             // more?
-            let mut attr_visitor = FmtVisitor::from_codemap(context.parse_session,
-                                                            context.config,
-                                                            None);
+            let mut attr_visitor = FmtVisitor::from_codemap(context.parse_session, context.config);
             attr_visitor.block_indent = context.block_indent;
             attr_visitor.last_pos = attrs[0].span.lo;
             if attr_visitor.visit_attrs(attrs) {

src/filemap.rs

@@ -31,18 +31,14 @@ pub fn append_newlines(file_map: &mut FileMap) {
     }
 }
 
-pub fn write_all_files<T>(file_map: &FileMap,
-                          mut out: T,
-                          mode: WriteMode,
-                          config: &Config)
-                          -> Result<(), io::Error>
+pub fn write_all_files<T>(file_map: &FileMap, mut out: T, config: &Config) -> Result<(), io::Error>
     where T: Write
 {
-    output_header(&mut out, mode).ok();
+    output_header(&mut out, config.write_mode).ok();
     for filename in file_map.keys() {
-        try!(write_file(&file_map[filename], filename, &mut out, mode, config));
+        try!(write_file(&file_map[filename], filename, &mut out, config));
     }
-    output_footer(&mut out, mode).ok();
+    output_footer(&mut out, config.write_mode).ok();
 
     Ok(())
 }
@@ -87,7 +83,6 @@ pub fn write_system_newlines<T>(writer: T,
 pub fn write_file<T>(text: &StringBuffer,
                      filename: &str,
                      out: &mut T,
-                     mode: WriteMode,
                      config: &Config)
                      -> Result<Option<String>, io::Error>
     where T: Write
@@ -114,7 +109,7 @@ pub fn write_file<T>(text: &StringBuffer,
         Ok(make_diff(&ori, &fmt, 3))
     }
 
-    match mode {
+    match config.write_mode {
         WriteMode::Replace => {
             if let Ok((ori, fmt)) = source_and_formatted_text(text, filename, config) {
                 if fmt != ori {
@@ -157,9 +152,6 @@ pub fn write_file<T>(text: &StringBuffer,
                            |line_num| format!("\nDiff at line {}:", line_num));
             }
         }
-        WriteMode::Default => {
-            unreachable!("The WriteMode should NEVER Be default at this point!");
-        }
         WriteMode::Checkstyle => {
             let diff = try!(create_diff(filename, text, config));
             try!(output_checkstyle_file(out, filename, diff));

src/items.rs

@@ -530,7 +530,7 @@ pub fn format_impl(context: &RewriteContext, item: &ast::Item, offset: Indent) -
         let open_pos = try_opt!(snippet.find_uncommented("{")) + 1;
 
         if !items.is_empty() || contains_comment(&snippet[open_pos..]) {
-            let mut visitor = FmtVisitor::from_codemap(context.parse_session, context.config, None);
+            let mut visitor = FmtVisitor::from_codemap(context.parse_session, context.config);
             visitor.block_indent = context.block_indent.block_indent(context.config);
             visitor.last_pos = item.span.lo + BytePos(open_pos as u32);