From d9a8a8a778e33f2ce3cd18fad0da112ae713e602 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Thu, 16 May 2019 07:31:56 +0200 Subject: [PATCH] Add a stderr file length check to clippy_dev This adds a check to `clippy_dev` that enforces a maximum line count for `stderr` files. CI will fail if the line count is exceeded. It's currently set to `320` lines. Ideally this would be implemented in `compiletest-rs` but there are plans to move Rust's `compiletest` into the `compiletest-rs` repository and I don't want to do the work in `compiletest` twice. However, I also don't want to wait until the move is done, so I added the check to `clippy_dev` until it makes sense to add it to compiletest-rs. cc #2038 --- ci/base-tests.sh | 1 + clippy_dev/src/main.rs | 12 +++++- clippy_dev/src/stderr_length_check.rs | 55 +++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 clippy_dev/src/stderr_length_check.rs diff --git a/ci/base-tests.sh b/ci/base-tests.sh index 64214e3be86..d67541f7df0 100755 --- a/ci/base-tests.sh +++ b/ci/base-tests.sh @@ -23,6 +23,7 @@ export CARGO_TARGET_DIR=`pwd`/target/ # Perform various checks for lint registration ./util/dev update_lints --check +./util/dev --limit-stderr-length cargo +nightly fmt --all -- --check # Check running clippy-driver without cargo diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 3f21b9dee11..45d4d13ed86 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -2,8 +2,9 @@ extern crate clap; extern crate clippy_dev; extern crate regex; -use clap::{App, AppSettings, Arg, SubCommand}; +use clap::{App, Arg, SubCommand}; use clippy_dev::*; +mod stderr_length_check; #[derive(PartialEq)] enum UpdateMode { @@ -13,7 +14,6 @@ enum UpdateMode { fn main() { let matches = App::new("Clippy developer tooling") - .setting(AppSettings::SubcommandRequiredElseHelp) .subcommand( SubCommand::with_name("update_lints") .about("Updates lint registration and information from the source code") @@ -36,8 +36,16 @@ fn main() { .help("Checks that util/dev update_lints has been run. Used on CI."), ), ) + .arg( + Arg::with_name("limit-stderr-length") + .long("limit-stderr-length") + .help("Ensures that stderr files do not grow longer than a certain amount of lines."), + ) .get_matches(); + if matches.is_present("limit-stderr-length") { + stderr_length_check::check(); + } if let Some(matches) = matches.subcommand_matches("update_lints") { if matches.is_present("print-only") { print_lints(); diff --git a/clippy_dev/src/stderr_length_check.rs b/clippy_dev/src/stderr_length_check.rs new file mode 100644 index 00000000000..9222ed84167 --- /dev/null +++ b/clippy_dev/src/stderr_length_check.rs @@ -0,0 +1,55 @@ +use std::ffi::OsStr; +use walkdir::WalkDir; + +use std::fs::File; +use std::io::prelude::*; + +// The maximum length allowed for stderr files. +// +// We limit this because small files are easier to deal with than bigger files. +const LIMIT: usize = 320; + +pub fn check() { + let stderr_files = stderr_files(); + let exceeding_files = exceeding_stderr_files(stderr_files); + + if !exceeding_files.is_empty() { + println!("Error: stderr files exceeding limit of {} lines:", LIMIT); + for path in exceeding_files { + println!("{}", path); + } + std::process::exit(1); + } +} + +fn exceeding_stderr_files(files: impl Iterator) -> Vec { + files + .filter_map(|file| { + let path = file.path().to_str().expect("Could not convert path to str").to_string(); + let linecount = count_linenumbers(&path); + if linecount > LIMIT { + Some(path) + } else { + None + } + }) + .collect() +} + +fn stderr_files() -> impl Iterator { + // We use `WalkDir` instead of `fs::read_dir` here in order to recurse into subdirectories. + WalkDir::new("../tests/ui") + .into_iter() + .filter_map(std::result::Result::ok) + .filter(|f| f.path().extension() == Some(OsStr::new("stderr"))) +} + +fn count_linenumbers(filepath: &str) -> usize { + if let Ok(mut file) = File::open(filepath) { + let mut content = String::new(); + file.read_to_string(&mut content).expect("Failed to read file?"); + content.lines().count() + } else { + 0 + } +}