Merge pull request #2292 from davidalber/configurations-checking

Configurations checking
This commit is contained in:
Nick Cameron 2018-01-09 18:39:15 +13:00 committed by GitHub
commit d60a6958f8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 288 additions and 20 deletions

1
Cargo.lock generated
View File

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

View File

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

View File

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

View File

@ -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() {
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),
}
}

View File

@ -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>>) {
let mut t = term::stdout().unwrap();
fn print_mismatches_default_message(
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 {
print_diff(
diff,
|line_num| format!("\nMismatch at {}:{}:", file_name.display(), line_num),
Color::Auto,
);
fn print_mismatches<T: Fn(u32) -> String>(
result: HashMap<PathBuf, Vec<Mismatch>>,
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<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);
}