diff --git a/src/changes.rs b/src/changes.rs index 97133ab67fb..7a16a5bca66 100644 --- a/src/changes.rs +++ b/src/changes.rs @@ -21,6 +21,7 @@ use std::fs::File; use std::io::{Write, stdout}; use WriteMode; use NewlineStyle; +use config::Config; use utils::round_up_to_power_of_two; // This is basically a wrapper around a bunch of Ropes which makes it convenient @@ -130,11 +131,12 @@ impl<'a> ChangeSet<'a> { } pub fn write_all_files(&self, - mode: WriteMode) + mode: WriteMode, + config: &Config) -> Result<(HashMap), ::std::io::Error> { let mut result = HashMap::new(); for filename in self.file_map.keys() { - let one_result = try!(self.write_file(filename, mode)); + let one_result = try!(self.write_file(filename, mode, config)); if let Some(r) = one_result { result.insert(filename.clone(), r); } @@ -145,18 +147,20 @@ impl<'a> ChangeSet<'a> { pub fn write_file(&self, filename: &str, - mode: WriteMode) + mode: WriteMode, + config: &Config) -> Result, ::std::io::Error> { let text = &self.file_map[filename]; // prints all newlines either as `\n` or as `\r\n` fn write_system_newlines( mut writer: T, - text: &StringBuffer) + text: &StringBuffer, + config: &Config) -> Result<(), ::std::io::Error> where T: Write, { - match config!(newline_style) { + match config.newline_style { NewlineStyle::Unix => write!(writer, "{}", text), NewlineStyle::Windows => { for (c, _) in text.chars() { @@ -181,7 +185,7 @@ impl<'a> ChangeSet<'a> { { // Write text to temp file let tmp_file = try!(File::create(&tmp_name)); - try!(write_system_newlines(tmp_file, text)); + try!(write_system_newlines(tmp_file, text, config)); } try!(::std::fs::rename(filename, bk_name)); @@ -190,18 +194,18 @@ impl<'a> ChangeSet<'a> { WriteMode::NewFile(extn) => { let filename = filename.to_owned() + "." + extn; let file = try!(File::create(&filename)); - try!(write_system_newlines(file, text)); + try!(write_system_newlines(file, text, config)); } WriteMode::Display => { println!("{}:\n", filename); let stdout = stdout(); let stdout_lock = stdout.lock(); - try!(write_system_newlines(stdout_lock, text)); + try!(write_system_newlines(stdout_lock, text, config)); } 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)); + try!(write_system_newlines(&mut v, text, config)); // won't panic, we are writing correct utf8 return Ok(Some(String::from_utf8(v).unwrap())); } diff --git a/src/config.rs b/src/config.rs index c803a07d9d0..23285167689 100644 --- a/src/config.rs +++ b/src/config.rs @@ -14,7 +14,7 @@ use {NewlineStyle, BraceStyle, ReturnIndent}; use lists::SeparatorTactic; use issues::ReportTactic; -#[derive(RustcDecodable)] +#[derive(RustcDecodable, Clone)] pub struct Config { pub max_width: usize, pub ideal_width: usize, @@ -32,20 +32,8 @@ pub struct Config { } impl Config { - fn from_toml(toml: &str) -> Config { + pub fn from_toml(toml: &str) -> Config { let parsed = toml.parse().unwrap(); toml::decode(parsed).unwrap() } } - -pub fn set_config(toml: &str) { - unsafe { - ::CONFIG = Some(Config::from_toml(toml)); - } -} - -macro_rules! config { - ($name: ident) => { - unsafe { ::CONFIG.as_ref().unwrap().$name } - }; -} diff --git a/src/expr.rs b/src/expr.rs index 85ea41fb060..dae64891ab3 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -62,7 +62,7 @@ fn rewrite_string_lit(context: &RewriteContext, s: &str, span: Span, width: usiz // strings, or if the string is too long for the line. let l_loc = context.codemap.lookup_char_pos(span.lo); let r_loc = context.codemap.lookup_char_pos(span.hi); - if l_loc.line == r_loc.line && r_loc.col.to_usize() <= config!(max_width) { + if l_loc.line == r_loc.line && r_loc.col.to_usize() <= context.config.max_width { return context.codemap.span_to_snippet(span).ok(); } @@ -82,7 +82,7 @@ fn rewrite_string_lit(context: &RewriteContext, s: &str, span: Span, width: usiz // First line. width - 2 // 2 = " + \ } else { - config!(max_width) - offset - 1 // 1 = either \ or ; + context.config.max_width - offset - 1 // 1 = either \ or ; }; let mut cur_end = cur_start + max_chars; @@ -206,7 +206,7 @@ fn rewrite_struct_lit(context: &RewriteContext, trailing_separator: if base.is_some() { SeparatorTactic::Never } else { - config!(struct_lit_trailing_comma) + context.config.struct_lit_trailing_comma }, indent: indent, h_width: budget, @@ -247,7 +247,7 @@ fn rewrite_tuple_lit(context: &RewriteContext, let rem_width = if i == items.len() - 1 { width - 2 } else { - config!(max_width) - indent - 2 + context.config.max_width - indent - 2 }; item.rewrite(context, rem_width, indent) }) diff --git a/src/items.rs b/src/items.rs index 29c950f1464..2dfa8a5fe65 100644 --- a/src/items.rs +++ b/src/items.rs @@ -144,7 +144,7 @@ impl<'a> FmtVisitor<'a> { // Check if vertical layout was forced by compute_budget_for_args. if one_line_budget <= 0 { - if config!(fn_args_paren_newline) { + if self.config.fn_args_paren_newline { result.push('\n'); result.push_str(&make_indent(arg_indent)); arg_indent = arg_indent + 1; // extra space for `(` @@ -170,8 +170,8 @@ impl<'a> FmtVisitor<'a> { // If we've already gone multi-line, or the return type would push // over the max width, then put the return type on a new line. if result.contains("\n") || - result.len() + indent + ret_str.len() > config!(max_width) { - let indent = match config!(fn_return_indent) { + result.len() + indent + ret_str.len() > self.config.max_width { + let indent = match self.config.fn_return_indent { ReturnIndent::WithWhereClause => indent + 4, // TODO we might want to check that using the arg indent doesn't // blow our budget, and if it does, then fallback to the where @@ -363,15 +363,15 @@ impl<'a> FmtVisitor<'a> { if !newline_brace { used_space += 2; } - let one_line_budget = if used_space > config!(max_width) { + let one_line_budget = if used_space > self.config.max_width { 0 } else { - config!(max_width) - used_space + self.config.max_width - used_space }; // 2 = `()` let used_space = indent + result.len() + 2; - let max_space = config!(ideal_width) + config!(leeway); + let max_space = self.config.ideal_width + self.config.leeway; debug!("compute_budgets_for_args: used_space: {}, max_space: {}", used_space, max_space); if used_space < max_space { @@ -383,9 +383,9 @@ impl<'a> FmtVisitor<'a> { // Didn't work. we must force vertical layout and put args on a newline. if let None = budgets { - let new_indent = indent + config!(tab_spaces); + let new_indent = indent + self.config.tab_spaces; let used_space = new_indent + 2; // account for `(` and `)` - let max_space = config!(ideal_width) + config!(leeway); + let max_space = self.config.ideal_width + self.config.leeway; if used_space > max_space { // Whoops! bankrupt. // TODO take evasive action, perhaps kill the indent or something. @@ -398,7 +398,7 @@ impl<'a> FmtVisitor<'a> { } fn newline_for_brace(&self, where_clause: &ast::WhereClause) -> bool { - match config!(fn_brace_style) { + match self.config.fn_brace_style { BraceStyle::AlwaysNextLine => true, BraceStyle::SameLineWhere if where_clause.predicates.len() > 0 => true, _ => false, @@ -421,7 +421,7 @@ impl<'a> FmtVisitor<'a> { self.changes.push_str_span(span, &generics_str); self.last_pos = body_start; - self.block_indent += config!(tab_spaces); + self.block_indent += self.config.tab_spaces; for (i, f) in enum_def.variants.iter().enumerate() { let next_span_start: BytePos = if i == enum_def.variants.len() - 1 { span.hi @@ -431,7 +431,7 @@ impl<'a> FmtVisitor<'a> { self.visit_variant(f, i == enum_def.variants.len() - 1, next_span_start); } - self.block_indent -= config!(tab_spaces); + self.block_indent -= self.config.tab_spaces; self.format_missing_with_indent(span.lo + BytePos(enum_snippet.rfind('}').unwrap() as u32)); self.changes.push_str_span(span, "}"); @@ -478,8 +478,8 @@ impl<'a> FmtVisitor<'a> { + field.node.name.to_string().len() + 1; // 1 = ( - let comma_cost = if config!(enum_trailing_comma) { 1 } else { 0 }; - let budget = config!(ideal_width) - indent - comma_cost - 1; // 1 = ) + let comma_cost = if self.config.enum_trailing_comma { 1 } else { 0 }; + let budget = self.config.ideal_width - indent - comma_cost - 1; // 1 = ) let fmt = ListFormatting { tactic: ListTactic::HorizontalVertical, @@ -500,13 +500,13 @@ impl<'a> FmtVisitor<'a> { // Make sure we do not exceed column limit // 4 = " = ," - assert!(config!(max_width) >= vis.len() + name.len() + expr_snippet.len() + 4, + assert!(self.config.max_width >= vis.len() + name.len() + expr_snippet.len() + 4, "Enum variant exceeded column limit"); } self.changes.push_str_span(field.span, &result); - if !last_field || config!(enum_trailing_comma) { + if !last_field || self.config.enum_trailing_comma { self.changes.push_str_span(field.span, ","); } } @@ -543,11 +543,11 @@ impl<'a> FmtVisitor<'a> { // This will drop the comment in between the header and body. self.last_pos = span.lo + BytePos(struct_snippet.find_uncommented("{").unwrap() as u32 + 1); - self.block_indent += config!(tab_spaces); + self.block_indent += self.config.tab_spaces; for (i, f) in struct_def.fields.iter().enumerate() { self.visit_field(f, i == struct_def.fields.len() - 1, span.lo, &struct_snippet); } - self.block_indent -= config!(tab_spaces); + self.block_indent -= self.config.tab_spaces; self.format_missing_with_indent(span.lo + BytePos(struct_snippet.rfind('}').unwrap() as u32)); self.changes.push_str_span(span, "}"); @@ -608,13 +608,13 @@ impl<'a> FmtVisitor<'a> { let mut field_str = match name { Some(name) => { - let budget = config!(ideal_width) - self.block_indent; + let budget = self.config.ideal_width - self.block_indent; // 3 is being conservative and assuming that there will be a trailing comma. if self.block_indent + vis.len() + name.len() + typ.len() + 3 > budget { format!("{}{}:\n{}{}", vis, name, - &make_indent(self.block_indent + config!(tab_spaces)), + &make_indent(self.block_indent + self.config.tab_spaces), typ) } else { format!("{}{}: {}", vis, name, typ) @@ -622,7 +622,7 @@ impl<'a> FmtVisitor<'a> { } None => format!("{}{}", vis, typ), }; - if !last_field || config!(struct_trailing_comma) { + if !last_field || self.config.struct_trailing_comma { field_str.push(','); } self.changes.push_str_span(field.span, &field_str); @@ -647,7 +647,7 @@ impl<'a> FmtVisitor<'a> { return result; } - let budget = config!(max_width) - indent - 2; + let budget = self.config.max_width - indent - 2; // TODO might need to insert a newline if the generics are really long result.push('<'); @@ -723,7 +723,7 @@ impl<'a> FmtVisitor<'a> { .zip(comments.into_iter()) .collect(); - let budget = config!(ideal_width) + config!(leeway) - indent - 10; + let budget = self.config.ideal_width + self.config.leeway - indent - 10; let fmt = ListFormatting { tactic: ListTactic::Vertical, separator: ",", diff --git a/src/lib.rs b/src/lib.rs index 1b360433e94..dfcf4c70424 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -41,10 +41,12 @@ use syntax::visit; use std::path::PathBuf; use std::collections::HashMap; use std::fmt; +use std::mem::swap; use issues::{BadIssueSeeker, Issue}; use changes::ChangeSet; use visitor::FmtVisitor; +use config::Config; #[macro_use] mod config; @@ -65,8 +67,6 @@ const MIN_STRING: usize = 10; // When we get scoped annotations, we should have rustfmt::skip. const SKIP_ANNOTATION: &'static str = "rustfmt_skip"; -static mut CONFIG: Option = None; - #[derive(Copy, Clone)] pub enum WriteMode { Overwrite, @@ -109,7 +109,7 @@ pub enum ReturnIndent { impl_enum_decodable!(ReturnIndent, WithArgs, WithWhereClause); enum ErrorKind { - // Line has more than config!(max_width) characters + // Line has exceeded character limit LineOverflow, // Line ends in whitespace TrailingWhitespace, @@ -161,8 +161,8 @@ impl fmt::Display for FormatReport { } // Formatting which depends on the AST. -fn fmt_ast<'a>(krate: &ast::Crate, codemap: &'a CodeMap) -> ChangeSet<'a> { - let mut visitor = FmtVisitor::from_codemap(codemap); +fn fmt_ast<'a>(krate: &ast::Crate, codemap: &'a CodeMap, config: &'a Config) -> ChangeSet<'a> { + let mut visitor = FmtVisitor::from_codemap(codemap, config); visit::walk_crate(&mut visitor, krate); let files = codemap.files.borrow(); if let Some(last) = files.last() { @@ -175,7 +175,7 @@ fn fmt_ast<'a>(krate: &ast::Crate, codemap: &'a CodeMap) -> ChangeSet<'a> { // Formatting done on a char by char or line by line basis. // TODO warn on bad license // TODO other stuff for parity with make tidy -fn fmt_lines(changes: &mut ChangeSet) -> FormatReport { +fn fmt_lines(changes: &mut ChangeSet, config: &Config) -> FormatReport { let mut truncate_todo = Vec::new(); let mut report = FormatReport { file_error_map: HashMap::new() }; @@ -187,8 +187,8 @@ fn fmt_lines(changes: &mut ChangeSet) -> FormatReport { let mut cur_line = 1; let mut newline_count = 0; let mut errors = vec![]; - let mut issue_seeker = BadIssueSeeker::new(config!(report_todo), - config!(report_fixme)); + let mut issue_seeker = BadIssueSeeker::new(config.report_todo, + config.report_fixme); for (c, b) in text.chars() { if c == '\r' { continue; } @@ -208,7 +208,7 @@ fn fmt_lines(changes: &mut ChangeSet) -> FormatReport { line_len -= b - lw; } // Check for any line width errors we couldn't correct. - if line_len > config!(max_width) { + if line_len > config.max_width { errors.push(FormattingError { line: cur_line, kind: ErrorKind::LineOverflow @@ -256,6 +256,7 @@ fn fmt_lines(changes: &mut ChangeSet) -> FormatReport { struct RustFmtCalls { input_path: Option, write_mode: WriteMode, + config: Option>, } impl<'a> CompilerCalls<'a> for RustFmtCalls { @@ -302,18 +303,23 @@ impl<'a> CompilerCalls<'a> for RustFmtCalls { fn build_controller(&mut self, _: &Session) -> driver::CompileController<'a> { let write_mode = self.write_mode; + + let mut config_option = None; + swap(&mut self.config, &mut config_option); + let config = config_option.unwrap(); + let mut control = driver::CompileController::basic(); control.after_parse.stop = Compilation::Stop; control.after_parse.callback = Box::new(move |state| { let krate = state.krate.unwrap(); let codemap = state.session.codemap(); - let mut changes = fmt_ast(krate, codemap); + let mut changes = fmt_ast(krate, codemap, &*config); // For some reason, the codemap does not include terminating newlines // so we must add one on for each file. This is sad. changes.append_newlines(); - println!("{}", fmt_lines(&mut changes)); + println!("{}", fmt_lines(&mut changes, &*config)); - let result = changes.write_all_files(write_mode); + let result = changes.write_all_files(write_mode, &*config); match result { Err(msg) => println!("Error writing files: {}", msg), @@ -335,8 +341,7 @@ impl<'a> CompilerCalls<'a> for RustFmtCalls { // WriteMode. // default_config is a string of toml data to be used to configure rustfmt. pub fn run(args: Vec, write_mode: WriteMode, default_config: &str) { - config::set_config(default_config); - - let mut call_ctxt = RustFmtCalls { input_path: None, write_mode: write_mode }; + let config = Some(Box::new(config::Config::from_toml(default_config))); + let mut call_ctxt = RustFmtCalls { input_path: None, write_mode: write_mode, config: config }; rustc_driver::run_compiler(&args, &mut call_ctxt); } diff --git a/src/rewrite.rs b/src/rewrite.rs index 354e9b17061..78d611253dd 100644 --- a/src/rewrite.rs +++ b/src/rewrite.rs @@ -12,6 +12,8 @@ use syntax::codemap::CodeMap; +use config::Config; + pub trait Rewrite { /// Rewrite self into offset and width. /// `offset` is the indentation of the first line. The next lines @@ -25,4 +27,5 @@ pub trait Rewrite { pub struct RewriteContext<'a> { pub codemap: &'a CodeMap, + pub config: &'a Config, } diff --git a/src/visitor.rs b/src/visitor.rs index e70ecc95b4f..88b5d4b18dc 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -13,6 +13,7 @@ use syntax::codemap::{self, CodeMap, Span, BytePos}; use syntax::visit; use utils; +use config::Config; use SKIP_ANNOTATION; use changes::ChangeSet; @@ -24,6 +25,7 @@ pub struct FmtVisitor<'a> { pub last_pos: BytePos, // TODO RAII util for indenting pub block_indent: usize, + pub config: &'a Config, } impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> { @@ -33,14 +35,12 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> { self.codemap.lookup_char_pos(ex.span.hi)); self.format_missing(ex.span.lo); let offset = self.changes.cur_offset_span(ex.span); - match ex.rewrite(&RewriteContext { codemap: self.codemap }, - config!(max_width) - offset, - offset) { - Some(new_str) => { - self.changes.push_str_span(ex.span, &new_str); - self.last_pos = ex.span.hi; - } - None => { self.last_pos = ex.span.lo; } + let context = RewriteContext { codemap: self.codemap, config: self.config }; + let rewrite = ex.rewrite(&context, self.config.max_width - offset, offset); + + if let Some(new_str) = rewrite { + self.changes.push_str_span(ex.span, &new_str); + self.last_pos = ex.span.hi; } } @@ -72,7 +72,7 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> { self.changes.push_str_span(b.span, "{"); self.last_pos = self.last_pos + BytePos(1); - self.block_indent += config!(tab_spaces); + self.block_indent += self.config.tab_spaces; for stmt in &b.stmts { self.visit_stmt(&stmt) @@ -85,7 +85,7 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> { None => {} } - self.block_indent -= config!(tab_spaces); + self.block_indent -= self.config.tab_spaces; // TODO we should compress any newlines here to just one self.format_missing_with_indent(b.span.hi - BytePos(1)); self.changes.push_str_span(b.span, "}"); @@ -162,8 +162,8 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> { match vp.node { ast::ViewPath_::ViewPathList(ref path, ref path_list) => { let block_indent = self.block_indent; - let one_line_budget = config!(max_width) - block_indent; - let multi_line_budget = config!(ideal_width) - block_indent; + let one_line_budget = self.config.max_width - block_indent; + let multi_line_budget = self.config.ideal_width - block_indent; let formatted = self.rewrite_use_list(block_indent, one_line_budget, multi_line_budget, @@ -183,6 +183,7 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> { }; self.format_missing(span_end); } + self.last_pos = item.span.hi; } ast::ViewPath_::ViewPathGlob(_) => { @@ -195,9 +196,9 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> { ast::Item_::ItemImpl(..) | ast::Item_::ItemMod(_) | ast::Item_::ItemTrait(..) => { - self.block_indent += config!(tab_spaces); + self.block_indent += self.config.tab_spaces; visit::walk_item(self, item); - self.block_indent -= config!(tab_spaces); + self.block_indent -= self.config.tab_spaces; } ast::Item_::ItemExternCrate(_) => { self.format_missing_with_indent(item.span.lo); @@ -273,12 +274,13 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> { } impl<'a> FmtVisitor<'a> { - pub fn from_codemap<'b>(codemap: &'b CodeMap) -> FmtVisitor<'b> { + pub fn from_codemap<'b>(codemap: &'b CodeMap, config: &'b Config) -> FmtVisitor<'b> { FmtVisitor { codemap: codemap, changes: ChangeSet::from_codemap(codemap), last_pos: BytePos(0), block_indent: 0, + config: config } } diff --git a/tests/system.rs b/tests/system.rs index d0769a855ac..4b29376ffa7 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -26,18 +26,34 @@ fn get_path_string(dir_entry: io::Result) -> String { path.to_str().expect("Couldn't stringify path.").to_owned() } -// Integration tests and idempotence tests. The files in the tests/source are -// formatted and compared to their equivalent in tests/target. The target file -// and config can be overriden by annotations in the source file. The input and -// output must match exactly. -// Files in tests/target are checked to be unaltered by rustfmt. -// FIXME(#28) would be good to check for error messages and fail on them, or at least report. +// Integration tests. The files in the tests/source are formatted and compared +// to their equivalent in tests/target. The target file and config can be +// overriden by annotations in the source file. The input and output must match +// exactly. +// FIXME(#28) would be good to check for error messages and fail on them, or at +// least report. #[test] fn system_tests() { + // Get all files in the tests/source directory + let files = fs::read_dir("tests/source").ok().expect("Couldn't read source dir."); + // turn a DirEntry into a String that represents the relative path to the file + let files = files.map(get_path_string); + + let (count, fails) = check_files(files); + + // Display results + println!("Ran {} system tests.", count); + assert!(fails == 0, "{} system tests failed", fails); +} + +// Idempotence tests. Files in tests/target are checked to be unaltered by +// rustfmt. +#[test] +fn idempotence_tests() { // Get all files in the tests/target directory - let files = fs::read_dir("tests/target").ok().expect("Couldn't read dir 1."); - let files = files.chain(fs::read_dir("tests").ok().expect("Couldn't read dir 2.")); - let files = files.chain(fs::read_dir("src/bin").ok().expect("Couldn't read dir 3.")); + let files = fs::read_dir("tests/target").ok().expect("Couldn't read target dir."); + let files = files.chain(fs::read_dir("tests").ok().expect("Couldn't read tests dir.")); + let files = files.chain(fs::read_dir("src/bin").ok().expect("Couldn't read src dir.")); // turn a DirEntry into a String that represents the relative path to the file let files = files.map(get_path_string); // hack because there's no `IntoIterator` impl for `[T; N]` @@ -48,17 +64,6 @@ fn system_tests() { // Display results println!("Ran {} idempotent tests.", count); assert!(fails == 0, "{} idempotent tests failed", fails); - - // Get all files in the tests/source directory - let files = fs::read_dir("tests/source").ok().expect("Couldn't read dir 4."); - // turn a DirEntry into a String that represents the relative path to the file - let files = files.map(get_path_string); - - let (count, fails) = check_files(files); - - // Display results - println!("Ran {} system tests.", count); - assert!(fails == 0, "{} system tests failed", fails); } // For each file, run rustfmt and collect the output. @@ -71,12 +76,9 @@ fn check_files(files: I) -> (u32, u32) for file_name in files.filter(|f| f.ends_with(".rs")) { println!("Testing '{}'...", file_name); - match idempotent_check(file_name) { - Ok(()) => {}, - Err(m) => { - print_mismatches(m); - fails += 1; - }, + if let Err(msg) = idempotent_check(file_name) { + print_mismatches(msg); + fails += 1; } count += 1; }