diff --git a/Cargo.lock b/Cargo.lock index 672c5f59da3..3349ad3410a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -191,6 +191,7 @@ dependencies = [ "env_logger 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "getopts 0.2.15 (registry+https://github.com/rust-lang/crates.io-index)", "kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", + "lazy_static 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.34 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)", "regex 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/Cargo.toml b/Cargo.toml index b2ecd547784..dd748d46854 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,9 @@ getopts = "0.2" derive-new = "0.5" cargo_metadata = "0.4" +[dev-dependencies] +lazy_static = "1.0.0" + [target.'cfg(unix)'.dependencies] libc = "0.2.11" diff --git a/Configurations.md b/Configurations.md index 37e6fa0a998..b91f39f31ca 100644 --- a/Configurations.md +++ b/Configurations.md @@ -511,12 +511,12 @@ Maximum length of comments. No effect unless`wrap_comments = true`. **Note:** A value of `0` results in [`wrap_comments`](#wrap_comments) being applied regardless of a line's width. -#### Comments shorter than `comment_width`: +#### `80` (default; comments shorter than `comment_width`): ```rust // Lorem ipsum dolor sit amet, consectetur adipiscing elit. ``` -#### Comments longer than `comment_width`: +#### `60` (comments longer than `comment_width`): ```rust // Lorem ipsum dolor sit amet, // consectetur adipiscing elit. diff --git a/src/rustfmt_diff.rs b/src/rustfmt_diff.rs index 69633568755..f37ab5c16ef 100644 --- a/src/rustfmt_diff.rs +++ b/src/rustfmt_diff.rs @@ -97,15 +97,28 @@ pub fn make_diff(expected: &str, actual: &str, context_size: usize) -> Vec Self { + match term::stdout() { + Some(ref t) if use_colored_tty(color) && t.supports_color() => PrintType::Fancy, + _ => PrintType::Basic, + } + } +} + pub fn print_diff(diff: Vec, get_section_title: F, color: Color) where F: Fn(u32) -> String, { - match term::stdout() { - Some(ref t) if use_colored_tty(color) && t.supports_color() => { - print_diff_fancy(diff, get_section_title, term::stdout().unwrap()) - } - _ => print_diff_basic(diff, get_section_title), + match PrintType::get(color) { + PrintType::Fancy => print_diff_fancy(diff, get_section_title, term::stdout().unwrap()), + PrintType::Basic => print_diff_basic(diff, get_section_title), } } diff --git a/tests/system.rs b/tests/system.rs index dceb5600634..a551ca67eae 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -10,6 +10,8 @@ #![feature(rustc_private)] +#[macro_use] +extern crate lazy_static; #[macro_use] extern crate log; extern crate regex; @@ -19,7 +21,7 @@ extern crate term; use std::collections::HashMap; use std::fs; use std::io::{self, BufRead, BufReader, Read}; -use std::iter::Peekable; +use std::iter::{Enumerate, Peekable}; use std::path::{Path, PathBuf}; use std::str::Chars; @@ -29,6 +31,7 @@ use rustfmt::filemap::{write_system_newlines, FileMap}; use rustfmt::rustfmt_diff::*; const DIFF_CONTEXT_SIZE: usize = 3; +const CONFIGURATIONS_FILE_NAME: &str = "Configurations.md"; // Returns a `Vec` containing `PathBuf`s of files with a rs extension in the // given path. The `recursive` argument controls if files from subdirectories @@ -98,6 +101,17 @@ fn verify_config_test_names() { } } +// This writes to the terminal using the same approach (via term::stdout or +// println!) that is used by `rustfmt::rustfmt_diff::print_diff`. Writing +// using only one or the other will cause the output order to differ when +// `print_diff` selects the approach not used. +fn write_message(msg: String) { + match PrintType::get(Color::Auto) { + PrintType::Fancy => writeln!(term::stdout().unwrap(), "{}", msg).unwrap(), + PrintType::Basic => println!("{}", msg), + } +} + // 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 // overridden by annotations in the source file. The input and output must match @@ -152,7 +166,7 @@ fn assert_output(source: &Path, expected_filename: &Path) { if !compare.is_empty() { let mut failures = HashMap::new(); failures.insert(source.to_owned(), compare); - print_mismatches(failures); + print_mismatches_default_message(failures, source.display()); assert!(false, "Text does not match expected output"); } } @@ -259,7 +273,7 @@ fn check_files(files: Vec) -> (Vec, u32, u32) { for file_name in files { debug!("Testing '{}'...", file_name.display()); - match idempotent_check(file_name) { + match idempotent_check(&file_name) { Ok(ref report) if report.has_warnings() => { print!("{}", report); fails += 1; @@ -267,7 +281,7 @@ fn check_files(files: Vec) -> (Vec, u32, u32) { Ok(report) => reports.push(report), Err(err) => { if let IdempotentCheckError::Mismatch(msg) = err { - print_mismatches(msg); + print_mismatches_default_message(msg, file_name.display()); } fails += 1; } @@ -279,15 +293,22 @@ fn check_files(files: Vec) -> (Vec, u32, u32) { (reports, count, fails) } -fn print_mismatches(result: HashMap>) { - let mut t = term::stdout().unwrap(); +fn print_mismatches_default_message( + result: HashMap>, + file_name: std::path::Display, +) { + print_mismatches(result, |line_num| { + format!("\nMismatch at {}:{}:", file_name, line_num) + }); +} - for (file_name, diff) in result { - print_diff( - diff, - |line_num| format!("\nMismatch at {}:{}:", file_name.display(), line_num), - Color::Auto, - ); +fn print_mismatches String>( + result: HashMap>, + mismatch_msg_formatter: T, +) { + let mut t = term::stdout().unwrap(); + for (_file_name, diff) in result { + print_diff(diff, &mismatch_msg_formatter, Color::Auto); } t.reset().unwrap(); @@ -327,7 +348,7 @@ pub enum IdempotentCheckError { Parse, } -pub fn idempotent_check(filename: PathBuf) -> Result { +pub fn idempotent_check(filename: &PathBuf) -> Result { let sig_comments = read_significant_comments(&filename); let config = read_config(&filename); let (error_summary, file_map, format_report) = format_file(filename, &config); @@ -536,3 +557,233 @@ fn string_eq_ignore_newline_repr_test() { assert!(string_eq_ignore_newline_repr("a\r\n\r\n\r\nb", "a\n\n\nb")); assert!(!string_eq_ignore_newline_repr("a\r\nbcd", "a\nbcdefghijk")); } + +// This enum is used to represent one of three text features in Configurations.md: a block of code +// with its starting line number, the name of a rustfmt configuration option, or the value of a +// rustfmt configuration option. +enum ConfigurationSection { + CodeBlock((String, u32)), // (String: block of code, u32: line number of code block start) + ConfigName(String), + ConfigValue(String), +} + +impl ConfigurationSection { + fn get_section>( + file: &mut Enumerate, + ) -> Option { + lazy_static! { + static ref CONFIG_NAME_REGEX: regex::Regex = regex::Regex::new(r"^## `([^`]+)`").expect("Failed creating configuration pattern"); + static ref CONFIG_VALUE_REGEX: regex::Regex = regex::Regex::new(r#"^#### `"?([^`"]+)"?`"#).expect("Failed creating configuration value pattern"); + } + + loop { + match file.next() { + Some((i, line)) => { + if line.starts_with("```rust") { + // Get the lines of the code block. + let lines: Vec = file.map(|(_i, l)| l) + .take_while(|l| !l.starts_with("```")) + .collect(); + let block = format!("{}\n", lines.join("\n")); + + // +1 to translate to one-based indexing + // +1 to get to first line of code (line after "```") + let start_line = (i + 2) as u32; + + return Some(ConfigurationSection::CodeBlock((block, start_line))); + } else if let Some(c) = CONFIG_NAME_REGEX.captures(&line) { + return Some(ConfigurationSection::ConfigName(String::from(&c[1]))); + } else if let Some(c) = CONFIG_VALUE_REGEX.captures(&line) { + return Some(ConfigurationSection::ConfigValue(String::from(&c[1]))); + } + } + None => return None, // reached the end of the file + } + } + } +} + +// This struct stores the information about code blocks in the configurations +// file, formats the code blocks, and prints formatting errors. +struct ConfigCodeBlock { + config_name: Option, + config_value: Option, + code_block: Option, + code_block_start: Option, +} + +impl ConfigCodeBlock { + fn new() -> ConfigCodeBlock { + ConfigCodeBlock { + config_name: None, + config_value: None, + code_block: None, + code_block_start: None, + } + } + + fn set_config_name(&mut self, name: Option) { + self.config_name = name; + self.config_value = None; + } + + fn set_config_value(&mut self, value: Option) { + self.config_value = value; + } + + fn set_code_block(&mut self, code_block: String, code_block_start: u32) { + self.code_block = Some(code_block); + self.code_block_start = Some(code_block_start); + } + + fn get_block_config(&self) -> Config { + let mut config = Config::default(); + config.override_value( + self.config_name.as_ref().unwrap(), + self.config_value.as_ref().unwrap(), + ); + config + } + + fn code_block_valid(&self) -> bool { + // We never expect to not have a code block. + assert!(self.code_block.is_some() && self.code_block_start.is_some()); + + if self.config_name.is_none() { + write_message(format!( + "configuration name not found for block beginning at line {}", + self.code_block_start.unwrap() + )); + return false; + } + if self.config_value.is_none() { + write_message(format!( + "configuration value not found for block beginning at line {}", + self.code_block_start.unwrap() + )); + return false; + } + true + } + + fn has_parsing_errors(&self, error_summary: Summary) -> bool { + if error_summary.has_parsing_errors() { + write_message(format!( + "\u{261d}\u{1f3fd} Failed to format block starting at Line {} in {}", + self.code_block_start.unwrap(), + CONFIGURATIONS_FILE_NAME + )); + return true; + } + + false + } + + fn print_diff(&self, compare: Vec) { + let mut mismatches = HashMap::new(); + mismatches.insert(PathBuf::from(CONFIGURATIONS_FILE_NAME), compare); + print_mismatches(mismatches, |line_num| { + format!( + "\nMismatch at {}:{}:", + CONFIGURATIONS_FILE_NAME, + line_num + self.code_block_start.unwrap() - 1 + ) + }); + } + + fn formatted_has_diff(&self, file_map: FileMap) -> bool { + let &(ref _file_name, ref text) = file_map.first().unwrap(); + let compare = make_diff(self.code_block.as_ref().unwrap(), text, DIFF_CONTEXT_SIZE); + if !compare.is_empty() { + self.print_diff(compare); + return true; + } + + false + } + + // Return a bool indicating if formatting this code block is an idempotent + // operation. This function also triggers printing any formatting failure + // messages. + fn formatted_is_idempotent(&self) -> bool { + // Verify that we have all of the expected information. + if !self.code_block_valid() { + return false; + } + + let input = Input::Text(self.code_block.as_ref().unwrap().to_owned()); + let config = self.get_block_config(); + + let (error_summary, file_map, _report) = + format_input::(input, &config, None).unwrap(); + + !self.has_parsing_errors(error_summary) && !self.formatted_has_diff(file_map) + } + + // Extract a code block from the iterator. Behavior: + // - Rust code blocks are identifed by lines beginning with "```rust". + // - One explicit configuration setting is supported per code block. + // - Rust code blocks with no configuration setting are illegal and cause an + // assertion failure. + // - Configuration names in Configurations.md must be in the form of + // "## `NAME`". + // - Configuration values in Configurations.md must be in the form of + // "#### `VALUE`". + fn extract>( + file: &mut Enumerate, + prev: Option<&ConfigCodeBlock>, + ) -> Option { + let mut code_block = ConfigCodeBlock::new(); + code_block.config_name = prev.map_or(None, |cb| cb.config_name.clone()); + + loop { + match ConfigurationSection::get_section(file) { + Some(ConfigurationSection::CodeBlock((block, start_line))) => { + code_block.set_code_block(block, start_line); + break; + } + Some(ConfigurationSection::ConfigName(name)) => { + code_block.set_config_name(Some(name)); + } + Some(ConfigurationSection::ConfigValue(value)) => { + code_block.set_config_value(Some(value)); + } + None => return None, // end of file was reached + } + } + + Some(code_block) + } +} + +#[test] +#[ignore] +fn configuration_snippet_tests() { + // Read Configurations.md and build a `Vec` of `ConfigCodeBlock` structs with one + // entry for each Rust code block found. + fn get_code_blocks() -> Vec { + let mut file_iter = BufReader::new( + fs::File::open(CONFIGURATIONS_FILE_NAME) + .expect(&format!("Couldn't read file {}", CONFIGURATIONS_FILE_NAME)), + ).lines() + .map(|l| l.unwrap()) + .enumerate(); + let mut code_blocks: Vec = Vec::new(); + + while let Some(cb) = ConfigCodeBlock::extract(&mut file_iter, code_blocks.last()) { + code_blocks.push(cb); + } + + code_blocks + } + + let blocks = get_code_blocks(); + let failures = blocks + .iter() + .map(|b| b.formatted_is_idempotent()) + .fold(0, |acc, r| acc + (!r as u32)); + + // Display results. + println!("Ran {} configurations tests.", blocks.len()); + assert_eq!(failures, 0, "{} configurations tests failed", failures); +}