From 79177f10c989490b78efda757664aba5c79726df Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Sat, 10 Jul 2021 07:36:29 -0700 Subject: [PATCH] Fixed clippy defects (#827) --- cargo/cargo_build_script_runner/bin.rs | 63 ++++++++++--------- cargo/cargo_build_script_runner/lib.rs | 34 ++++++---- proto/optional_output_wrapper.rs | 2 +- test/chained_direct_deps/mod3.rs | 2 +- test/renamed_deps/mod3.rs | 2 +- .../rust_project_json_test.rs | 2 +- .../rust_project_json_test.rs | 2 +- .../rust_project_json_test.rs | 2 +- tools/runfiles/runfiles.rs | 12 ++-- tools/rustfmt/srcs/lib.rs | 12 ++-- tools/rustfmt/srcs/test_main.rs | 3 - util/label/label.rs | 20 +++--- util/launcher/launcher_main.rs | 10 ++- 13 files changed, 88 insertions(+), 78 deletions(-) diff --git a/cargo/cargo_build_script_runner/bin.rs b/cargo/cargo_build_script_runner/bin.rs index 4eda6b41b0..b42683e300 100644 --- a/cargo/cargo_build_script_runner/bin.rs +++ b/cargo/cargo_build_script_runner/bin.rs @@ -50,10 +50,8 @@ fn run_buildrs() -> Result<(), String> { let out_dir_abs = exec_root.join(&out_dir); // For some reason Google's RBE does not create the output directory, force create it. - create_dir_all(&out_dir_abs).expect(&format!( - "Failed to make output directory: {:?}", - out_dir_abs - )); + create_dir_all(&out_dir_abs) + .unwrap_or_else(|_| panic!("Failed to make output directory: {:?}", out_dir_abs)); let target_env_vars = get_target_env_vars(&rustc_env).expect("Error getting target env vars from rustc"); @@ -119,47 +117,52 @@ fn run_buildrs() -> Result<(), String> { } } - let (buildrs_outputs, process_output) = - BuildScriptOutput::from_command(&mut command).map_err(|process_output| { - format!( - "Build script process failed{}\n--stdout:\n{}\n--stderr:\n{}", - if let Some(exit_code) = process_output.status.code() { - format!(" with exit code {}", exit_code) - } else { - String::new() - }, - String::from_utf8(process_output.stdout) - .expect("Failed to parse stdout of child process"), - String::from_utf8(process_output.stderr) - .expect("Failed to parse stdout of child process"), - ) - })?; + let (buildrs_outputs, process_output) = BuildScriptOutput::outputs_from_command(&mut command) + .map_err(|process_output| { + format!( + "Build script process failed{}\n--stdout:\n{}\n--stderr:\n{}", + if let Some(exit_code) = process_output.status.code() { + format!(" with exit code {}", exit_code) + } else { + String::new() + }, + String::from_utf8(process_output.stdout) + .expect("Failed to parse stdout of child process"), + String::from_utf8(process_output.stderr) + .expect("Failed to parse stdout of child process"), + ) + })?; write( &env_file, - BuildScriptOutput::to_env(&buildrs_outputs, &exec_root.to_string_lossy()).as_bytes(), + BuildScriptOutput::outputs_to_env(&buildrs_outputs, &exec_root.to_string_lossy()) + .as_bytes(), ) - .expect(&format!("Unable to write file {:?}", env_file)); + .unwrap_or_else(|_| panic!("Unable to write file {:?}", env_file)); write( &output_dep_env_path, - BuildScriptOutput::to_dep_env(&buildrs_outputs, &crate_links, &exec_root.to_string_lossy()) - .as_bytes(), + BuildScriptOutput::outputs_to_dep_env( + &buildrs_outputs, + &crate_links, + &exec_root.to_string_lossy(), + ) + .as_bytes(), ) - .expect(&format!("Unable to write file {:?}", output_dep_env_path)); + .unwrap_or_else(|_| panic!("Unable to write file {:?}", output_dep_env_path)); write(&stdout_path, process_output.stdout) - .expect(&format!("Unable to write file {:?}", stdout_path)); + .unwrap_or_else(|_| panic!("Unable to write file {:?}", stdout_path)); write(&stderr_path, process_output.stderr) - .expect(&format!("Unable to write file {:?}", stderr_path)); + .unwrap_or_else(|_| panic!("Unable to write file {:?}", stderr_path)); let CompileAndLinkFlags { compile_flags, link_flags, - } = BuildScriptOutput::to_flags(&buildrs_outputs, &exec_root.to_string_lossy()); + } = BuildScriptOutput::outputs_to_flags(&buildrs_outputs, &exec_root.to_string_lossy()); write(&compile_flags_file, compile_flags.as_bytes()) - .expect(&format!("Unable to write file {:?}", compile_flags_file)); + .unwrap_or_else(|_| panic!("Unable to write file {:?}", compile_flags_file)); write(&link_flags_file, link_flags.as_bytes()) - .expect(&format!("Unable to write file {:?}", link_flags_file)); + .unwrap_or_else(|_| panic!("Unable to write file {:?}", link_flags_file)); Ok(()) } @@ -243,7 +246,7 @@ fn get_target_env_vars>(rustc: &P) -> Result= 2 { values .entry(key) - .or_insert(vec![]) + .or_insert_with(Vec::new) .push(value[1..(value.len() - 1)].to_owned()); } } diff --git a/cargo/cargo_build_script_runner/lib.rs b/cargo/cargo_build_script_runner/lib.rs index 273c0f4578..e2d1b7033d 100644 --- a/cargo/cargo_build_script_runner/lib.rs +++ b/cargo/cargo_build_script_runner/lib.rs @@ -94,7 +94,7 @@ impl BuildScriptOutput { } /// Converts a [BufReader] into a vector of [BuildScriptOutput] enums. - fn from_reader(mut reader: BufReader) -> Vec { + fn outputs_from_reader(mut reader: BufReader) -> Vec { let mut result = Vec::::new(); let mut line = String::new(); while reader.read_line(&mut line).expect("Cannot read line") != 0 { @@ -107,11 +107,13 @@ impl BuildScriptOutput { } /// Take a [Command], execute it and converts its input into a vector of [BuildScriptOutput] - pub fn from_command(cmd: &mut Command) -> Result<(Vec, Output), Output> { + pub fn outputs_from_command( + cmd: &mut Command, + ) -> Result<(Vec, Output), Output> { let child_output = cmd.output().expect("Unable to start binary"); if child_output.status.success() { let reader = BufReader::new(child_output.stdout.as_slice()); - let output = Self::from_reader(reader); + let output = Self::outputs_from_reader(reader); Ok((output, child_output)) } else { Err(child_output) @@ -119,8 +121,9 @@ impl BuildScriptOutput { } /// Convert a vector of [BuildScriptOutput] into a list of environment variables. - pub fn to_env(v: &Vec, exec_root: &str) -> String { - v.iter() + pub fn outputs_to_env(outputs: &[BuildScriptOutput], exec_root: &str) -> String { + outputs + .iter() .filter_map(|x| { if let BuildScriptOutput::Env(env) = x { Some(Self::escape_for_serializing(Self::redact_exec_root( @@ -135,9 +138,14 @@ impl BuildScriptOutput { } /// Convert a vector of [BuildScriptOutput] into a list of dependencies environment variables. - pub fn to_dep_env(v: &Vec, crate_links: &str, exec_root: &str) -> String { + pub fn outputs_to_dep_env( + outputs: &[BuildScriptOutput], + crate_links: &str, + exec_root: &str, + ) -> String { let prefix = format!("DEP_{}_", crate_links.replace("-", "_").to_uppercase()); - v.iter() + outputs + .iter() .filter_map(|x| { if let BuildScriptOutput::DepEnv(env) = x { Some(format!( @@ -154,11 +162,11 @@ impl BuildScriptOutput { } /// Convert a vector of [BuildScriptOutput] into a flagfile. - pub fn to_flags(v: &Vec, exec_root: &str) -> CompileAndLinkFlags { + pub fn outputs_to_flags(outputs: &[BuildScriptOutput], exec_root: &str) -> CompileAndLinkFlags { let mut compile_flags = Vec::new(); let mut link_flags = Vec::new(); - for flag in v { + for flag in outputs { match flag { BuildScriptOutput::Cfg(e) => compile_flags.push(format!("--cfg={}", e)), BuildScriptOutput::Flags(e) => compile_flags.push(e.to_owned()), @@ -214,7 +222,7 @@ cargo:rustc-env=SOME_PATH=/some/absolute/path/beep ", ); let reader = BufReader::new(buff); - let result = BuildScriptOutput::from_reader(reader); + let result = BuildScriptOutput::outputs_from_reader(reader); assert_eq!(result.len(), 10); assert_eq!(result[0], BuildScriptOutput::LinkLib("sdfsdf".to_owned())); assert_eq!(result[1], BuildScriptOutput::Env("FOO=BAR".to_owned())); @@ -242,15 +250,15 @@ cargo:rustc-env=SOME_PATH=/some/absolute/path/beep ); assert_eq!( - BuildScriptOutput::to_dep_env(&result, "ssh2", "/some/absolute/path"), + BuildScriptOutput::outputs_to_dep_env(&result, "ssh2", "/some/absolute/path"), "DEP_SSH2_VERSION=123\nDEP_SSH2_VERSION_NUMBER=1010107f\nDEP_SSH2_INCLUDE_PATH=${pwd}/include".to_owned() ); assert_eq!( - BuildScriptOutput::to_env(&result, "/some/absolute/path"), + BuildScriptOutput::outputs_to_env(&result, "/some/absolute/path"), "FOO=BAR\nBAR=FOO\nSOME_PATH=${pwd}/beep".to_owned() ); assert_eq!( - BuildScriptOutput::to_flags(&result, "/some/absolute/path"), + BuildScriptOutput::outputs_to_flags(&result, "/some/absolute/path"), CompileAndLinkFlags { // -Lblah was output as a rustc-flags, so even though it probably _should_ be a link // flag, we don't treat it like one. diff --git a/proto/optional_output_wrapper.rs b/proto/optional_output_wrapper.rs index 60ff1cd580..59eba1b944 100644 --- a/proto/optional_output_wrapper.rs +++ b/proto/optional_output_wrapper.rs @@ -26,7 +26,7 @@ fn ensure() -> Result> { let optional_outputs = args().take(index).collect::>(); let exe = args().nth(index + 1).ok_or("no exe")?; let exe_args = args().skip(index + 2).collect::>(); - if exe_args.len() < 1 { + if exe_args.is_empty() { return Err("no exe args".into()); } match Command::new(exe).args(exe_args).status()?.code() { diff --git a/test/chained_direct_deps/mod3.rs b/test/chained_direct_deps/mod3.rs index 1719ae98a3..2f31e6dbc8 100644 --- a/test/chained_direct_deps/mod3.rs +++ b/test/chained_direct_deps/mod3.rs @@ -25,7 +25,7 @@ pub fn greet_default() { /// ); /// ``` pub fn am_i_the_world(me: &str) -> bool { - return me == mod1::world(); + me == mod1::world() } #[cfg(test)] diff --git a/test/renamed_deps/mod3.rs b/test/renamed_deps/mod3.rs index 4942bf2086..4a424827b4 100644 --- a/test/renamed_deps/mod3.rs +++ b/test/renamed_deps/mod3.rs @@ -25,7 +25,7 @@ pub fn greet_default() { /// ); /// ``` pub fn am_i_the_world(me: &str) -> bool { - return me == alias_a::world(); + me == alias_a::world() } #[cfg(test)] diff --git a/test/rust_analyzer/aspect_traversal_test/rust_project_json_test.rs b/test/rust_analyzer/aspect_traversal_test/rust_project_json_test.rs index a2cb8f9309..a297a1449b 100644 --- a/test/rust_analyzer/aspect_traversal_test/rust_project_json_test.rs +++ b/test/rust_analyzer/aspect_traversal_test/rust_project_json_test.rs @@ -9,7 +9,7 @@ mod tests { r.rlocation("rules_rust/test/rust_analyzer/aspect_traversal_test/rust-project.json"); let content = std::fs::read_to_string(&rust_project_path) - .expect(&format!("couldn't open {:?}", &rust_project_path)); + .unwrap_or_else(|_| panic!("couldn't open {:?}", &rust_project_path)); for dep in &[ "lib_dep", diff --git a/test/rust_analyzer/merging_crates_test/rust_project_json_test.rs b/test/rust_analyzer/merging_crates_test/rust_project_json_test.rs index 88d0ce884b..417f72f25b 100644 --- a/test/rust_analyzer/merging_crates_test/rust_project_json_test.rs +++ b/test/rust_analyzer/merging_crates_test/rust_project_json_test.rs @@ -9,7 +9,7 @@ mod tests { r.rlocation("rules_rust/test/rust_analyzer/merging_crates_test/rust-project.json"); let content = std::fs::read_to_string(&rust_project_path) - .expect(&format!("couldn't open {:?}", &rust_project_path)); + .unwrap_or_else(|_| panic!("couldn't open {:?}", &rust_project_path)); assert!( content.contains(r#""root_module":"test/rust_analyzer/merging_crates_test/mylib.rs","deps":[{"crate":0,"name":"lib_dep"},{"crate":2,"name":"extra_test_dep"}]"#), diff --git a/test/rust_analyzer/merging_crates_test_reversed/rust_project_json_test.rs b/test/rust_analyzer/merging_crates_test_reversed/rust_project_json_test.rs index 3e8600e8ba..f77c2a6ef5 100644 --- a/test/rust_analyzer/merging_crates_test_reversed/rust_project_json_test.rs +++ b/test/rust_analyzer/merging_crates_test_reversed/rust_project_json_test.rs @@ -10,7 +10,7 @@ mod tests { ); let content = std::fs::read_to_string(&rust_project_path) - .expect(&format!("couldn't open {:?}", &rust_project_path)); + .unwrap_or_else(|_| panic!("couldn't open {:?}", &rust_project_path)); assert!( content.contains(r#""root_module":"test/rust_analyzer/merging_crates_test_reversed/mylib.rs","deps":[{"crate":0,"name":"lib_dep"},{"crate":1,"name":"extra_test_dep"}]"#), diff --git a/tools/runfiles/runfiles.rs b/tools/runfiles/runfiles.rs index 6486c2e0e3..7b6fc86582 100644 --- a/tools/runfiles/runfiles.rs +++ b/tools/runfiles/runfiles.rs @@ -97,10 +97,9 @@ impl Runfiles { Mode::DirectoryBased(runfiles_dir) => runfiles_dir.join(path), Mode::ManifestBased(path_mapping) => path_mapping .get(path) - .expect(&format!( - "Path {} not found among runfiles.", - path.to_string_lossy() - )) + .unwrap_or_else(|| { + panic!("Path {} not found among runfiles.", path.to_string_lossy()) + }) .clone(), } } @@ -109,10 +108,11 @@ impl Runfiles { /// Returns the .runfiles directory for the currently executing binary. pub fn find_runfiles_dir() -> io::Result { assert_ne!( - std::env::var_os("RUNFILES_MANIFEST_ONLY").unwrap_or(OsString::from("0")), + std::env::var_os("RUNFILES_MANIFEST_ONLY").unwrap_or_else(|| OsString::from("0")), "1" ); - let exec_path = std::env::args().nth(0).expect("arg 0 was not set"); + // Consume the first argument (argv[0]) + let exec_path = std::env::args().next().expect("arg 0 was not set"); let mut binary_path = PathBuf::from(&exec_path); loop { diff --git a/tools/rustfmt/srcs/lib.rs b/tools/rustfmt/srcs/lib.rs index 35ccf78a94..ad2e86a5d6 100644 --- a/tools/rustfmt/srcs/lib.rs +++ b/tools/rustfmt/srcs/lib.rs @@ -3,7 +3,7 @@ use std::fs; use std::path::{Path, PathBuf}; /// The expected extension of rustfmt manifest files generated by `rustfmt_aspect`. -pub const RUSTFMT_MANIFEST_EXTENSION: &'static str = "rustfmt"; +pub const RUSTFMT_MANIFEST_EXTENSION: &str = "rustfmt"; /// Generate an absolute path to a file without resolving symlinks fn absolutify_existing>(path: &T) -> std::io::Result { @@ -50,13 +50,11 @@ pub struct RustfmtManifest { /// Parse rustfmt flags from a manifest generated by builds using `rustfmt_aspect`. pub fn parse_rustfmt_manifest(manifest: &Path) -> RustfmtManifest { - let content = fs::read_to_string(manifest).expect(&format!( - "Failed to read rustfmt manifest: {}", - manifest.display() - )); + let content = fs::read_to_string(manifest) + .unwrap_or_else(|_| panic!("Failed to read rustfmt manifest: {}", manifest.display())); let mut lines: Vec = content - .split("\n") + .split('\n') .into_iter() .filter(|s| !s.is_empty()) .map(|s| s.to_owned()) @@ -70,7 +68,7 @@ pub fn parse_rustfmt_manifest(manifest: &Path) -> RustfmtManifest { .expect("The edition should be a numeric value. eg `2018`."); RustfmtManifest { - edition: edition, + edition, sources: lines, } } diff --git a/tools/rustfmt/srcs/test_main.rs b/tools/rustfmt/srcs/test_main.rs index d1520d7a0f..87c145fa2a 100644 --- a/tools/rustfmt/srcs/test_main.rs +++ b/tools/rustfmt/srcs/test_main.rs @@ -2,9 +2,6 @@ use std::fs; use std::path::{Path, PathBuf}; use std::process::Command; -use runfiles; -use rustfmt_lib; - fn main() { // Gather all and environment settings let options = parse_args(); diff --git a/util/label/label.rs b/util/label/label.rs index 6b441c58c8..e25770d5db 100644 --- a/util/label/label.rs +++ b/util/label/label.rs @@ -8,7 +8,7 @@ use label_error::LabelError; /// /// TODO: validate . and .. in target name /// TODO: validate used characters in target name -pub fn analyze<'s>(input: &'s str) -> Result> { +pub fn analyze(input: &'_ str) -> Result> { let label = input; let (input, repository_name) = consume_repository_name(input, label)?; let (input, package_name) = consume_package_name(input, label)?; @@ -40,7 +40,7 @@ impl<'s> Label<'s> { pub fn packages(&self) -> Vec<&'s str> { match self.package_name { - Some(name) => name.split("/").collect(), + Some(name) => name.split('/').collect(), None => vec![], } } @@ -57,13 +57,13 @@ fn consume_repository_name<'s>( input: &'s str, label: &'s str, ) -> Result<(&'s str, Option<&'s str>)> { - if !input.starts_with("@") { + if !input.starts_with('@') { return Ok((input, None)); } let slash_pos = input .find("//") - .ok_or(err(label, "labels with repository must contain //."))?; + .ok_or_else(|| err(label, "labels with repository must contain //."))?; let repository_name = &input[1..slash_pos]; if repository_name.is_empty() { return Ok((&input[1..], None)); @@ -93,7 +93,7 @@ fn consume_repository_name<'s>( } fn consume_package_name<'s>(input: &'s str, label: &'s str) -> Result<(&'s str, Option<&'s str>)> { - let colon_pos = input.find(":"); + let colon_pos = input.find(':'); let start_pos; let mut is_absolute = false; if input.starts_with("//") { @@ -143,7 +143,7 @@ fn consume_package_name<'s>(input: &'s str, label: &'s str) -> Result<(&'s str, a-z, 0-9, '/', '-', '.', ' ', '$', '(', ')' and '_'.", ))); } - if package_name.ends_with("/") { + if package_name.ends_with('/') { return Err(LabelError(err( label, "package names may not end with '/'.", @@ -154,7 +154,7 @@ fn consume_package_name<'s>(input: &'s str, label: &'s str) -> Result<(&'s str, // This label doesn't contain the target name, we have to use // last segment of the package name as target name. return Ok(( - match package_name.rfind("/") { + match package_name.rfind('/') { Some(pos) => &package_name[pos..], None => package_name, }, @@ -169,15 +169,15 @@ fn consume_name<'s>(input: &'s str, label: &'s str) -> Result<&'s str> { if input.is_empty() { return Err(LabelError(err(label, "empty target name."))); } - let name = if input.starts_with(":") { - &input[1..] + let name = if let Some(stripped) = input.strip_prefix(':') { + stripped } else { input }; if name.is_empty() { return Err(LabelError(err(label, "empty target name."))); } - if name.starts_with("/") { + if name.starts_with('/') { return Err(LabelError(err( label, "target names may not start with '/'.", diff --git a/util/launcher/launcher_main.rs b/util/launcher/launcher_main.rs index 02ffbf2d7d..006d051911 100644 --- a/util/launcher/launcher_main.rs +++ b/util/launcher/launcher_main.rs @@ -9,7 +9,7 @@ use std::vec::Vec; use std::os::unix::process::CommandExt; /// This string must match the one found in `_create_test_launcher` -const LAUNCHFILES_ENV_PATH: &'static str = ".launchfiles/env"; +const LAUNCHFILES_ENV_PATH: &str = ".launchfiles/env"; /// Load environment variables from a uniquly formatted fn environ() -> BTreeMap { @@ -17,8 +17,11 @@ fn environ() -> BTreeMap { let mut key: Option = None; + // Consume the first argument (argv[0]) + let exe_path = std::env::args().next().expect("arg 0 was not set"); + // Load the environment file into a map - let env_path = std::env::args().nth(0).expect("arg 0 was not set") + LAUNCHFILES_ENV_PATH; + let env_path = exe_path + LAUNCHFILES_ENV_PATH; let file = File::open(env_path).expect("Failed to load the environment file"); // Variables will have the `${pwd}` variable replaced which is rendered by @@ -47,7 +50,8 @@ fn environ() -> BTreeMap { /// Locate the executable based on the name of the launcher executable fn executable() -> PathBuf { - let mut exec_path = std::env::args().nth(0).expect("arg 0 was not set"); + // Consume the first argument (argv[0]) + let mut exec_path = std::env::args().next().expect("arg 0 was not set"); let stem_index = exec_path .rfind(".launcher") .expect("This executable should always contain `.launcher`");