From becd0e8896079016cd615def3799b8133636b39e Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Mon, 14 Dec 2020 23:10:15 +0900 Subject: [PATCH] Replace some `println!` with `tidy_error!` to simplify --- src/tools/tidy/src/deps.rs | 29 +++++++++++++---------------- src/tools/tidy/src/extdeps.rs | 3 +-- src/tools/tidy/src/features.rs | 1 - src/tools/tidy/src/lib.rs | 4 ++++ src/tools/tidy/src/ui_tests.rs | 6 ++---- 5 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 057b0884e287a..952782175f13c 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -214,12 +214,12 @@ fn check_exceptions(metadata: &Metadata, bad: &mut bool) { for (name, license) in EXCEPTIONS { // Check that the package actually exists. if !metadata.packages.iter().any(|p| p.name == *name) { - println!( + tidy_error!( + bad, "could not find exception package `{}`\n\ Remove from EXCEPTIONS list if it is no longer used.", name ); - *bad = true; } // Check that the license hasn't changed. for pkg in metadata.packages.iter().filter(|p| p.name == *name) { @@ -232,11 +232,11 @@ fn check_exceptions(metadata: &Metadata, bad: &mut bool) { } match &pkg.license { None => { - println!( + tidy_error!( + bad, "dependency exception `{}` does not declare a license expression", pkg.id ); - *bad = true; } Some(pkg_license) => { if pkg_license.as_str() != *license { @@ -273,8 +273,7 @@ fn check_exceptions(metadata: &Metadata, bad: &mut bool) { let license = match &pkg.license { Some(license) => license, None => { - println!("dependency `{}` does not define a license expression", pkg.id,); - *bad = true; + tidy_error!(bad, "dependency `{}` does not define a license expression", pkg.id); continue; } }; @@ -286,8 +285,7 @@ fn check_exceptions(metadata: &Metadata, bad: &mut bool) { // general, these should never be added. continue; } - println!("invalid license `{}` in `{}`", license, pkg.id); - *bad = true; + tidy_error!(bad, "invalid license `{}` in `{}`", license, pkg.id); } } } @@ -300,12 +298,12 @@ fn check_dependencies(metadata: &Metadata, bad: &mut bool) { // Check that the PERMITTED_DEPENDENCIES does not have unused entries. for name in PERMITTED_DEPENDENCIES { if !metadata.packages.iter().any(|p| p.name == *name) { - println!( + tidy_error!( + bad, "could not find allowed package `{}`\n\ Remove from PERMITTED_DEPENDENCIES list if it is no longer used.", name ); - *bad = true; } } // Get the list in a convenient form. @@ -322,11 +320,10 @@ fn check_dependencies(metadata: &Metadata, bad: &mut bool) { } if !unapproved.is_empty() { - println!("Dependencies not explicitly permitted:"); + tidy_error!(bad, "Dependencies not explicitly permitted:"); for dep in unapproved { println!("* {}", dep); } - *bad = true; } } @@ -381,16 +378,17 @@ fn check_crate_duplicate(metadata: &Metadata, bad: &mut bool) { let matches: Vec<_> = metadata.packages.iter().filter(|pkg| pkg.name == name).collect(); match matches.len() { 0 => { - println!( + tidy_error!( + bad, "crate `{}` is missing, update `check_crate_duplicate` \ if it is no longer used", name ); - *bad = true; } 1 => {} _ => { - println!( + tidy_error!( + bad, "crate `{}` is duplicated in `Cargo.lock`, \ it is too expensive to build multiple times, \ so make sure only one version appears across all dependencies", @@ -399,7 +397,6 @@ fn check_crate_duplicate(metadata: &Metadata, bad: &mut bool) { for pkg in matches { println!(" * {}", pkg.id); } - *bad = true; } } } diff --git a/src/tools/tidy/src/extdeps.rs b/src/tools/tidy/src/extdeps.rs index 93d4d3d8047d8..aad57cacbb41e 100644 --- a/src/tools/tidy/src/extdeps.rs +++ b/src/tools/tidy/src/extdeps.rs @@ -27,8 +27,7 @@ pub fn check(root: &Path, bad: &mut bool) { // Ensure source is allowed. if !ALLOWED_SOURCES.contains(&&*source) { - println!("invalid source: {}", source); - *bad = true; + tidy_error!(bad, "invalid source: {}", source); } } } diff --git a/src/tools/tidy/src/features.rs b/src/tools/tidy/src/features.rs index 3c2880d0d5e26..d78af2cd6164c 100644 --- a/src/tools/tidy/src/features.rs +++ b/src/tools/tidy/src/features.rs @@ -330,7 +330,6 @@ fn collect_lang_features_in(base: &Path, file: &str, bad: &mut bool) -> Features let issue_str = parts.next().unwrap().trim(); let tracking_issue = if issue_str.starts_with("None") { if level == Status::Unstable && !next_feature_omits_tracking_issue { - *bad = true; tidy_error!( bad, "{}:{}: no tracking issue for feature {}", diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index e11d293210b0a..d282d240d8234 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -28,6 +28,10 @@ macro_rules! t { } macro_rules! tidy_error { + ($bad:expr, $fmt:expr) => ({ + *$bad = true; + eprintln!("tidy error: {}", $fmt); + }); ($bad:expr, $fmt:expr, $($arg:tt)*) => ({ *$bad = true; eprint!("tidy error: "); diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 72ffdabd5222f..d8d2b449fee87 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -67,14 +67,12 @@ pub fn check(path: &Path, bad: &mut bool) { let testname = file_path.file_name().unwrap().to_str().unwrap().split_once('.').unwrap().0; if !file_path.with_file_name(testname).with_extension("rs").exists() { - println!("Stray file with UI testing output: {:?}", file_path); - *bad = true; + tidy_error!(bad, "Stray file with UI testing output: {:?}", file_path); } if let Ok(metadata) = fs::metadata(file_path) { if metadata.len() == 0 { - println!("Empty file with UI testing output: {:?}", file_path); - *bad = true; + tidy_error!(bad, "Empty file with UI testing output: {:?}", file_path); } } }