Skip to content

Commit

Permalink
Rollup merge of rust-lang#53895 - joshtriplett:tidy-tidy, r=nikomatsakis
Browse files Browse the repository at this point in the history
tidy: Cleanups and clippy warning fixes

This eliminates various clippy warnings in the tidy tool, as well as
making some related cleanups. These changes should not introduce any
functional differences.
  • Loading branch information
kennytm committed Sep 7, 2018
2 parents 3c77043 + a5c86fe commit d2a0f98
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 57 deletions.
5 changes: 2 additions & 3 deletions src/tools/tidy/src/cargo.rs
Expand Up @@ -65,9 +65,8 @@ 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() {
if line.starts_with("[") {
for line in deps.lines() {
if line.starts_with('[') {
break
}

Expand Down
15 changes: 8 additions & 7 deletions src/tools/tidy/src/deps.rs
Expand Up @@ -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",
Expand All @@ -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
Expand All @@ -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"),
Expand Down Expand Up @@ -208,12 +208,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;
}

Expand Down Expand Up @@ -242,7 +243,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());
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/errors.rs
Expand Up @@ -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::<u32>()
Expand Down
8 changes: 3 additions & 5 deletions src/tools/tidy/src/extdeps.rs
Expand Up @@ -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\"",
];

Expand All @@ -29,17 +29,15 @@ 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 = ") {
continue;
}

// 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) {
Expand Down
31 changes: 12 additions & 19 deletions src/tools/tidy/src/features.rs
Expand Up @@ -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);
Expand All @@ -88,13 +88,9 @@ 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()..line[i+1..].find(' ').unwrap_or(line.len())]
line[i+gate_test_str.len()..].splitn(2, ' ').next().unwrap()
},
None => continue,
};
Expand Down Expand Up @@ -133,7 +129,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());
}

Expand Down Expand Up @@ -259,14 +255,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
Expand Down Expand Up @@ -332,11 +325,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
Expand Down Expand Up @@ -394,7 +387,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));
Expand Down
7 changes: 2 additions & 5 deletions src/tools/tidy/src/main.rs
Expand Up @@ -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<String> = env::args().skip(1).collect();

Expand Down
18 changes: 9 additions & 9 deletions src/tools/tidy/src/pal.rs
Expand Up @@ -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",
Expand Down Expand Up @@ -88,22 +88,22 @@ const EXCEPTION_PATHS: &'static [&'static 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 }

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,
Expand All @@ -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 }
Expand Down Expand Up @@ -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]);
}
}
_ => { }
Expand Down
8 changes: 4 additions & 4 deletions src/tools/tidy/src/style.rs
Expand Up @@ -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)
Expand Down Expand Up @@ -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" {
Expand Down
7 changes: 3 additions & 4 deletions src/tools/tidy/src/unstable_book.rs
Expand Up @@ -56,12 +56,11 @@ pub fn collect_unstable_feature_names(features: &Features) -> BTreeSet<String> {
pub fn collect_unstable_book_section_file_names(dir: &path::Path) -> BTreeSet<String> {
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()
}

Expand Down

0 comments on commit d2a0f98

Please sign in to comment.