From 4f8938c616af167e47a05abafdfa67eed5f13533 Mon Sep 17 00:00:00 2001 From: "Victor M. Suarez" Date: Tue, 12 Jan 2016 18:12:48 -0500 Subject: [PATCH] Allow for setting of write-mode via config file. FIxes #215 Also from @marcusklaas: Refactor code output functions Specifically, `write_all_files` no longer returns a HashMap. It would sometimes contain items, and sometimes be empty. When "fixed" newlines are required, this must now be done with a separate call. The tests use this strategy and should now pass! --- src/bin/rustfmt.rs | 6 +-- src/config.rs | 20 +++++++++ src/filemap.rs | 98 ++++++++++++++++++++------------------------- src/lib.rs | 57 ++++++++------------------ src/missed_spans.rs | 2 +- src/visitor.rs | 4 +- tests/system.rs | 21 +++++++--- 7 files changed, 100 insertions(+), 108 deletions(-) diff --git a/src/bin/rustfmt.rs b/src/bin/rustfmt.rs index 3e369e1ef23..c795d529409 100644 --- a/src/bin/rustfmt.rs +++ b/src/bin/rustfmt.rs @@ -17,8 +17,8 @@ extern crate toml; extern crate env_logger; extern crate getopts; -use rustfmt::{WriteMode, run, run_from_stdin}; -use rustfmt::config::Config; +use rustfmt::{run, run_from_stdin}; +use rustfmt::config::{Config, WriteMode}; use std::env; use std::fs::{self, File}; @@ -216,7 +216,7 @@ fn determine_operation(matches: &Matches) -> Operation { Err(..) => return Operation::InvalidInput("Unrecognized write mode".into()), } } - None => WriteMode::Replace, + None => WriteMode::Default, }; let files: Vec<_> = matches.free.iter().map(PathBuf::from).collect(); diff --git a/src/config.rs b/src/config.rs index 7c8505e5e09..4576ecc5d2a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -120,6 +120,24 @@ configuration_option_enum! { ReportTactic: Never, } +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. + Overwrite, + // Write the output to stdout. + Display, + // Write the diff to stdout. + Diff, + // Display how much of the input file was processed + Coverage, + // Unfancy stdout + Plain, +} + // This trait and the following impl blocks are there so that we an use // UCFS inside the get_docs() function on types for configs. pub trait ConfigType { @@ -323,4 +341,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, + "What Write Mode to use when none is supplied: Replace, Overwrite, Display, Diff, Coverage"; } diff --git a/src/filemap.rs b/src/filemap.rs index 95ae95e66bc..b518eaaa344 100644 --- a/src/filemap.rs +++ b/src/filemap.rs @@ -15,10 +15,9 @@ use strings::string_buffer::StringBuffer; use std::collections::HashMap; use std::fs::{self, File}; -use std::io::{self, Write, Read, stdout}; +use std::io::{self, Write, Read, stdout, BufWriter}; -use WriteMode; -use config::{NewlineStyle, Config}; +use config::{NewlineStyle, Config, WriteMode}; use rustfmt_diff::{make_diff, print_diff}; // A map of the files of a crate, with their new content @@ -34,16 +33,48 @@ pub fn append_newlines(file_map: &mut FileMap) { pub fn write_all_files(file_map: &FileMap, mode: WriteMode, config: &Config) - -> Result<(HashMap), io::Error> { - let mut result = HashMap::new(); + -> Result<(), io::Error> { for filename in file_map.keys() { - let one_result = try!(write_file(&file_map[filename], filename, mode, config)); - if let Some(r) = one_result { - result.insert(filename.clone(), r); - } + try!(write_file(&file_map[filename], filename, mode, config)); } - Ok(result) + Ok(()) +} + +// Prints all newlines either as `\n` or as `\r\n`. +pub fn write_system_newlines(writer: T, + text: &StringBuffer, + config: &Config) + -> Result<(), io::Error> + where T: Write +{ + // Buffer output, since we're writing a since char at a time. + let mut writer = BufWriter::new(writer); + + let style = if config.newline_style == NewlineStyle::Native { + if cfg!(windows) { + NewlineStyle::Windows + } else { + NewlineStyle::Unix + } + } else { + config.newline_style + }; + + match style { + NewlineStyle::Unix => write!(writer, "{}", text), + NewlineStyle::Windows => { + for (c, _) in text.chars() { + match c { + '\n' => try!(write!(writer, "\r\n")), + '\r' => continue, + c => try!(write!(writer, "{}", c)), + } + } + Ok(()) + } + NewlineStyle::Native => unreachable!(), + } } pub fn write_file(text: &StringBuffer, @@ -52,39 +83,6 @@ pub fn write_file(text: &StringBuffer, config: &Config) -> Result, io::Error> { - // prints all newlines either as `\n` or as `\r\n` - fn write_system_newlines(mut writer: T, - text: &StringBuffer, - config: &Config) - -> Result<(), io::Error> - where T: Write - { - let style = if config.newline_style == NewlineStyle::Native { - if cfg!(windows) { - NewlineStyle::Windows - } else { - NewlineStyle::Unix - } - } else { - config.newline_style - }; - - match style { - NewlineStyle::Unix => write!(writer, "{}", text), - NewlineStyle::Windows => { - for (c, _) in text.chars() { - match c { - '\n' => try!(write!(writer, "\r\n")), - '\r' => continue, - c => try!(write!(writer, "{}", c)), - } - } - Ok(()) - } - NewlineStyle::Native => unreachable!(), - } - } - fn source_and_formatted_text(text: &StringBuffer, filename: &str, config: &Config) @@ -123,11 +121,6 @@ pub fn write_file(text: &StringBuffer, let file = try!(File::create(filename)); try!(write_system_newlines(file, text, config)); } - WriteMode::NewFile(extn) => { - let filename = filename.to_owned() + "." + extn; - let file = try!(File::create(&filename)); - try!(write_system_newlines(file, text, config)); - } WriteMode::Plain => { let stdout = stdout(); let stdout = stdout.lock(); @@ -146,13 +139,8 @@ pub fn write_file(text: &StringBuffer, |line_num| format!("\nDiff at line {}:", line_num)); } } - WriteMode::Return => { - // io::Write is not implemented for String, working around with - // Vec - let mut v = Vec::new(); - try!(write_system_newlines(&mut v, text, config)); - // won't panic, we are writing correct utf8 - return Ok(Some(String::from_utf8(v).unwrap())); + WriteMode::Default => { + unreachable!("The WriteMode should NEVER Be default at this point!"); } } diff --git a/src/lib.rs b/src/lib.rs index 3ed7c3cad8c..fccc0f0934c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -34,12 +34,11 @@ use std::ops::{Add, Sub}; use std::path::Path; use std::collections::HashMap; use std::fmt; -use std::str::FromStr; use issues::{BadIssueSeeker, Issue}; use filemap::FileMap; use visitor::FmtVisitor; -use config::Config; +use config::{Config, WriteMode}; #[macro_use] mod utils; @@ -187,42 +186,6 @@ impl Sub for Indent { } } -#[derive(Copy, Clone)] -pub enum WriteMode { - // Backsup the original file and overwrites the orignal. - Replace, - // Overwrites original file without backup. - Overwrite, - // str is the extension of the new file. - NewFile(&'static str), - // Write the output to stdout. - Display, - // Write the diff to stdout. - Diff, - // Return the result as a mapping from filenames to Strings. - Return, - // Display how much of the input file was processed - Coverage, - // Unfancy stdout - Plain, -} - -impl FromStr for WriteMode { - type Err = (); - - fn from_str(s: &str) -> Result { - match s { - "replace" => Ok(WriteMode::Replace), - "display" => Ok(WriteMode::Display), - "overwrite" => Ok(WriteMode::Overwrite), - "diff" => Ok(WriteMode::Diff), - "coverage" => Ok(WriteMode::Coverage), - "plain" => Ok(WriteMode::Plain), - _ => Err(()), - } - } -} - pub enum ErrorKind { // Line has exceeded character limit LineOverflow, @@ -445,16 +408,27 @@ 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, + } +} + // args are the arguments passed on the command line, generally passed through // to the compiler. // write_mode determines what happens to the result of running rustfmt, see // WriteMode. pub fn run(file: &Path, write_mode: WriteMode, config: &Config) { - let mut result = format(file, config, write_mode); + let mode = check_write_mode(write_mode, config.write_mode); + let mut result = format(file, config, mode); print!("{}", fmt_lines(&mut result, config)); - let write_result = filemap::write_all_files(&result, write_mode, config); + let write_result = filemap::write_all_files(&result, mode, config); if let Err(msg) = write_result { println!("Error writing files: {}", msg); @@ -462,7 +436,8 @@ pub fn run(file: &Path, write_mode: WriteMode, config: &Config) { } // Similar to run, but takes an input String instead of a file to format -pub fn run_from_stdin(input: String, mode: WriteMode, config: &Config) { +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); fmt_lines(&mut result, config); diff --git a/src/missed_spans.rs b/src/missed_spans.rs index 67661143ddf..cd69b6dcc80 100644 --- a/src/missed_spans.rs +++ b/src/missed_spans.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use WriteMode; +use config::WriteMode; use visitor::FmtVisitor; use syntax::codemap::{self, BytePos, Span, Pos}; use comment::{CodeCharKind, CommentCodeSlices, rewrite_comment}; diff --git a/src/visitor.rs b/src/visitor.rs index f0efde9de64..1f7de66f91c 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -15,9 +15,9 @@ use syntax::visit; use strings::string_buffer::StringBuffer; -use {Indent, WriteMode}; +use Indent; use utils; -use config::Config; +use config::{Config, WriteMode}; use rewrite::{Rewrite, RewriteContext}; use comment::rewrite_comment; use macros::rewrite_macro; diff --git a/tests/system.rs b/tests/system.rs index d6f1ba806f5..378998e30a9 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -19,7 +19,8 @@ use std::io::{self, Read, BufRead, BufReader}; use std::path::Path; use rustfmt::*; -use rustfmt::config::{Config, ReportTactic}; +use rustfmt::filemap::write_system_newlines; +use rustfmt::config::{Config, ReportTactic, WriteMode}; use rustfmt::rustfmt_diff::*; static DIFF_CONTEXT_SIZE: usize = 3; @@ -43,7 +44,7 @@ fn system_tests() { // Turn a DirEntry into a String that represents the relative path to the // file. let files = files.map(get_path_string); - let (_reports, count, fails) = check_files(files, WriteMode::Return); + let (_reports, count, fails) = check_files(files, WriteMode::Default); // Display results. println!("Ran {} system tests.", count); @@ -71,7 +72,7 @@ fn idempotence_tests() { .ok() .expect("Couldn't read target dir.") .map(get_path_string); - let (_reports, count, fails) = check_files(files, WriteMode::Return); + let (_reports, count, fails) = check_files(files, WriteMode::Default); // Display results. println!("Ran {} idempotent tests.", count); @@ -90,7 +91,7 @@ fn self_tests() { // Hack because there's no `IntoIterator` impl for `[T; N]`. let files = files.chain(Some("src/lib.rs".to_owned()).into_iter()); - let (reports, count, fails) = check_files(files, WriteMode::Return); + let (reports, count, fails) = check_files(files, WriteMode::Default); let mut warnings = 0; // Display results. @@ -162,8 +163,16 @@ pub fn idempotent_check(filename: String, let mut file_map = format(Path::new(&filename), &config, write_mode); let format_report = fmt_lines(&mut file_map, &config); - // Won't panic, as we're not doing any IO. - let write_result = filemap::write_all_files(&file_map, WriteMode::Return, &config).unwrap(); + let mut write_result = HashMap::new(); + for (filename, text) in file_map.iter() { + let mut v = Vec::new(); + // Won't panic, as we're not doing any IO. + write_system_newlines(&mut v, text, &config).unwrap(); + // Won't panic, we are writing correct utf8. + let one_result = String::from_utf8(v).unwrap(); + write_result.insert(filename.clone(), one_result); + } + let target = sig_comments.get("target").map(|x| &(*x)[..]); handle_result(write_result, target, write_mode).map(|_| format_report)