From cd51523715a77a30660dfb46898eb407937ec9d7 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Fri, 31 Aug 2018 20:57:46 -0700 Subject: [PATCH 01/15] tidy: Use chars for single-character patterns Fixes the clippy "single_char_pattern" lint, and (marginally) improves performance. --- src/tools/tidy/src/cargo.rs | 2 +- src/tools/tidy/src/errors.rs | 2 +- src/tools/tidy/src/extdeps.rs | 2 +- src/tools/tidy/src/features.rs | 10 +++++----- src/tools/tidy/src/style.rs | 8 ++++---- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/tools/tidy/src/cargo.rs b/src/tools/tidy/src/cargo.rs index f40fea60f40a8..1f0379f1ea80b 100644 --- a/src/tools/tidy/src/cargo.rs +++ b/src/tools/tidy/src/cargo.rs @@ -67,7 +67,7 @@ fn verify(tomlfile: &Path, libfile: &Path, bad: &mut bool) { }; let mut lines = deps.lines().peekable(); while let Some(line) = lines.next() { - if line.starts_with("[") { + if line.starts_with('[') { break } diff --git a/src/tools/tidy/src/errors.rs b/src/tools/tidy/src/errors.rs index 3dccffddf9388..9f55d6f9ad636 100644 --- a/src/tools/tidy/src/errors.rs +++ b/src/tools/tidy/src/errors.rs @@ -50,7 +50,7 @@ pub fn check(path: &Path, bad: &mut bool) { } let mut search = line; - while let Some(i) = search.find("E") { + while let Some(i) = search.find('E') { search = &search[i + 1..]; let code = if search.len() > 4 { search[..4].parse::() diff --git a/src/tools/tidy/src/extdeps.rs b/src/tools/tidy/src/extdeps.rs index 761803a45b8b3..74f3a41047a32 100644 --- a/src/tools/tidy/src/extdeps.rs +++ b/src/tools/tidy/src/extdeps.rs @@ -38,7 +38,7 @@ pub fn check(path: &Path, bad: &mut bool) { } // extract source value - let parts: Vec<&str> = line.splitn(2, "=").collect(); + let parts: Vec<&str> = line.splitn(2, '=').collect(); let source = parts[1].trim(); // ensure source is whitelisted diff --git a/src/tools/tidy/src/features.rs b/src/tools/tidy/src/features.rs index 74f66c0a05169..c95f1c7b32a94 100644 --- a/src/tools/tidy/src/features.rs +++ b/src/tools/tidy/src/features.rs @@ -75,7 +75,7 @@ pub fn check(path: &Path, bad: &mut bool, quiet: bool) { return; } - let filen_underscore = filename.replace("-","_").replace(".rs",""); + let filen_underscore = filename.replace('-',"_").replace(".rs",""); let filename_is_gate_test = test_filen_gate(&filen_underscore, &mut features); contents.truncate(0); @@ -332,11 +332,11 @@ fn map_lib_features(base_src_path: &Path, f.tracking_issue = find_attr_val(line, "issue") .map(|s| s.parse().unwrap()); } - if line.ends_with("]") { + if line.ends_with(']') { mf(Ok((name, f.clone())), file, i + 1); - } else if !line.ends_with(",") && !line.ends_with("\\") { + } else if !line.ends_with(',') && !line.ends_with('\\') { // We need to bail here because we might have missed the - // end of a stability attribute above because the "]" + // end of a stability attribute above because the ']' // might not have been at the end of the line. // We could then get into the very unfortunate situation that // we continue parsing the file assuming the current stability @@ -394,7 +394,7 @@ fn map_lib_features(base_src_path: &Path, has_gate_test: false, tracking_issue, }; - if line.contains("]") { + if line.contains(']') { mf(Ok((feature_name, feature)), file, i + 1); } else { becoming_feature = Some((feature_name.to_owned(), feature)); diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index 6b431ccda0883..33cd8b5dcd9ab 100644 --- a/src/tools/tidy/src/style.rs +++ b/src/tools/tidy/src/style.rs @@ -69,7 +69,7 @@ fn line_is_url(line: &str) -> bool { (EXP_COMMENT_START, "//!") => state = EXP_LINK_LABEL_OR_URL, (EXP_LINK_LABEL_OR_URL, w) - if w.len() >= 4 && w.starts_with("[") && w.ends_with("]:") + if w.len() >= 4 && w.starts_with('[') && w.ends_with("]:") => state = EXP_URL, (EXP_LINK_LABEL_OR_URL, w) @@ -128,13 +128,13 @@ pub fn check(path: &Path, bad: &mut bool) { && !long_line_is_ok(line) { err(&format!("line longer than {} chars", COLS)); } - if line.contains("\t") && !skip_tab { + if line.contains('\t') && !skip_tab { err("tab character"); } - if !skip_end_whitespace && (line.ends_with(" ") || line.ends_with("\t")) { + if !skip_end_whitespace && (line.ends_with(' ') || line.ends_with('\t')) { err("trailing whitespace"); } - if line.contains("\r") && !skip_cr { + if line.contains('\r') && !skip_cr { err("CR character"); } if filename != "style.rs" { From 0f40a12ea35d0a33cef29dc50787ed6efd74cbfd Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 1 Sep 2018 12:17:57 -0700 Subject: [PATCH 02/15] tidy: Use is_empty() instead of len tests Fixes a clippy warning, and improves readability. --- src/tools/tidy/src/deps.rs | 2 +- src/tools/tidy/src/features.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 68a4b7898575d..5bff8480d591e 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -239,7 +239,7 @@ pub fn check_whitelist(path: &Path, cargo: &Path, bad: &mut bool) { unapproved.append(&mut bad); } - if unapproved.len() > 0 { + if !unapproved.is_empty() { println!("Dependencies not on the whitelist:"); for dep in unapproved { println!("* {}", dep.id_str()); diff --git a/src/tools/tidy/src/features.rs b/src/tools/tidy/src/features.rs index c95f1c7b32a94..05eeae7d6b016 100644 --- a/src/tools/tidy/src/features.rs +++ b/src/tools/tidy/src/features.rs @@ -133,7 +133,7 @@ pub fn check(path: &Path, bad: &mut bool, quiet: bool) { name); } - if gate_untested.len() > 0 { + if !gate_untested.is_empty() { tidy_error!(bad, "Found {} features without a gate test.", gate_untested.len()); } From 2893a2127d02de058bb41bde44eaa8c8cbd473a4 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 1 Sep 2018 12:24:03 -0700 Subject: [PATCH 03/15] tidy: Clean up argument handling Use `.nth(n)` rather than `.skip(n).next()` (also fixes a clippy warning), and use `.into()` and a type signature rather than `PathBuf::from`. --- src/tools/tidy/src/main.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 4fe77f8b58feb..22fa954e5fa55 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -24,11 +24,8 @@ use std::path::PathBuf; use std::env; fn main() { - let path = env::args_os().skip(1).next().expect("need path to src"); - let path = PathBuf::from(path); - - let cargo = env::args_os().skip(2).next().expect("need path to cargo"); - let cargo = PathBuf::from(cargo); + let path: PathBuf = env::args_os().nth(1).expect("need path to src").into(); + let cargo: PathBuf = env::args_os().nth(2).expect("need path to cargo").into(); let args: Vec = env::args().skip(1).collect(); From fb317aaf9e90cb3ca5ce0a9f48b1b36cf231f162 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 1 Sep 2018 16:40:49 -0700 Subject: [PATCH 04/15] tidy: cargo.rs: Clean up loop to use "for" instead of "while let" Eliminates a clippy warning. Also drop the unnecessary `.peekable()`. --- src/tools/tidy/src/cargo.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tools/tidy/src/cargo.rs b/src/tools/tidy/src/cargo.rs index 1f0379f1ea80b..69f61bc248dbb 100644 --- a/src/tools/tidy/src/cargo.rs +++ b/src/tools/tidy/src/cargo.rs @@ -65,8 +65,7 @@ fn verify(tomlfile: &Path, libfile: &Path, bad: &mut bool) { Some(i) => &toml[i+1..], None => return, }; - let mut lines = deps.lines().peekable(); - while let Some(line) = lines.next() { + for line in deps.lines() { if line.starts_with('[') { break } From 896c3ce2f17acc90f43c026618731cf82a42a9d9 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 1 Sep 2018 16:45:14 -0700 Subject: [PATCH 05/15] tidy: extdeps.rs: Clean up loop iteration to use "for" Also eliminates a clippy lint. --- src/tools/tidy/src/extdeps.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tools/tidy/src/extdeps.rs b/src/tools/tidy/src/extdeps.rs index 74f3a41047a32..bb375c17682a9 100644 --- a/src/tools/tidy/src/extdeps.rs +++ b/src/tools/tidy/src/extdeps.rs @@ -29,8 +29,7 @@ pub fn check(path: &Path, bad: &mut bool) { t!(t!(File::open(path)).read_to_string(&mut cargo_lock)); // process each line - let mut lines = cargo_lock.lines(); - while let Some(line) = lines.next() { + for line in cargo_lock.lines() { // consider only source entries if ! line.starts_with("source = ") { From decc3b0eba163a45accc4e1157301f528dfc7e50 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 1 Sep 2018 16:46:40 -0700 Subject: [PATCH 06/15] tidy: extdeps.rs: Avoid an unnecessary collect() --- src/tools/tidy/src/extdeps.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tools/tidy/src/extdeps.rs b/src/tools/tidy/src/extdeps.rs index bb375c17682a9..831eb47858af1 100644 --- a/src/tools/tidy/src/extdeps.rs +++ b/src/tools/tidy/src/extdeps.rs @@ -37,8 +37,7 @@ pub fn check(path: &Path, bad: &mut bool) { } // extract source value - let parts: Vec<&str> = line.splitn(2, '=').collect(); - let source = parts[1].trim(); + let source = line.splitn(2, '=').nth(1).unwrap().trim(); // ensure source is whitelisted if !WHITELISTED_SOURCES.contains(&&*source) { From cd20cdf7e07d87cd346f6b71d0a46311b79218ab Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 1 Sep 2018 16:46:57 -0700 Subject: [PATCH 07/15] tidy: Use "const" instead of "static, and remove implied `'static` lifetimes Dropping the redundant lifetimes also eliminates a clippy warning. --- src/tools/tidy/src/deps.rs | 8 ++++---- src/tools/tidy/src/extdeps.rs | 2 +- src/tools/tidy/src/pal.rs | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 5bff8480d591e..ee1fc0ca510d9 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -18,7 +18,7 @@ use std::process::Command; use serde_json; -static LICENSES: &'static [&'static str] = &[ +const LICENSES: &[&str] = &[ "MIT/Apache-2.0", "MIT / Apache-2.0", "Apache-2.0/MIT", @@ -33,7 +33,7 @@ static LICENSES: &'static [&'static str] = &[ /// should be considered bugs. Exceptions are only allowed in Rust /// tooling. It is _crucial_ that no exception crates be dependencies /// of the Rust runtime (std / test). -static EXCEPTIONS: &'static [&'static str] = &[ +const EXCEPTIONS: &[&str] = &[ "mdbook", // MPL2, mdbook "openssl", // BSD+advertising clause, cargo, mdbook "pest", // MPL2, mdbook via handlebars @@ -54,13 +54,13 @@ static EXCEPTIONS: &'static [&'static str] = &[ ]; /// Which crates to check against the whitelist? -static WHITELIST_CRATES: &'static [CrateVersion] = &[ +const WHITELIST_CRATES: &[CrateVersion] = &[ CrateVersion("rustc", "0.0.0"), CrateVersion("rustc_codegen_llvm", "0.0.0"), ]; /// Whitelist of crates rustc is allowed to depend on. Avoid adding to the list if possible. -static WHITELIST: &'static [Crate] = &[ +const WHITELIST: &[Crate] = &[ Crate("aho-corasick"), Crate("arrayvec"), Crate("atty"), diff --git a/src/tools/tidy/src/extdeps.rs b/src/tools/tidy/src/extdeps.rs index 831eb47858af1..7f58b440a833e 100644 --- a/src/tools/tidy/src/extdeps.rs +++ b/src/tools/tidy/src/extdeps.rs @@ -15,7 +15,7 @@ use std::io::Read; use std::path::Path; /// List of whitelisted sources for packages -static WHITELISTED_SOURCES: &'static [&'static str] = &[ +const WHITELISTED_SOURCES: &[&str] = &[ "\"registry+https://github.com/rust-lang/crates.io-index\"", ]; diff --git a/src/tools/tidy/src/pal.rs b/src/tools/tidy/src/pal.rs index 8071f07d81195..3cddfb8fde84c 100644 --- a/src/tools/tidy/src/pal.rs +++ b/src/tools/tidy/src/pal.rs @@ -50,7 +50,7 @@ use std::path::Path; use std::iter::Iterator; // Paths that may contain platform-specific code -const EXCEPTION_PATHS: &'static [&'static str] = &[ +const EXCEPTION_PATHS: &[&str] = &[ // std crates "src/liballoc_jemalloc", "src/liballoc_system", From 226d79c30aed5c0fb58c99047f88f98ac2eed689 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 1 Sep 2018 16:55:53 -0700 Subject: [PATCH 08/15] tidy: Avoid "let ref mut = ..." This also eliminates a clippy warning. --- src/tools/tidy/src/pal.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tools/tidy/src/pal.rs b/src/tools/tidy/src/pal.rs index 3cddfb8fde84c..52913d7cfba3a 100644 --- a/src/tools/tidy/src/pal.rs +++ b/src/tools/tidy/src/pal.rs @@ -88,10 +88,10 @@ const EXCEPTION_PATHS: &[&str] = &[ ]; pub fn check(path: &Path, bad: &mut bool) { - let ref mut contents = String::new(); + let mut contents = String::new(); // Sanity check that the complex parsing here works - let ref mut saw_target_arch = false; - let ref mut saw_cfg_bang = false; + let mut saw_target_arch = false; + let mut saw_cfg_bang = false; super::walk(path, &mut super::filter_dirs, &mut |file| { let filestr = file.to_string_lossy().replace("\\", "/"); if !filestr.ends_with(".rs") { return } @@ -99,11 +99,11 @@ pub fn check(path: &Path, bad: &mut bool) { let is_exception_path = EXCEPTION_PATHS.iter().any(|s| filestr.contains(&**s)); if is_exception_path { return } - check_cfgs(contents, &file, bad, saw_target_arch, saw_cfg_bang); + check_cfgs(&mut contents, &file, bad, &mut saw_target_arch, &mut saw_cfg_bang); }); - assert!(*saw_target_arch); - assert!(*saw_cfg_bang); + assert!(saw_target_arch); + assert!(saw_cfg_bang); } fn check_cfgs(contents: &mut String, file: &Path, From 40ea9999155bb4c48dc537ef066cb736956914e1 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 1 Sep 2018 17:11:00 -0700 Subject: [PATCH 09/15] tidy: unstable_book.rs: Clean up directory iteration Drop unnecessary .into_iter() (also fixing a clippy warning), and use path functions to handle file extensions. --- src/tools/tidy/src/unstable_book.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/tools/tidy/src/unstable_book.rs b/src/tools/tidy/src/unstable_book.rs index 62296f73f016b..520879df620ee 100644 --- a/src/tools/tidy/src/unstable_book.rs +++ b/src/tools/tidy/src/unstable_book.rs @@ -56,12 +56,11 @@ pub fn collect_unstable_feature_names(features: &Features) -> BTreeSet { pub fn collect_unstable_book_section_file_names(dir: &path::Path) -> BTreeSet { fs::read_dir(dir) .expect("could not read directory") - .into_iter() .map(|entry| entry.expect("could not read directory entry")) .filter(dir_entry_is_file) - .map(|entry| entry.file_name().into_string().unwrap()) - .filter(|n| n.ends_with(".md")) - .map(|n| n.trim_right_matches(".md").to_owned()) + .map(|entry| entry.path()) + .filter(|path| path.extension().map(|e| e.to_str().unwrap()) == Some("md")) + .map(|path| path.file_stem().unwrap().to_str().unwrap().into()) .collect() } From fc3419c762dd23788f1daa723dd6172619212612 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 1 Sep 2018 17:36:37 -0700 Subject: [PATCH 10/15] tidy: deps: Hoist a complex multi-line if condition into a let This makes the code more readable, and eliminates a clippy warning. --- src/tools/tidy/src/deps.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index ee1fc0ca510d9..7278bbf3595c8 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -205,12 +205,13 @@ pub fn check(path: &Path, bad: &mut bool) { let dir = t!(dir); // skip our exceptions - if EXCEPTIONS.iter().any(|exception| { + let is_exception = EXCEPTIONS.iter().any(|exception| { dir.path() .to_str() .unwrap() .contains(&format!("src/vendor/{}", exception)) - }) { + }); + if is_exception { continue; } From d5b1dee4728e5166d5e4478d27b9c4fb940e3dae Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 1 Sep 2018 17:38:09 -0700 Subject: [PATCH 11/15] tidy: Eliminate an unnecessary .into_iter() --- src/tools/tidy/src/pal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/tidy/src/pal.rs b/src/tools/tidy/src/pal.rs index 52913d7cfba3a..a7abaf7a5b875 100644 --- a/src/tools/tidy/src/pal.rs +++ b/src/tools/tidy/src/pal.rs @@ -130,7 +130,7 @@ fn check_cfgs(contents: &mut String, file: &Path, tidy_error!(bad, "{}:{}: platform-specific cfg: {}", file.display(), line, cfg); }; - for (idx, cfg) in cfgs.into_iter() { + for (idx, cfg) in cfgs { // Sanity check that the parsing here works if !*saw_target_arch && cfg.contains("target_arch") { *saw_target_arch = true } if !*saw_cfg_bang && cfg.contains("cfg!") { *saw_cfg_bang = true } From 87658bb667be731395b45b89ffad2ceba17d88c4 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 1 Sep 2018 17:39:07 -0700 Subject: [PATCH 12/15] tidy: Use an inclusive range rather than a +1 bound This improves readability and eliminates a clippy warning. --- src/tools/tidy/src/pal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/tidy/src/pal.rs b/src/tools/tidy/src/pal.rs index a7abaf7a5b875..69a4c09c2285d 100644 --- a/src/tools/tidy/src/pal.rs +++ b/src/tools/tidy/src/pal.rs @@ -216,7 +216,7 @@ fn parse_cfgs<'a>(contents: &'a str) -> Vec<(usize, &'a str)> { b')' => { depth -= 1; if depth == 0 { - return (i, &contents_from[.. j + 1]); + return (i, &contents_from[..=j]); } } _ => { } From 0e65aeb95d230ef5b49b42eebdc3b294bc71fd7a Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 1 Sep 2018 17:41:45 -0700 Subject: [PATCH 13/15] tidy: features.rs: collect_lib_features: Simplify Use `if let` to simplify a match, and use `contains_key` instead of `get`. --- src/tools/tidy/src/features.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/tools/tidy/src/features.rs b/src/tools/tidy/src/features.rs index 05eeae7d6b016..91e6423a6b074 100644 --- a/src/tools/tidy/src/features.rs +++ b/src/tools/tidy/src/features.rs @@ -259,14 +259,11 @@ pub fn collect_lib_features(base_src_path: &Path) -> Features { map_lib_features(base_src_path, &mut |res, _, _| { - match res { - Ok((name, feature)) => { - if lib_features.get(name).is_some() { - return; - } - lib_features.insert(name.to_owned(), feature); - }, - Err(_) => (), + if let Ok((name, feature)) = res { + if lib_features.contains_key(name) { + return; + } + lib_features.insert(name.to_owned(), feature); } }); lib_features From ebdc1bda7154703aa48157839dfb44e77d396dd9 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 1 Sep 2018 17:43:26 -0700 Subject: [PATCH 14/15] tidy: features.rs: Use splitn rather than a complex slice --- src/tools/tidy/src/features.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/tidy/src/features.rs b/src/tools/tidy/src/features.rs index 91e6423a6b074..732eade263d91 100644 --- a/src/tools/tidy/src/features.rs +++ b/src/tools/tidy/src/features.rs @@ -94,7 +94,7 @@ pub fn check(path: &Path, bad: &mut bool, quiet: bool) { let feature_name = match line.find(gate_test_str) { Some(i) => { - &line[i+gate_test_str.len()..line[i+1..].find(' ').unwrap_or(line.len())] + line[i+gate_test_str.len()..].splitn(2, ' ').next().unwrap() }, None => continue, }; From a5c86fe6e1693add443ca94616bd262e393f6c7b Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 1 Sep 2018 17:44:16 -0700 Subject: [PATCH 15/15] tidy: features.rs: Remove a redundant .contains The match expression immediately below it checks the same condition. --- src/tools/tidy/src/features.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/tools/tidy/src/features.rs b/src/tools/tidy/src/features.rs index 732eade263d91..85b123e4af51f 100644 --- a/src/tools/tidy/src/features.rs +++ b/src/tools/tidy/src/features.rs @@ -88,10 +88,6 @@ pub fn check(path: &Path, bad: &mut bool, quiet: bool) { let gate_test_str = "gate-test-"; - if !line.contains(gate_test_str) { - continue; - } - let feature_name = match line.find(gate_test_str) { Some(i) => { line[i+gate_test_str.len()..].splitn(2, ' ').next().unwrap()