Adding test to verify code block idempotency in Configurations.md

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)",

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"
 

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.

src/rustfmt_diff.rs

@@ -97,15 +97,28 @@ pub fn make_diff(expected: &str, actual: &str, context_size: usize) -> Vec<Misma
     results
 }
 
+// A representation of how to write output.
+pub enum PrintType {
+    Fancy, // want to output color and the terminal supports it
+    Basic, // do not want to output color or the terminal does not support color
+}
+
+impl PrintType {
+    pub fn get(color: Color) -> Self {
+        match term::stdout() {
+            Some(ref t) if use_colored_tty(color) && t.supports_color() => PrintType::Fancy,
+            _ => PrintType::Basic,
+        }
+    }
+}
+
 pub fn print_diff<F>(diff: Vec<Mismatch>, get_section_title: F, color: Color)
 where
     F: Fn(u32) -> String,
 {
-    match term::stdout() {
+    match PrintType::get(color) {
-        Some(ref t) if use_colored_tty(color) && t.supports_color() => {
+        PrintType::Fancy => print_diff_fancy(diff, get_section_title, term::stdout().unwrap()),
-            print_diff_fancy(diff, get_section_title, term::stdout().unwrap())
+        PrintType::Basic => print_diff_basic(diff, get_section_title),
-        }
-        _ => print_diff_basic(diff, get_section_title),
     }
 }
 

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<PathBuf>) -> (Vec<FormatReport>, 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<PathBuf>) -> (Vec<FormatReport>, 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<PathBuf>) -> (Vec<FormatReport>, u32, u32) {
     (reports, count, fails)
 }
 
-fn print_mismatches(result: HashMap<PathBuf, Vec<Mismatch>>) {
+fn print_mismatches_default_message(
-    let mut t = term::stdout().unwrap();
+    result: HashMap<PathBuf, Vec<Mismatch>>,
+    file_name: std::path::Display,
+) {
+    print_mismatches(result, |line_num| {
+        format!("\nMismatch at {}:{}:", file_name, line_num)
+    });
+}
 
-    for (file_name, diff) in result {
+fn print_mismatches<T: Fn(u32) -> String>(
-        print_diff(
+    result: HashMap<PathBuf, Vec<Mismatch>>,
-            diff,
+    mismatch_msg_formatter: T,
-            |line_num| format!("\nMismatch at {}:{}:", file_name.display(), line_num),
+) {
-            Color::Auto,
+    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<FormatReport, IdempotentCheckError> {
+pub fn idempotent_check(filename: &PathBuf) -> Result<FormatReport, IdempotentCheckError> {
     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<I: Iterator<Item = String>>(
+        file: &mut Enumerate<I>,
+    ) -> Option<ConfigurationSection> {
+        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<String> = 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<String>,
+    config_value: Option<String>,
+    code_block: Option<String>,
+    code_block_start: Option<u32>,
+}
+
+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<String>) {
+        self.config_name = name;
+        self.config_value = None;
+    }
+
+    fn set_config_value(&mut self, value: Option<String>) {
+        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<Mismatch>) {
+        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::<io::Stdout>(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<I: Iterator<Item = String>>(
+        file: &mut Enumerate<I>,
+        prev: Option<&ConfigCodeBlock>,
+    ) -> Option<ConfigCodeBlock> {
+        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<ConfigCodeBlock> {
+        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<ConfigCodeBlock> = 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);
+}