Merge pull request #812 from kamalmarhubi/write-mode-from-config

nrc · nrc · commit 347cbb1b3456 · 2016-02-09T17:16:54.000+13:00
config: Use write_mode from config
diff --git a/src/bin/rustfmt.rs b/src/bin/rustfmt.rs
@@ -24,13 +24,14 @@ use std::env;
 use std::fs::{self, File};
 use std::io::{self, ErrorKind, Read, Write};
 use std::path::{Path, PathBuf};
+use std::str::FromStr;
 
 use getopts::{Matches, Options};
 
 /// Rustfmt operations.
 enum Operation {
     /// Format files and their child modules.
-    Format(Vec<PathBuf>, WriteMode, Option<PathBuf>),
+    Format(Vec<PathBuf>, Option<PathBuf>),
     /// Print the help message.
     Help,
     // Print version information
@@ -40,7 +41,7 @@ enum Operation {
     /// Invalid program input, including reason.
     InvalidInput(String),
     /// No file specified, read from stdin
-    Stdin(String, WriteMode, Option<PathBuf>),
+    Stdin(String, Option<PathBuf>),
 }
 
 /// Try to find a project file in the given directory and its parents. Returns the path of a the
@@ -109,9 +110,19 @@ fn match_cli_path_or_file(config_path: Option<PathBuf>,
     resolve_config(input_file)
 }
 
-fn update_config(config: &mut Config, matches: &Matches) {
+fn update_config(config: &mut Config, matches: &Matches) -> Result<(), String> {
     config.verbose = matches.opt_present("verbose");
     config.skip_children = matches.opt_present("skip-children");
+
+    let write_mode = matches.opt_str("write-mode");
+    match matches.opt_str("write-mode").map(|wm| WriteMode::from_str(&wm)) {
+        None => Ok(()),
+        Some(Ok(write_mode)) => {
+            config.write_mode = write_mode;
+            Ok(())
+        }
+        Some(Err(_)) => Err(format!("Invalid write-mode: {}", write_mode.expect("cannot happen"))),
+    }
 }
 
 fn execute() -> i32 {
@@ -161,15 +172,18 @@ fn execute() -> i32 {
             Config::print_docs();
             0
         }
-        Operation::Stdin(input, write_mode, config_path) => {
+        Operation::Stdin(input, config_path) => {
             // try to read config from local directory
-            let (config, _) = match_cli_path_or_file(config_path, &env::current_dir().unwrap())
-                                  .expect("Error resolving config");
+            let (mut config, _) = match_cli_path_or_file(config_path, &env::current_dir().unwrap())
+                                      .expect("Error resolving config");
 
-            run_from_stdin(input, write_mode, &config);
+            // write_mode is always Plain for Stdin.
+            config.write_mode = WriteMode::Plain;
+
+            run_from_stdin(input, &config);
             0
         }
-        Operation::Format(files, write_mode, config_path) => {
+        Operation::Format(files, config_path) => {
             let mut config = Config::default();
             let mut path = None;
             // Load the config path file if provided
@@ -198,8 +212,11 @@ fn execute() -> i32 {
                     config = config_tmp;
                 }
 
-                update_config(&mut config, &matches);
-                run(&file, write_mode, &config);
+                if let Err(e) = update_config(&mut config, &matches) {
+                    print_usage(&opts, &e);
+                    return 1;
+                }
+                run(&file, &config);
             }
             0
         }
@@ -267,21 +284,10 @@ 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, config_path);
+        return Operation::Stdin(buffer, config_path);
     }
 
-    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, config_path)
+    Operation::Format(files, config_path)
 }
diff --git a/src/config.rs b/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.
@@ -349,6 +346,6 @@ 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";
 }
diff --git a/src/expr.rs b/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) {
diff --git a/src/filemap.rs b/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));
diff --git a/src/items.rs b/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);
 
diff --git a/src/lib.rs b/src/lib.rs
@@ -39,7 +39,7 @@ use std::fmt;
 use issues::{BadIssueSeeker, Issue};
 use filemap::FileMap;
 use visitor::FmtVisitor;
-use config::{Config, WriteMode};
+use config::Config;
 
 #[macro_use]
 mod utils;
