From 0c67d742f0094786f301e089955552004b8731cd Mon Sep 17 00:00:00 2001 From: WindSoilder Date: Mon, 9 Oct 2023 22:31:15 +0800 Subject: [PATCH] fix clippy (#10659) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This pr fix clippy warnings in latest clippy version(1.72.0): Unfortunally it's not easy to handle for [try fold](https://rust-lang.github.io/rust-clippy/master/index.html#/manual_try_fold) warning in `start command` Refer to known issue: > This lint doesn’t take into account whether a function does something on the failure case, i.e., whether short-circuiting will affect behavior. Refactoring to try_fold is not desirable in those cases. That's the case for our code, which does something on the failure case. So this pr is making a little refactor on `try_commands`. --- crates/nu-command/src/date/parser.rs | 2 +- crates/nu-command/src/debug/explain.rs | 1 - crates/nu-command/src/filesystem/ls.rs | 6 +- crates/nu-command/src/filesystem/start.rs | 66 +++++++++---------- .../nu-command/src/strings/detect_columns.rs | 2 +- .../src/strings/encode_decode/base64.rs | 3 +- crates/nu-command/tests/commands/glob.rs | 2 +- crates/nu-engine/src/glob_from.rs | 2 +- crates/nu-glob/src/lib.rs | 32 ++------- crates/nu-system/src/macos.rs | 5 +- crates/nu-test-support/src/fs.rs | 2 +- crates/nu-test-support/src/macros.rs | 9 +-- 12 files changed, 56 insertions(+), 76 deletions(-) diff --git a/crates/nu-command/src/date/parser.rs b/crates/nu-command/src/date/parser.rs index d2d1f3a851..66872b2e24 100644 --- a/crates/nu-command/src/date/parser.rs +++ b/crates/nu-command/src/date/parser.rs @@ -98,7 +98,7 @@ fn timezone_offset_internal( }; match s.len() { len if len >= 2 => &s[2..], - len if len == 0 => s, + 0 => s, _ => return Err(ParseErrorKind::TooShort), }; diff --git a/crates/nu-command/src/debug/explain.rs b/crates/nu-command/src/debug/explain.rs index 22937cfbef..a88fc78c10 100644 --- a/crates/nu-command/src/debug/explain.rs +++ b/crates/nu-command/src/debug/explain.rs @@ -90,7 +90,6 @@ pub fn get_pipeline_elements( let value_span = value.span(); let value_span_start = value_span.start as i64; let value_span_end = value_span.end as i64; - let command_name = command_name; let record = record! { "cmd_index" => Value::string(index, span), diff --git a/crates/nu-command/src/filesystem/ls.rs b/crates/nu-command/src/filesystem/ls.rs index 8964f61f62..8d356b5c34 100644 --- a/crates/nu-command/src/filesystem/ls.rs +++ b/crates/nu-command/src/filesystem/ls.rs @@ -158,8 +158,10 @@ impl Command for Ls { let glob_options = if all { None } else { - let mut glob_options = MatchOptions::new(); - glob_options.recursive_match_hidden_dir = false; + let glob_options = MatchOptions { + recursive_match_hidden_dir: false, + ..Default::default() + }; Some(glob_options) }; let (prefix, paths) = nu_engine::glob_from(&glob_path, &cwd, call_span, glob_options)?; diff --git a/crates/nu-command/src/filesystem/start.rs b/crates/nu-command/src/filesystem/start.rs index 5e96ffa568..35ec73e829 100644 --- a/crates/nu-command/src/filesystem/start.rs +++ b/crates/nu-command/src/filesystem/start.rs @@ -151,40 +151,38 @@ fn try_commands( span: Span, ) -> Result<(), ShellError> { let env_vars_str = env_to_strings(engine_state, stack)?; - commands - .into_iter() - .map(|mut cmd| { - let status = cmd - .envs(&env_vars_str) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .status(); - match status { - Ok(status) if status.success() => Ok(()), - Ok(status) => Err(format!( - "\nCommand `{}` failed with {}", - format_command(&cmd), - status - )), - Err(err) => Err(format!( - "\nCommand `{}` failed with {}", - format_command(&cmd), - err - )), - } - }) - .take_while_inclusive(|result| result.is_err()) - .fold(Err("".to_string()), |combined_result, next_result| { - combined_result.or_else(|combined_message| { - next_result.map_err(|next_message| combined_message + &next_message) - }) - }) - .map_err(|message| ShellError::ExternalCommand { - label: "No command found to start with this path".to_string(), - help: "Try different path or install appropriate command\n".to_string() + &message, - span, - }) + let cmd_run_result = commands.into_iter().map(|mut cmd| { + let status = cmd + .envs(&env_vars_str) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); + match status { + Ok(status) if status.success() => Ok(()), + Ok(status) => Err(format!( + "\nCommand `{}` failed with {}", + format_command(&cmd), + status + )), + Err(err) => Err(format!( + "\nCommand `{}` failed with {}", + format_command(&cmd), + err + )), + } + }); + + for one_result in cmd_run_result { + if let Err(err_msg) = one_result { + return Err(ShellError::ExternalCommand { + label: "No command found to start with this path".to_string(), + help: "Try different path or install appropriate command\n".to_string() + &err_msg, + span, + }); + } + } + Ok(()) } fn format_command(command: &std::process::Command) -> String { diff --git a/crates/nu-command/src/strings/detect_columns.rs b/crates/nu-command/src/strings/detect_columns.rs index 7c520e81e2..7a24d23aa1 100644 --- a/crates/nu-command/src/strings/detect_columns.rs +++ b/crates/nu-command/src/strings/detect_columns.rs @@ -232,7 +232,7 @@ fn detect_columns( .iter() .take(end_index) .skip(start_index) - .map(|v| v.as_string().unwrap_or(String::default())) + .map(|v| v.as_string().unwrap_or_default()) .join(" "); let binding = Value::string(combined, Span::unknown()); let last_seg = vals.split_off(end_index); diff --git a/crates/nu-command/src/strings/encode_decode/base64.rs b/crates/nu-command/src/strings/encode_decode/base64.rs index f44d5f9ed3..cb017236cc 100644 --- a/crates/nu-command/src/strings/encode_decode/base64.rs +++ b/crates/nu-command/src/strings/encode_decode/base64.rs @@ -107,8 +107,7 @@ fn action( Value::Error { .. } => input.clone(), Value::Binary { val, .. } => match base64_config.action_type { ActionType::Encode => { - let mut enc_vec = Vec::new(); - enc_vec.resize(val.len() * 4 / 3 + 4, 0); + let mut enc_vec = vec![0; val.len() * 4 / 3 + 4]; let bytes_written = match base64_engine.encode_slice(val, &mut enc_vec) { Ok(bytes_written) => bytes_written, Err(err) => { diff --git a/crates/nu-command/tests/commands/glob.rs b/crates/nu-command/tests/commands/glob.rs index 8b0f886ecf..0ab35158b0 100644 --- a/crates/nu-command/tests/commands/glob.rs +++ b/crates/nu-command/tests/commands/glob.rs @@ -133,7 +133,7 @@ pub fn create_file_at(full_path: impl AsRef) -> Result<(), std::io::Error> #[rstest] #[case(".", r#"'*z'"#, &["ablez", "baker", "charliez"], &["ablez", "charliez"], "simple glob")] #[case(".", r#"'qqq'"#, &["ablez", "baker", "charliez"], &[], "glob matches none")] -#[case("foo/bar", r#"'*[\]}]*'"#, &[r#"foo/bar/ab}le"#, "foo/bar/baker", r#"foo/bar/cha]rlie"#], &[r#"foo/bar/ab}le"#, r#"foo/bar/cha]rlie"#], "glob has quoted metachars")] +#[case("foo/bar", r"'*[\]}]*'", &[r#"foo/bar/ab}le"#, "foo/bar/baker", r#"foo/bar/cha]rlie"#], &[r#"foo/bar/ab}le"#, r#"foo/bar/cha]rlie"#], "glob has quoted metachars")] #[case("foo/bar", r#"'../*'"#, &["foo/able", "foo/bar/baker", "foo/charlie"], &["foo/able", "foo/bar", "foo/charlie"], "glob matches files in parent")] #[case("foo", r#"'./{a,b}*'"#, &["foo/able", "foo/bar/baker", "foo/charlie"], &["foo/able", "foo/bar"], "glob with leading ./ matches peer files")] fn glob_files_in_parent( diff --git a/crates/nu-engine/src/glob_from.rs b/crates/nu-engine/src/glob_from.rs index 38729544db..bc53aef1ef 100644 --- a/crates/nu-engine/src/glob_from.rs +++ b/crates/nu-engine/src/glob_from.rs @@ -82,7 +82,7 @@ pub fn glob_from( }; let pattern = pattern.to_string_lossy().to_string(); - let glob_options = options.unwrap_or_else(MatchOptions::new); + let glob_options = options.unwrap_or_default(); let glob = nu_glob::glob_with(&pattern, glob_options).map_err(|err| { nu_protocol::ShellError::GenericError( diff --git a/crates/nu-glob/src/lib.rs b/crates/nu-glob/src/lib.rs index d9bdf4e66b..eb8860e7a1 100644 --- a/crates/nu-glob/src/lib.rs +++ b/crates/nu-glob/src/lib.rs @@ -161,7 +161,7 @@ pub struct Paths { /// ``` /// Paths are yielded in alphabetical order. pub fn glob(pattern: &str) -> Result { - glob_with(pattern, MatchOptions::new()) + glob_with(pattern, MatchOptions::default()) } /// Return an iterator that produces all the `Path`s that match the given @@ -716,7 +716,7 @@ impl Pattern { /// assert!(Pattern::new("d*g").unwrap().matches("doog")); /// ``` pub fn matches(&self, str: &str) -> bool { - self.matches_with(str, MatchOptions::new()) + self.matches_with(str, MatchOptions::default()) } /// Return if the given `Path`, when converted to a `str`, matches this @@ -1020,7 +1020,7 @@ fn chars_eq(a: char, b: char, case_sensitive: bool) -> bool { /// Configuration options to modify the behaviour of `Pattern::matches_with(..)`. #[allow(missing_copy_implementations)] -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Default)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct MatchOptions { /// Whether or not patterns should be matched in a case-sensitive manner. /// This currently only considers upper/lower case relationships between @@ -1045,27 +1045,9 @@ pub struct MatchOptions { pub recursive_match_hidden_dir: bool, } -impl MatchOptions { - /// Constructs a new `MatchOptions` with default field values. This is used - /// when calling functions that do not take an explicit `MatchOptions` - /// parameter. - /// - /// This function always returns this value: - /// - /// ```rust,ignore - /// MatchOptions { - /// case_sensitive: true, - /// require_literal_separator: false, - /// require_literal_leading_dot: false - /// recursive_match_hidden_dir: true, - /// } - /// ``` - /// - /// # Note - /// The behavior of this method doesn't match `default()`'s. This returns - /// `case_sensitive` as `true` while `default()` does it as `false`. - // FIXME: Consider unity the behavior with `default()` in a next major release. - pub fn new() -> Self { +// Overwrite default behavior, because we want to make `recursive_match_hidden_dir` to true. +impl Default for MatchOptions { + fn default() -> Self { Self { case_sensitive: true, require_literal_separator: false, @@ -1274,7 +1256,7 @@ mod test { for c in "ABCDEFGHIJKLMNOPQRSTUVWXYZ".chars() { let options = MatchOptions { case_sensitive: false, - ..MatchOptions::new() + ..MatchOptions::default() }; assert!(pat.matches_with(&c.to_string(), options)); } diff --git a/crates/nu-system/src/macos.rs b/crates/nu-system/src/macos.rs index 0842f42fed..8a7c2ba3af 100644 --- a/crates/nu-system/src/macos.rs +++ b/crates/nu-system/src/macos.rs @@ -9,7 +9,6 @@ use libproc::libproc::thread_info::ThreadInfo; use libproc::processes::{pids_by_type, ProcFilter}; use mach2::mach_time; use std::cmp; -use std::ffi::OsStr; use std::path::{Path, PathBuf}; use std::thread; use std::time::{Duration, Instant}; @@ -186,7 +185,7 @@ fn get_path_info(pid: i32, mut size: size_t) -> Option { let exe = Path::new(get_unchecked_str(cp, start).as_str()).to_path_buf(); let name = exe .file_name() - .unwrap_or_else(|| OsStr::new("")) + .unwrap_or_default() .to_str() .unwrap_or("") .to_owned(); @@ -406,7 +405,7 @@ impl ProcessInfo { self.curr_path .as_ref() .map(|cur_path| cur_path.cwd.display().to_string()) - .unwrap_or_else(|| "".to_string()) + .unwrap_or_default() } } diff --git a/crates/nu-test-support/src/fs.rs b/crates/nu-test-support/src/fs.rs index bd23386316..00321e6590 100644 --- a/crates/nu-test-support/src/fs.rs +++ b/crates/nu-test-support/src/fs.rs @@ -233,7 +233,7 @@ pub fn root() -> PathBuf { } pub fn binaries() -> PathBuf { - let build_target = std::env::var("CARGO_BUILD_TARGET").unwrap_or(String::new()); + let build_target = std::env::var("CARGO_BUILD_TARGET").unwrap_or_default(); let profile = if let Ok(env_profile) = std::env::var("NUSHELL_CARGO_PROFILE") { env_profile diff --git a/crates/nu-test-support/src/macros.rs b/crates/nu-test-support/src/macros.rs index ca7012e55a..8b8a418f3d 100644 --- a/crates/nu-test-support/src/macros.rs +++ b/crates/nu-test-support/src/macros.rs @@ -212,6 +212,7 @@ macro_rules! nu_with_plugins { } use crate::{Outcome, NATIVE_PATH_ENV_VAR}; +use std::fmt::Write; use std::{ path::Path, process::{Command, Stdio}, @@ -299,14 +300,14 @@ pub fn nu_with_plugin_run_test(cwd: impl AsRef, plugins: &[&str], command: let registrations: String = plugins .iter() - .map(|plugin_name| { + .fold(String::new(), |mut output, plugin_name| { let plugin = with_exe(plugin_name); let plugin_path = nu_path::canonicalize_with(&plugin, &test_bins) .unwrap_or_else(|_| panic!("failed to canonicalize plugin {} path", &plugin)); let plugin_path = plugin_path.to_string_lossy(); - format!("register {plugin_path};") - }) - .collect(); + let _ = write!(output, "register {plugin_path};"); + output + }); let commands = format!("{registrations}{command}"); let target_cwd = crate::fs::in_directory(&cwd);