Skip to content

Commit

Permalink
Fixed clippy defects (#827)
Browse files Browse the repository at this point in the history
  • Loading branch information
UebelAndre authored Jul 10, 2021
1 parent 0c74f0e commit 79177f1
Show file tree
Hide file tree
Showing 13 changed files with 88 additions and 78 deletions.
63 changes: 33 additions & 30 deletions cargo/cargo_build_script_runner/bin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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(())
}

Expand Down Expand Up @@ -243,7 +246,7 @@ fn get_target_env_vars<P: AsRef<Path>>(rustc: &P) -> Result<BTreeMap<String, Str
if value.starts_with('"') && value.ends_with('"') && value.len() >= 2 {
values
.entry(key)
.or_insert(vec![])
.or_insert_with(Vec::new)
.push(value[1..(value.len() - 1)].to_owned());
}
}
Expand Down
34 changes: 21 additions & 13 deletions cargo/cargo_build_script_runner/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ impl BuildScriptOutput {
}

/// Converts a [BufReader] into a vector of [BuildScriptOutput] enums.
fn from_reader<T: Read>(mut reader: BufReader<T>) -> Vec<BuildScriptOutput> {
fn outputs_from_reader<T: Read>(mut reader: BufReader<T>) -> Vec<BuildScriptOutput> {
let mut result = Vec::<BuildScriptOutput>::new();
let mut line = String::new();
while reader.read_line(&mut line).expect("Cannot read line") != 0 {
Expand All @@ -107,20 +107,23 @@ 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<BuildScriptOutput>, Output), Output> {
pub fn outputs_from_command(
cmd: &mut Command,
) -> Result<(Vec<BuildScriptOutput>, 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)
}
}

/// Convert a vector of [BuildScriptOutput] into a list of environment variables.
pub fn to_env(v: &Vec<BuildScriptOutput>, 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(
Expand All @@ -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<BuildScriptOutput>, 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!(
Expand All @@ -154,11 +162,11 @@ impl BuildScriptOutput {
}

/// Convert a vector of [BuildScriptOutput] into a flagfile.
pub fn to_flags(v: &Vec<BuildScriptOutput>, 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()),
Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion proto/optional_output_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ fn ensure() -> Result<i32, Box<dyn error::Error>> {
let optional_outputs = args().take(index).collect::<Vec<String>>();
let exe = args().nth(index + 1).ok_or("no exe")?;
let exe_args = args().skip(index + 2).collect::<Vec<String>>();
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() {
Expand Down
2 changes: 1 addition & 1 deletion test/chained_direct_deps/mod3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion test/renamed_deps/mod3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"}]"#),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"}]"#),
Expand Down
12 changes: 6 additions & 6 deletions tools/runfiles/runfiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
}
Expand All @@ -109,10 +108,11 @@ impl Runfiles {
/// Returns the .runfiles directory for the currently executing binary.
pub fn find_runfiles_dir() -> io::Result<PathBuf> {
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 {
Expand Down
12 changes: 5 additions & 7 deletions tools/rustfmt/srcs/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: AsRef<Path>>(path: &T) -> std::io::Result<PathBuf> {
Expand Down Expand Up @@ -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<String> = content
.split("\n")
.split('\n')
.into_iter()
.filter(|s| !s.is_empty())
.map(|s| s.to_owned())
Expand All @@ -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,
}
}
3 changes: 0 additions & 3 deletions tools/rustfmt/srcs/test_main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading

0 comments on commit 79177f1

Please sign in to comment.