@@ -264,8 +264,7 @@ impl fmt::Display for FormatReport {
 fn fmt_ast(krate: &ast::Crate,
            parse_session: &ParseSess,
            main_file: &Path,
-           config: &Config,
-           mode: WriteMode)
+           config: &Config)
            -> FileMap {
     let mut file_map = FileMap::new();
     for (path, module) in modules::list_files(krate, parse_session.codemap()) {
@@ -276,7 +275,7 @@ fn fmt_ast(krate: &ast::Crate,
         if config.verbose {
             println!("Formatting {}", path);
         }
-        let mut visitor = FmtVisitor::from_codemap(parse_session, config, Some(mode));
+        let mut visitor = FmtVisitor::from_codemap(parse_session, config);
         visitor.format_separate_mod(module);
         file_map.insert(path.to_owned(), visitor.buffer);
     }
@@ -366,7 +365,7 @@ pub fn fmt_lines(file_map: &mut FileMap, config: &Config) -> FormatReport {
     report
 }
 
-pub fn format_string(input: String, config: &Config, mode: WriteMode) -> FileMap {
+pub fn format_string(input: String, config: &Config) -> FileMap {
     let path = "stdin";
     let mut parse_session = ParseSess::new();
     let krate = parse::parse_crate_from_source_str(path.to_owned(),
@@ -383,7 +382,7 @@ pub fn format_string(input: String, config: &Config, mode: WriteMode) -> FileMap
     let mut file_map = FileMap::new();
 
     // do the actual formatting
-    let mut visitor = FmtVisitor::from_codemap(&parse_session, config, Some(mode));
+    let mut visitor = FmtVisitor::from_codemap(&parse_session, config);
     visitor.format_separate_mod(&krate.module);
 
     // append final newline
@@ -393,15 +392,15 @@ pub fn format_string(input: String, config: &Config, mode: WriteMode) -> FileMap
     file_map
 }
 
-pub fn format(file: &Path, config: &Config, mode: WriteMode) -> FileMap {
+pub fn format(file: &Path, config: &Config) -> FileMap {
     let mut parse_session = ParseSess::new();
     let krate = parse::parse_crate_from_file(file, Vec::new(), &parse_session);
 
     // Suppress error output after parsing.
     let emitter = Box::new(EmitterWriter::new(Box::new(Vec::new()), None));
     parse_session.span_diagnostic.handler = Handler::with_emitter(false, emitter);
 
-    let mut file_map = fmt_ast(&krate, &parse_session, file, config, mode);
+    let mut file_map = fmt_ast(&krate, &parse_session, file, config);
 
     // For some reason, the codemap does not include terminating
     // newlines so we must add one on for each file. This is sad.
@@ -410,39 +409,25 @@ pub fn format(file: &Path, config: &Config, mode: WriteMode) -> FileMap {
     file_map
 }
 
-// Make sure that we are using the correct WriteMode,
-// preferring what is passed as an argument
-fn check_write_mode(arg: WriteMode, config: WriteMode) -> WriteMode {
-    match (arg, config) {
-        (WriteMode::Default, WriteMode::Default) => WriteMode::Replace,
-        (WriteMode::Default, mode) => mode,
-        (mode, _) => mode,
-    }
-}
-
-// write_mode determines what happens to the result of running rustfmt, see
-// WriteMode.
-pub fn run(file: &Path, write_mode: WriteMode, config: &Config) {
-    let mode = check_write_mode(write_mode, config.write_mode);
-    let mut result = format(file, config, mode);
+pub fn run(file: &Path, config: &Config) {
+    let mut result = format(file, config);
 
     print!("{}", fmt_lines(&mut result, config));
     let out = stdout();
-    let write_result = filemap::write_all_files(&result, out, mode, config);
+    let write_result = filemap::write_all_files(&result, out, config);
 
     if let Err(msg) = write_result {
         println!("Error writing files: {}", msg);
     }
 }
 
 // Similar to run, but takes an input String instead of a file to format
-pub fn run_from_stdin(input: String, write_mode: WriteMode, config: &Config) {
-    let mode = check_write_mode(write_mode, config.write_mode);
-    let mut result = format_string(input, config, mode);
+pub fn run_from_stdin(input: String, config: &Config) {
+    let mut result = format_string(input, config);
     fmt_lines(&mut result, config);
 
     let mut out = stdout();
-    let write_result = filemap::write_file(&result["stdin"], "stdin", &mut out, mode, config);
+    let write_result = filemap::write_file(&result["stdin"], "stdin", &mut out, config);
 
     if let Err(msg) = write_result {
         panic!("Error writing to stdout: {}", msg);
diff --git a/src/missed_spans.rs b/src/missed_spans.rs
@@ -103,14 +103,9 @@ impl<'a> FmtVisitor<'a> {
                   .collect()
         }
 
-        let replaced = match self.write_mode {
-            Some(mode) => {
-                match mode {
-                    WriteMode::Coverage => replace_chars(old_snippet),
-                    _ => old_snippet.to_owned(),
-                }
-            }
-            None => old_snippet.to_owned(),
+        let replaced = match self.config.write_mode {
+            WriteMode::Coverage => replace_chars(old_snippet),
+            _ => old_snippet.to_owned(),
         };
         let snippet = &*replaced;
 
diff --git a/src/visitor.rs b/src/visitor.rs
@@ -17,7 +17,7 @@ use strings::string_buffer::StringBuffer;
 
 use Indent;
 use utils;
-use config::{Config, WriteMode};
+use config::Config;
 use rewrite::{Rewrite, RewriteContext};
 use comment::rewrite_comment;
 use macros::rewrite_macro;
@@ -31,7 +31,6 @@ pub struct FmtVisitor<'a> {
     // FIXME: use an RAII util or closure for indenting
     pub block_indent: Indent,
     pub config: &'a Config,
-    pub write_mode: Option<WriteMode>,
 }
 
 impl<'a> FmtVisitor<'a> {
@@ -380,10 +379,7 @@ impl<'a> FmtVisitor<'a> {
         self.last_pos = span.hi;
     }
 
-    pub fn from_codemap(parse_session: &'a ParseSess,
-                        config: &'a Config,
-                        mode: Option<WriteMode>)
-                        -> FmtVisitor<'a> {
+    pub fn from_codemap(parse_session: &'a ParseSess, config: &'a Config) -> FmtVisitor<'a> {
         FmtVisitor {
             parse_session: parse_session,
             codemap: parse_session.codemap(),
@@ -394,7 +390,6 @@ impl<'a> FmtVisitor<'a> {
                 alignment: 0,
             },
             config: config,
-            write_mode: mode,
         }
     }
 
diff --git a/tests/system.rs b/tests/system.rs

Original file line number	Diff line number	Diff line change
`@@ -31,18 +31,14 @@ pub fn append_newlines(file_map: &mut FileMap) {`
`31`	`31`	`}`
`32`	`32`	`}`
`33`	`33`
`34`		`-pub fn write_all_files<T>(file_map: &FileMap,`
`35`		`- mut out: T,`
`36`		`- mode: WriteMode,`
`37`		`- config: &Config)`
`38`		`- -> Result<(), io::Error>`
	`34`	`+pub fn write_all_files<T>(file_map: &FileMap, mut out: T, config: &Config) -> Result<(), io::Error>`
`39`	`35`	`where T: Write`
`40`	`36`	`{`
`41`		`- output_header(&mut out, mode).ok();`
	`37`	`+ output_header(&mut out, config.write_mode).ok();`
`42`	`38`	`for filename in file_map.keys() {`
`43`		`- try!(write_file(&file_map[filename], filename, &mut out, mode, config));`
	`39`	`+ try!(write_file(&file_map[filename], filename, &mut out, config));`
`44`	`40`	`}`
`45`		`- output_footer(&mut out, mode).ok();`
	`41`	`+ output_footer(&mut out, config.write_mode).ok();`
`46`	`42`
`47`	`43`	`Ok(())`
`48`	`44`	`}`
`@@ -87,7 +83,6 @@ pub fn write_system_newlines<T>(writer: T,`
`87`	`83`	`pub fn write_file<T>(text: &StringBuffer,`
`88`	`84`	`filename: &str,`
`89`	`85`	`out: &mut T,`
`90`		`- mode: WriteMode,`
`91`	`86`	`config: &Config)`
`92`	`87`	`-> Result<Option<String>, io::Error>`
`93`	`88`	`where T: Write`
`@@ -114,7 +109,7 @@ pub fn write_file<T>(text: &StringBuffer,`
`114`	`109`	`Ok(make_diff(&ori, &fmt, 3))`
`115`	`110`	`}`
`116`	`111`
`117`		`- match mode {`
	`112`	`+ match config.write_mode {`
`118`	`113`	`WriteMode::Replace => {`
`119`	`114`	`if let Ok((ori, fmt)) = source_and_formatted_text(text, filename, config) {`
`120`	`115`	`if fmt != ori {`
`@@ -157,9 +152,6 @@ pub fn write_file<T>(text: &StringBuffer,`
`157`	`152`	`\|line_num\| format!("\nDiff at line {}:", line_num));`
`158`	`153`	`}`
`159`	`154`	`}`
`160`		`- WriteMode::Default => {`
`161`		`- unreachable!("The WriteMode should NEVER Be default at this point!");`
`162`		`- }`
`163`	`155`	`WriteMode::Checkstyle => {`
`164`	`156`	`let diff = try!(create_diff(filename, text, config));`
`165`	`157`	`try!(output_checkstyle_file(out, filename, diff));`