Skip to content

Commit

Permalink
Deduplicate warnings from the same config file
Browse files Browse the repository at this point in the history 
Closes rust-lang#3872
  • Loading branch information
@ayazhafiz
ayazhafiz committed May 20, 2020
1 parent 45895dc commit 641dbec
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 13 deletions.
46 changes: 39 additions & 7 deletions rustfmt-core/rustfmt-bin/src/bin/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#[macro_use]
extern crate lazy_static;

use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::env;
use std::fmt;
use std::io::{self, stdin, stdout, Error as IoError, Read, Write};
Expand Down Expand Up @@ -496,12 +496,21 @@ fn format(opt: Opt) -> Result<i32> {
verbosity: opt.verbosity(),
};

let mut config_warnings: HashMap<&Option<PathBuf>, HashSet<String>> = HashMap::new();
let inputs = FileConfigPairIter::new(&opt, config_path.is_some()).collect::<Vec<_>>();
let format_report = format_inputs(
inputs.iter().map(|p| {
(
Input::File(p.file.to_path_buf()),
if let FileConfig::Local(ref config, _) = p.config {
if let FileConfig::Local(ref config, ref path) = p.config {
let seen_warnings = config_warnings.entry(path).or_insert(HashSet::new());
for warning in config.warnings.iter() {
if seen_warnings.contains(warning) {
continue;
}
eprintln!("{}", warning);
seen_warnings.insert(warning.to_string());
}
config
} else {
&default_config
Expand Down Expand Up @@ -542,17 +551,18 @@ mod test {
path: PathBuf,
}

fn make_temp_file(file_name: &'static str) -> TempFile {
fn make_temp_file(file_name: &'static str, content: &'static str) -> TempFile {
use std::env::var;
use std::fs::File;

// Used in the Rust build system.
let target_dir = var("RUSTFMT_TEST_DIR").unwrap_or_else(|_| ".".to_owned());
let target_dir = var("RUSTFMT_TEST_DIR").map_or_else(|_| env::temp_dir(), PathBuf::from);
let path = Path::new(&target_dir).join(file_name);

std::fs::create_dir_all(path.parent().unwrap()).expect("couldn't create temp file");
let mut file = File::create(&path).expect("couldn't create temp file");
let content = b"fn main() {}\n";
file.write_all(content).expect("couldn't write temp file");
file.write_all(content.as_bytes())
.expect("couldn't write temp file");
TempFile { path }
}

Expand Down Expand Up @@ -596,7 +606,7 @@ mod test {
#[test]
fn verify_check_works() {
init_log();
let temp_file = make_temp_file("temp_check.rs");
let temp_file = make_temp_file("temp_check.rs", "fn main() {}\n");

Command::new(rustfmt())
.arg("--check")
Expand Down Expand Up @@ -679,4 +689,26 @@ mod test {
.expect("Failed to wait on rustfmt child");
assert!(output.status.success());
}

#[test]
fn deduplicate_warning_messages() {
init_log();

let temp_file_1 = make_temp_file("temp_check_1.rs", "fn main() {}\n");
let temp_file_2 = make_temp_file("temp_check_2.rs", "fn main() {}\n");
let _temp_config = make_temp_file("rustfmt.toml", "max_width = 60\nfn_call_width = 100\n");

let child = Command::new(rustfmt())
.arg(&temp_file_1.path)
.arg(&temp_file_2.path)
.output()
.expect("rustfmt failed");

let stderr = String::from_utf8_lossy(&child.stderr);
assert_eq!(
stderr,
"`fn_call_width` cannot have a value that exceeds `max_width`. \
`fn_call_width` will be set to the same value as `max_width`\n"
);
}
}
27 changes: 21 additions & 6 deletions rustfmt-core/rustfmt-lib/src/config/config_type.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ impl ConfigType for IgnoreList {

macro_rules! create_config {
($($i:ident: $Ty:ty, $def:expr, $is_stable:literal, $dstring:literal;)+) => (
use std::collections::HashSet;
use std::io::Write;

use serde::{Deserialize, Serialize};
Expand All @@ -62,6 +63,7 @@ macro_rules! create_config {
// if a license_template_path has been specified, successfully read, parsed and compiled
// into a regex, it will be stored here
pub license_template: Option<Regex>,
pub warnings: HashSet<String>,
// For each config item, we store a bool indicating whether it has
// been accessed and the value, and a bool whether the option was
// manually initialized, or taken from the default,
Expand Down Expand Up @@ -156,8 +158,9 @@ macro_rules! create_config {
self.$i.1 = true;
self.$i.2 = val;
} else {
eprintln!("Warning: can't set `{} = {:?}`, unstable features are only \
available in nightly channel.", stringify!($i), val);
self.warnings.insert(
format!("Warning: can't set `{} = {:?}`, unstable features are \
only available in nightly channel.", stringify!($i), val));
}
}
}
Expand Down Expand Up @@ -302,16 +305,17 @@ macro_rules! create_config {
override_value: usize,
heuristic_value: usize,
config_key: &str,
warnings: &mut HashSet<String>,
| -> usize {
if !was_set {
return heuristic_value;
}
if override_value > max_width {
eprintln!(
warnings.insert(format!(
"`{0}` cannot have a value that exceeds `max_width`. \
`{0}` will be set to the same value as `max_width`",
config_key,
);
));
return max_width;
}
override_value
Expand All @@ -322,6 +326,7 @@ macro_rules! create_config {
self.fn_call_width.2,
heuristics.fn_call_width,
"fn_call_width",
&mut self.warnings,
);
self.fn_call_width.2 = fn_call_width;

Expand All @@ -330,6 +335,7 @@ macro_rules! create_config {
self.attr_fn_like_width.2,
heuristics.attr_fn_like_width,
"attr_fn_like_width",
&mut self.warnings,
);
self.attr_fn_like_width.2 = attr_fn_like_width;

Expand All @@ -338,6 +344,7 @@ macro_rules! create_config {
self.struct_lit_width.2,
heuristics.struct_lit_width,
"struct_lit_width",
&mut self.warnings,
);
self.struct_lit_width.2 = struct_lit_width;

Expand All @@ -346,6 +353,7 @@ macro_rules! create_config {
self.struct_variant_width.2,
heuristics.struct_variant_width,
"struct_variant_width",
&mut self.warnings,
);
self.struct_variant_width.2 = struct_variant_width;

Expand All @@ -354,6 +362,7 @@ macro_rules! create_config {
self.array_width.2,
heuristics.array_width,
"array_width",
&mut self.warnings,
);
self.array_width.2 = array_width;

Expand All @@ -362,6 +371,7 @@ macro_rules! create_config {
self.chain_width.2,
heuristics.chain_width,
"chain_width",
&mut self.warnings,
);
self.chain_width.2 = chain_width;

Expand All @@ -370,6 +380,7 @@ macro_rules! create_config {
self.single_line_if_else_max_width.2,
heuristics.single_line_if_else_max_width,
"single_line_if_else_max_width",
&mut self.warnings,
);
self.single_line_if_else_max_width.2 = single_line_if_else_max_width;
}
Expand All @@ -390,8 +401,11 @@ macro_rules! create_config {
if lt_path.len() > 0 {
match license::load_and_compile_template(&lt_path) {
Ok(re) => self.license_template = Some(re),
Err(msg) => eprintln!("Warning for license template file {:?}: {}",
lt_path, msg),
Err(msg) => {
self.warnings.insert(
format!("Warning for license template file {:?}: {}",
lt_path, msg));
}
}
}
}
Expand All @@ -418,6 +432,7 @@ macro_rules! create_config {
fn default() -> Self {
Self {
license_template: None,
warnings: HashSet::new(),
$(
$i: (Cell::new(false), false, $def, $is_stable),
)+
Expand Down

0 comments on commit 641dbec

Please sign in to comment.