Merge pull request #112 from marcusklaas/config-fix

Remove global mutable config to allow for concurrency
This commit is contained in:
Nick Cameron 2015-06-23 06:18:07 -07:00
commit 66c6fe5334
8 changed files with 109 additions and 105 deletions

View File

@ -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<String, String>), ::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<Option<String>, ::std::io::Error> {
let text = &self.file_map[filename];
// prints all newlines either as `\n` or as `\r\n`
fn write_system_newlines<T>(
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<u8>
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()));
}

View File

@ -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 }
};
}

View File

@ -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)
})

View File

@ -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: ",",

View File

@ -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<config::Config> = 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<PathBuf>,
write_mode: WriteMode,
config: Option<Box<config::Config>>,
}
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<String>, 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);
}

View File

@ -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,
}

View File

@ -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
}
}

View File

@ -26,18 +26,34 @@ fn get_path_string(dir_entry: io::Result<fs::DirEntry>) -> 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<I>(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;
}